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

feat: Optimize helm deploy by using goroutines #9451

Merged
merged 2 commits into from
Nov 15, 2024

Conversation

idsulik
Copy link
Contributor

@idsulik idsulik commented Jun 25, 2024

Description

  1. Optimized helm install by using goroutines which reduced deploy time by more than 50%
  2. Added config option to control the concurrency, by default it keeps current logic - sequential installation
  3. Minor fixes

in my case:
before: Deploy completed in 3 minutes 52.351 seconds
after: Deploy completed in 1 minute 57.148 seconds

colleague:
before: Deploy completed in 4 minutes 4.594 seconds
after: Deploy completed in 53.423 seconds

@idsulik idsulik marked this pull request as draft June 25, 2024 08:22
@pull-request-size pull-request-size bot added size/L and removed size/M labels Jun 25, 2024
@idsulik idsulik force-pushed the helm-deploy-optimization branch 3 times, most recently from 44fb2b8 to d1f1046 Compare June 25, 2024 13:49
@idsulik idsulik marked this pull request as ready for review July 10, 2024 07:20
@idsulik idsulik marked this pull request as draft August 7, 2024 08:19
@idsulik idsulik force-pushed the helm-deploy-optimization branch 4 times, most recently from f3adb33 to 383b26e Compare September 6, 2024 15:04
@idsulik idsulik marked this pull request as ready for review September 6, 2024 15:52

if h.Concurrency != nil && *h.Concurrency == 1 {
Copy link
Contributor

@alphanota alphanota Nov 15, 2024

Choose a reason for hiding this comment

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

Hi @idsulik, thanks for this PR! Would you be able to add tests for both Sequential and Concurrent installation cases?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hi @alphanota, pushed changes. by default it keeps the old behavior (sequential order), but a user can change it using the config file.
Please check this PR as well #9520, because it's very important, if something goes wrong with helm package installation, user will have package in pending state, because skaffold kills the helm command, so the user needs to manually uninstall/rollback the pending release

@idsulik idsulik force-pushed the helm-deploy-optimization branch from 383b26e to b07b960 Compare November 15, 2024 17:15
Signed-off-by: Suleiman Dibirov <idsulik@gmail.com>
@idsulik idsulik force-pushed the helm-deploy-optimization branch from b07b960 to 252d836 Compare November 15, 2024 17:16
@idsulik idsulik requested a review from alphanota November 15, 2024 17:22
@alphanota alphanota self-assigned this Nov 15, 2024
Signed-off-by: Suleiman Dibirov <idsulik@gmail.com>
@alphanota alphanota merged commit 3f2e8df into GoogleContainerTools:main Nov 15, 2024
11 checks passed
plumpy added a commit to plumpy/skaffold that referenced this pull request Nov 20, 2024
@idsulik idsulik deleted the helm-deploy-optimization branch November 27, 2024 09:33
@idsulik
Copy link
Contributor Author

idsulik commented Nov 27, 2024

Hey, I'm testing the latest version of skaffold with parallel helm, it's wonderful! I had a deployment that usually takes 25-40 minutes and now it takes 10 minutes!

(c) skaffold slack https://kubernetes.slack.com/archives/CABQMSZA6/p1732688941159699

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants