Skip to content

Commit

Permalink
chore: instance profile tags optional
Browse files Browse the repository at this point in the history
  • Loading branch information
jigisha620 committed Nov 12, 2024
1 parent 7e8269b commit 8ae2b8c
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 46 deletions.
33 changes: 0 additions & 33 deletions pkg/controllers/nodeclass/status/instanceprofile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,39 +99,6 @@ var _ = Describe("NodeClass InstanceProfile Status Controller", func() {
Expect(nodeClass.Status.InstanceProfile).To(Equal(profileName))
Expect(nodeClass.StatusConditions().IsTrue(v1.ConditionTypeInstanceProfileReady)).To(BeTrue())
})
It("should add the eks:eks-cluster-name tag when the tag doesn't exist", func() {
awsEnv.IAMAPI.InstanceProfiles = map[string]*iamtypes.InstanceProfile{
profileName: {
InstanceProfileId: aws.String(fake.InstanceProfileID()),
InstanceProfileName: aws.String(profileName),
Roles: []iamtypes.Role{
{
RoleName: aws.String("other-role"),
},
},
Tags: []iamtypes.Tag{
{
Key: lo.ToPtr(fmt.Sprintf("kubernetes.io/cluster/%s", options.FromContext(ctx).ClusterName)),
Value: lo.ToPtr("owned"),
},
{
Key: lo.ToPtr(v1.LabelNodeClass),
Value: lo.ToPtr(nodeClass.Name),
},
},
},
}

ExpectApplied(ctx, env.Client, nodeClass)
ExpectObjectReconciled(ctx, env.Client, statusController, nodeClass)

Expect(awsEnv.IAMAPI.InstanceProfiles).To(HaveLen(1))
Expect(awsEnv.IAMAPI.InstanceProfiles[profileName].Tags).To(ContainElements(
iamtypes.Tag{Key: lo.ToPtr(fmt.Sprintf("kubernetes.io/cluster/%s", options.FromContext(ctx).ClusterName)), Value: lo.ToPtr("owned")},
iamtypes.Tag{Key: lo.ToPtr(v1.LabelNodeClass), Value: lo.ToPtr(nodeClass.Name)},
iamtypes.Tag{Key: lo.ToPtr(v1.EKSClusterNameTagKey), Value: lo.ToPtr(options.FromContext(ctx).ClusterName)},
))
})
It("should not call CreateInstanceProfile or AddRoleToInstanceProfile when instance profile exists with correct role", func() {
awsEnv.IAMAPI.InstanceProfiles = map[string]*iamtypes.InstanceProfile{
profileName: {
Expand Down
17 changes: 4 additions & 13 deletions pkg/providers/instanceprofile/instanceprofile.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import (
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/types"

v1 "github.com/aws/karpenter-provider-aws/pkg/apis/v1"
sdk "github.com/aws/karpenter-provider-aws/pkg/aws"
awserrors "github.com/aws/karpenter-provider-aws/pkg/errors"
"github.com/aws/karpenter-provider-aws/pkg/operator/options"
Expand Down Expand Up @@ -61,8 +60,10 @@ func NewDefaultProvider(region string, iamapi sdk.IAMAPI, cache *cache.Cache) *D

func (p *DefaultProvider) Create(ctx context.Context, m ResourceOwner) (string, error) {
profileName := m.InstanceProfileName(options.FromContext(ctx).ClusterName, p.region)
tags := lo.Assign(m.InstanceProfileTags(options.FromContext(ctx).ClusterName), map[string]string{corev1.LabelTopologyRegion: p.region})

tags := map[string]string{}
if len(m.InstanceProfileTags(options.FromContext(ctx).ClusterName)) != 0 {
tags = lo.Assign(m.InstanceProfileTags(options.FromContext(ctx).ClusterName), map[string]string{corev1.LabelTopologyRegion: p.region})
}
// An instance profile exists for this NodeClass
if _, ok := p.cache.Get(string(m.GetUID())); ok {
return profileName, nil
Expand All @@ -83,16 +84,6 @@ func (p *DefaultProvider) Create(ctx context.Context, m ResourceOwner) (string,
}
instanceProfile = o.InstanceProfile
} else {
if !lo.ContainsBy(out.InstanceProfile.Tags, func(t iamtypes.Tag) bool {
return lo.FromPtr(t.Key) == v1.EKSClusterNameTagKey
}) {
if _, err = p.iamapi.TagInstanceProfile(ctx, &iam.TagInstanceProfileInput{
InstanceProfileName: aws.String(profileName),
Tags: lo.MapToSlice(tags, func(k, v string) iamtypes.Tag { return iamtypes.Tag{Key: aws.String(k), Value: aws.String(v)} }),
}); err != nil {
return "", fmt.Errorf("tagging instance profile %q, %w", profileName, err)
}
}
instanceProfile = out.InstanceProfile
}
// Instance profiles can only have a single role assigned to them so this profile either has 1 or 0 roles
Expand Down

0 comments on commit 8ae2b8c

Please sign in to comment.