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

chore: Improve image build speed #1919 #1948

Merged
merged 4 commits into from
Apr 11, 2022

Conversation

34fathombelow
Copy link
Member

Signed-off-by: Justin Marquis [email protected]

This is a follow-up to PR#1919. Cache created from the controller-image should be used to build the plugin-image. Buildx uses Buildkit under the hood and uses "Automatic garbage collection" which has a default policy of 10% of free disk space. Disabling the garbage collection ensures that the cache is temporarily stored for the duration of the runner.

I also made a small change to the Dockerfile to provide support for older versions of Docker and Podman which do not support multiple args on a single line.

Checklist:

  • Either (a) I've created an enhancement proposal and discussed it with the community, (b) this is a bug fix, or (c) this is a chore.
  • The title of the PR is (a) conventional, (b) states what changed, and (c) suffixes the related issues number. E.g. "fix(controller): Updates such and such. Fixes #1234".
  • I've signed my commits with DCO
  • I have written unit and/or e2e tests for my change. PRs without these are unlikely to be merged.
  • My builds are green. Try syncing with master if they are not.
  • My organization is added to USERS.md.

@codecov
Copy link

codecov bot commented Apr 2, 2022

Codecov Report

Merging #1948 (5921b1c) into master (beae624) will increase coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #1948      +/-   ##
==========================================
+ Coverage   82.42%   82.43%   +0.01%     
==========================================
  Files         119      119              
  Lines       16913    16913              
==========================================
+ Hits        13940    13943       +3     
+ Misses       2282     2280       -2     
+ Partials      691      690       -1     
Impacted Files Coverage Δ
rollout/trafficrouting/istio/controller.go 52.43% <0.00%> (+1.62%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update beae624...5921b1c. Read the comment docs.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Apr 3, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@34fathombelow
Copy link
Member Author

34fathombelow commented Apr 3, 2022

I noticed line# 66 of the Dockerfile we were compiling controller plugin plugin-linux plugin-darwin plugin-windows. There is no need to compile plugin-linux plugin-darwin plugin-windows binaries each time we build images, as they are not in any of the final images. They are only used during a release. With two different architectures being built, we were compiling six additional unneeded binaries. They are added back into the release workflow by including them as a --build-arg in /hack/build-release-plugins.sh

The release workflow now also supports all of the optimizations. Image build times should be about 10 minutes now!!! All these changes have been fully tested, I have ran them on GitHub runners, built images locally and verified images sizes and binaries work on both architectures.

Please add a label of "test-arm-image" to this PR.

@34fathombelow 34fathombelow changed the title chore: Improve image build speed #2 chore: Improve image build speed #1919 Apr 5, 2022
@34fathombelow
Copy link
Member Author

@harikrongali can you please review this when you get a chance?

@harikrongali
Copy link
Contributor

@crenshaw-dev can you review?

Copy link
Member

@crenshaw-dev crenshaw-dev left a comment

Choose a reason for hiding this comment

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

lgtm!

@harikrongali harikrongali added the ready-for-review Ready for final review label Apr 11, 2022
Copy link
Contributor

@alexmt alexmt left a comment

Choose a reason for hiding this comment

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

thank you!

@alexmt alexmt merged commit a8ae46a into argoproj:master Apr 11, 2022
@34fathombelow 34fathombelow deleted the image-build-improvement branch April 12, 2022 21:40
jenciso pushed a commit to jenciso/argo-rollouts that referenced this pull request Oct 25, 2022
* chore: Improve image build speed argoproj#2

Signed-off-by: Justin Marquis <[email protected]>

* chore: Added test-arm-image test for arm64

Signed-off-by: Justin Marquis <[email protected]>

* chore: enabled buildkit for docker build commands

Signed-off-by: Justin Marquis <[email protected]>

* chore: support for releases and further docker optimizations

Signed-off-by: Justin Marquis <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-for-review Ready for final review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants