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: use new pod identity based tags #6319

Closed
wants to merge 10 commits into from
Closed
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')"
rschalo marked this conversation as resolved.
Show resolved Hide resolved
// +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"
rschalo marked this conversation as resolved.
Show resolved Hide resolved

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
}
nodeClaim.Labels = labels
nodeClaim.Annotations = annotations
Expand Down
18 changes: 11 additions & 7 deletions pkg/controllers/nodeclaim/garbagecollection/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,22 +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"

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 @@ -61,18 +65,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 @@ -89,7 +93,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 @@ -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,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this adds the eks cluster annotation tag on now, is this right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct, according to @mikestef9, it should be added to all resources Karpenter creates.

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"
rschalo marked this conversation as resolved.
Show resolved Hide resolved
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:cluster-name, and Name tag", func() {
rschalo marked this conversation as resolved.
Show resolved Hide resolved
pod := coretest.Pod()

env.ExpectCreated(nodePool, nodeClass, pod)
Expand All @@ -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"))
})

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{"eks:eks-cluster-name": env.ClusterName}
rschalo marked this conversation as resolved.
Show resolved Hide resolved
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