Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Drop karpenter.sh/managed-by annotation #6379

Closed
wants to merge 13 commits into from
4 changes: 2 additions & 2 deletions pkg/apis/crds/karpenter.k8s.aws_ec2nodeclasses.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 2 additions & 3 deletions pkg/apis/v1beta1/ec2nodeclass.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -80,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
Expand Down Expand Up @@ -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,
rschalo marked this conversation as resolved.
Show resolved Hide resolved
LabelNodeClass: in.Name,
})
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/apis/v1beta1/ec2nodeclass_validation_cel_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
4 changes: 2 additions & 2 deletions pkg/apis/v1beta1/ec2nodeclass_validation_webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
})
Expand All @@ -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{
Expand Down
5 changes: 4 additions & 1 deletion pkg/apis/v1beta1/labels.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
4 changes: 2 additions & 2 deletions pkg/cloudprovider/cloudprovider.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
rschalo marked this conversation as resolved.
Show resolved Hide resolved
}
nodeClaim.Labels = labels
nodeClaim.Annotations = annotations
Expand Down
14 changes: 7 additions & 7 deletions pkg/controllers/nodeclaim/garbagecollection/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ 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"
Expand All @@ -32,10 +33,9 @@ import (
"sigs.k8s.io/controller-runtime/pkg/log"
"sigs.k8s.io/controller-runtime/pkg/manager"
"sigs.k8s.io/controller-runtime/pkg/reconcile"
corev1beta1 "sigs.k8s.io/karpenter/pkg/apis/v1beta1"
"sigs.k8s.io/karpenter/pkg/cloudprovider"
"sigs.k8s.io/karpenter/pkg/operator/injection"

"sigs.k8s.io/karpenter/pkg/apis/v1beta1"
)

type Controller struct {
Expand All @@ -62,18 +62,18 @@ func (c *Controller) Reconcile(ctx context.Context) (reconcile.Result, error) {
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
}
nodeList := &v1.NodeList{}
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))
Expand All @@ -90,7 +90,7 @@ func (c *Controller) Reconcile(ctx context.Context) (reconcile.Result, error) {
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)
Expand Down
10 changes: 5 additions & 5 deletions pkg/controllers/nodeclaim/garbagecollection/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,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),
},
},
Expand Down Expand Up @@ -181,7 +181,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),
},
},
Expand Down Expand Up @@ -284,9 +284,9 @@ 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) == corev1beta1.ManagedByAnnotationKey
return aws.StringValue(t.Key) == v1beta1.EKSClusterNameAnnotationKey
})

// Launch time was 1m ago
Expand Down Expand Up @@ -344,7 +344,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),
},
},
Expand Down
2 changes: 1 addition & 1 deletion pkg/controllers/nodeclaim/tagging/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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),
},
},
Expand Down
6 changes: 3 additions & 3 deletions pkg/providers/instance/instance.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/providers/instance/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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),
},
},
Expand Down
6 changes: 5 additions & 1 deletion pkg/providers/launchtemplate/launchtemplate.go
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,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,
}),
},
},
})
Expand Down
2 changes: 1 addition & 1 deletion test/hack/resource/pkg/resourcetypes/resourcetypes.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
3 changes: 2 additions & 1 deletion test/suites/integration/tags_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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:eks-cluster-name, and Name tag", func() {
pod := coretest.Pod()

env.ExpectCreated(nodePool, nodeClass, pod)
Expand All @@ -101,6 +101,7 @@ var _ = Describe("Tags", func() {
}, time.Minute)

nodeInstance := instance.NewInstance(lo.ToPtr(env.GetInstance(node.Name)))
Expect(nodeInstance.Tags).To(HaveKeyWithValue(v1beta1.EKSClusterNameAnnotationKey, env.ClusterName))
Expect(nodeInstance.Tags).To(HaveKeyWithValue("Name", node.Name))
Expect(nodeInstance.Tags).To(HaveKey("karpenter.sh/nodeclaim"))
})
Expand Down
2 changes: 1 addition & 1 deletion test/suites/integration/validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{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"}
Expand Down
4 changes: 2 additions & 2 deletions test/suites/nodeclaim/garbage_collection_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,12 +125,12 @@ 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{
{
Key: aws.String(corev1beta1.ManagedByAnnotationKey),
Key: aws.String(v1beta1.EKSClusterNameAnnotationKey),
Value: aws.String(env.ClusterName),
},
},
Expand Down
Loading