Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure Github workflows use up-to-date base Docker images #1951

Conversation

antoninbas
Copy link
Contributor

Relying on a CRON job to update the Antrea base images has proven
sub-optimal: we sometimes push base images manually which do not match
the checked-in Dockerfiles and CI tests run for PRs which update the
Dockerfiles ignore these updates (making the tests worthless).

We now ensure that Github workflows always build the base images befgore
building the Antrea image, thanks to a new helper script. By relying on
Docker caching (using the Dockerhub registry as the cache), we ensure
that build times are not increased: in the absence of any change, we
only add a handful of seconds to the build time.

For now, we only update CI jobs run as Github workflow. Once this is
merged, we should consider doing the same for Jenkins scripts. We could
add support for DOCKER_REGISTRY to the new helper script
(hack/build-antrea-ubuntu-all.sh).

One question that we could ask now is whether these base images are even
necessary: if caching works well, using one large Dockerfile should be
just as fast, while simplifying build architecture. This is something we
may want to revisit in the future. Maybe using base images only makes
sense if we are going to share them across multiple images.

See #1540

Relying on a CRON job to update the Antrea base images has proven
sub-optimal: we sometimes push base images manually which do not match
the checked-in Dockerfiles and CI tests run for PRs which update the
Dockerfiles ignore these updates (making the tests worthless).

We now ensure that Github workflows always build the base images befgore
building the Antrea image, thanks to a new helper script. By relying on
Docker caching (using the Dockerhub registry as the cache), we ensure
that build times are not increased: in the absence of any change, we
only add a handful of seconds to the build time.

For now, we only update CI jobs run as Github workflow. Once this is
merged, we should consider doing the same for Jenkins scripts. We could
add support for DOCKER_REGISTRY to the new helper script
(hack/build-antrea-ubuntu-all.sh).

One question that we could ask now is whether these base images are even
necessary: if caching works well, using one large Dockerfile should be
just as fast, while simplifying build architecture. This is something we
may want to revisit in the future. Maybe using base images only makes
sense if we are going to share them across multiple images.

See antrea-io#1540
@antoninbas antoninbas force-pushed the ensure-Github-workflows-use-up-to-date-base-images branch from 298fd12 to 89f0b30 Compare March 13, 2021 03:06
@codecov-io
Copy link

codecov-io commented Mar 13, 2021

Codecov Report

Merging #1951 (89f0b30) into main (d77f8ff) will increase coverage by 0.06%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1951      +/-   ##
==========================================
+ Coverage   64.47%   64.54%   +0.06%     
==========================================
  Files         193      193              
  Lines       16893    16893              
==========================================
+ Hits        10892    10903      +11     
+ Misses       4837     4830       -7     
+ Partials     1164     1160       -4     
Flag Coverage Δ
kind-e2e-tests 55.44% <ø> (+0.06%) ⬆️
unit-tests 41.37% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
pkg/agent/util/ipset/ipset.go 65.38% <0.00%> (+3.84%) ⬆️
pkg/controller/networkpolicy/tier.go 52.50% <0.00%> (+5.00%) ⬆️
pkg/controller/networkpolicy/status_controller.go 88.81% <0.00%> (+5.26%) ⬆️

Copy link
Contributor

@jianjuns jianjuns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@antoninbas
Copy link
Contributor Author

@tnqn do you want to take a look at this, since you're the one that brought #1540 to my attention a while back?

Copy link
Member

@tnqn tnqn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, LGTM.
A typo in commit message: s/befgore/before

@antoninbas
Copy link
Contributor Author

/test-all

@antoninbas antoninbas merged commit cdbd4fc into antrea-io:main Mar 22, 2021
@antoninbas antoninbas deleted the ensure-Github-workflows-use-up-to-date-base-images branch March 22, 2021 22:08
@antoninbas
Copy link
Contributor Author

@tnqn thanks, fixed the typo when I squashed

antoninbas added a commit to antoninbas/antrea that referenced this pull request Mar 30, 2021
In antrea-io#1951 we ensured that the
Github workflows would use up-to-date base Docker images by building all
images in every run and leveraging Docker caching to reduce build
times. With this new change, we do the same thing for Jenkins jobs.

See antrea-io#1540
antoninbas added a commit to antoninbas/antrea that referenced this pull request Apr 2, 2021
In antrea-io#1951 we ensured that the
Github workflows would use up-to-date base Docker images by building all
images in every run and leveraging Docker caching to reduce build
times. With this new change, we do the same thing for Jenkins jobs.

See antrea-io#1540
antoninbas added a commit that referenced this pull request Apr 2, 2021
* Ensure Jenkins CI jobs use up-to-date base Docker images

In #1951 we ensured that the
Github workflows would use up-to-date base Docker images by building all
images in every run and leveraging Docker caching to reduce build
times. With this new change, we do the same thing for Jenkins jobs.

See #1540

* Increase integration tests timeout

* Correct file permissions to enable Docker caching
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants