fix(eks): route standalone lifecycle commands#6201
Conversation
Readiness evidence for exact head
This remains a draft until current-head CI and one green CodeRabbit/Codex review clear the full readiness gate. |
✅MegaLinter analysis: Success✅ Linters with no issuesactionlint, bash-exec, git_diff, hadolint, jscpd, jsonlint, lychee, markdown-table-formatter, markdownlint, prettier, prettier, shellcheck, shfmt, stylelint, syft, trivy-sbom, trufflehog, v8r, v8r, yamllint Notices📣 MegaLinter 9.5.0 is out! Discover the new features and security recommendations in the release announcement. (Skip this info by defining See detailed reports in MegaLinter artifacts
|
Code Coverage OverviewLanguages: Go Go / code-coverage/goThe overall coverage in the branch remains at 67%, unchanged from the branch. Show a code coverage summary of the most impacted files.
Updated |
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: adff1ade9d
ℹ️ 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".
Route EKS delete, start, and stop through exact name-region ownership checks, preserve custom AWS credentials, and bind create, update, and TTL flows to canonical config provenance. Refs #6087
Exact-head readiness evidence for c29922c RED / GREEN
Programmatic verification
User evaluation Built the exact head and exercised the real CLI against an isolated fake-eksctl fixture with custom AWS credential aliases and stale canonical AWS variables. cluster start, cluster stop, and cluster delete --force all succeeded as a user. The observed child calls stayed bound to eval-eks in eu-north-1:
Follow-up boundaries
This remains draft until ordinary CI reaches a terminal state and one exact-head Codex review is green. |
Extract EKS metadata parsing and split invalid-file cases without changing lifecycle validation semantics. Refs #6087
Pushed a narrow CI-lint follow-up at |
Persist exact EKS nodegroup capacities before stop, restore and verify them on start, and fail closed on stale or incomplete lifecycle state. Refs #6087
Pushed reviewer-driven capacity preservation at Validation: affected normal/race suites green; vet and full build green; full changed-code lint 0 issues; two final exact-diff audits READY at staged hash |
Current-head user evaluation — 56001ec
Local validation on this head: affected normal tests, affected race tests, changed-line golangci-lint, build, and zero-clone jscpd all pass. The repository-wide process-helper package also passes alone; broad concurrent local sweeps can exhaust this host process budget, so the exact-head GitHub matrix remains the authoritative unconstrained run. |
@codex review for EKS lifecycle target binding, capacity preservation, and fail-closed ownership |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0eb6de39d6
ℹ️ 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 Head is now |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 56001ecc91
ℹ️ 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".
| cleanupErr := state.DeleteClusterState(name) | ||
| if cleanupErr != nil { | ||
| err = fmt.Errorf("clean up local EKS cluster state after deletion: %w", cleanupErr) |
There was a problem hiding this comment.
When EKS deletion succeeds but ~/.ksail/clusters/<name> cannot be removed (for example because of permissions or a read-only home directory), assigning the cleanup error to err sends the already-deleted cluster through the failure branch below. The job is then retained as a Failed ghost even though the cloud cluster is gone, contradicting the idempotent cleanup behavior described below; state cleanup should be reported as a warning while the completed deletion still clears the job.
Useful? React with 👍 / 👎.
| if nodegroup.DesiredCap == 0 && nodegroup.MinSize <= 0 { | ||
| return fmt.Errorf( | ||
| "cannot restore stopped EKS nodegroup %q without the pre-stop snapshot: %w", | ||
| nodegroup.Name, | ||
| errNodegroupStateMissing, | ||
| ) |
There was a problem hiding this comment.
For a cluster stopped by the previous KSail release, StopNodes set both desired and minimum capacity to zero but did not write the newly introduced snapshot. After upgrading, every such cluster enters this no-snapshot branch and cluster start now returns an error instead of using the former max(MinSize, 1) fallback, so users cannot restart any EKS nodegroup that was already stopped before this change without manually scaling it outside KSail.
Useful? React with 👍 / 👎.
| func eksNodegroupStatePath(clusterName string) (string, error) { | ||
| dir, err := clusterStateDir(clusterName) | ||
| if err != nil { | ||
| return "", err | ||
| } | ||
|
|
||
| return filepath.Join(dir, eksNodegroupStateFileName), nil |
There was a problem hiding this comment.
When a user manages same-named EKS clusters in multiple regions, the first stop writes its capacity snapshot to this name-only path. A stop or start targeting the second region then loads the first region's snapshot and fails the region identity check, and deleting either same-named cluster removes the shared state directory and can discard the other cluster's restore data. The snapshot path needs to include at least the resolved region in addition to the cluster name.
Useful? React with 👍 / 👎.
| err := resolveFromConfig(cmd, &resolved) | ||
| if err != nil { | ||
| return nil, err |
There was a problem hiding this comment.
When a malformed ksail.yaml or distribution config is present, returning this load error now also blocks every read-only caller of ResolveClusterInfo, including cluster info and cluster diagnose, even when the user supplied an explicit --name, --provider, and kubeconfig. The fail-closed rationale applies to destructive lifecycle operations, but placing it in this shared resolver regresses the documented ability to use read-only operations while local project configuration is broken; strict loading should be selected by mutating callers rather than imposed globally.
Useful? React with 👍 / 👎.
1 participant
Summary
Make the standalone
cluster delete,cluster start, andcluster stopcommands reach the existing EKS/AWS lifecycle implementation.Custom AWS credential mappings and region precedence now flow through the same isolated provider factory used by the rest of EKS. Explicit cluster names remain authoritative, so a local
eks.yamlcannot redirect a destructive delete to a different cluster.Destructive writes require matching local KSail evidence and an exact eksctl-created cluster result. Stop/start also preserve each managed nodegroup's exact desired, minimum, and maximum capacity across separate CLI processes, retaining recovery state until restoration is verified.
User impact
AWS-backed clusters can now be safely deleted and have their managed nodegroups stopped and restored from the documented standalone CLI surface. Ambiguous ownership, region, config, or lifecycle state fails before mutation.
Fixes #6087
Part of #4328