Merge branch 'aws:release-v0.32.x' into release-v0.32.x

cmd/controller/main.go

@@ -47,7 +47,6 @@ func main() {
 		WithControllers(ctx, corecontrollers.NewControllers(
 			op.Clock,
 			op.GetClient(),
-			op.KubernetesInterface,
 			state.NewCluster(op.Clock, op.GetClient(), cloudProvider),
 			op.EventRecorder,
 			cloudProvider,

go.mod

@@ -7,7 +7,7 @@ require (
 	github.com/PuerkitoBio/goquery v1.8.1
 	github.com/avast/retry-go v3.0.0+incompatible
 	github.com/aws/aws-sdk-go v1.48.0
-	github.com/aws/karpenter-core v0.32.5
+	github.com/aws/karpenter-core v0.32.9
 	github.com/aws/karpenter/tools/kompat v0.0.0-20231010173459-62c25a3ea85c
 	github.com/go-logr/zapr v1.3.0
 	github.com/imdario/mergo v0.3.16
@@ -42,7 +42,6 @@ require (
 	github.com/census-instrumentation/opencensus-proto v0.4.1 // indirect
 	github.com/cespare/xxhash/v2 v2.2.0 // indirect
 	github.com/davecgh/go-spew v1.1.1 // indirect
-	github.com/deckarep/golang-set v1.8.0 // indirect
 	github.com/emicklei/go-restful/v3 v3.11.0 // indirect
 	github.com/evanphx/json-patch v5.7.0+incompatible // indirect
 	github.com/evanphx/json-patch/v5 v5.7.0 // indirect

go.sum

@@ -57,8 +57,8 @@ github.com/avast/retry-go v3.0.0+incompatible h1:4SOWQ7Qs+oroOTQOYnAHqelpCO0biHS
 github.com/avast/retry-go v3.0.0+incompatible/go.mod h1:XtSnn+n/sHqQIpZ10K1qAevBhOOCWBLXXy3hyiqqBrY=
 github.com/aws/aws-sdk-go v1.48.0 h1:1SeJ8agckRDQvnSCt1dGZYAwUaoD2Ixj6IaXB4LCv8Q=
 github.com/aws/aws-sdk-go v1.48.0/go.mod h1:LF8svs817+Nz+DmiMQKTO3ubZ/6IaTpq3TjupRn3Eqk=
-github.com/aws/karpenter-core v0.32.5 h1:8XEMiPX80TwtMJpGVMXkxkZ7YW4CzgD6X1OGfSft2Es=
-github.com/aws/karpenter-core v0.32.5/go.mod h1:RNih2g6qCiah8rFaZ7HkmClIK66Hjj38z3DbWnWGM2w=
+github.com/aws/karpenter-core v0.32.9 h1:KICh1aJfzhTbri3b0kH/KdVuupcdeKTDcSebDbnPv40=
+github.com/aws/karpenter-core v0.32.9/go.mod h1:DY6qKd/bLlTrWFDtEdYQtapSUtCArP72DpkseN5EB8U=
 github.com/aws/karpenter/tools/kompat v0.0.0-20231010173459-62c25a3ea85c h1:oXWwIttmjYLbBKhLazG21aQvpJ3NOOr8IXhCJ/p6e/M=
 github.com/aws/karpenter/tools/kompat v0.0.0-20231010173459-62c25a3ea85c/go.mod h1:l/TIBsaCx/IrOr0Xvlj/cHLOf05QzuQKEZ1hx2XWmfU=
 github.com/beorn7/perks v0.0.0-20180321164747-3a771d992973/go.mod h1:Dwedo/Wpr24TaqPxmxbtue+5NUziq4I4S80YR8gNf3Q=
@@ -86,8 +86,6 @@ github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ3
 github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
 github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
-github.com/deckarep/golang-set v1.8.0 h1:sk9/l/KqpunDwP7pSjUg0keiOOLEnOBHzykLrsPppp4=
-github.com/deckarep/golang-set v1.8.0/go.mod h1:5nI87KwE7wgsBU1F4GKAw2Qod7p5kyS383rP6+o6qqo=
 github.com/emicklei/go-restful/v3 v3.11.0 h1:rAQeMHw1c7zTmncogyy8VvRZwtkmkZ4FxERmMY4rD+g=
 github.com/emicklei/go-restful/v3 v3.11.0/go.mod h1:6n3XBCmQQb25CM2LCACGz8ukIrRry+4bhvbpWn3mrbc=
 github.com/envoyproxy/go-control-plane v0.9.0/go.mod h1:YTl/9mNaCwkRvm6d1a2C3ymFceY/DCBVvsKhRF0iEA4=

hack/toolchain.sh

@@ -16,7 +16,7 @@ tools() {
     go install github.com/mikefarah/yq/v4@latest
     go install github.com/norwoodj/helm-docs/cmd/helm-docs@latest
     go install sigs.k8s.io/controller-runtime/tools/setup-envtest@latest
-    go install sigs.k8s.io/controller-tools/cmd/controller-gen@latest
+    go install sigs.k8s.io/controller-tools/cmd/controller-gen@v0.13.0
     go install github.com/sigstore/cosign/cmd/cosign@latest
     go install -tags extended github.com/gohugoio/[email protected]
     go install golang.org/x/vuln/cmd/govulncheck@latest

pkg/apis/v1beta1/ec2nodeclass.go

@@ -336,6 +336,12 @@ type EC2NodeClass struct {
 	IsNodeTemplate bool `json:"-" hash:"ignore"`
 }
 
+// We need to bump the EC2NodeClassHashVersion when we make an update to the EC2NodeClass CRD under these conditions:
+// 1. A field changes its default value for an existing field that is already hashed
+// 2. A field is added to the hash calculation with an already-set value
+// 3. A field is removed from the hash calculations
+const EC2NodeClassHashVersion = "v1"
+
 func (in *EC2NodeClass) Hash() string {
 	return fmt.Sprint(lo.Must(hashstructure.Hash(in.Spec, hashstructure.FormatV2, &hashstructure.HashOptions{
 		SlicesAsSets:    true,

pkg/apis/v1beta1/labels.go

@@ -111,6 +111,7 @@ var (
 	LabelInstanceAcceleratorName              = Group + "/instance-accelerator-name"
 	LabelInstanceAcceleratorManufacturer      = Group + "/instance-accelerator-manufacturer"
 	LabelInstanceAcceleratorCount             = Group + "/instance-accelerator-count"
-	AnnotationNodeClassHash                   = Group + "/ec2nodeclass-hash"
+	AnnotationEC2NodeClassHash                = Group + "/ec2nodeclass-hash"
+	AnnotationEC2NodeClassHashVersion         = Group + "/ec2nodeclass-hash-version"
 	AnnotationInstanceTagged                  = Group + "/tagged"
 )

pkg/cloudprovider/drift.go

@@ -135,13 +135,26 @@ func (c *CloudProvider) areStaticFieldsDrifted(nodeClaim *corev1beta1.NodeClaim,
 	if nodeClaim.IsMachine {
 		ownerHashKey = v1alpha1.AnnotationNodeTemplateHash
 	} else {
-		ownerHashKey = v1beta1.AnnotationNodeClassHash
+		ownerHashKey = v1beta1.AnnotationEC2NodeClassHash
 	}
 	nodeClassHash, foundHashNodeClass := nodeClass.Annotations[ownerHashKey]
 	nodeClaimHash, foundHashNodeClaim := nodeClaim.Annotations[ownerHashKey]
 	if !foundHashNodeClass || !foundHashNodeClaim {
 		return ""
 	}
+
+	if !nodeClass.IsNodeTemplate {
+		nodeClassHashVersion, foundNodeClassHashVersion := nodeClass.Annotations[v1beta1.AnnotationEC2NodeClassHashVersion]
+		nodeClaimHashVersion, foundNodeClaimHashVersion := nodeClaim.Annotations[v1beta1.AnnotationEC2NodeClassHashVersion]
+		if !foundNodeClassHashVersion || !foundNodeClaimHashVersion {
+			return ""
+		}
+		// validate that the hash version for the EC2NodeClass is the same as the NodeClaim before evaluating for static drift
+		if nodeClassHashVersion != nodeClaimHashVersion {
+			return ""
+		}
+	}
+
 	if nodeClassHash != nodeClaimHash {
 		return lo.Ternary(nodeClaim.IsMachine, NodeTemplateDrift, NodeClassDrift)
 	}

pkg/cloudprovider/nodeclaim_test.go

@@ -99,7 +99,7 @@ var _ = Describe("NodeClaim/CloudProvider", func() {
 		cloudProviderNodeClaim, err := cloudProvider.Create(ctx, nodeClaim)
 		Expect(err).To(BeNil())
 		Expect(cloudProviderNodeClaim).ToNot(BeNil())
-		_, ok := cloudProviderNodeClaim.ObjectMeta.Annotations[v1beta1.AnnotationNodeClassHash]
+		_, ok := cloudProviderNodeClaim.ObjectMeta.Annotations[v1beta1.AnnotationEC2NodeClassHash]
 		Expect(ok).To(BeTrue())
 	})
 	Context("EC2 Context", func() {
@@ -194,10 +194,14 @@ var _ = Describe("NodeClaim/CloudProvider", func() {
 				Reservations: []*ec2.Reservation{{Instances: []*ec2.Instance{instance}}},
 			})
 			nodeClass.Annotations = lo.Assign(nodeClass.Annotations, map[string]string{
-				v1beta1.AnnotationNodeClassHash: nodeClass.Hash(),
+				v1beta1.AnnotationEC2NodeClassHash:        nodeClass.Hash(),
+				v1beta1.AnnotationEC2NodeClassHashVersion: v1beta1.EC2NodeClassHashVersion,
 			})
 			nodeClaim.Status.ProviderID = fake.ProviderID(lo.FromPtr(instance.InstanceId))
-			nodeClaim.Annotations = lo.Assign(nodeClaim.Annotations, map[string]string{v1beta1.AnnotationNodeClassHash: nodeClass.Hash()})
+			nodeClaim.Annotations = lo.Assign(nodeClaim.Annotations, map[string]string{
+				v1beta1.AnnotationEC2NodeClassHash:        nodeClass.Hash(),
+				v1beta1.AnnotationEC2NodeClassHashVersion: v1beta1.EC2NodeClassHashVersion,
+			})
 			nodeClaim.Labels = lo.Assign(nodeClaim.Labels, map[string]string{v1.LabelInstanceTypeStable: selectedInstanceType.Name})
 		})
 		It("should not fail if NodeClass does not exist", func() {
@@ -226,7 +230,7 @@ var _ = Describe("NodeClaim/CloudProvider", func() {
 			instance.SecurityGroups = []*ec2.GroupIdentifier{{GroupId: aws.String(fake.SecurityGroupID())}}
 			// Assign a fake hash
 			nodeClass.Annotations = lo.Assign(nodeClass.Annotations, map[string]string{
-				v1beta1.AnnotationNodeClassHash: "abcdefghijkl",
+				v1beta1.AnnotationEC2NodeClassHash: "abcdefghijkl",
 			})
 			ExpectApplied(ctx, env.Client, nodeClass)
 			isDrifted, err := cloudProvider.IsDrifted(ctx, nodeClaim)
@@ -327,7 +331,7 @@ var _ = Describe("NodeClaim/CloudProvider", func() {
 					Expect(isDrifted).To(BeEmpty())
 
 					Expect(mergo.Merge(nodeClass, changes, mergo.WithOverride))
-					nodeClass.Annotations = lo.Assign(nodeClass.Annotations, map[string]string{v1beta1.AnnotationNodeClassHash: nodeClass.Hash()})
+					nodeClass.Annotations = lo.Assign(nodeClass.Annotations, map[string]string{v1beta1.AnnotationEC2NodeClassHash: nodeClass.Hash()})
 
 					ExpectApplied(ctx, env.Client, nodeClass)
 					isDrifted, err = cloudProvider.IsDrifted(ctx, nodeClaim)
@@ -350,7 +354,7 @@ var _ = Describe("NodeClaim/CloudProvider", func() {
 					Expect(isDrifted).To(BeEmpty())
 
 					Expect(mergo.Merge(nodeClass, changes, mergo.WithOverride))
-					nodeClass.Annotations = lo.Assign(nodeClass.Annotations, map[string]string{v1beta1.AnnotationNodeClassHash: nodeClass.Hash()})
+					nodeClass.Annotations = lo.Assign(nodeClass.Annotations, map[string]string{v1beta1.AnnotationEC2NodeClassHash: nodeClass.Hash()})
 
 					ExpectApplied(ctx, env.Client, nodeClass)
 					isDrifted, err = cloudProvider.IsDrifted(ctx, nodeClaim)
@@ -371,6 +375,54 @@ var _ = Describe("NodeClaim/CloudProvider", func() {
 				Expect(err).NotTo(HaveOccurred())
 				Expect(isDrifted).To(BeEmpty())
 			})
+			It("should not return drifted if the NodeClaim's karpenter.k8s.aws/ec2nodeclass-hash-version annotation does not match the EC2NodeClass's", func() {
+				nodeClass.ObjectMeta.Annotations = map[string]string{
+					v1beta1.AnnotationEC2NodeClassHash:        "test-hash-111111",
+					v1beta1.AnnotationEC2NodeClassHashVersion: "test-hash-version-1",
+				}
+				nodeClaim.ObjectMeta.Annotations = map[string]string{
+					v1beta1.AnnotationEC2NodeClassHash:        "test-hash-222222",
+					v1beta1.AnnotationEC2NodeClassHashVersion: "test-hash-version-2",
+				}
+				ExpectApplied(ctx, env.Client, nodePool, nodeClass)
+				isDrifted, err := cloudProvider.IsDrifted(ctx, nodeClaim)
+				Expect(err).NotTo(HaveOccurred())
+				Expect(isDrifted).To(BeEmpty())
+			})
+			It("should not return drifted if karpenter.k8s.aws/ec2nodeclass-hash-version annotation is not present on the NodeClass", func() {
+				nodeClass.ObjectMeta.Annotations = map[string]string{
+					v1beta1.AnnotationEC2NodeClassHash: "test-hash-111111",
+				}
+				nodeClaim.ObjectMeta.Annotations = map[string]string{
+					v1beta1.AnnotationEC2NodeClassHash:        "test-hash-222222",
+					v1beta1.AnnotationEC2NodeClassHashVersion: "test-hash-version-2",
+				}
+				// should trigger drift
+				nodeClass.Spec.Tags = map[string]string{
+					"Test Key": "Test Value",
+				}
+				ExpectApplied(ctx, env.Client, nodePool, nodeClass)
+				isDrifted, err := cloudProvider.IsDrifted(ctx, nodeClaim)
+				Expect(err).NotTo(HaveOccurred())
+				Expect(isDrifted).To(BeEmpty())
+			})
+			It("should not return drifted if karpenter.k8s.aws/ec2nodeclass-hash-version annotation is not present on the NodeClaim", func() {
+				nodeClass.ObjectMeta.Annotations = map[string]string{
+					v1beta1.AnnotationEC2NodeClassHash:        "test-hash-111111",
+					v1beta1.AnnotationEC2NodeClassHashVersion: "test-hash-version-1",
+				}
+				nodeClaim.ObjectMeta.Annotations = map[string]string{
+					v1beta1.AnnotationEC2NodeClassHash: "test-hash-222222",
+				}
+				// should trigger drift
+				nodeClass.Spec.Tags = map[string]string{
+					"Test Key": "Test Value",
+				}
+				ExpectApplied(ctx, env.Client, nodePool, nodeClass)
+				isDrifted, err := cloudProvider.IsDrifted(ctx, nodeClaim)
+				Expect(err).NotTo(HaveOccurred())
+				Expect(isDrifted).To(BeEmpty())
+			})
 		})
 	})
 	Context("Subnet Compatibility", func() {

pkg/cloudprovider/suite_test.go

@@ -72,7 +72,7 @@ var _ = BeforeSuite(func() {
 	cloudProvider = cloudprovider.New(awsEnv.InstanceTypesProvider, awsEnv.InstanceProvider, recorder,
 		env.Client, awsEnv.AMIProvider, awsEnv.SecurityGroupProvider, awsEnv.SubnetProvider)
 	cluster = state.NewCluster(fakeClock, env.Client, cloudProvider)
-	prov = provisioning.NewProvisioner(env.Client, env.KubernetesInterface.CoreV1(), recorder, cloudProvider, cluster)
+	prov = provisioning.NewProvisioner(env.Client, recorder, cloudProvider, cluster)
 })
 
 var _ = AfterSuite(func() {

pkg/controllers/nodeclass/controller.go

@@ -76,7 +76,13 @@ func (c *Controller) Reconcile(ctx context.Context, nodeClass *v1beta1.EC2NodeCl
 	if !nodeClass.IsNodeTemplate {
 		controllerutil.AddFinalizer(nodeClass, v1beta1.TerminationFinalizer)
 	}
+	if nodeClass.Annotations[v1beta1.AnnotationEC2NodeClassHashVersion] != v1beta1.EC2NodeClassHashVersion {
+		if err := c.updateNodeClaimHash(ctx, nodeClass); err != nil {
+			return reconcile.Result{}, err
+		}
+	}
 	nodeClass.Annotations = lo.Assign(nodeClass.Annotations, nodeclassutil.HashAnnotation(nodeClass))
+
 	err := multierr.Combine(
 		c.resolveSubnets(ctx, nodeClass),
 		c.resolveSecurityGroups(ctx, nodeClass),
@@ -216,6 +222,49 @@ func (c *Controller) resolveInstanceProfile(ctx context.Context, nodeClass *v1be
 	return nil
 }
 
+// Updating `ec2nodeclass-hash-version` annotation inside the karpenter controller means a breaking change has been made to the hash calculation.
+// `ec2nodeclass-hash` annotation on the EC2NodeClass will be updated, due to the breaking change, making the `ec2nodeclass-hash` on the NodeClaim different from
+// EC2NodeClass. Since, we cannot rely on the `ec2nodeclass-hash` on the NodeClaims, due to the breaking change, we will need to re-calculate the hash and update the annotation.
+// For more information on the Drift Hash Versioning: https://github.com/kubernetes-sigs/karpenter/blob/main/designs/drift-hash-versioning.md
+func (c *Controller) updateNodeClaimHash(ctx context.Context, nodeClass *v1beta1.EC2NodeClass) error {
+	if nodeClass.IsNodeTemplate {
+		return nil
+	}
+
+	ncList := &corev1beta1.NodeClaimList{}
+	if err := c.kubeClient.List(ctx, ncList, client.MatchingFields{"spec.nodeClassRef.name": nodeClass.Name}); err != nil {
+		return err
+	}
+
+	errs := make([]error, len(ncList.Items))
+	for i := range ncList.Items {
+		nc := ncList.Items[i]
+		stored := nc.DeepCopy()
+
+		if nc.Annotations[v1beta1.AnnotationEC2NodeClassHashVersion] != v1beta1.EC2NodeClassHashVersion {
+			nc.Annotations = lo.Assign(nc.Annotations, map[string]string{
+				v1beta1.AnnotationEC2NodeClassHashVersion: v1beta1.EC2NodeClassHashVersion,
+			})
+
+			// Any NodeClaim that is already drifted will remain drifted if the karpenter.k8s.aws/nodepool-hash-version doesn't match
+			// Since the hashing mechanism has changed we will not be able to determine if the drifted status of the NodeClaim has changed
+			if nc.StatusConditions().GetCondition(corev1beta1.Drifted) == nil {
+				nc.Annotations = lo.Assign(nc.Annotations, map[string]string{
+					v1beta1.AnnotationEC2NodeClassHash: nodeClass.Hash(),
+				})
+			}
+
+			if !equality.Semantic.DeepEqual(stored, nc) {
+				if err := c.kubeClient.Patch(ctx, &nc, client.MergeFrom(stored)); err != nil {
+					errs[i] = client.IgnoreNotFound(err)
+				}
+			}
+		}
+	}
+
+	return multierr.Combine(errs...)
+}
+
 var _ corecontroller.FinalizingTypedController[*v1beta1.EC2NodeClass] = (*NodeClassController)(nil)
 
 //nolint:revive
@@ -230,7 +279,7 @@ func NewNodeClassController(kubeClient client.Client, recorder events.Recorder,
 	})
 }
 
-func (c *NodeClassController) Name() string {
+func (c *Controller) Name() string {
 	return "nodeclass"
 }