fix(eks): helm uninstall in custom resource handler does not respect Wait
#28830
Conversation
I have a stack that installs and uninstalls helm charts, where the underlying resources use [Kubernetes Finalizers](https://kubernetes.io/docs/concepts/overview/working-with-objects/finalizers/). CDK currently ignores `--wait`, which means that any object finalization in not respected in my dependency ordering.
github-actions
bot
added
beginning-contributor
ellistarn
changed the title
There was a problem hiding this comment.
The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.
A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.
|
Exemption Request: #28830 (review). This code does not have testing built out yet AFAICT. |
aws-cdk-automation
added
pr-linter/exemption-requested
mrgrain
changed the title
Wait
There was a problem hiding this comment.
I would classify this issue as a bug and accept the minor breaking changes, so going to approve this PR.
My reasoning for the approval is that prior to this proposed modification, setting wait=True resulted in identical behavior to wait=False or not set at all. Consequently, this update is reasonable. However, it will alter the existing behavior for users who have previously set wait=True, as they will now experience a delay while waiting for the uninstallation process to complete. The change is confined to CLI change and its primary impact is merely extending the duration users will need to wait for the command’s execution to finish.
aws-cdk-automation
removed
the
pr/needs-community-review
|
Anything I need to do to dismiss the testing linter errors? |
GavinZZ
added
pr-linter/exempt-test
aws-cdk-automation
dismissed stale reviews from themself
✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.
|
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
|
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
…`Wait` (aws#28830) I have a stack that installs and uninstalls helm charts, where the underlying resources use [Kubernetes Finalizers](https://kubernetes.io/docs/concepts/overview/working-with-objects/finalizers/). CDK's helm construct currently ignores `--wait`, which means that any object finalization in those helm charts is not respected in my dependency ordering. ## Compatibility I could see some debate around whether or not this is a breaking change. I'm currently viewing it as a bug fix. I'm a bit cautious about how valuable it would be to gate this behavior compared to the additional complexity. ## Testing Tested on a local fork and it worked like a charm Closes aws#28831 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
…`Wait` (#28830) I have a stack that installs and uninstalls helm charts, where the underlying resources use [Kubernetes Finalizers](https://kubernetes.io/docs/concepts/overview/working-with-objects/finalizers/). CDK's helm construct currently ignores `--wait`, which means that any object finalization in those helm charts is not respected in my dependency ordering. ## Compatibility I could see some debate around whether or not this is a breaking change. I'm currently viewing it as a bug fix. I'm a bit cautious about how valuable it would be to gate this behavior compared to the additional complexity. ## Testing Tested on a local fork and it worked like a charm Closes #28831 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
I have a stack that installs and uninstalls helm charts, where the underlying resources use Kubernetes Finalizers. CDK's helm construct currently ignores
--wait, which means that any object finalization in those helm charts is not respected in my dependency ordering.Compatibility
I could see some debate around whether or not this is a breaking change. I'm currently viewing it as a bug fix. I'm a bit cautious about how valuable it would be to gate this behavior compared to the additional complexity.
Testing
Tested on a local fork and it worked like a charm
Closes #28831
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license