Skip to content

feat: Optimize helm deploy by using goroutines#9451

Merged
alphanota merged 2 commits intoGoogleContainerTools:mainfrom
idsulik:helm-deploy-optimization
Nov 15, 2024
Merged

feat: Optimize helm deploy by using goroutines#9451
alphanota merged 2 commits intoGoogleContainerTools:mainfrom
idsulik:helm-deploy-optimization

Conversation

@idsulik
Copy link
Copy Markdown
Contributor

@idsulik idsulik commented Jun 25, 2024 •
edited
Loading

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 mentioned this pull request Jul 19, 2024
Merged
@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
alphanota
alphanota requested changes Nov 15, 2024
View reviewed changes
pkg/skaffold/deploy/helm/helm.go Outdated
manifests := manifest.ManifestList{}
g, ctx := errgroup.WithContext(ctx)

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

@alphanota alphanota Nov 15, 2024 •
edited
Loading

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
Copy Markdown
Contributor Author

@idsulik idsulik Nov 15, 2024

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
@idsulik
feat: Optimize helm deploy by using goroutines
252d836 
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
@idsulik
fix lint
e7d9702 
Signed-off-by: Suleiman Dibirov <idsulik@gmail.com>
@alphanota alphanota merged commit 3f2e8df into GoogleContainerTools:main Nov 15, 2024
@idsulik idsulik mentioned this pull request Nov 18, 2024
Merged
plumpy added a commit to plumpy/skaffold that referenced this pull request Nov 20, 2024
@plumpy
Revert "feat: Optimize helm deploy by using goroutines (GoogleContain…
81ec56b 
…erTools#9451)"

This reverts commit 3f2e8df.
@idsulik idsulik deleted the helm-deploy-optimization branch November 27, 2024 09:33
@idsulik
Copy link
Copy Markdown
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

This was referenced Dec 20, 2024
Closed
Merged
@idsulik
Copy link
Copy Markdown
Contributor Author

idsulik commented Feb 6, 2025

the result is perfect!)
image

@Oden22
Copy link
Copy Markdown

Oden22 commented Feb 11, 2025

Is there any documentation around how to use this feature ?

@idsulik
Copy link
Copy Markdown
Contributor Author

idsulik commented Feb 11, 2025

@Oden22 hi! It's pretty simple:

  1. specify concurrency - https://skaffold.dev/docs/references/yaml/#deploy-helm-concurrency, 0 for no limit
  2. if you have a release that depends on other releases - https://skaffold.dev/docs/references/yaml/#deploy-helm-releases-dependsOn - add dependsOn

That's it

@menahyouyeah menahyouyeah mentioned this pull request Mar 3, 2026
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alphanota alphanota alphanota approved these changes

Assignees

@alphanota alphanota

Labels

size/XL

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@idsulik @Oden22 @alphanota