Skip to content

Commit 6154cdc

Browse files
authored
fix: use interruptionQueue Helm value for Karpenter >= v0.33.0 (#8844)
* fix: use interruptionQueue Helm value for Karpenter >= v0.33.0 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> * fix: only advertise the interruption queue when eksctl created it Correcting the misspelled Helm key activated a second, previously masked defect. pkg/cfn/builder creates the SQS queue -- and grants the controller role sqs:ReceiveMessage on it -- only when withSpotInterruptionQueue is enabled, and that flag has no defaulting, so it is off by default. Sending the queue name unconditionally therefore pointed a default-configuration cluster on >= 0.33.0 at a queue that was never created and that it has no permission to poll. The name is now sent only when the queue was provisioned; when the flag is enabled the value is unchanged, so no working setup is affected. Adds specs for the disabled case on both chart layouts, and asserts the whole values map on the flattened path as the legacy specs already do. --------- Signed-off-by: warren <warren.chen830@gmail.com> Co-authored-by: warren830 <warren830@users.noreply.github.com>
1 parent a4732a5 commit 6154cdc

2 files changed

Lines changed: 85 additions & 15 deletions

File tree

pkg/karpenter/karpenter.go

Lines changed: 36 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ const (
3030
serviceAccountName = "name"
3131
settings = "settings"
3232
interruptionQueueName = "interruptionQueueName"
33+
interruptionQueue = "interruptionQueue"
3334
)
3435

3536
// Options contains values which Karpenter uses to configure the installation.
@@ -72,29 +73,50 @@ func (k *Installer) Install(ctx context.Context, serviceAccountRoleARN string, i
7273
serviceAccountName: DefaultServiceAccountName,
7374
}
7475

76+
settingsValues := map[string]interface{}{
77+
defaultInstanceProfile: instanceProfileName,
78+
clusterName: k.ClusterConfig.Metadata.Name,
79+
clusterEndpoint: k.ClusterConfig.Status.Endpoint,
80+
}
81+
82+
// The Karpenter chart renamed the interruption queue Helm value when it
83+
// flattened `settings`: charts before v0.33.0 read
84+
// `settings.aws.interruptionQueueName`, while the flattened layout reads
85+
// `settings.interruptionQueue`. A flattened chart silently ignores the old
86+
// spelling, which leaves INTERRUPTION_QUEUE unset on the Karpenter pod and
87+
// disables spot interruption handling without reporting an error.
88+
version := k.ClusterConfig.Karpenter.Version
89+
compareVersions, err := utils.CompareVersions(version, "0.33.0")
90+
legacyChart := err == nil && compareVersions < 0
91+
92+
// Only advertise the interruption queue when eksctl actually provisioned it.
93+
// pkg/cfn/builder creates the SQS queue -- and grants the controller role
94+
// sqs:ReceiveMessage on it -- only when withSpotInterruptionQueue is enabled,
95+
// so sending the name unconditionally would point Karpenter at a queue that
96+
// does not exist and that it has no permission to poll.
97+
queueEnabled := api.IsEnabled(k.ClusterConfig.Karpenter.WithSpotInterruptionQueue)
98+
99+
if legacyChart {
100+
if queueEnabled {
101+
settingsValues[interruptionQueueName] = k.ClusterConfig.Metadata.Name
102+
}
103+
settingsValues = map[string]interface{}{
104+
aws: settingsValues,
105+
}
106+
} else if queueEnabled {
107+
settingsValues[interruptionQueue] = k.ClusterConfig.Metadata.Name
108+
}
109+
75110
values := map[string]interface{}{
76111
clusterName: k.ClusterConfig.Metadata.Name,
77112
clusterEndpoint: k.ClusterConfig.Status.Endpoint,
78113
aws: map[string]interface{}{
79114
defaultInstanceProfile: instanceProfileName,
80115
},
81-
settings: map[string]interface{}{
82-
defaultInstanceProfile: instanceProfileName,
83-
clusterName: k.ClusterConfig.Metadata.Name,
84-
clusterEndpoint: k.ClusterConfig.Status.Endpoint,
85-
interruptionQueueName: k.ClusterConfig.Metadata.Name,
86-
},
116+
settings: settingsValues,
87117
serviceAccount: serviceAccountMap,
88118
}
89119

90-
version := k.ClusterConfig.Karpenter.Version
91-
compareVersions, err := utils.CompareVersions(version, "0.33.0")
92-
if err == nil && compareVersions < 0 {
93-
values[settings] = map[string]interface{}{
94-
aws: values[settings],
95-
}
96-
}
97-
98120
registryClient, err := registry.NewClient(
99121
registry.ClientOptEnableCache(true),
100122
)

pkg/karpenter/karpenter_test.go

Lines changed: 49 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,9 @@ var _ = Describe("Install", func() {
3030
Version: "0.15.3",
3131
CreateServiceAccount: api.Disabled(),
3232
DefaultInstanceProfile: nil,
33+
// The queue name is only sent when eksctl provisioned the queue,
34+
// so the specs that assert on it enable it explicitly.
35+
WithSpotInterruptionQueue: api.Enabled(),
3336
}
3437
cfg.Status = &api.ClusterStatus{
3538
Endpoint: "https://endpoint.com",
@@ -90,10 +93,55 @@ var _ = Describe("Install", func() {
9093
defaultInstanceProfile: "dummy",
9194
clusterName: cfg.Metadata.Name,
9295
clusterEndpoint: cfg.Status.Endpoint,
93-
interruptionQueueName: cfg.Metadata.Name,
96+
// The flattened Karpenter chart names this value
97+
// "interruptionQueue", not "interruptionQueueName" --
98+
// see charts/karpenter/values.yaml from v0.32.0 onwards.
99+
// Asserted as a literal rather than via a constant so the
100+
// test pins the key the chart actually reads.
101+
"interruptionQueue": cfg.Metadata.Name,
94102
},
95103
}
96104
Expect(opts.Values[settings]).To(Equal(values[settings]))
105+
// The legacy specs assert the whole values map; do the same here so
106+
// the top-level keys are guarded on the flattened path too.
107+
Expect(opts.Values[aws]).To(Equal(map[string]interface{}{defaultInstanceProfile: "dummy"}))
108+
Expect(opts.Values[clusterName]).To(Equal(cfg.Metadata.Name))
109+
Expect(opts.Values[clusterEndpoint]).To(Equal(cfg.Status.Endpoint))
110+
})
111+
112+
When("withSpotInterruptionQueue is disabled", func() {
113+
114+
BeforeEach(func() {
115+
cfg.Karpenter.WithSpotInterruptionQueue = api.Disabled()
116+
})
117+
118+
// pkg/cfn/builder only creates the SQS queue, and only grants the
119+
// controller role sqs:ReceiveMessage on it, when the queue is
120+
// enabled. Advertising a queue name in either chart layout would
121+
// point Karpenter at a queue that does not exist and that it
122+
// cannot poll.
123+
It("omits the queue name from the legacy settings.aws values", func() {
124+
Expect(installerUnderTest.Install(context.Background(), "dummy", "dummy")).To(Succeed())
125+
_, opts := fakeHelmInstaller.InstallChartArgsForCall(0)
126+
Expect(opts.Values[settings]).To(Equal(map[string]interface{}{
127+
aws: map[string]interface{}{
128+
defaultInstanceProfile: "dummy",
129+
clusterName: cfg.Metadata.Name,
130+
clusterEndpoint: cfg.Status.Endpoint,
131+
},
132+
}))
133+
})
134+
135+
It("omits the queue name from the flattened settings values", func() {
136+
installerUnderTest.ClusterConfig.Karpenter.Version = "0.33.0"
137+
Expect(installerUnderTest.Install(context.Background(), "dummy", "dummy")).To(Succeed())
138+
_, opts := fakeHelmInstaller.InstallChartArgsForCall(0)
139+
Expect(opts.Values[settings]).To(Equal(map[string]interface{}{
140+
defaultInstanceProfile: "dummy",
141+
clusterName: cfg.Metadata.Name,
142+
clusterEndpoint: cfg.Status.Endpoint,
143+
}))
144+
})
97145
})
98146

99147
When("install chart fails", func() {

0 commit comments

Comments
 (0)