diff --git a/pkg/karpenter/karpenter.go b/pkg/karpenter/karpenter.go index fd8293664b..13ed4bae5a 100644 --- a/pkg/karpenter/karpenter.go +++ b/pkg/karpenter/karpenter.go @@ -30,6 +30,7 @@ const ( serviceAccountName = "name" settings = "settings" interruptionQueueName = "interruptionQueueName" + interruptionQueue = "interruptionQueue" ) // Options contains values which Karpenter uses to configure the installation. @@ -72,29 +73,50 @@ func (k *Installer) Install(ctx context.Context, serviceAccountRoleARN string, i serviceAccountName: DefaultServiceAccountName, } + settingsValues := map[string]interface{}{ + defaultInstanceProfile: instanceProfileName, + clusterName: k.ClusterConfig.Metadata.Name, + clusterEndpoint: k.ClusterConfig.Status.Endpoint, + } + + // The Karpenter chart renamed the interruption queue Helm value when it + // flattened `settings`: charts before v0.33.0 read + // `settings.aws.interruptionQueueName`, while the flattened layout reads + // `settings.interruptionQueue`. A flattened chart silently ignores the old + // spelling, which leaves INTERRUPTION_QUEUE unset on the Karpenter pod and + // disables spot interruption handling without reporting an error. + version := k.ClusterConfig.Karpenter.Version + compareVersions, err := utils.CompareVersions(version, "0.33.0") + 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 would point 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 if queueEnabled { + settingsValues[interruptionQueue] = k.ClusterConfig.Metadata.Name + } + values := map[string]interface{}{ clusterName: k.ClusterConfig.Metadata.Name, clusterEndpoint: k.ClusterConfig.Status.Endpoint, aws: map[string]interface{}{ defaultInstanceProfile: instanceProfileName, }, - settings: map[string]interface{}{ - defaultInstanceProfile: instanceProfileName, - clusterName: k.ClusterConfig.Metadata.Name, - clusterEndpoint: k.ClusterConfig.Status.Endpoint, - interruptionQueueName: k.ClusterConfig.Metadata.Name, - }, + settings: settingsValues, serviceAccount: serviceAccountMap, } - version := k.ClusterConfig.Karpenter.Version - compareVersions, err := utils.CompareVersions(version, "0.33.0") - if err == nil && compareVersions < 0 { - values[settings] = map[string]interface{}{ - aws: values[settings], - } - } - registryClient, err := registry.NewClient( registry.ClientOptEnableCache(true), ) diff --git a/pkg/karpenter/karpenter_test.go b/pkg/karpenter/karpenter_test.go index 34fd944687..2dda144885 100644 --- 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(), } cfg.Status = &api.ClusterStatus{ Endpoint: "https://endpoint.com", @@ -90,10 +93,55 @@ var _ = Describe("Install", func() { defaultInstanceProfile: "dummy", clusterName: cfg.Metadata.Name, clusterEndpoint: cfg.Status.Endpoint, - interruptionQueueName: cfg.Metadata.Name, + // The flattened Karpenter chart names this value + // "interruptionQueue", not "interruptionQueueName" -- + // see charts/karpenter/values.yaml from v0.32.0 onwards. + // Asserted as a literal rather than via a constant so the + // test pins the key the chart actually reads. + "interruptionQueue": cfg.Metadata.Name, }, } Expect(opts.Values[settings]).To(Equal(values[settings])) + // The legacy specs assert the whole values map; do the same here so + // the top-level keys are guarded on the flattened path too. + Expect(opts.Values[aws]).To(Equal(map[string]interface{}{defaultInstanceProfile: "dummy"})) + Expect(opts.Values[clusterName]).To(Equal(cfg.Metadata.Name)) + Expect(opts.Values[clusterEndpoint]).To(Equal(cfg.Status.Endpoint)) + }) + + 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() {