fix(helm): Fix helm package installation order (#9693)

[plumpy]

• fix(helm): Fix helm package installation order

• fix copyright

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

• tests

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

• lint

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

• fixes

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

• fix linters

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

---

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

[code-review-assist]

Hi @plumpy, reviewers, and future readers!

I'm Gemini, and I'm currently reviewing this pull request. In the meantime, I'm providing a summary for everyone to quickly grasp the changes and intent.

This pull request, "fix(helm): Fix helm package installation order (#9693)", aims to improve the installation order of Helm packages. The description mentions fixing the installation order, copyright issues, and linters, along with adding tests. Let's break down the changes:

• pkg/skaffold/deploy/helm/dependencygraph.go: This file has been completely rewritten (220 lines added). It introduces a DependencyGraph struct and associated methods to manage Helm release dependencies. The key improvement is the GetReleasesByLevel function, which now returns releases grouped by their dependency level, ensuring correct installation order. This addresses the core issue of incorrect installation order.
• pkg/skaffold/deploy/helm/dependencygraph_test.go: This file also received a major update (320 lines added), adding comprehensive tests for the new DependencyGraph functionality. These tests cover various scenarios, including simple and complex dependency graphs, cycles, and duplicate release names.
• pkg/skaffold/deploy/helm/helm.go: This file has been modified to leverage the new DependencyGraph (lines 261-323). The changes primarily involve replacing the old, less robust dependency handling with the new, level-based approach. The concurrency handling has also been slightly improved for clarity and to allow for unlimited concurrency.
• pkg/skaffold/deploy/helm/helm_test.go: Additional tests were added to helm_test.go to verify the new deployment order logic (lines 479-567).
• pkg/skaffold/schema/defaults/defaults.go: The cycle detection logic has been removed from this file (lines 129-140), as it is now handled within the DependencyGraph.
• pkg/skaffold/deploy/helm/util.go: This file was completely removed, as its functionality is now incorporated into dependencygraph.go.
• pkg/skaffold/deploy/helm/util_test.go: This file was also completely removed.

I'll be providing a more detailed review shortly. In the meantime, please let me know if you have any questions.

And now, a little haiku to lighten the mood:

Helm charts align,
Dependencies now in flow,
Smooth deployment starts.

[code-review-assist]

This pull request refactors the Helm dependency management to introduce a topological sort and ensure deployments happen in the correct order, respecting dependencies between releases. The changes also remove redundant code and improve error handling. The addition of tests for the new DependencyGraph is commendable and covers various scenarios, including cycle detection and level-based deployment. The changes in helm.go correctly utilize the new DependencyGraph and simplify the deployment logic. Overall, the changes are well-structured and improve the reliability of Helm deployments with dependencies.

Here's a summary of the elements I referenced from the implicit Google Go style guide:

• Error Handling: Checked for appropriate error handling and suggested improvements where necessary.
• Naming Conventions: Verified adherence to Go naming conventions for functions and variables.
• Code Clarity: Suggested improvements for code readability and conciseness.
• Testing: Reviewed the test cases for comprehensiveness and correctness.