From 795343af6895981824a65d9fb47d0b4d47941961 Mon Sep 17 00:00:00 2001 From: Reed Schalo Date: Tue, 4 Jun 2024 16:17:16 -0700 Subject: [PATCH 1/9] chore: use new pod identity based tags --- pkg/apis/v1beta1/ec2nodeclass.go | 3 +-- pkg/apis/v1beta1/labels.go | 5 ++++- pkg/cloudprovider/cloudprovider.go | 4 ++-- .../nodeclaim/garbagecollection/controller.go | 14 ++++++++------ pkg/providers/instance/instance.go | 6 +++--- pkg/providers/launchtemplate/launchtemplate.go | 6 +++++- test/suites/integration/kubelet_config_test.go | 2 +- test/suites/integration/tags_test.go | 3 ++- 8 files changed, 26 insertions(+), 17 deletions(-) diff --git a/pkg/apis/v1beta1/ec2nodeclass.go b/pkg/apis/v1beta1/ec2nodeclass.go index 66a2926b2ced..d16d08eef962 100644 --- a/pkg/apis/v1beta1/ec2nodeclass.go +++ b/pkg/apis/v1beta1/ec2nodeclass.go @@ -21,7 +21,6 @@ import ( "github.com/samber/lo" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - corev1beta1 "sigs.k8s.io/karpenter/pkg/apis/v1beta1" ) // EC2NodeClassSpec is the top level specification for the AWS Karpenter Provider. @@ -356,7 +355,7 @@ func (in *EC2NodeClass) InstanceProfileRole() string { func (in *EC2NodeClass) InstanceProfileTags(clusterName string) map[string]string { return lo.Assign(in.Spec.Tags, map[string]string{ fmt.Sprintf("kubernetes.io/cluster/%s", clusterName): "owned", - corev1beta1.ManagedByAnnotationKey: clusterName, + EksClusterNameAnnotationKey: clusterName, LabelNodeClass: in.Name, }) } diff --git a/pkg/apis/v1beta1/labels.go b/pkg/apis/v1beta1/labels.go index ac72ab48b908..c17c90311d9e 100644 --- a/pkg/apis/v1beta1/labels.go +++ b/pkg/apis/v1beta1/labels.go @@ -69,10 +69,13 @@ var ( // https://docs.aws.amazon.com/eks/latest/APIReference/API_CreateCluster.html regexp.MustCompile(`^kubernetes\.io/cluster/[0-9A-Za-z][A-Za-z0-9\-_]*$`), regexp.MustCompile(fmt.Sprintf("^%s$", regexp.QuoteMeta(v1beta1.NodePoolLabelKey))), - regexp.MustCompile(fmt.Sprintf("^%s$", regexp.QuoteMeta(v1beta1.ManagedByAnnotationKey))), + regexp.MustCompile(fmt.Sprintf("^%s$", regexp.QuoteMeta(EksClusterNameAnnotationKey))), regexp.MustCompile(fmt.Sprintf("^%s$", regexp.QuoteMeta(LabelNodeClass))), regexp.MustCompile(fmt.Sprintf("^%s$", regexp.QuoteMeta(TagNodeClaim))), } + + EksClusterNameAnnotationKey = "eks:eks-cluster-name" + AMIFamilyBottlerocket = "Bottlerocket" AMIFamilyAL2 = "AL2" AMIFamilyAL2023 = "AL2023" diff --git a/pkg/cloudprovider/cloudprovider.go b/pkg/cloudprovider/cloudprovider.go index a4c0c83fff16..3f6f09c90ad9 100644 --- a/pkg/cloudprovider/cloudprovider.go +++ b/pkg/cloudprovider/cloudprovider.go @@ -345,8 +345,8 @@ func (c *CloudProvider) instanceToNodeClaim(i *instance.Instance, instanceType * if v, ok := i.Tags[corev1beta1.NodePoolLabelKey]; ok { labels[corev1beta1.NodePoolLabelKey] = v } - if v, ok := i.Tags[corev1beta1.ManagedByAnnotationKey]; ok { - annotations[corev1beta1.ManagedByAnnotationKey] = v + if v, ok := i.Tags[v1beta1.EksClusterNameAnnotationKey]; ok { + annotations[v1beta1.EksClusterNameAnnotationKey] = v } nodeClaim.Labels = labels nodeClaim.Annotations = annotations diff --git a/pkg/controllers/nodeclaim/garbagecollection/controller.go b/pkg/controllers/nodeclaim/garbagecollection/controller.go index 71748eaaa02c..f63d6e525df4 100644 --- a/pkg/controllers/nodeclaim/garbagecollection/controller.go +++ b/pkg/controllers/nodeclaim/garbagecollection/controller.go @@ -31,7 +31,9 @@ import ( "sigs.k8s.io/controller-runtime/pkg/reconcile" "sigs.k8s.io/karpenter/pkg/cloudprovider" - "sigs.k8s.io/karpenter/pkg/apis/v1beta1" + "github.com/aws/karpenter-provider-aws/pkg/apis/v1beta1" + + corev1beta1 "sigs.k8s.io/karpenter/pkg/apis/v1beta1" "sigs.k8s.io/karpenter/pkg/operator/controller" ) @@ -57,10 +59,10 @@ func (c *Controller) Reconcile(ctx context.Context, _ reconcile.Request) (reconc if err != nil { return reconcile.Result{}, fmt.Errorf("listing cloudprovider machines, %w", err) } - managedRetrieved := lo.Filter(retrieved, func(nc *v1beta1.NodeClaim, _ int) bool { - return nc.Annotations[v1beta1.ManagedByAnnotationKey] != "" && nc.DeletionTimestamp.IsZero() + managedRetrieved := lo.Filter(retrieved, func(nc *corev1beta1.NodeClaim, _ int) bool { + return nc.Annotations[v1beta1.EksClusterNameAnnotationKey] != "" && nc.DeletionTimestamp.IsZero() }) - nodeClaimList := &v1beta1.NodeClaimList{} + nodeClaimList := &corev1beta1.NodeClaimList{} if err = c.kubeClient.List(ctx, nodeClaimList); err != nil { return reconcile.Result{}, err } @@ -68,7 +70,7 @@ func (c *Controller) Reconcile(ctx context.Context, _ reconcile.Request) (reconc if err = c.kubeClient.List(ctx, nodeList); err != nil { return reconcile.Result{}, err } - resolvedProviderIDs := sets.New[string](lo.FilterMap(nodeClaimList.Items, func(n v1beta1.NodeClaim, _ int) (string, bool) { + resolvedProviderIDs := sets.New[string](lo.FilterMap(nodeClaimList.Items, func(n corev1beta1.NodeClaim, _ int) (string, bool) { return n.Status.ProviderID, n.Status.ProviderID != "" })...) errs := make([]error, len(retrieved)) @@ -85,7 +87,7 @@ func (c *Controller) Reconcile(ctx context.Context, _ reconcile.Request) (reconc return reconcile.Result{RequeueAfter: lo.Ternary(c.successfulCount <= 20, time.Second*10, time.Minute*2)}, nil } -func (c *Controller) garbageCollect(ctx context.Context, nodeClaim *v1beta1.NodeClaim, nodeList *v1.NodeList) error { +func (c *Controller) garbageCollect(ctx context.Context, nodeClaim *corev1beta1.NodeClaim, nodeList *v1.NodeList) error { ctx = log.IntoContext(ctx, log.FromContext(ctx).WithValues("provider-id", nodeClaim.Status.ProviderID)) if err := c.cloudProvider.Delete(ctx, nodeClaim); err != nil { return cloudprovider.IgnoreNodeClaimNotFoundError(err) diff --git a/pkg/providers/instance/instance.go b/pkg/providers/instance/instance.go index 95c17cefb206..f5e25b4751cf 100644 --- a/pkg/providers/instance/instance.go +++ b/pkg/providers/instance/instance.go @@ -261,9 +261,9 @@ func (p *DefaultProvider) launchInstance(ctx context.Context, nodeClass *v1beta1 func getTags(ctx context.Context, nodeClass *v1beta1.EC2NodeClass, nodeClaim *corev1beta1.NodeClaim) map[string]string { staticTags := map[string]string{ fmt.Sprintf("kubernetes.io/cluster/%s", options.FromContext(ctx).ClusterName): "owned", - corev1beta1.NodePoolLabelKey: nodeClaim.Labels[corev1beta1.NodePoolLabelKey], - corev1beta1.ManagedByAnnotationKey: options.FromContext(ctx).ClusterName, - v1beta1.LabelNodeClass: nodeClass.Name, + corev1beta1.NodePoolLabelKey: nodeClaim.Labels[corev1beta1.NodePoolLabelKey], + v1beta1.EksClusterNameAnnotationKey: options.FromContext(ctx).ClusterName, + v1beta1.LabelNodeClass: nodeClass.Name, } return lo.Assign(nodeClass.Spec.Tags, staticTags) } diff --git a/pkg/providers/launchtemplate/launchtemplate.go b/pkg/providers/launchtemplate/launchtemplate.go index d40fda648fb1..aa9dc078768c 100644 --- a/pkg/providers/launchtemplate/launchtemplate.go +++ b/pkg/providers/launchtemplate/launchtemplate.go @@ -271,7 +271,11 @@ func (p *DefaultProvider) createLaunchTemplate(ctx context.Context, options *ami TagSpecifications: []*ec2.TagSpecification{ { ResourceType: aws.String(ec2.ResourceTypeLaunchTemplate), - Tags: utils.MergeTags(options.Tags, map[string]string{v1beta1.TagManagedLaunchTemplate: options.ClusterName, v1beta1.LabelNodeClass: options.NodeClassName}), + Tags: utils.MergeTags(options.Tags, map[string]string{ + v1beta1.EksClusterNameAnnotationKey: options.ClusterName, + v1beta1.TagManagedLaunchTemplate: options.ClusterName, + v1beta1.LabelNodeClass: options.NodeClassName, + }), }, }, }) diff --git a/test/suites/integration/kubelet_config_test.go b/test/suites/integration/kubelet_config_test.go index 7f59366245ba..5030c9496394 100644 --- a/test/suites/integration/kubelet_config_test.go +++ b/test/suites/integration/kubelet_config_test.go @@ -157,7 +157,7 @@ var _ = Describe("KubeletConfiguration Overrides", func() { // Get the DS pod count and use it to calculate the DS pod overhead dsCount := env.GetDaemonSetCount(nodePool) nodePool.Spec.Template.Spec.Kubelet = &corev1beta1.KubeletConfiguration{ - MaxPods: lo.ToPtr(int32(1 + int32(dsCount))), + MaxPods: lo.ToPtr(1 + int32(dsCount)), } numPods := 3 diff --git a/test/suites/integration/tags_test.go b/test/suites/integration/tags_test.go index ab56b5abd312..850e2a9f6e9b 100644 --- a/test/suites/integration/tags_test.go +++ b/test/suites/integration/tags_test.go @@ -86,7 +86,7 @@ var _ = Describe("Tags", func() { }) Context("Tagging Controller", func() { - It("should tag with karpenter.sh/nodeclaim and Name tag", func() { + It("should tag with karpenter.sh/nodeclaim, eks:cluster-name, and Name tag", func() { pod := coretest.Pod() env.ExpectCreated(nodePool, nodeClass, pod) @@ -102,6 +102,7 @@ var _ = Describe("Tags", func() { nodeInstance := instance.NewInstance(lo.ToPtr(env.GetInstance(node.Name))) Expect(nodeInstance.Tags).To(HaveKeyWithValue("Name", node.Name)) + Expect(nodeInstance.Tags).To(HaveKeyWithValue(v1beta1.EksClusterNameAnnotationKey, node.Name)) Expect(nodeInstance.Tags).To(HaveKey("karpenter.sh/nodeclaim")) }) From f3f9ed84a22ef748013a217a53f5932b3b61fcef Mon Sep 17 00:00:00 2001 From: Reed Schalo Date: Sat, 8 Jun 2024 16:38:01 -0700 Subject: [PATCH 2/9] fix: tests --- pkg/apis/v1beta1/ec2nodeclass_validation_cel_test.go | 2 +- pkg/controllers/nodeclaim/garbagecollection/suite_test.go | 8 ++++---- pkg/controllers/nodeclaim/tagging/suite_test.go | 2 +- pkg/providers/instance/suite_test.go | 2 +- test/suites/nodeclaim/garbage_collection_test.go | 2 +- 5 files changed, 8 insertions(+), 8 deletions(-) diff --git a/pkg/apis/v1beta1/ec2nodeclass_validation_cel_test.go b/pkg/apis/v1beta1/ec2nodeclass_validation_cel_test.go index b0104c9c506a..f1d6bef3fc9c 100644 --- a/pkg/apis/v1beta1/ec2nodeclass_validation_cel_test.go +++ b/pkg/apis/v1beta1/ec2nodeclass_validation_cel_test.go @@ -80,7 +80,7 @@ var _ = Describe("CEL/Validation", func() { } Expect(env.Client.Create(ctx, nc)).To(Not(Succeed())) nc.Spec.Tags = map[string]string{ - corev1beta1.ManagedByAnnotationKey: "test", + v1beta1.EksClusterNameAnnotationKey: "test", } Expect(env.Client.Create(ctx, nc)).To(Not(Succeed())) nc.Spec.Tags = map[string]string{ diff --git a/pkg/controllers/nodeclaim/garbagecollection/suite_test.go b/pkg/controllers/nodeclaim/garbagecollection/suite_test.go index 0446d774da30..578c119e57c0 100644 --- a/pkg/controllers/nodeclaim/garbagecollection/suite_test.go +++ b/pkg/controllers/nodeclaim/garbagecollection/suite_test.go @@ -115,7 +115,7 @@ var _ = Describe("GarbageCollection", func() { Value: aws.String(nodeClass.Name), }, { - Key: aws.String(corev1beta1.ManagedByAnnotationKey), + Key: aws.String(v1beta1.EksClusterNameAnnotationKey), Value: aws.String(options.FromContext(ctx).ClusterName), }, }, @@ -183,7 +183,7 @@ var _ = Describe("GarbageCollection", func() { Value: aws.String("default"), }, { - Key: aws.String(corev1beta1.ManagedByAnnotationKey), + Key: aws.String(v1beta1.EksClusterNameAnnotationKey), Value: aws.String(options.FromContext(ctx).ClusterName), }, }, @@ -288,7 +288,7 @@ var _ = Describe("GarbageCollection", func() { It("should not delete an instance if it was not launched by a NodeClaim", func() { // Remove the "karpenter.sh/managed-by" tag (this isn't launched by a machine) instance.Tags = lo.Reject(instance.Tags, func(t *ec2.Tag, _ int) bool { - return aws.StringValue(t.Key) == corev1beta1.ManagedByAnnotationKey + return aws.StringValue(t.Key) == v1beta1.EksClusterNameAnnotationKey }) // Launch time was 1m ago @@ -346,7 +346,7 @@ var _ = Describe("GarbageCollection", func() { Value: aws.String("default"), }, { - Key: aws.String(corev1beta1.ManagedByAnnotationKey), + Key: aws.String(v1beta1.EksClusterNameAnnotationKey), Value: aws.String(options.FromContext(ctx).ClusterName), }, }, diff --git a/pkg/controllers/nodeclaim/tagging/suite_test.go b/pkg/controllers/nodeclaim/tagging/suite_test.go index b132b71d90e7..f82957d79029 100644 --- a/pkg/controllers/nodeclaim/tagging/suite_test.go +++ b/pkg/controllers/nodeclaim/tagging/suite_test.go @@ -91,7 +91,7 @@ var _ = Describe("TaggingController", func() { Value: aws.String("default"), }, { - Key: aws.String(corev1beta1.ManagedByAnnotationKey), + Key: aws.String(v1beta1.EksClusterNameAnnotationKey), Value: aws.String(options.FromContext(ctx).ClusterName), }, }, diff --git a/pkg/providers/instance/suite_test.go b/pkg/providers/instance/suite_test.go index 08da5f5053cf..5f1a9b74f91a 100644 --- a/pkg/providers/instance/suite_test.go +++ b/pkg/providers/instance/suite_test.go @@ -156,7 +156,7 @@ var _ = Describe("InstanceProvider", func() { Value: aws.String("default"), }, { - Key: aws.String(corev1beta1.ManagedByAnnotationKey), + Key: aws.String(v1beta1.EksClusterNameAnnotationKey), Value: aws.String(options.FromContext(ctx).ClusterName), }, }, diff --git a/test/suites/nodeclaim/garbage_collection_test.go b/test/suites/nodeclaim/garbage_collection_test.go index 0872116b00c4..e76c36d29819 100644 --- a/test/suites/nodeclaim/garbage_collection_test.go +++ b/test/suites/nodeclaim/garbage_collection_test.go @@ -130,7 +130,7 @@ var _ = Describe("GarbageCollection", func() { Resources: []*string{out.Instances[0].InstanceId}, Tags: []*ec2.Tag{ { - Key: aws.String(corev1beta1.ManagedByAnnotationKey), + Key: aws.String(v1beta1.EksClusterNameAnnotationKey), Value: aws.String(env.ClusterName), }, }, From 6a654d5e21fdc8324879be7dce38bd574773b148 Mon Sep 17 00:00:00 2001 From: Reed Schalo Date: Sat, 8 Jun 2024 16:46:34 -0700 Subject: [PATCH 3/9] fix: remove unused import --- pkg/controllers/nodeclaim/garbagecollection/controller.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/controllers/nodeclaim/garbagecollection/controller.go b/pkg/controllers/nodeclaim/garbagecollection/controller.go index a8123266e135..1404d105484b 100644 --- a/pkg/controllers/nodeclaim/garbagecollection/controller.go +++ b/pkg/controllers/nodeclaim/garbagecollection/controller.go @@ -37,7 +37,6 @@ import ( "github.com/aws/karpenter-provider-aws/pkg/apis/v1beta1" corev1beta1 "sigs.k8s.io/karpenter/pkg/apis/v1beta1" - "sigs.k8s.io/karpenter/pkg/operator/controller" ) type Controller struct { From 5809b7db9e9bc0f06b297dc74f9dbd5a14b1d1ac Mon Sep 17 00:00:00 2001 From: Reed Schalo Date: Sat, 8 Jun 2024 16:58:02 -0700 Subject: [PATCH 4/9] fix: additional references --- pkg/apis/crds/karpenter.k8s.aws_ec2nodeclasses.yaml | 4 ++-- pkg/apis/v1beta1/ec2nodeclass.go | 2 +- pkg/apis/v1beta1/ec2nodeclass_validation_webhook_test.go | 4 ++-- pkg/controllers/nodeclaim/garbagecollection/suite_test.go | 2 +- test/hack/resource/pkg/resourcetypes/resourcetypes.go | 2 +- test/suites/integration/validation_test.go | 2 +- test/suites/nodeclaim/garbage_collection_test.go | 2 +- 7 files changed, 9 insertions(+), 9 deletions(-) diff --git a/pkg/apis/crds/karpenter.k8s.aws_ec2nodeclasses.yaml b/pkg/apis/crds/karpenter.k8s.aws_ec2nodeclasses.yaml index aec3d01d61d4..1dc5d68af46e 100644 --- a/pkg/apis/crds/karpenter.k8s.aws_ec2nodeclasses.yaml +++ b/pkg/apis/crds/karpenter.k8s.aws_ec2nodeclasses.yaml @@ -427,8 +427,8 @@ spec: rule: self.all(k, !k.startsWith('kubernetes.io/cluster') ) - message: tag contains a restricted tag matching karpenter.sh/nodepool rule: self.all(k, k != 'karpenter.sh/nodepool') - - message: tag contains a restricted tag matching karpenter.sh/managed-by - rule: self.all(k, k !='karpenter.sh/managed-by') + - message: tag contains a restricted tag matching eks:eks-cluster-name + rule: self.all(k, k !='eks:eks-cluster-name') - message: tag contains a restricted tag matching karpenter.sh/nodeclaim rule: self.all(k, k !='karpenter.sh/nodeclaim') - message: tag contains a restricted tag matching karpenter.k8s.aws/ec2nodeclass diff --git a/pkg/apis/v1beta1/ec2nodeclass.go b/pkg/apis/v1beta1/ec2nodeclass.go index d16d08eef962..baf4c6ae823c 100644 --- a/pkg/apis/v1beta1/ec2nodeclass.go +++ b/pkg/apis/v1beta1/ec2nodeclass.go @@ -79,7 +79,7 @@ type EC2NodeClassSpec struct { // +kubebuilder:validation:XValidation:message="empty tag keys aren't supported",rule="self.all(k, k != '')" // +kubebuilder:validation:XValidation:message="tag contains a restricted tag matching kubernetes.io/cluster/",rule="self.all(k, !k.startsWith('kubernetes.io/cluster') )" // +kubebuilder:validation:XValidation:message="tag contains a restricted tag matching karpenter.sh/nodepool",rule="self.all(k, k != 'karpenter.sh/nodepool')" - // +kubebuilder:validation:XValidation:message="tag contains a restricted tag matching karpenter.sh/managed-by",rule="self.all(k, k !='karpenter.sh/managed-by')" + // +kubebuilder:validation:XValidation:message="tag contains a restricted tag matching eks:eks-cluster-name",rule="self.all(k, k !='eks:eks-cluster-name')" // +kubebuilder:validation:XValidation:message="tag contains a restricted tag matching karpenter.sh/nodeclaim",rule="self.all(k, k !='karpenter.sh/nodeclaim')" // +kubebuilder:validation:XValidation:message="tag contains a restricted tag matching karpenter.k8s.aws/ec2nodeclass",rule="self.all(k, k !='karpenter.k8s.aws/ec2nodeclass')" // +optional diff --git a/pkg/apis/v1beta1/ec2nodeclass_validation_webhook_test.go b/pkg/apis/v1beta1/ec2nodeclass_validation_webhook_test.go index 18035649e235..255eafa52b93 100644 --- a/pkg/apis/v1beta1/ec2nodeclass_validation_webhook_test.go +++ b/pkg/apis/v1beta1/ec2nodeclass_validation_webhook_test.go @@ -78,7 +78,7 @@ var _ = Describe("Webhook/Validation", func() { } Expect(nc.Validate(ctx)).To(Succeed()) nc.Spec.Tags = map[string]string{ - "karpenterzsh/managed-by": "test", + "ekszsh:eks-cluster-name": "test", } Expect(nc.Validate(ctx)).To(Succeed()) }) @@ -92,7 +92,7 @@ var _ = Describe("Webhook/Validation", func() { } Expect(nc.Validate(ctx)).To(Not(Succeed())) nc.Spec.Tags = map[string]string{ - "karpenter.sh/managed-by": "test", + "eks:eks-cluster-name": "test", } Expect(nc.Validate(ctx)).To(Not(Succeed())) nc.Spec.Tags = map[string]string{ diff --git a/pkg/controllers/nodeclaim/garbagecollection/suite_test.go b/pkg/controllers/nodeclaim/garbagecollection/suite_test.go index d811eea20b65..54e701afc61f 100644 --- a/pkg/controllers/nodeclaim/garbagecollection/suite_test.go +++ b/pkg/controllers/nodeclaim/garbagecollection/suite_test.go @@ -284,7 +284,7 @@ var _ = Describe("GarbageCollection", func() { Expect(err).NotTo(HaveOccurred()) }) It("should not delete an instance if it was not launched by a NodeClaim", func() { - // Remove the "karpenter.sh/managed-by" tag (this isn't launched by a machine) + // Remove the "eks:eks-cluster-name" tag (this isn't launched by a machine) instance.Tags = lo.Reject(instance.Tags, func(t *ec2.Tag, _ int) bool { return aws.StringValue(t.Key) == v1beta1.EksClusterNameAnnotationKey }) diff --git a/test/hack/resource/pkg/resourcetypes/resourcetypes.go b/test/hack/resource/pkg/resourcetypes/resourcetypes.go index ce8a7bfd9441..9757c14fcd26 100644 --- a/test/hack/resource/pkg/resourcetypes/resourcetypes.go +++ b/test/hack/resource/pkg/resourcetypes/resourcetypes.go @@ -20,7 +20,7 @@ import ( ) const ( - karpenterClusterNameTag = "karpenter.sh/managed-by" + karpenterClusterNameTag = "eks:eks-cluster-name" karpenterNodePoolTag = "karpenter.sh/nodepool" karpenterLaunchTemplateTag = "karpenter.k8s.aws/cluster" karpenterSecurityGroupTag = "karpenter.sh/discovery" diff --git a/test/suites/integration/validation_test.go b/test/suites/integration/validation_test.go index 522815d151ca..32df2e17f9cc 100644 --- a/test/suites/integration/validation_test.go +++ b/test/suites/integration/validation_test.go @@ -194,7 +194,7 @@ var _ = Describe("Validation", func() { nodeClass.Spec.Tags = map[string]string{"karpenter.sh/nodepool": "custom-value"} Expect(env.Client.Create(env.Context, nodeClass)).ToNot(Succeed()) - nodeClass.Spec.Tags = map[string]string{"karpenter.sh/managed-by": env.ClusterName} + nodeClass.Spec.Tags = map[string]string{"eks:eks-cluster-name": env.ClusterName} Expect(env.Client.Create(env.Context, nodeClass)).ToNot(Succeed()) nodeClass.Spec.Tags = map[string]string{fmt.Sprintf("kubernetes.io/cluster/%s", env.ClusterName): "owned"} diff --git a/test/suites/nodeclaim/garbage_collection_test.go b/test/suites/nodeclaim/garbage_collection_test.go index e76c36d29819..6317c4b75351 100644 --- a/test/suites/nodeclaim/garbage_collection_test.go +++ b/test/suites/nodeclaim/garbage_collection_test.go @@ -125,7 +125,7 @@ var _ = Describe("GarbageCollection", func() { // Wait for the node to register with the cluster node := env.EventuallyExpectCreatedNodeCount("==", 1)[0] - // Update the tags to add the karpenter.sh/managed-by tag + // Update the tags to add the eks:eks-cluster-name tag _, err = env.EC2API.CreateTagsWithContext(env.Context, &ec2.CreateTagsInput{ Resources: []*string{out.Instances[0].InstanceId}, Tags: []*ec2.Tag{ From acbe8c6095684962a0dacdf38e7549bca22feee1 Mon Sep 17 00:00:00 2001 From: Reed Schalo Date: Sat, 8 Jun 2024 17:18:05 -0700 Subject: [PATCH 5/9] fix: reorder imports --- .../nodeclaim/garbagecollection/controller.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/pkg/controllers/nodeclaim/garbagecollection/controller.go b/pkg/controllers/nodeclaim/garbagecollection/controller.go index 1404d105484b..3812a2a2c03c 100644 --- a/pkg/controllers/nodeclaim/garbagecollection/controller.go +++ b/pkg/controllers/nodeclaim/garbagecollection/controller.go @@ -19,24 +19,26 @@ import ( "fmt" "time" + "github.com/aws/karpenter-provider-aws/pkg/apis/v1beta1" "github.com/awslabs/operatorpkg/singleton" "github.com/samber/lo" + "go.uber.org/multierr" + v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/client-go/util/workqueue" "k8s.io/klog/v2" + controllerruntime "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/manager" "sigs.k8s.io/controller-runtime/pkg/reconcile" - "sigs.k8s.io/karpenter/pkg/cloudprovider" - "sigs.k8s.io/karpenter/pkg/operator/injection" - - "github.com/aws/karpenter-provider-aws/pkg/apis/v1beta1" corev1beta1 "sigs.k8s.io/karpenter/pkg/apis/v1beta1" + "sigs.k8s.io/karpenter/pkg/cloudprovider" + "sigs.k8s.io/karpenter/pkg/operator/injection" ) type Controller struct { From 58e44565e4fca415ffd63f2c038c29df6ed41688 Mon Sep 17 00:00:00 2001 From: Reed Schalo Date: Mon, 17 Jun 2024 11:17:38 -0700 Subject: [PATCH 6/9] Update pkg/apis/v1beta1/ec2nodeclass.go Co-authored-by: Nick Tran <10810510+njtran@users.noreply.github.com> --- pkg/apis/v1beta1/ec2nodeclass.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/apis/v1beta1/ec2nodeclass.go b/pkg/apis/v1beta1/ec2nodeclass.go index baf4c6ae823c..52275f67298f 100644 --- a/pkg/apis/v1beta1/ec2nodeclass.go +++ b/pkg/apis/v1beta1/ec2nodeclass.go @@ -355,7 +355,7 @@ func (in *EC2NodeClass) InstanceProfileRole() string { func (in *EC2NodeClass) InstanceProfileTags(clusterName string) map[string]string { return lo.Assign(in.Spec.Tags, map[string]string{ fmt.Sprintf("kubernetes.io/cluster/%s", clusterName): "owned", - EksClusterNameAnnotationKey: clusterName, + EKSClusterNameAnnotationKey: clusterName, LabelNodeClass: in.Name, }) } From a90517f87c536543c0bdd5121efb40b54444481e Mon Sep 17 00:00:00 2001 From: Reed Schalo Date: Mon, 17 Jun 2024 11:17:58 -0700 Subject: [PATCH 7/9] Update pkg/apis/v1beta1/labels.go Co-authored-by: Nick Tran <10810510+njtran@users.noreply.github.com> --- pkg/apis/v1beta1/labels.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/apis/v1beta1/labels.go b/pkg/apis/v1beta1/labels.go index c17c90311d9e..af9365b50357 100644 --- a/pkg/apis/v1beta1/labels.go +++ b/pkg/apis/v1beta1/labels.go @@ -74,7 +74,7 @@ var ( regexp.MustCompile(fmt.Sprintf("^%s$", regexp.QuoteMeta(TagNodeClaim))), } - EksClusterNameAnnotationKey = "eks:eks-cluster-name" + EKSClusterNameAnnotationKey = "eks:eks-cluster-name" AMIFamilyBottlerocket = "Bottlerocket" AMIFamilyAL2 = "AL2" From 79c97ea0236f422ce3c13bd1c16ae1be41cb5480 Mon Sep 17 00:00:00 2001 From: Reed Schalo Date: Mon, 17 Jun 2024 11:18:56 -0700 Subject: [PATCH 8/9] Update test/suites/integration/tags_test.go Co-authored-by: Nick Tran <10810510+njtran@users.noreply.github.com> --- test/suites/integration/tags_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/suites/integration/tags_test.go b/test/suites/integration/tags_test.go index 850e2a9f6e9b..8cc9a0466c44 100644 --- a/test/suites/integration/tags_test.go +++ b/test/suites/integration/tags_test.go @@ -86,7 +86,7 @@ var _ = Describe("Tags", func() { }) Context("Tagging Controller", func() { - It("should tag with karpenter.sh/nodeclaim, eks:cluster-name, and Name tag", func() { + It("should tag with karpenter.sh/nodeclaim, eks:eks-cluster-name, and Name tag", func() { pod := coretest.Pod() env.ExpectCreated(nodePool, nodeClass, pod) From 401171606a4d2a9451bf160843e6ca7928545b94 Mon Sep 17 00:00:00 2001 From: Reed Schalo Date: Mon, 17 Jun 2024 11:57:24 -0700 Subject: [PATCH 9/9] update for PR comments --- pkg/apis/v1beta1/ec2nodeclass_validation_cel_test.go | 2 +- pkg/apis/v1beta1/labels.go | 2 +- pkg/cloudprovider/cloudprovider.go | 4 ++-- pkg/controllers/nodeclaim/garbagecollection/controller.go | 2 +- pkg/controllers/nodeclaim/garbagecollection/suite_test.go | 8 ++++---- pkg/controllers/nodeclaim/tagging/suite_test.go | 2 +- pkg/providers/instance/instance.go | 2 +- pkg/providers/instance/suite_test.go | 2 +- pkg/providers/launchtemplate/launchtemplate.go | 2 +- test/suites/integration/tags_test.go | 2 +- test/suites/integration/validation_test.go | 2 +- test/suites/nodeclaim/garbage_collection_test.go | 2 +- 12 files changed, 16 insertions(+), 16 deletions(-) diff --git a/pkg/apis/v1beta1/ec2nodeclass_validation_cel_test.go b/pkg/apis/v1beta1/ec2nodeclass_validation_cel_test.go index f1d6bef3fc9c..02c0d741e5f3 100644 --- a/pkg/apis/v1beta1/ec2nodeclass_validation_cel_test.go +++ b/pkg/apis/v1beta1/ec2nodeclass_validation_cel_test.go @@ -80,7 +80,7 @@ var _ = Describe("CEL/Validation", func() { } Expect(env.Client.Create(ctx, nc)).To(Not(Succeed())) nc.Spec.Tags = map[string]string{ - v1beta1.EksClusterNameAnnotationKey: "test", + v1beta1.EKSClusterNameAnnotationKey: "test", } Expect(env.Client.Create(ctx, nc)).To(Not(Succeed())) nc.Spec.Tags = map[string]string{ diff --git a/pkg/apis/v1beta1/labels.go b/pkg/apis/v1beta1/labels.go index af9365b50357..1b2ab7dfa8a2 100644 --- a/pkg/apis/v1beta1/labels.go +++ b/pkg/apis/v1beta1/labels.go @@ -69,7 +69,7 @@ var ( // https://docs.aws.amazon.com/eks/latest/APIReference/API_CreateCluster.html regexp.MustCompile(`^kubernetes\.io/cluster/[0-9A-Za-z][A-Za-z0-9\-_]*$`), regexp.MustCompile(fmt.Sprintf("^%s$", regexp.QuoteMeta(v1beta1.NodePoolLabelKey))), - regexp.MustCompile(fmt.Sprintf("^%s$", regexp.QuoteMeta(EksClusterNameAnnotationKey))), + regexp.MustCompile(fmt.Sprintf("^%s$", regexp.QuoteMeta(EKSClusterNameAnnotationKey))), regexp.MustCompile(fmt.Sprintf("^%s$", regexp.QuoteMeta(LabelNodeClass))), regexp.MustCompile(fmt.Sprintf("^%s$", regexp.QuoteMeta(TagNodeClaim))), } diff --git a/pkg/cloudprovider/cloudprovider.go b/pkg/cloudprovider/cloudprovider.go index 3f6f09c90ad9..832653d980a3 100644 --- a/pkg/cloudprovider/cloudprovider.go +++ b/pkg/cloudprovider/cloudprovider.go @@ -345,8 +345,8 @@ func (c *CloudProvider) instanceToNodeClaim(i *instance.Instance, instanceType * if v, ok := i.Tags[corev1beta1.NodePoolLabelKey]; ok { labels[corev1beta1.NodePoolLabelKey] = v } - if v, ok := i.Tags[v1beta1.EksClusterNameAnnotationKey]; ok { - annotations[v1beta1.EksClusterNameAnnotationKey] = v + if v, ok := i.Tags[v1beta1.EKSClusterNameAnnotationKey]; ok { + annotations[v1beta1.EKSClusterNameAnnotationKey] = v } nodeClaim.Labels = labels nodeClaim.Annotations = annotations diff --git a/pkg/controllers/nodeclaim/garbagecollection/controller.go b/pkg/controllers/nodeclaim/garbagecollection/controller.go index 3812a2a2c03c..ae5f60abe2a6 100644 --- a/pkg/controllers/nodeclaim/garbagecollection/controller.go +++ b/pkg/controllers/nodeclaim/garbagecollection/controller.go @@ -66,7 +66,7 @@ func (c *Controller) Reconcile(ctx context.Context) (reconcile.Result, error) { return reconcile.Result{}, fmt.Errorf("listing cloudprovider machines, %w", err) } managedRetrieved := lo.Filter(retrieved, func(nc *corev1beta1.NodeClaim, _ int) bool { - return nc.Annotations[v1beta1.EksClusterNameAnnotationKey] != "" && nc.DeletionTimestamp.IsZero() + return nc.Annotations[v1beta1.EKSClusterNameAnnotationKey] != "" && nc.DeletionTimestamp.IsZero() }) nodeClaimList := &corev1beta1.NodeClaimList{} if err = c.kubeClient.List(ctx, nodeClaimList); err != nil { diff --git a/pkg/controllers/nodeclaim/garbagecollection/suite_test.go b/pkg/controllers/nodeclaim/garbagecollection/suite_test.go index 54e701afc61f..4c23a71e27bc 100644 --- a/pkg/controllers/nodeclaim/garbagecollection/suite_test.go +++ b/pkg/controllers/nodeclaim/garbagecollection/suite_test.go @@ -113,7 +113,7 @@ var _ = Describe("GarbageCollection", func() { Value: aws.String(nodeClass.Name), }, { - Key: aws.String(v1beta1.EksClusterNameAnnotationKey), + Key: aws.String(v1beta1.EKSClusterNameAnnotationKey), Value: aws.String(options.FromContext(ctx).ClusterName), }, }, @@ -181,7 +181,7 @@ var _ = Describe("GarbageCollection", func() { Value: aws.String("default"), }, { - Key: aws.String(v1beta1.EksClusterNameAnnotationKey), + Key: aws.String(v1beta1.EKSClusterNameAnnotationKey), Value: aws.String(options.FromContext(ctx).ClusterName), }, }, @@ -286,7 +286,7 @@ var _ = Describe("GarbageCollection", func() { It("should not delete an instance if it was not launched by a NodeClaim", func() { // Remove the "eks:eks-cluster-name" tag (this isn't launched by a machine) instance.Tags = lo.Reject(instance.Tags, func(t *ec2.Tag, _ int) bool { - return aws.StringValue(t.Key) == v1beta1.EksClusterNameAnnotationKey + return aws.StringValue(t.Key) == v1beta1.EKSClusterNameAnnotationKey }) // Launch time was 1m ago @@ -344,7 +344,7 @@ var _ = Describe("GarbageCollection", func() { Value: aws.String("default"), }, { - Key: aws.String(v1beta1.EksClusterNameAnnotationKey), + Key: aws.String(v1beta1.EKSClusterNameAnnotationKey), Value: aws.String(options.FromContext(ctx).ClusterName), }, }, diff --git a/pkg/controllers/nodeclaim/tagging/suite_test.go b/pkg/controllers/nodeclaim/tagging/suite_test.go index f82957d79029..dbaa29543802 100644 --- a/pkg/controllers/nodeclaim/tagging/suite_test.go +++ b/pkg/controllers/nodeclaim/tagging/suite_test.go @@ -91,7 +91,7 @@ var _ = Describe("TaggingController", func() { Value: aws.String("default"), }, { - Key: aws.String(v1beta1.EksClusterNameAnnotationKey), + Key: aws.String(v1beta1.EKSClusterNameAnnotationKey), Value: aws.String(options.FromContext(ctx).ClusterName), }, }, diff --git a/pkg/providers/instance/instance.go b/pkg/providers/instance/instance.go index f5e25b4751cf..ab015968409b 100644 --- a/pkg/providers/instance/instance.go +++ b/pkg/providers/instance/instance.go @@ -262,7 +262,7 @@ func getTags(ctx context.Context, nodeClass *v1beta1.EC2NodeClass, nodeClaim *co staticTags := map[string]string{ fmt.Sprintf("kubernetes.io/cluster/%s", options.FromContext(ctx).ClusterName): "owned", corev1beta1.NodePoolLabelKey: nodeClaim.Labels[corev1beta1.NodePoolLabelKey], - v1beta1.EksClusterNameAnnotationKey: options.FromContext(ctx).ClusterName, + v1beta1.EKSClusterNameAnnotationKey: options.FromContext(ctx).ClusterName, v1beta1.LabelNodeClass: nodeClass.Name, } return lo.Assign(nodeClass.Spec.Tags, staticTags) diff --git a/pkg/providers/instance/suite_test.go b/pkg/providers/instance/suite_test.go index 5f1a9b74f91a..ec8938598e8e 100644 --- a/pkg/providers/instance/suite_test.go +++ b/pkg/providers/instance/suite_test.go @@ -156,7 +156,7 @@ var _ = Describe("InstanceProvider", func() { Value: aws.String("default"), }, { - Key: aws.String(v1beta1.EksClusterNameAnnotationKey), + Key: aws.String(v1beta1.EKSClusterNameAnnotationKey), Value: aws.String(options.FromContext(ctx).ClusterName), }, }, diff --git a/pkg/providers/launchtemplate/launchtemplate.go b/pkg/providers/launchtemplate/launchtemplate.go index aa9dc078768c..3a5fb06a504e 100644 --- a/pkg/providers/launchtemplate/launchtemplate.go +++ b/pkg/providers/launchtemplate/launchtemplate.go @@ -272,7 +272,7 @@ func (p *DefaultProvider) createLaunchTemplate(ctx context.Context, options *ami { ResourceType: aws.String(ec2.ResourceTypeLaunchTemplate), Tags: utils.MergeTags(options.Tags, map[string]string{ - v1beta1.EksClusterNameAnnotationKey: options.ClusterName, + v1beta1.EKSClusterNameAnnotationKey: options.ClusterName, v1beta1.TagManagedLaunchTemplate: options.ClusterName, v1beta1.LabelNodeClass: options.NodeClassName, }), diff --git a/test/suites/integration/tags_test.go b/test/suites/integration/tags_test.go index 8cc9a0466c44..549ff2b0cd8c 100644 --- a/test/suites/integration/tags_test.go +++ b/test/suites/integration/tags_test.go @@ -102,7 +102,7 @@ var _ = Describe("Tags", func() { nodeInstance := instance.NewInstance(lo.ToPtr(env.GetInstance(node.Name))) Expect(nodeInstance.Tags).To(HaveKeyWithValue("Name", node.Name)) - Expect(nodeInstance.Tags).To(HaveKeyWithValue(v1beta1.EksClusterNameAnnotationKey, node.Name)) + Expect(nodeInstance.Tags).To(HaveKeyWithValue(v1beta1.EKSClusterNameAnnotationKey, node.Name)) Expect(nodeInstance.Tags).To(HaveKey("karpenter.sh/nodeclaim")) }) diff --git a/test/suites/integration/validation_test.go b/test/suites/integration/validation_test.go index 32df2e17f9cc..ae3495ffdeec 100644 --- a/test/suites/integration/validation_test.go +++ b/test/suites/integration/validation_test.go @@ -194,7 +194,7 @@ var _ = Describe("Validation", func() { nodeClass.Spec.Tags = map[string]string{"karpenter.sh/nodepool": "custom-value"} Expect(env.Client.Create(env.Context, nodeClass)).ToNot(Succeed()) - nodeClass.Spec.Tags = map[string]string{"eks:eks-cluster-name": env.ClusterName} + nodeClass.Spec.Tags = map[string]string{v1beta1.EKSClusterNameAnnotationKey: env.ClusterName} Expect(env.Client.Create(env.Context, nodeClass)).ToNot(Succeed()) nodeClass.Spec.Tags = map[string]string{fmt.Sprintf("kubernetes.io/cluster/%s", env.ClusterName): "owned"} diff --git a/test/suites/nodeclaim/garbage_collection_test.go b/test/suites/nodeclaim/garbage_collection_test.go index 6317c4b75351..c1f303ca5deb 100644 --- a/test/suites/nodeclaim/garbage_collection_test.go +++ b/test/suites/nodeclaim/garbage_collection_test.go @@ -130,7 +130,7 @@ var _ = Describe("GarbageCollection", func() { Resources: []*string{out.Instances[0].InstanceId}, Tags: []*ec2.Tag{ { - Key: aws.String(v1beta1.EksClusterNameAnnotationKey), + Key: aws.String(v1beta1.EKSClusterNameAnnotationKey), Value: aws.String(env.ClusterName), }, },