Skip to content

fix: use interruptionQueue Helm value for Karpenter >= v0.33.0 - #8844

Open
warren830 wants to merge 1 commit into
eksctl-io:mainfrom
warren830:fix/issue-7697-karpenter-interruption-queue
Open

fix: use interruptionQueue Helm value for Karpenter >= v0.33.0#8844
warren830 wants to merge 1 commit into
eksctl-io:mainfrom
warren830:fix/issue-7697-karpenter-interruption-queue

Conversation

@warren830

Copy link
Copy Markdown

Description

The Karpenter chart renamed its interruption queue Helm value when it flattened settings, and eksctl still sends the old spelling to new charts.

Symptom. With withSpotInterruptionQueue: true and a Karpenter version >= 0.33.0, spot instances are terminated without Karpenter draining them first. INTERRUPTION_QUEUE is absent from the Karpenter pod's environment and nothing is logged — the misconfiguration is completely silent.

Root cause. pkg/karpenter/karpenter.go built a single settings map and then re-nested it under aws for older charts:

settings: map[string]interface{}{
    ...
    interruptionQueueName: k.ClusterConfig.Metadata.Name,   // "interruptionQueueName"
},
...
if err == nil && compareVersions < 0 {
    values[settings] = map[string]interface{}{aws: values[settings]}   // re-nest, same key
}

Because both branches shared that one map, both sent interruptionQueueName. That is correct only for the pre-flattening charts. The flattened chart reads settings.interruptionQueue, so the key eksctl sends is unknown to it and Helm drops it without complaint. The chart gates the env var on that exact value:

{{- with .Values.settings.interruptionQueue }}
  - name: INTERRUPTION_QUEUE
    value: "{{ . }}"
{{- end }}

charts/karpenter/templates/deployment.yaml, unchanged from v0.33.0 through v1.2.1

Chart key by version, read from charts/karpenter/values.yaml at upstream tags:

chart tag key in the chart
v0.31.0 settings.aws.interruptionQueueName
v0.32.0 / v0.32.9 settings.interruptionQueue (plus a deprecated empty aws: {})
v0.33.0 → v1.2.1 settings.interruptionQueue

The flattened key has never been spelled interruptionQueueName.

Change. Select the queue key per version branch instead of sharing one map. The < 0.33.0 path keeps settings.aws.interruptionQueueName byte-for-byte as before; only the >= 0.33.0 path changes, to settings.interruptionQueue. Deliberately minimal: no API, flag, CloudFormation or documentation change, and withSpotInterruptionQueue is untouched.

Verified behaviour across every supported version (min is v0.20.0 per supportedKarpenterVersion)
karpenter.version shape sent to Helm queue key
0.20.0, v0.20.0, 0.28.0, 0.31.0 settings.aws{…} interruptionQueueName — unchanged
0.32.0, 0.32.9 settings.aws{…} interruptionQueueName — unchanged (see note below)
0.33.0, v0.33.0 settings{…} interruptionQueue — fixed
0.34.0, 0.37.0, 1.0.0, 1.2.1, v1.2.1 settings{…} interruptionQueue — fixed
0.33.0-rc.1 settings.aws{…} interruptionQueueName — pre-release sorts below 0.33.0; pre-existing boundary semantics, unchanged
unparseable / empty settings{…} interruptionQueue — same partition as before (rejected earlier by validation anyway)

The v prefix is handled identically on both sides of the boundary. Hoisting the map into a local also removes the pre-existing aliasing where the same map object was reachable at both settings and settings.aws; each Install now builds a fresh map, verified to not carry state across calls.

Two adjacent questions left out of this PR on purpose — both pre-existing, neither introduced here, happy to file separate issues if you'd like them tracked:

  1. 0.32.x boundary. Upstream flattened settings at chart v0.32.0, but eksctl pivots at 0.33.0, so 0.32.x still receives the deprecated settings.aws.* shape. Independent of the key rename.
  2. Queue name is sent unconditionally. pkg/karpenter never consults WithSpotInterruptionQueue (zero references), so the cluster name is passed as the queue name even when the user set withSpotInterruptionQueue: false — while pkg/cfn/builder/karpenter.go only creates the SQS queue when it is enabled. On the legacy path this has always been delivered (v0.31.0 flattens settings into the karpenter-global-settings ConfigMap). On >= 0.33.0 the typo was masking it, so this fix makes that pre-existing behaviour effective there: a user on >= 0.33.0 with withSpotInterruptionQueue: false will now get INTERRUPTION_QUEUE set to a queue that was never created, and Karpenter will log SQS polling errors. That is the same behaviour the < 0.33.0 path already has, so this PR brings the two branches to parity rather than diverging them — but gating the value on WithSpotInterruptionQueue is a real behaviour change affecting both branches, so I have not made that call here. Flagging it explicitly as a maintainer decision.

Checklist

  • Added tests that cover your change (if possible)
  • Added/modified documentation as required (such as the README.md, or the userdocs directory)
  • Manually tested
  • Made sure the title of the PR is a good description that can go into the release notes
  • (Core team) Added labels for change area (e.g. area/nodegroup) and kind (e.g. kind/improvement)

Documentation: not applicable. The bug is entirely in the Helm value name eksctl sends internally; no user-facing field, flag, or default changes, so userdocs/src/usage/eksctl-karpenter.md stays accurate as written.

Manual testing: not done — I have no AWS account able to stand up an EKS cluster with spot capacity, so I could not observe a real interruption end to end. What I did verify, entirely offline with no AWS/EKS/cluster/Docker/credentials:

  • The existing >= 0.33.0 unit expectation encoded the wrong key. Correcting it to the key the chart reads fails against unmodified production and passes with this change — a genuine RED → GREEN, not a test written to fit the code:
    [FAILED] Install [It] installs karpenter with expanded settings.aws values for version greater or equal to v0.33.0
      Expected  "interruptionQueueName": "test-cluster"
      to equal  "interruptionQueue":     "test-cluster"
    
    Reverting only karpenter.go reproduces the failure (3 passed / 1 failed); restoring it returns ok.
  • go test -tags=release ./pkg/karpenter/ok.
  • ./pkg/actions/karpenter/, ./pkg/cfn/builder/, ./pkg/apis/eksctl.io/v1alpha5/, ./pkg/utils/ → all ok.
  • Broad sweep ./pkg/...85 packages ok, 2 failing: pkg/karpenter/providers/helm and pkg/iam/oidc. Both fail identically on unmodified main in this environment (they reach public.ecr.aws and need a Docker credential helper), so they are environmental and unrelated to this diff.
  • gofmt clean, go vet -tags=release ./pkg/karpenter/ clean, go build -tags=release ./... clean, integration tests compile (go test -tags integration -run=^$ ./integration/...).
  • go mod tidy produces no go.mod/go.sum drift; regenerating assets/schema.json produces a byte-identical file, so check-gomod and check-schema are unaffected.
  • Chart keys confirmed by reading charts/karpenter/values.yaml and templates/deployment.yaml at upstream tags v0.31.0, v0.32.0, v0.32.9, v0.33.0, v0.34.0, v0.37.0, v1.0.0 and v1.2.1 — not from memory.

Verified against main at 99984adb6164ece593c0728523b7f497ea61d60e.

BONUS POINTS checklist: complete for good vibes and maybe prizes?! 🤯

  • Backfilled missing tests for code in same general area 🎉
  • Refactored something and made the world a better place 🌟

The version branch now owns the key choice rather than mutating a shared map after the fact, which is what allowed one spelling to serve two incompatible chart layouts in the first place.

Fixes #7697

The Karpenter chart renamed its interruption queue Helm value when it
flattened `settings`. Charts before v0.33.0 read
`settings.aws.interruptionQueueName`; the flattened layout reads
`settings.interruptionQueue`.

eksctl built a single `settings` map and re-nested it under `aws` for older
charts, so both version branches shared the `interruptionQueueName`
spelling. On charts >= v0.33.0 Helm silently ignores that unknown key, so
`INTERRUPTION_QUEUE` is never set on the Karpenter pod and spot
interruption handling is disabled with no error surfaced to the user.
Instances are terminated without Karpenter draining them first.

Select the queue key per version branch instead of sharing one map: the
`< 0.33.0` path keeps `settings.aws.interruptionQueueName` unchanged, and
the `>= 0.33.0` path now sends `settings.interruptionQueue`. No API, flag
or documentation change; `withSpotInterruptionQueue` is untouched.

The existing `>= 0.33.0` unit expectation encoded the wrong key, so it is
corrected to the key the chart actually reads. The two
`settings.aws.interruptionQueueName` specs are left as-is and act as the
regression guard for the legacy contract.

Signed-off-by: warren <warren.chen830@gmail.com>
@github-actions

Copy link
Copy Markdown
Contributor

Hello warren830 👋 Thank you for opening a Pull Request in eksctl project. The team will review the Pull Request and aim to respond within 1-10 business days. Meanwhile, please read about the Contribution and Code of Conduct guidelines here. You can find out more information about eksctl on our website

@warren830
warren830 marked this pull request as ready for review August 31, 2026 02:09

@gustavodiaz7722 gustavodiaz7722 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note

AI-generated review. Produced by an AI agent (Kiro) at a maintainer's request. Findings were verified by execution against upstream charts, not by inspection alone.

Summary

The diagnosis is correct and well-evidenced, the fix is minimal, and the 0.33.0 pivot is exactly right. One change is required before merge: gate the queue name on withSpotInterruptionQueue.

Verification performed

Checks were run against 199c36195 (one commit on main at 99984adb6).

  • Chart keys read at upstream tags v0.31.0 → v1.2.1: settings.aws.interruptionQueueName before flattening, settings.interruptionQueue from v0.32.0 onward. Root cause confirmed.
  • The real published OCI charts were rendered with the values Install emits, which reproduces the bug and confirms the fix:
    • charts v0.33.0 and 1.2.1 + current mainINTERRUPTION_QUEUE absent
    • charts v0.33.0 and 1.2.1 + this PR → INTERRUPTION_QUEUE=<cluster>
    • chart v0.31.0 → no such env var exists; the queue arrives via the karpenter-global-settings ConfigMap, and the full 689-line rendered manifest is byte-identical between main and this PR. The legacy path is unchanged at the Kubernetes-object level, not merely at the values-map level.
  • Reverting only karpenter.go while keeping the new test reproduces the failure quoted in the description (3 passed / 1 failed), so the test is genuinely bug-first.
  • The version table was reproduced independently across 16 version strings, including the 0.33.0-rc.1 pre-release boundary and unparseable input. No map aliasing and no state carryover across repeated Install calls.
  • go build -tags=release ./..., gofmt, go vet clean; integration tests compile; no go.mod/go.sum drift; 85 packages pass and the 2 failures are environmental.

Required change: gate on withSpotInterruptionQueue

Adjacent question 2 should be resolved here rather than deferred, because the affected configuration is the default one:

  • WithSpotInterruptionQueue has no defaulting, IsEnabled(nil) is false, and the documented default is false.
  • pkg/cfn/builder/karpenter.go:197 creates the SQS queue and grants the controller role sqs:ReceiveMessage only when the flag is enabled.

The misspelled key was suppressing this second defect on >= 0.33.0. Correcting the key alone activates it: a default-configuration cluster on >= 0.33.0 begins pointing Karpenter at a queue that was never created and that it has no permission to poll, producing continuous SQS errors. Parity with the legacy branch is reached by propagating the defect rather than containing it.

Gating is purely additive. The name sent is already correct when the flag is enabled — CFN creates the queue as QueueName: <cluster name> — so no working value is ever suppressed.

Suggested fix

--- a/pkg/karpenter/karpenter.go
+++ b/pkg/karpenter/karpenter.go
@@ -87,12 +87,23 @@ func (k *Installer) Install(ctx context.Context, serviceAccountRoleARN string, i
 	// disables spot interruption handling without reporting an error.
 	version := k.ClusterConfig.Karpenter.Version
 	compareVersions, err := utils.CompareVersions(version, "0.33.0")
-	if err == nil && compareVersions < 0 {
-		settingsValues[interruptionQueueName] = k.ClusterConfig.Metadata.Name
+	legacyChart := err == nil && compareVersions < 0
+
+	// Only advertise the interruption queue when eksctl actually provisioned
+	// it. pkg/cfn/builder creates the SQS queue -- and grants the controller
+	// role sqs:ReceiveMessage on it -- only when withSpotInterruptionQueue is
+	// enabled, so sending the name unconditionally points Karpenter at a queue
+	// that does not exist and that it has no permission to poll.
+	queueEnabled := api.IsEnabled(k.ClusterConfig.Karpenter.WithSpotInterruptionQueue)
+
+	if legacyChart {
+		if queueEnabled {
+			settingsValues[interruptionQueueName] = k.ClusterConfig.Metadata.Name
+		}
 		settingsValues = map[string]interface{}{
 			aws: settingsValues,
 		}
-	} else {
+	} else if queueEnabled {
 		settingsValues[interruptionQueue] = k.ClusterConfig.Metadata.Name
 	}

Three existing specs assert the queue is delivered while the flag is unset, so the current suite encodes the ungated behaviour. Enabling the flag in BeforeEach preserves their intent, and two new specs cover the disabled case:

--- a/pkg/karpenter/karpenter_test.go
+++ b/pkg/karpenter/karpenter_test.go
@@ -30,6 +30,9 @@ var _ = Describe("Install", func() {
 				Version:                "0.15.3",
 				CreateServiceAccount:   api.Disabled(),
 				DefaultInstanceProfile: nil,
+				// The queue name is only sent when eksctl provisioned the
+				// queue, so the specs that assert on it enable it explicitly.
+				WithSpotInterruptionQueue: api.Enabled(),
 			}
@@ -101,6 +104,41 @@ var _ = Describe("Install", func() {
 			Expect(opts.Values[settings]).To(Equal(values[settings]))
 		})
 
+		When("withSpotInterruptionQueue is disabled", func() {
+
+			BeforeEach(func() {
+				cfg.Karpenter.WithSpotInterruptionQueue = api.Disabled()
+			})
+
+			// pkg/cfn/builder only creates the SQS queue, and only grants the
+			// controller role sqs:ReceiveMessage on it, when the queue is
+			// enabled. Advertising a queue name in either chart layout would
+			// point Karpenter at a queue that does not exist and that it
+			// cannot poll.
+			It("omits the queue name from the legacy settings.aws values", func() {
+				Expect(installerUnderTest.Install(context.Background(), "dummy", "dummy")).To(Succeed())
+				_, opts := fakeHelmInstaller.InstallChartArgsForCall(0)
+				Expect(opts.Values[settings]).To(Equal(map[string]interface{}{
+					aws: map[string]interface{}{
+						defaultInstanceProfile: "dummy",
+						clusterName:            cfg.Metadata.Name,
+						clusterEndpoint:        cfg.Status.Endpoint,
+					},
+				}))
+			})
+
+			It("omits the queue name from the flattened settings values", func() {
+				installerUnderTest.ClusterConfig.Karpenter.Version = "0.33.0"
+				Expect(installerUnderTest.Install(context.Background(), "dummy", "dummy")).To(Succeed())
+				_, opts := fakeHelmInstaller.InstallChartArgsForCall(0)
+				Expect(opts.Values[settings]).To(Equal(map[string]interface{}{
+					defaultInstanceProfile: "dummy",
+					clusterName:            cfg.Metadata.Name,
+					clusterEndpoint:        cfg.Status.Endpoint,
+				}))
+			})
+		})
+
 		When("install chart fails", func() {

The gated build was rendered against the real charts. All four combinations behave correctly:

chart withSpotInterruptionQueue rendered result
v0.31.0 true ConfigMap aws.interruptionQueueName: <cluster>
v0.31.0 false ConfigMap key empty — the chart's own default, handling off
1.2.1 true INTERRUPTION_QUEUE=<cluster>
1.2.1 false no queue advertised

6/6 specs pass; go build -tags=release ./..., gofmt and go vet clean.

Minor

  1. Adjacent question 1 can be withdrawn rather than filed. The 0.32.x charts read {{- with or .Values.settings.aws.interruptionQueueName .Values.settings.interruptionQueue }} — confirmed present in v0.32.0, v0.32.5, v0.32.9 and v0.32.10, and absent from v0.33.0. 0.32.x accepts both spellings and was never broken, so the 0.33.0 pivot is exact rather than approximate. The description currently undersells the fix.
  2. One test failure is misattributed. pkg/iam/oidc fails because cfssl/cfssljson are not installed, not for want of a Docker credential helper; only pkg/karpenter/providers/helm needs registry auth. The "environmental and unrelated" conclusion holds — neither package depends on pkg/karpenter.
  3. Optional: the flattened-path spec asserts only opts.Values[settings], while the legacy specs assert the whole values map, leaving top-level aws and serviceAccount unguarded on the >= 0.33.0 path.

Escalating the queue-gating question instead of silently resolving it was the right call, and moving the key choice into the version branch removes the shared-map mutation that allowed one spelling to serve two incompatible chart layouts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Karpenter setting interruptionQueueName renamed to interruptionQueue

2 participants