Skip to content

Move core logic of helm install into independent helm module#4127

Merged
ighosh98 merged 1 commit into
GoogleCloudPlatform:developfrom
ighosh98:helm-migration
May 15, 2025
Merged

Move core logic of helm install into independent helm module#4127
ighosh98 merged 1 commit into
GoogleCloudPlatform:developfrom
ighosh98:helm-migration

Conversation

@ighosh98

Copy link
Copy Markdown
Contributor

Submission Checklist

Required to create isolation between the kubectl and helm modules.

NOTE: Community submissions can take up to 2 weeks to be reviewed.

Please take the following actions before submitting this pull request.

  • Fork your PR branch from the Toolkit "develop" branch (not main)
  • Test all changes with pre-commit in a local branch #
  • Confirm that "make tests" passes all tests
  • Add or modify unit tests to cover code changes
  • Ensure that unit test coverage remains above 80%
  • Update all applicable documentation
  • Follow Cluster Toolkit Contribution guidelines #

@ighosh98 ighosh98 requested review from a team and samskillman as code owners May 15, 2025 04:12

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hello @ighosh98, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

Summary of Changes

Hello team, gemini-code-assist here to provide a summary of this pull request. This PR introduces a new, independent Terraform module located under modules/management/helm-install/helm_install. The primary goal, as stated in the title and description, is to isolate the core logic for installing Helm charts from other modules, specifically aiming to separate it from kubectl-related logic. This new module encapsulates the functionality of the helm_release Terraform resource, providing a comprehensive set of input variables to configure various aspects of a Helm chart installation or upgrade.

Highlights

  • New Helm Install Module: A completely new Terraform module modules/management/helm-install/helm_install has been created to handle Helm chart installations.
  • Logic Isolation: The core logic for deploying Helm charts via the helm_release resource is now contained within this dedicated module, improving separation from other concerns like kubectl operations.
  • Comprehensive Configuration: The module exposes a wide range of input variables, allowing users to configure almost all parameters available for the helm_release resource, mirroring many helm install and helm upgrade CLI options.
  • Standard Module Structure: The new module follows standard Terraform module practices with dedicated files for main.tf (resource definition), variables.tf (inputs), versions.tf (provider/Terraform constraints), and README.md (documentation).

Changelog

Click here to see the changelog
  • modules/management/helm-install/helm_install/README.md
    • Added README documentation for the new Helm install module, including requirements, providers, resources, and detailed input variable descriptions.
  • modules/management/helm-install/helm_install/main.tf
    • Added the main Terraform file defining the helm_release resource and mapping input variables to its arguments.
    • Implemented dynamic blocks for set and postrender arguments based on input variables.
  • modules/management/helm-install/helm_install/metadata.yaml
    • Added module metadata, specifying required services like container.googleapis.com.
  • modules/management/helm-install/helm_install/variables.tf
    • Added definitions for all input variables controlling the Helm release configuration, including types, descriptions, and default values.
  • modules/management/helm-install/helm_install/versions.tf
    • Added version constraints for the required Terraform version (>= 1.3) and the Helm provider (~> 2.17).
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in issue comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.


Module built anew,
Helm charts deploy with ease,
Logic stands apart.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

The pull request introduces a new Helm install module, which aims to isolate Helm-related logic from the kubectl module. This is a good step towards modularity. The module appears well-structured, with clear separation of concerns. However, there are a few areas that could be improved for better maintainability and clarity.

Summary of Findings

  • Code Formatting: Inconsistent code formatting can hinder readability. Consider using terraform fmt for automatic formatting.
  • Helm Version Compatibility: The recreate_pods option is deprecated in Helm CLI v3. Implement conditional logic to skip this option for Helm v3 or higher.
  • Variable Default Value Rationale: Expand on the comments explaining the rationale behind the default values for variables. This will improve maintainability and understanding of the module.

Merge Readiness

The pull request is well-structured and introduces a valuable new module. However, addressing the identified issues related to code formatting and Helm version compatibility will further enhance the module's quality and maintainability. I am unable to directly approve this pull request, and recommend that others review and approve this code before merging. I recommend addressing the comments before merging.

Comment thread modules/management/helm-install/helm_install/main.tf
@ighosh98 ighosh98 added the release-improvements Added to release notes under the "Improvements" heading. label May 15, 2025

@SwarnaBharathiMantena SwarnaBharathiMantena left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Shouldn't helm be part of community modules?

Comment thread modules/management/helm-install/helm_install/main.tf Outdated
@ighosh98

Copy link
Copy Markdown
Contributor Author

Shouldn't helm be part of community modules?

Given that we have aligned support for it, I think we can just start by adding it to the core module. This module will not be used anytime soon, so we should be fine. But, we could go the community->core migration route.

@ighosh98 ighosh98 force-pushed the helm-migration branch 2 times, most recently from 59315ed to 59e9182 Compare May 15, 2025 06:09

@SwarnaBharathiMantena SwarnaBharathiMantena left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@ighosh98 ighosh98 merged commit 8a4748b into GoogleCloudPlatform:develop May 15, 2025
11 of 63 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release-improvements Added to release notes under the "Improvements" heading.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants