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

(aws-eks): Support --take-ownership flag in new helm version #32918

Open
1 of 2 tasks
hanseltime opened this issue Jan 14, 2025 · 3 comments
Open
1 of 2 tasks

(aws-eks): Support --take-ownership flag in new helm version #32918

hanseltime opened this issue Jan 14, 2025 · 3 comments
Labels
@aws-cdk/aws-eks Related to Amazon Elastic Kubernetes Service effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p2

Comments

@hanseltime
Copy link

hanseltime commented Jan 14, 2025

Describe the feature

The newest helm release (scheduled to come out tomorrow Jan 15, 2025), has a --take-ownership flag provided for the upgrade and install commands. We would want to open up this flag as a parameter for the HelmChart Custom Resource so that (provided there is the correct helm version in the KubectlLayer), IAC can specify something like:

new eks.HelmChart(this, 'my-chart', {
  ...previousProps,
  // We are okay with taking over resources that we may have already applied in manual migrations
  takeOwnership: true,
};

Use Case

A general use case is that a Devops team is testing particular configurations for some version upgrade of a helm chart or for turning on some previously unturned on feature. In this case, the team might manually deploy some manifests so that they can vary different values and then soak test afterwards. The devops team would expect that they could add these features to the helm chart construct after the fact and have helm take over the zero-sum change by applying its upgrade.

The last assumption right now does not work though because helm will not succeed if ownership annotations are missing and will break your IAC deploy pipeline despite you thinking you were just bringing things up to parity. The new --take-ownership feature solves this problem and will allow DevOps personnel to make the conscious choice to run the flag and avoid deployment triage that either involves finding all discrepancies and patching helm-specific ownership or deleting resources to have them recreated (potentially creating some downtime).

A direct use case for me is that ArgoCD does not have the ability to add annotations for a helm chart (this traces back to how the helm library implements its template API). Because of this, experimentation through the better visibility tool of ArgoCD for a helm chart that CDK deploys ends up blocking the next cluster deployment that tries to also create the new manifest that was previously tested.

Proposed Solution

  1. Add a takeOwnership?: boolean option to the eks.HelmChart Construct
  2. Update the KubectlHandler code to apply the take-ownership option
    3. Since this is version specific, it would be ideal to verify the helm version during synth and throw an error if the option wouldn't be supported - I believe this could be achieved by updating the KubectlLayer libraries with a "helmVersion" field and retrieving that?
    4. If the synth time option isn't possible, then at least providing a descriptive thrown error from the custom resource handler

An obvious requirement here is that this needs to be an opt-in flag, so that default behavior of all other existing HelmCharts are preserved.

I think that would be about it, unless I'm missing something.

Other Information

No response

Acknowledgements

  • I may be able to implement this feature request
  • This feature might incur a breaking change

CDK version used

2.175.1

Environment details (OS name and version, etc.)

Any environment

@hanseltime hanseltime added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Jan 14, 2025
@github-actions github-actions bot added the @aws-cdk/aws-eks Related to Amazon Elastic Kubernetes Service label Jan 14, 2025
@ashishdhingra ashishdhingra self-assigned this Jan 14, 2025
@ashishdhingra ashishdhingra added p2 investigating This issue is being investigated and/or work is in progress to resolve the issue. and removed needs-triage This issue or PR still needs to be triaged. labels Jan 14, 2025
@ashishdhingra
Copy link
Contributor

Appears to be valid feature request. Helm chart custom resource handler is defined here.

@ashishdhingra ashishdhingra added effort/medium Medium work item – several days of effort and removed investigating This issue is being investigated and/or work is in progress to resolve the issue. labels Jan 14, 2025
@ashishdhingra ashishdhingra removed their assignment Jan 14, 2025
@hanseltime
Copy link
Author

@ashishdhingra - I see that you unassigned yourself from this ticket. If you're not actively doing the work, would it be okay if I submitted a PR?

@ashishdhingra
Copy link
Contributor

@ashishdhingra - I see that you unassigned yourself from this ticket. If you're not actively doing the work, would it be okay if I submitted a PR?

@hanseltime Sure. Feel free to contribute PR that could be reviewed by the team.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-eks Related to Amazon Elastic Kubernetes Service effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p2
Projects
None yet
Development

No branches or pull requests

2 participants