-
Notifications
You must be signed in to change notification settings - Fork 129
[CASCL-647] Add a subcommand to kubectl plugin to uninstall Karpenter #2424
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
base: main
Are you sure you want to change the base?
Conversation
be38b1c to
03da607
Compare
|
@codex review |
…c/CASCL-647_karpenter_uninstaller
03da607 to
f208c10
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if err = deleteKarpenterNodePools(ctx, clients); err != nil { | ||
| log.Printf("Warning: failed to delete NodePools: %v", err) | ||
| } | ||
|
|
||
| if err = deleteKarpenterEC2NodeClasses(ctx, clients); err != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fail uninstall when cleanup steps error
Errors from each cleanup step are only logged and ignored (deleteKarpenterNodePools/deleteKarpenterEC2NodeClasses at these lines), yet the command later prints a success banner and returns nil. If the user lacks RBAC or CRDs are missing, those deletions will fail but the CLI will still exit 0, leaving Karpenter resources and AWS infrastructure behind with no signal to automation that the uninstall was incomplete. This should surface failures rather than treating them as warnings.
Useful? React with 👍 / 👎.
What does this PR do?
Add a subcommand to the kubectl plugin to uninstall Karpenter on a cluster:
Motivation
#2301 added a subcommand to install Karpenter.
This PR introduce a subcommend to do the reverse operation.
Additional Notes
Minimum Agent Versions
Are there minimum versions of the Datadog Agent and/or Cluster Agent required?
No
Describe your test plan
kubectl datadog autoscaling cluster install;kubectl datadog autoscaling cluster uninstall.Checklist
bug,enhancement,refactoring,documentation,tooling, and/ordependenciesqa/skip-qalabel