Avoid panics when cleaning up a deleted EKS cluster - #8873
Open
nbaliyan260 wants to merge 1 commit into
Open
nbaliyan260 wants to merge 1 commit into
nbaliyan260 wants to merge 1 commit into
Conversation
ClusterInfo is unavailable once an EKS control plane has been removed. Guard cluster-state access in the action constructor and stack cleanup, and skip Auto Mode role cleanup when that state is unavailable. Add regression coverage for missing clusters and preserve existing owned/unowned cluster selection. Fixes eksctl-io#7450
Contributor
|
Hello nbaliyan260 👋 Thank you for opening a Pull Request in |
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The nil-state paths are guarded and covered by focused regression tests without introducing unresolved issues.
Review effort: Lite
Findings: None
What changed in this PR
Fixes nil-pointer panics during cleanup when the EKS control plane is deleted but CloudFormation resources remain.
Changes:
- Safely handle missing cluster state in constructors and cleanup.
- Skip Auto Mode role cleanup when cluster state is unavailable.
- Add regression coverage for deleted, owned, unowned, and existing clusters.
| File | Description |
|---|---|
pkg/automode/role_deleter.go |
Guards absent cluster state. |
pkg/automode/role_deleter_test.go |
Tests missing cluster behavior. |
pkg/actions/cluster/owned.go |
Passes nil cluster state safely to cleanup tasks. |
pkg/actions/cluster/owned_test.go |
Verifies cleanup continues without Kubernetes access. |
pkg/actions/cluster/cluster.go |
Safely initializes Auto Mode cleanup. |
pkg/actions/cluster/cluster_test.go |
Covers constructor outcomes. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fixes #7450.
When an EKS control plane has already been deleted but its CloudFormation stack remains, cluster status has no
ClusterInfo. Cleanup currently panics while constructing the cluster action or passing cluster state to the stack deletion tasks. Auto Mode role cleanup also assumes that cluster state is available.Guard those three accesses so cleanup can continue through the existing inoperable-cluster path. Preserve the missing cluster state, allowing Kubernetes operations to remain skipped, and retain the normal error when neither the EKS cluster nor its stack exists.
Regression tests cover both missing-cluster constructor outcomes, continued execution of stack cleanup without Kubernetes access, and absent cluster state in Auto Mode role cleanup. The constructor tests also cover existing owned and unowned clusters.
Validation
f131ac5(using a Go overlay).CGO_ENABLED=0 go test ./pkg/actions/cluster ./pkg/automodepasses with the fix.make unit-test-no-generatepasses: 87 test packages, using Go 1.27.1. OIDC certificate fixtures were generated first withmake -C pkg/iam/oidc/testdata allto avoid its existing three-second setup timeout under local host load.CGO_ENABLED=0 go build ./cmd/eksctland the resulting binary'sversioncommand pass.CGO_ENABLED=0 go test -tags integration -run='^$' ./integration/...passes (compile-only).GOTOOLCHAIN=go1.26.5 golangci-lint run --concurrency=2 --timeout=30mpasses with the repository's pinned linter: zero issues.No live AWS integration tests were run.
Checklist
BONUS POINTS checklist