fix: (helm) Add expand env template for dependsOn, fix concurrent installation (#9689)

[plumpy]

• fix(helm): Add expand env template for dependsOn and fix concurrent installation

• tests

Signed-off-by: Suleiman Dibirov idsulik@gmail.com

---

Signed-off-by: Suleiman Dibirov idsulik@gmail.com

[ghost]

Hi team,

I'm currently reviewing this pull request and will provide feedback shortly. In the meantime, here's a summary for you and other reviewers to quickly get up to speed:

This pull request addresses concurrent installation issues in the Helm deployer and improves the handling of environment variables in release names and dependencies.

Intent:

The primary goal is to fix concurrent installation problems and enhance the robustness of environment variable expansion within Helm deployments. The changes ensure that release names and dependsOn fields correctly handle environment variables, preventing unexpected behavior during concurrent deployments.

Changes:

• pkg/skaffold/deploy/helm/helm.go: Modified the deployment logic to handle concurrency more effectively. Specifically, lines 288-298 were changed to allow for configurable concurrency, with a default of sequential execution. The handling of release names (lines 338-344) now includes environment variable expansion for release names. The logging (lines 309-312) was also slightly adjusted for clarity.
• pkg/skaffold/deploy/helm/helm_test.go: Added a new test case (testDeployWithDependsOnConfig) on lines 69-82 to verify the correct functionality of dependsOn with environment variables. This involved adding a new test case to the TestHelmDeploy function (lines 464-481) to cover the new scenario.
• pkg/skaffold/deploy/helm/util.go: Removed unnecessary environment variable expansion from the BuildDependencyGraph function (lines 27-29). This simplifies the dependency graph creation and removes a potential source of errors.
• pkg/skaffold/deploy/helm/util_test.go: Updated a test case description (lines 69-79) to reflect the changes in the BuildDependencyGraph function.

Activity:

The pull request has been updated with additional tests to cover the changes made. The author, Suleiman Dibirov, has signed off on the changes.

I'll provide a more detailed review shortly. Thanks!

---

And now, a little haiku to brighten your day:

Code flows like a stream,
Fixes merge, the build turns green,
Joyful, smooth release.

[ghost]

This pull request addresses a concurrency issue in the Helm deployer and allows for environment variable expansion in dependsOn. The changes are well-structured and improve the functionality and flexibility of the Helm deployer. The added test case effectively covers the new dependsOn behavior. A few minor suggestions for improvement are listed below.

I've referenced the Effective Go style guide in this review.