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

Merged
merged 2 commits into from
Apr 1, 2022

Conversation

34fathombelow
Copy link
Member

@34fathombelow 34fathombelow commented Mar 22, 2022

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

Disabled cache, it is configured to only cache the export layers that are in the final build stages. Not taking advantage of the cache on disk for the second run of the kubectl-argo-rollouts image This will reduce build times by half. I recommend a second PR to cleanup the existing cache steps from the docker-publish.yml as they are not needed anymore.

I have tested these builds locally. I'm expecting the complete Workflow should be complete in approximately 20-25 minutes on GitHub runners.

The flowchart below shows how the Dockerfile is processed using cross-compilation. Orange nodes are x86_64 and opaque orange nodes are arm64.

flowchart TB

id1([builder]) --> id2([argo-rollouts-ui]) --> id3([argo-rollouts-build]) --> id4([kubectl-argo-rollouts]) --> id5([x86_64 Final_image])
id3([argo-rollouts-build]) --> id6([kubectl-argo-rollouts]) --> id7([arm64 Final_Image])


style id6 fill:#5C74C6,stroke:#333,stroke-width:2px,color:#ffffff
classDef x86 fill:#EF7B4D,stroke:#EF7B4D,stroke-width:2px,color:#ffffff
classDef arm64 fill:#FCE5DB,stroke:#EF7B4D,stroke-width:2px,color:#EF7B4D
class id1,id2,id3,id4,id5 x86
class id6,id7,id8 arm64

Loading

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.

Signed-off-by: Justin Marquis <[email protected]>
@codecov
Copy link

codecov bot commented Mar 22, 2022

Codecov Report

Merging #1919 (1209b0a) into master (08cf10e) will increase coverage by 0.03%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #1919      +/-   ##
==========================================
+ Coverage   82.36%   82.40%   +0.03%     
==========================================
  Files         119      119              
  Lines       16872    16910      +38     
==========================================
+ Hits        13897    13934      +37     
- Misses       2284     2285       +1     
  Partials      691      691              
Impacted Files Coverage Δ
controller/metrics/metrics.go 100.00% <0.00%> (ø)
controller/metrics/prommetrics.go 100.00% <0.00%> (ø)
utils/record/record.go 80.55% <0.00%> (+2.83%) ⬆️

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 08cf10e...1209b0a. Read the comment docs.

Signed-off-by: Justin Marquis <[email protected]>
@sonarcloud
Copy link

sonarcloud bot commented Mar 22, 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

@@ -23,13 +23,13 @@ jobs:
- name: Set up Docker Buildx
uses: docker/setup-buildx-action@v1

- name: Cache Docker layers
Copy link
Contributor

Choose a reason for hiding this comment

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

is this intentional to remove cache?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, the cache is only caching the exported layers in the final build stage. Which provides no real benefit. This could be changed to mode=max to export all interminate layers but GitHub has a limit of 10GB across a single repo. Also the time to prossess approximately 20gb of cache and compress it really outweighs the benefits. A full cache will also not prossess RUN commands in the dockerfile.

Copy link
Member Author

Choose a reason for hiding this comment

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

Also this cache is being accessed during pull requests. It can create conflicts and slow the processes down.

Copy link
Contributor

Choose a reason for hiding this comment

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

got it..thanks

@harikrongali
Copy link
Contributor

thanks for the PR

@jessesuen jessesuen merged commit beae624 into argoproj:master Apr 1, 2022
@34fathombelow
Copy link
Member Author

@harikrongali @jessesuen Thank you for the review and merge. Image build times are much better, but the plugin-image should have been built from the runners cache of the controller-image. I will look into this and have a solution by Monday. We should be able to cut off an additional 20+ minutes.

@34fathombelow 34fathombelow deleted the image-speed-improvement branch April 2, 2022 00:32
RaviHari pushed a commit to RaviHari/argo-rollouts that referenced this pull request Apr 5, 2022
RaviHari pushed a commit to RaviHari/argo-rollouts that referenced this pull request Apr 5, 2022
Signed-off-by: Justin Marquis <[email protected]>
Signed-off-by: Ravi Hari <[email protected]>
alexmt pushed a commit that referenced this pull request Apr 11, 2022
* chore: Improve image build speed #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]>
jenciso pushed a commit to jenciso/argo-rollouts that referenced this pull request Oct 25, 2022
Signed-off-by: Justin Marquis <[email protected]>
Signed-off-by: Juan Enciso <[email protected]>
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
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants