From c30ac3eec02e97e58c89bff5d4358dad55629193 Mon Sep 17 00:00:00 2001 From: Amanuel Engeda Date: Thu, 27 Jul 2023 08:36:36 -0700 Subject: [PATCH] Adding AWSNodeTemplate drift --- .github/actions/e2e/cleanup/action.yaml | 12 + pkg/cloudprovider/drift.go | 12 +- pkg/cloudprovider/suite_test.go | 262 ++++++------------ test/cloudformation/iam_cloudformation.yaml | 8 + test/pkg/environment/common/environment.go | 2 + test/suites/drift/suite_test.go | 77 ++++- .../en/preview/concepts/deprovisioning.md | 16 +- website/content/en/preview/upgrade-guide.md | 3 + 8 files changed, 209 insertions(+), 183 deletions(-) diff --git a/.github/actions/e2e/cleanup/action.yaml b/.github/actions/e2e/cleanup/action.yaml index 84bc1ea8cb42..41ffdef11317 100644 --- a/.github/actions/e2e/cleanup/action.yaml +++ b/.github/actions/e2e/cleanup/action.yaml @@ -30,6 +30,18 @@ runs: - uses: ./.github/actions/e2e/install-eksctl with: eksctl_version: v0.147.0 + - name: delete-instance-profiles + shell: + run: | + for name in $(aws iam list-instance-profiles --query "InstanceProfiles[*].{Name:InstanceProfileName}" --output text); do + tags=$(aws iam list-instance-profile-tags --instance-profile-name $name --output json) + if [[ $(echo $tags | jq -r '.Tags[] | select(.Key == "testing.karpenter.sh/discovery") | .Value') == "${{ inputs.cluster_name }}" ]]; then + roleName=$(aws iam get-instance-profile --instance-profile-name $name --query "InstanceProfile.Roles[*].{Name:RoleName}" --output text) + aws iam remove-role-from-instance-profile --instance-profile-name $name --role-name $roleName + aws iam delete-instance-profile --instance-profile-name $name + break + fi + done - name: delete-cluster shell: bash run: | diff --git a/pkg/cloudprovider/drift.go b/pkg/cloudprovider/drift.go index d98e8fbc81f4..02afdf7a401f 100644 --- a/pkg/cloudprovider/drift.go +++ b/pkg/cloudprovider/drift.go @@ -48,7 +48,7 @@ func (c *CloudProvider) isNodeTemplateDrifted(ctx context.Context, machine *v1al return false, fmt.Errorf("calculating subnet drift, %w", err) } - return amiDrifted || securitygroupDrifted || subnetDrifted, nil + return amiDrifted || securitygroupDrifted || subnetDrifted || c.areStaticFieldsDrifted(machine, nodeTemplate), nil } func (c *CloudProvider) isAMIDrifted(ctx context.Context, machine *v1alpha5.Machine, provisioner *v1alpha5.Provisioner, @@ -106,6 +106,16 @@ func (c *CloudProvider) areSecurityGroupsDrifted(ec2Instance *instance.Instance, return !securityGroupIds.Equal(sets.New(ec2Instance.SecurityGroupIDs...)), nil } +func (c *CloudProvider) areStaticFieldsDrifted(machine *v1alpha5.Machine, nodeTemplate *v1alpha1.AWSNodeTemplate) bool { + nodeTemplateHash, foundHashNodeTemplate := nodeTemplate.ObjectMeta.Annotations[v1alpha1.AnnotationNodeTemplateHash] + machineHash, foundHashMachine := machine.ObjectMeta.Annotations[v1alpha1.AnnotationNodeTemplateHash] + if !foundHashNodeTemplate || !foundHashMachine { + return false + } + + return nodeTemplateHash != machineHash +} + func (c *CloudProvider) getInstance(ctx context.Context, providerID string) (*instance.Instance, error) { // Get InstanceID to fetch from EC2 instanceID, err := utils.ParseInstanceID(providerID) diff --git a/pkg/cloudprovider/suite_test.go b/pkg/cloudprovider/suite_test.go index 8fc6c348ae0d..c806db39a257 100644 --- a/pkg/cloudprovider/suite_test.go +++ b/pkg/cloudprovider/suite_test.go @@ -224,6 +224,7 @@ var _ = Describe("CloudProvider", func() { var validSecurityGroup string var selectedInstanceType *corecloudproivder.InstanceType var instance *ec2.Instance + var machine *v1alpha5.Machine var validSubnet1 string var validSubnet2 string BeforeEach(func() { @@ -235,12 +236,14 @@ var _ = Describe("CloudProvider", func() { Parameter: &ssm.Parameter{Value: aws.String(validAMI)}, } awsEnv.EC2API.DescribeImagesOutput.Set(&ec2.DescribeImagesOutput{ - Images: []*ec2.Image{{ - Name: aws.String(coretest.RandomName()), - ImageId: aws.String(validAMI), - Architecture: aws.String("x86_64"), - CreationDate: aws.String("2022-08-15T12:00:00Z"), - }}, + Images: []*ec2.Image{ + { + Name: aws.String(coretest.RandomName()), + ImageId: aws.String(validAMI), + Architecture: aws.String("arm64"), + CreationDate: aws.String("2022-08-15T12:00:00Z"), + }, + }, }) nodeTemplate.Status.SecurityGroups = []v1alpha1.SecurityGroup{ { @@ -281,10 +284,11 @@ var _ = Describe("CloudProvider", func() { awsEnv.EC2API.DescribeInstancesBehavior.Output.Set(&ec2.DescribeInstancesOutput{ Reservations: []*ec2.Reservation{{Instances: []*ec2.Instance{instance}}}, }) - }) - It("should not fail if node template does not exist", func() { - ExpectDeleted(ctx, env.Client, nodeTemplate) - machine := coretest.Machine(v1alpha5.Machine{ + nodeTemplateHash := nodeTemplate.Hash() + nodeTemplate.Annotations = lo.Assign(nodeTemplate.Annotations, map[string]string{ + v1alpha1.AnnotationNodeTemplateHash: nodeTemplateHash, + }) + machine = coretest.Machine(v1alpha5.Machine{ Status: v1alpha5.MachineStatus{ ProviderID: fake.ProviderID(lo.FromPtr(instance.InstanceId)), }, @@ -293,8 +297,14 @@ var _ = Describe("CloudProvider", func() { v1alpha5.ProvisionerNameLabelKey: provisioner.Name, v1.LabelInstanceTypeStable: selectedInstanceType.Name, }, + Annotations: map[string]string{ + v1alpha1.AnnotationNodeTemplateHash: nodeTemplateHash, + }, }, }) + }) + It("should not fail if node template does not exist", func() { + ExpectDeleted(ctx, env.Client, nodeTemplate) drifted, err := cloudProvider.IsMachineDrifted(ctx, machine) Expect(err).ToNot(HaveOccurred()) Expect(drifted).To(BeFalse()) @@ -302,50 +312,17 @@ var _ = Describe("CloudProvider", func() { It("should return false if providerRef is not defined", func() { provisioner.Spec.ProviderRef = nil ExpectApplied(ctx, env.Client, provisioner) - machine := coretest.Machine(v1alpha5.Machine{ - Status: v1alpha5.MachineStatus{ - ProviderID: fake.ProviderID(lo.FromPtr(instance.InstanceId)), - }, - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{ - v1alpha5.ProvisionerNameLabelKey: provisioner.Name, - v1.LabelInstanceTypeStable: selectedInstanceType.Name, - }, - }, - }) drifted, err := cloudProvider.IsMachineDrifted(ctx, machine) Expect(err).ToNot(HaveOccurred()) Expect(drifted).To(BeFalse()) }) It("should not fail if provisioner does not exist", func() { ExpectDeleted(ctx, env.Client, provisioner) - machine := coretest.Machine(v1alpha5.Machine{ - Status: v1alpha5.MachineStatus{ - ProviderID: fake.ProviderID(lo.FromPtr(instance.InstanceId)), - }, - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{ - v1alpha5.ProvisionerNameLabelKey: provisioner.Name, - v1.LabelInstanceTypeStable: selectedInstanceType.Name, - }, - }, - }) drifted, err := cloudProvider.IsMachineDrifted(ctx, machine) Expect(err).ToNot(HaveOccurred()) Expect(drifted).To(BeFalse()) }) It("should return drifted if the AMI is not valid", func() { - machine := coretest.Machine(v1alpha5.Machine{ - Status: v1alpha5.MachineStatus{ - ProviderID: fake.ProviderID(lo.FromPtr(instance.InstanceId)), - }, - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{ - v1alpha5.ProvisionerNameLabelKey: provisioner.Name, - v1.LabelInstanceTypeStable: selectedInstanceType.Name, - }, - }, - }) // Instance is a reference to what we return in the GetInstances call instance.ImageId = aws.String(fake.ImageID()) isDrifted, err := cloudProvider.IsMachineDrifted(ctx, machine) @@ -353,68 +330,24 @@ var _ = Describe("CloudProvider", func() { Expect(isDrifted).To(BeTrue()) }) It("should return drifted if the subnet is not valid", func() { - machine := coretest.Machine(v1alpha5.Machine{ - Status: v1alpha5.MachineStatus{ - ProviderID: fake.ProviderID(lo.FromPtr(instance.InstanceId)), - }, - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{ - v1alpha5.ProvisionerNameLabelKey: provisioner.Name, - v1.LabelInstanceTypeStable: selectedInstanceType.Name, - }, - }, - }) instance.SubnetId = aws.String(fake.SubnetID()) isDrifted, err := cloudProvider.IsMachineDrifted(ctx, machine) Expect(err).ToNot(HaveOccurred()) Expect(isDrifted).To(BeTrue()) }) It("should return an error if AWSNodeTemplate subnets are empty", func() { - machine := coretest.Machine(v1alpha5.Machine{ - Status: v1alpha5.MachineStatus{ - ProviderID: fake.ProviderID(lo.FromPtr(instance.InstanceId)), - }, - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{ - v1alpha5.ProvisionerNameLabelKey: provisioner.Name, - v1.LabelInstanceTypeStable: selectedInstanceType.Name, - }, - }, - }) nodeTemplate.Status.Subnets = []v1alpha1.Subnet{} ExpectApplied(ctx, env.Client, nodeTemplate) _, err := cloudProvider.IsMachineDrifted(ctx, machine) Expect(err).To(HaveOccurred()) }) It("should not return drifted if the machine is valid", func() { - machine := coretest.Machine(v1alpha5.Machine{ - Status: v1alpha5.MachineStatus{ - ProviderID: fake.ProviderID(lo.FromPtr(instance.InstanceId)), - }, - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{ - v1alpha5.ProvisionerNameLabelKey: provisioner.Name, - v1.LabelInstanceTypeStable: selectedInstanceType.Name, - }, - }, - }) isDrifted, err := cloudProvider.IsMachineDrifted(ctx, machine) Expect(err).ToNot(HaveOccurred()) Expect(isDrifted).To(BeFalse()) }) It("should return an error if the AWSNodeTemplate securitygroup are empty", func() { nodeTemplate.Status.SecurityGroups = []v1alpha1.SecurityGroup{} - machine := coretest.Machine(v1alpha5.Machine{ - Status: v1alpha5.MachineStatus{ - ProviderID: fake.ProviderID(lo.FromPtr(instance.InstanceId)), - }, - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{ - v1alpha5.ProvisionerNameLabelKey: provisioner.Name, - v1.LabelInstanceTypeStable: selectedInstanceType.Name, - }, - }, - }) ExpectApplied(ctx, env.Client, nodeTemplate) // Instance is a reference to what we return in the GetInstances call instance.SecurityGroups = []*ec2.GroupIdentifier{{GroupId: aws.String(fake.SecurityGroupID())}} @@ -422,17 +355,6 @@ var _ = Describe("CloudProvider", func() { Expect(err).To(HaveOccurred()) }) It("should return drifted if the instance securitygroup do not match the AWSNodeTemplateStatus", func() { - machine := coretest.Machine(v1alpha5.Machine{ - Status: v1alpha5.MachineStatus{ - ProviderID: fake.ProviderID(lo.FromPtr(instance.InstanceId)), - }, - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{ - v1alpha5.ProvisionerNameLabelKey: provisioner.Name, - v1.LabelInstanceTypeStable: selectedInstanceType.Name, - }, - }, - }) // Instance is a reference to what we return in the GetInstances call instance.SecurityGroups = []*ec2.GroupIdentifier{{GroupId: aws.String(fake.SecurityGroupID())}} isDrifted, err := cloudProvider.IsMachineDrifted(ctx, machine) @@ -440,17 +362,6 @@ var _ = Describe("CloudProvider", func() { Expect(isDrifted).To(BeTrue()) }) It("should return drifted if there are more instance securitygroups are present than AWSNodeTemplate Status", func() { - machine := coretest.Machine(v1alpha5.Machine{ - Status: v1alpha5.MachineStatus{ - ProviderID: fake.ProviderID(lo.FromPtr(instance.InstanceId)), - }, - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{ - v1alpha5.ProvisionerNameLabelKey: provisioner.Name, - v1.LabelInstanceTypeStable: selectedInstanceType.Name, - }, - }, - }) // Instance is a reference to what we return in the GetInstances call instance.SecurityGroups = []*ec2.GroupIdentifier{{GroupId: aws.String(fake.SecurityGroupID())}, {GroupId: aws.String(validSecurityGroup)}} isDrifted, err := cloudProvider.IsMachineDrifted(ctx, machine) @@ -458,17 +369,6 @@ var _ = Describe("CloudProvider", func() { Expect(isDrifted).To(BeTrue()) }) It("should return drifted if more AWSNodeTemplate securitygroups are present than instance securitygroups", func() { - machine := coretest.Machine(v1alpha5.Machine{ - Status: v1alpha5.MachineStatus{ - ProviderID: fake.ProviderID(lo.FromPtr(instance.InstanceId)), - }, - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{ - v1alpha5.ProvisionerNameLabelKey: provisioner.Name, - v1.LabelInstanceTypeStable: selectedInstanceType.Name, - }, - }, - }) nodeTemplate.Status.SecurityGroups = []v1alpha1.SecurityGroup{ { ID: validSecurityGroup, @@ -485,17 +385,6 @@ var _ = Describe("CloudProvider", func() { Expect(isDrifted).To(BeTrue()) }) It("should not return drifted if launchTemplateName is defined", func() { - machine := coretest.Machine(v1alpha5.Machine{ - Status: v1alpha5.MachineStatus{ - ProviderID: fake.ProviderID(lo.FromPtr(instance.InstanceId)), - }, - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{ - v1alpha5.ProvisionerNameLabelKey: provisioner.Name, - v1.LabelInstanceTypeStable: selectedInstanceType.Name, - }, - }, - }) nodeTemplate.Spec.LaunchTemplateName = aws.String("validLaunchTemplateName") nodeTemplate.Spec.SecurityGroupSelector = nil nodeTemplate.Status.SecurityGroups = nil @@ -504,44 +393,19 @@ var _ = Describe("CloudProvider", func() { Expect(isDrifted).To(BeFalse()) }) It("should not return drifted if the securitygroups match", func() { - machine := coretest.Machine(v1alpha5.Machine{ - Status: v1alpha5.MachineStatus{ - ProviderID: fake.ProviderID(lo.FromPtr(instance.InstanceId)), - }, - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{ - v1alpha5.ProvisionerNameLabelKey: provisioner.Name, - v1.LabelInstanceTypeStable: selectedInstanceType.Name, - }, - }, - }) isDrifted, err := cloudProvider.IsMachineDrifted(ctx, machine) Expect(err).ToNot(HaveOccurred()) Expect(isDrifted).To(BeFalse()) }) It("should error if the machine doesn't have the instance-type label", func() { - machine := coretest.Machine(v1alpha5.Machine{ - Status: v1alpha5.MachineStatus{ - ProviderID: fake.ProviderID(lo.FromPtr(instance.InstanceId)), - }, - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{ - v1alpha5.ProvisionerNameLabelKey: provisioner.Name, - }, - }, - }) + machine.Labels = map[string]string{ + v1alpha5.ProvisionerNameLabelKey: provisioner.Name, + } _, err := cloudProvider.IsMachineDrifted(ctx, machine) Expect(err).To(HaveOccurred()) }) It("should error drift if machine doesn't have provider id", func() { - machine := coretest.Machine(v1alpha5.Machine{ - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{ - v1alpha5.ProvisionerNameLabelKey: provisioner.Name, - v1.LabelInstanceTypeStable: selectedInstanceType.Name, - }, - }, - }) + machine.Status = v1alpha5.MachineStatus{} isDrifted, err := cloudProvider.IsMachineDrifted(ctx, machine) Expect(err).To(HaveOccurred()) Expect(isDrifted).To(BeFalse()) @@ -550,20 +414,76 @@ var _ = Describe("CloudProvider", func() { awsEnv.EC2API.DescribeInstancesBehavior.Output.Set(&ec2.DescribeInstancesOutput{ Reservations: []*ec2.Reservation{{Instances: []*ec2.Instance{}}}, }) - machine := coretest.Machine(v1alpha5.Machine{ - Status: v1alpha5.MachineStatus{ - ProviderID: fake.ProviderID(lo.FromPtr(instance.InstanceId)), - }, - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{ - v1alpha5.ProvisionerNameLabelKey: provisioner.Name, - v1.LabelInstanceTypeStable: selectedInstanceType.Name, - }, - }, - }) _, err := cloudProvider.IsMachineDrifted(ctx, machine) Expect(err).To(HaveOccurred()) }) + Context("Static Drift Detection", func() { + BeforeEach(func() { + provisioner = test.Provisioner(coretest.ProvisionerOptions{ + ProviderRef: &v1alpha5.MachineTemplateRef{Kind: nodeTemplate.Kind, Name: nodeTemplate.Name}, + }) + machine.ObjectMeta.Labels = lo.Assign(machine.ObjectMeta.Labels, map[string]string{ + v1alpha5.ProvisionerNameLabelKey: provisioner.Name, + }) + }) + DescribeTable("should return drifted if the AWSNodeTemplate.Spec is updated", + func(awsnodetemplatespec v1alpha1.AWSNodeTemplateSpec) { + ExpectApplied(ctx, env.Client, provisioner, nodeTemplate) + isDrifted, err := cloudProvider.IsMachineDrifted(ctx, machine) + Expect(err).NotTo(HaveOccurred()) + Expect(isDrifted).To(BeFalse()) + + updatedAWSNodeTemplate := test.AWSNodeTemplate(*nodeTemplate.Spec.DeepCopy(), awsnodetemplatespec) + updatedAWSNodeTemplate.ObjectMeta = nodeTemplate.ObjectMeta + updatedAWSNodeTemplate.Status = nodeTemplate.Status + updatedAWSNodeTemplate.Annotations = map[string]string{v1alpha1.AnnotationNodeTemplateHash: updatedAWSNodeTemplate.Hash()} + + ExpectApplied(ctx, env.Client, updatedAWSNodeTemplate) + isDrifted, err = cloudProvider.IsMachineDrifted(ctx, machine) + Expect(err).NotTo(HaveOccurred()) + Expect(isDrifted).To(BeTrue()) + }, + Entry("InstanceProfile Drift", v1alpha1.AWSNodeTemplateSpec{AWS: v1alpha1.AWS{InstanceProfile: aws.String("profile-2")}}), + Entry("UserData Drift", v1alpha1.AWSNodeTemplateSpec{UserData: aws.String("userdata-test-2")}), + Entry("Tags Drift", v1alpha1.AWSNodeTemplateSpec{AWS: v1alpha1.AWS{Tags: map[string]string{"keyTag-test-3": "valueTag-test-3"}}}), + Entry("MetadataOptions Drift", v1alpha1.AWSNodeTemplateSpec{AWS: v1alpha1.AWS{LaunchTemplate: v1alpha1.LaunchTemplate{MetadataOptions: &v1alpha1.MetadataOptions{HTTPEndpoint: aws.String("test-metadata-2")}}}}), + Entry("BlockDeviceMappings Drift", v1alpha1.AWSNodeTemplateSpec{AWS: v1alpha1.AWS{LaunchTemplate: v1alpha1.LaunchTemplate{BlockDeviceMappings: []*v1alpha1.BlockDeviceMapping{{DeviceName: aws.String("map-device-test-3")}}}}}), + Entry("Context Drift", v1alpha1.AWSNodeTemplateSpec{AWS: v1alpha1.AWS{Context: aws.String("context-2")}}), + Entry("DetailedMonitoring Drift", v1alpha1.AWSNodeTemplateSpec{DetailedMonitoring: aws.Bool(true)}), + Entry("AMIFamily Drift", v1alpha1.AWSNodeTemplateSpec{AWS: v1alpha1.AWS{AMIFamily: aws.String(v1alpha1.AMIFamilyBottlerocket)}}), + ) + DescribeTable("should not return drifted if dynamic felids are updated", + func(awsnodetemplatespec v1alpha1.AWSNodeTemplateSpec) { + ExpectApplied(ctx, env.Client, provisioner, nodeTemplate) + isDrifted, err := cloudProvider.IsMachineDrifted(ctx, machine) + Expect(err).NotTo(HaveOccurred()) + Expect(isDrifted).To(BeFalse()) + + updatedAWSNodeTemplate := test.AWSNodeTemplate(*nodeTemplate.Spec.DeepCopy(), awsnodetemplatespec) + updatedAWSNodeTemplate.ObjectMeta = nodeTemplate.ObjectMeta + updatedAWSNodeTemplate.Status = nodeTemplate.Status + updatedAWSNodeTemplate.Annotations = map[string]string{v1alpha1.AnnotationNodeTemplateHash: updatedAWSNodeTemplate.Hash()} + + ExpectApplied(ctx, env.Client, updatedAWSNodeTemplate) + isDrifted, err = cloudProvider.IsMachineDrifted(ctx, machine) + Expect(err).NotTo(HaveOccurred()) + Expect(isDrifted).To(BeFalse()) + }, + Entry("AMISelector Drift", v1alpha1.AWSNodeTemplateSpec{AMISelector: map[string]string{"aws::ids": validAMI}}), + Entry("SubnetSelector Drift", v1alpha1.AWSNodeTemplateSpec{AWS: v1alpha1.AWS{SubnetSelector: map[string]string{"aws-ids": "subnet-test1"}}}), + Entry("SecurityGroupSelector Drift", v1alpha1.AWSNodeTemplateSpec{AWS: v1alpha1.AWS{SecurityGroupSelector: map[string]string{"sg-key": "sg-value"}}}), + ) + It("should not return drifted if karpenter.k8s.aws/nodetemplate-hash annotation is not present on the machine", func() { + machine.Annotations = map[string]string{} + nodeTemplate.Spec.Tags = map[string]string{ + "Test Key": "Test Value", + } + ExpectApplied(ctx, env.Client, provisioner, nodeTemplate) + isDrifted, err := cloudProvider.IsMachineDrifted(ctx, machine) + Expect(err).NotTo(HaveOccurred()) + Expect(isDrifted).To(BeFalse()) + }) + }) }) Context("Provider Backwards Compatibility", func() { It("should launch a machine using provider defaults", func() { diff --git a/test/cloudformation/iam_cloudformation.yaml b/test/cloudformation/iam_cloudformation.yaml index e82b983e59bd..9e828d58f90e 100644 --- a/test/cloudformation/iam_cloudformation.yaml +++ b/test/cloudformation/iam_cloudformation.yaml @@ -76,16 +76,24 @@ Resources: - Effect: Allow Action: iam:PassRole Resource: !Sub "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/eksctl-*" + - Effect: Allow + Action: + - iam:ListInstanceProfiles + - iam:ListInstanceProfileTags + Resource: + - !Sub "arn:${AWS::Partition}:iam::${AWS::AccountId}:instance-profile/*" - Effect: Allow Action: - iam:AddRoleToInstanceProfile - iam:CreateInstanceProfile + - iam:TagInstanceProfile - iam:RemoveRoleFromInstanceProfile - iam:DeleteInstanceProfile - iam:GetInstanceProfile Resource: - !Sub "arn:${AWS::Partition}:iam::${AWS::AccountId}:instance-profile/KarpenterNodeInstanceProfile-*" - !Sub "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/KarpenterNodeRole-*" + - !Sub "arn:${AWS::Partition}:iam::${AWS::AccountId}:instance-profile/KarpenterNodeInstanceProfile-Drift-*" - Effect: Allow Action: - iam:CreateOpenIDConnectProvider diff --git a/test/pkg/environment/common/environment.go b/test/pkg/environment/common/environment.go index fb5a4b62c985..2c9c2581d1b0 100644 --- a/test/pkg/environment/common/environment.go +++ b/test/pkg/environment/common/environment.go @@ -43,6 +43,8 @@ type ContextKey string const ( GitRefContextKey = ContextKey("gitRef") + TestingDiscovery = "testing.karpenter.sh/id" + TestingType = "testing.karpenter.sh/type" ) type Environment struct { diff --git a/test/suites/drift/suite_test.go b/test/suites/drift/suite_test.go index 7c86d3adb1c5..910e8c21ec4f 100644 --- a/test/suites/drift/suite_test.go +++ b/test/suites/drift/suite_test.go @@ -25,12 +25,14 @@ import ( . "github.com/onsi/gomega" "github.com/samber/lo" v1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" awssdk "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ec2" "github.com/aws/aws-sdk-go/service/eks" + "github.com/aws/aws-sdk-go/service/iam" "github.com/aws/aws-sdk-go/service/ssm" "github.com/aws/karpenter-core/pkg/apis/v1alpha5" @@ -39,6 +41,7 @@ import ( "github.com/aws/karpenter/pkg/apis/v1alpha1" awstest "github.com/aws/karpenter/pkg/test" "github.com/aws/karpenter/test/pkg/environment/aws" + "github.com/aws/karpenter/test/pkg/environment/common" ) var env *aws.Environment @@ -171,6 +174,10 @@ var _ = Describe("Drift", Label("AWS"), func() { Key: awssdk.String("karpenter.sh/discovery"), Value: awssdk.String(settings.FromContext(env.Context).ClusterName), }, + { + Key: awssdk.String(common.TestingDiscovery), + Value: awssdk.String(settings.FromContext(env.Context).ClusterName), + }, }, }, }, @@ -270,7 +277,7 @@ var _ = Describe("Drift", Label("AWS"), func() { env.ExpectUpdated(pod) env.EventuallyExpectNotFound(pod, node) }) - Describe("Provisioner Drift", func() { + Describe("Static Drift", func() { var pod *v1.Pod var nodeTemplate *v1alpha1.AWSNodeTemplate var provisioner *v1alpha5.Provisioner @@ -289,7 +296,7 @@ var _ = Describe("Drift", Label("AWS"), func() { }, }) }) - DescribeTable("provisioner static drift", func(fieldName string, provisionerOption test.ProvisionerOptions) { + DescribeTable("Provisioner Drift", func(fieldName string, provisionerOption test.ProvisionerOptions) { provisionerOption.ObjectMeta = provisioner.ObjectMeta updatedProvisioner := test.Provisioner( test.ProvisionerOptions{ProviderRef: &v1alpha5.MachineTemplateRef{Name: nodeTemplate.Name}}, @@ -335,5 +342,71 @@ var _ = Describe("Drift", Label("AWS"), func() { }}), Entry("Start-up Taints Drift", "Start-up Taint", test.ProvisionerOptions{StartupTaints: []v1.Taint{{Key: "example.com/another-taint-2", Effect: v1.TaintEffectPreferNoSchedule}}}), ) + DescribeTable("AWSNodeTemplate Drift", func(fieldName string, nodeTemplateSpec v1alpha1.AWSNodeTemplateSpec) { + if fieldName == "InstanceProfile" { + ExpectInstanceProfileCreated(nodeTemplateSpec) + } + + updatedAWSNodeTemplate := awstest.AWSNodeTemplate(*nodeTemplate.Spec.DeepCopy(), nodeTemplateSpec) + updatedAWSNodeTemplate.ObjectMeta = nodeTemplate.ObjectMeta + updatedAWSNodeTemplate.Annotations = map[string]string{v1alpha1.AnnotationNodeTemplateHash: updatedAWSNodeTemplate.Hash()} + + env.ExpectCreated(pod, nodeTemplate, provisioner) + machine := env.EventuallyExpectCreatedMachineCount("==", 1)[0] + node := env.EventuallyExpectCreatedNodeCount("==", 1)[0] + env.EventuallyExpectHealthy(pod) + + env.ExpectCreatedOrUpdated(updatedAWSNodeTemplate) + + By("validating the drifted status condition has propagated") + EventuallyWithOffset(1, func(g Gomega) { + g.Expect(env.Client.Get(env, client.ObjectKeyFromObject(machine), machine)).To(Succeed()) + g.Expect(machine.StatusConditions().GetCondition(v1alpha5.MachineDrifted)).ToNot(BeNil()) + g.Expect(machine.StatusConditions().GetCondition(v1alpha5.MachineDrifted).IsTrue()).To(BeTrue()) + }).Should(Succeed()) + + delete(pod.Annotations, v1alpha5.DoNotEvictPodAnnotationKey) + env.ExpectUpdated(pod) + env.EventuallyExpectNotFound(pod, node) + }, + Entry("InstanceProfile Drift", "InstanceProfile", v1alpha1.AWSNodeTemplateSpec{}), + Entry("UserData Drift", "UserData", v1alpha1.AWSNodeTemplateSpec{UserData: awssdk.String("#!/bin/bash\n/etc/eks/bootstrap.sh")}), + Entry("Tags Drift", "Tags", v1alpha1.AWSNodeTemplateSpec{AWS: v1alpha1.AWS{Tags: map[string]string{"keyTag-test-3": "valueTag-test-3"}}}), + Entry("MetadataOptions Drift", "MetadataOptions", v1alpha1.AWSNodeTemplateSpec{AWS: v1alpha1.AWS{LaunchTemplate: v1alpha1.LaunchTemplate{MetadataOptions: &v1alpha1.MetadataOptions{HTTPPutResponseHopLimit: awssdk.Int64(1)}}}}), + Entry("BlockDeviceMappings Drift", "BlockDeviceMappings", v1alpha1.AWSNodeTemplateSpec{AWS: v1alpha1.AWS{LaunchTemplate: v1alpha1.LaunchTemplate{BlockDeviceMappings: []*v1alpha1.BlockDeviceMapping{ + { + DeviceName: awssdk.String("/dev/xvda"), + EBS: &v1alpha1.BlockDevice{ + VolumeSize: resource.NewScaledQuantity(20, resource.Giga), + VolumeType: awssdk.String("gp3"), + Encrypted: awssdk.Bool(true), + }, + }}}}}), + Entry("DetailedMonitoring Drift", "DetailedMonitoring", v1alpha1.AWSNodeTemplateSpec{DetailedMonitoring: awssdk.Bool(true)}), + Entry("AMIFamily Drift", "AMIFamily", v1alpha1.AWSNodeTemplateSpec{AWS: v1alpha1.AWS{AMIFamily: awssdk.String(v1alpha1.AMIFamilyBottlerocket)}}), + ) }) }) + +func ExpectInstanceProfileCreated(ants v1alpha1.AWSNodeTemplateSpec) { + By("creating an instance profile") + ants.AWS.InstanceProfile = awssdk.String(fmt.Sprintf("KarpenterNodeInstanceProfile-Drift-%s", settings.FromContext(env.Context).ClusterName)) + createInstanceProfile := &iam.CreateInstanceProfileInput{ + InstanceProfileName: ants.AWS.InstanceProfile, + Tags: []*iam.Tag{ + { + Key: awssdk.String(common.TestingDiscovery), + Value: awssdk.String(settings.FromContext(env.Context).ClusterName), + }, + }, + } + By("adding the karpenter role to new instance profile") + _, err := env.IAMAPI.CreateInstanceProfile(createInstanceProfile) + Expect(err).ToNot(HaveOccurred()) + addInstanceProfile := &iam.AddRoleToInstanceProfileInput{ + InstanceProfileName: ants.AWS.InstanceProfile, + RoleName: awssdk.String(fmt.Sprintf("KarpenterNodeRole-%s", settings.FromContext(env.Context).ClusterName)), + } + _, err = env.IAMAPI.AddRoleToInstanceProfile(addInstanceProfile) + Expect(err).ToNot(HaveOccurred()) +} diff --git a/website/content/en/preview/concepts/deprovisioning.md b/website/content/en/preview/concepts/deprovisioning.md index 47a765a6e322..89be26dc6682 100644 --- a/website/content/en/preview/concepts/deprovisioning.md +++ b/website/content/en/preview/concepts/deprovisioning.md @@ -171,16 +171,14 @@ Read the [Drift Design](https://github.com/aws/karpenter-core/pull/366/files) fo |----------------------------| :---: | :---: | :---: | :---: | | Subnet Selector | | x | | x | | Security Group Selector | | x | | x | -| Instance Profile | x | | | | - AMI Family | x | | | | +| Instance Profile | x | | | x | +| AMI Family | x | | | x | | AMI Selector | | x | | x | -| UserData | x | | | | -| Tags | x | | | | -| Metadata Options | x | | | | -| Block Device Mappings | x | | | | -| Detailed Monitoring | x | | | | -| | | | | | - +| UserData | x | | | x | +| Tags | x | | | x | +| Metadata Options | x | | | x | +| Block Device Mappings | x | | | x | +| Detailed Monitoring | x | | | x | To enable the drift feature flag, refer to the [Settings Feature Gates]({{}}). diff --git a/website/content/en/preview/upgrade-guide.md b/website/content/en/preview/upgrade-guide.md index fe609d8e960f..8c63de50c308 100644 --- a/website/content/en/preview/upgrade-guide.md +++ b/website/content/en/preview/upgrade-guide.md @@ -102,6 +102,9 @@ Snapshot releases are tagged with the git commit hash prefixed by the Karpenter ## Released Upgrade Notes +### Upgrading to v0.30.0+ + +* Karpenter will now [statically drift]({{}}) on both Provisioner and AWSNodeTemplate Fields. For Provisioner Static Drift, the `karpenter.sh/provisioner-hash` annotation must be present on both the Provisioner and Machine. For AWSNodeTemplate drift, the `karpenter.k8s.aws/nodetemplate-hash` annotation must be present on the AWSNodeTemplate and Machine. Karpenter will not add these annotations to pre-existing nodes, so each of these nodes will need to be recycled one time for the annotations to be added. ### Upgrading to v0.29.0+ {{% alert title="Warning" color="warning" %}}