diff --git a/pkg/apis/v1alpha1/awsnodetemplate.go b/pkg/apis/v1alpha1/awsnodetemplate.go index e4f2344bdb25..7722bdf8443b 100644 --- a/pkg/apis/v1alpha1/awsnodetemplate.go +++ b/pkg/apis/v1alpha1/awsnodetemplate.go @@ -15,6 +15,9 @@ limitations under the License. package v1alpha1 import ( + "fmt" + + "github.com/mitchellh/hashstructure/v2" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -79,7 +82,7 @@ type AWSNodeTemplateSpec struct { AWS `json:",inline"` // AMISelector discovers AMIs to be used by Amazon EC2 tags. // +optional - AMISelector map[string]string `json:"amiSelector,omitempty"` + AMISelector map[string]string `json:"amiSelector,omitempty" hash:"ignore"` // DetailedMonitoring controls if detailed monitoring is enabled for instances that are launched // +optional DetailedMonitoring *bool `json:"detailedMonitoring,omitempty"` @@ -97,6 +100,16 @@ type AWSNodeTemplate struct { Status AWSNodeTemplateStatus `json:"status,omitempty"` } +func (a *AWSNodeTemplate) Hash() string { + hash, _ := hashstructure.Hash(a.Spec, hashstructure.FormatV2, &hashstructure.HashOptions{ + SlicesAsSets: true, + IgnoreZeroValue: true, + ZeroNil: true, + }) + + return fmt.Sprint(hash) +} + // AWSNodeTemplateList contains a list of AWSNodeTemplate // +kubebuilder:object:root=true type AWSNodeTemplateList struct { diff --git a/pkg/apis/v1alpha1/provider.go b/pkg/apis/v1alpha1/provider.go index abc4c02e0a1b..d5737d522e79 100644 --- a/pkg/apis/v1alpha1/provider.go +++ b/pkg/apis/v1alpha1/provider.go @@ -24,10 +24,10 @@ import ( type AWS struct { // TypeMeta includes version and kind of the extensions, inferred if not provided. // +optional - metav1.TypeMeta `json:",inline"` + metav1.TypeMeta `json:",inline" hash:"ignore"` // AMIFamily is the AMI family that instances use. // +optional - AMIFamily *string `json:"amiFamily,omitempty"` + AMIFamily *string `json:"amiFamily,omitempty" hash:"ignore"` // Context is a Reserved field in EC2 APIs // https://docs.aws.amazon.com/AWSEC2/latest/APIReference/API_CreateFleet.html // +optional @@ -37,10 +37,10 @@ type AWS struct { InstanceProfile *string `json:"instanceProfile,omitempty"` // SubnetSelector discovers subnets by tags. A value of "" is a wildcard. // +optional - SubnetSelector map[string]string `json:"subnetSelector,omitempty"` + SubnetSelector map[string]string `json:"subnetSelector,omitempty" hash:"ignore"` // SecurityGroups specify the names of the security groups. // +optional - SecurityGroupSelector map[string]string `json:"securityGroupSelector,omitempty"` + SecurityGroupSelector map[string]string `json:"securityGroupSelector,omitempty" hash:"ignore"` // Tags to be applied on ec2 resources like instances and launch templates. // +optional Tags map[string]string `json:"tags,omitempty"` @@ -53,7 +53,7 @@ type LaunchTemplate struct { // NOTE: This field is for specifying a custom launch template and is exposed in the Spec // as `launchTemplate` for backwards compatibility. // +optional - LaunchTemplateName *string `json:"launchTemplate,omitempty"` + LaunchTemplateName *string `json:"launchTemplate,omitempty" hash:"ignore"` // MetadataOptions for the generated launch template of provisioned nodes. // // This specifies the exposure of the Instance Metadata Service to diff --git a/pkg/apis/v1alpha1/register.go b/pkg/apis/v1alpha1/register.go index e1543b4823b7..ea3cbd478016 100644 --- a/pkg/apis/v1alpha1/register.go +++ b/pkg/apis/v1alpha1/register.go @@ -107,6 +107,7 @@ var ( LabelInstanceAcceleratorName = LabelDomain + "/instance-accelerator-name" LabelInstanceAcceleratorManufacturer = LabelDomain + "/instance-accelerator-manufacturer" LabelInstanceAcceleratorCount = LabelDomain + "/instance-accelerator-count" + AnnotationNodeTemplateHash = LabelDomain + "/nodetemplate-hash" ) var ( diff --git a/pkg/apis/v1alpha1/suite_test.go b/pkg/apis/v1alpha1/suite_test.go index 024843a5433a..f4f0ec117ba2 100644 --- a/pkg/apis/v1alpha1/suite_test.go +++ b/pkg/apis/v1alpha1/suite_test.go @@ -16,10 +16,12 @@ package v1alpha1_test import ( "context" + "fmt" "strings" "testing" "github.com/Pallinder/go-randomdata" + "github.com/mitchellh/hashstructure/v2" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" . "knative.dev/pkg/logging/testing" @@ -27,7 +29,10 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/karpenter/pkg/apis/v1alpha1" + "github.com/aws/karpenter/pkg/test" ) var ctx context.Context @@ -115,4 +120,93 @@ var _ = Describe("Validation", func() { Expect(ant.Validate(ctx)).To(Not(Succeed())) }) }) + var _ = Describe("AWSNodeTemplate Hash", func() { + var awsnodetemplatespec v1alpha1.AWSNodeTemplateSpec + var awsnodetemplate *v1alpha1.AWSNodeTemplate + BeforeEach(func() { + awsnodetemplatespec = v1alpha1.AWSNodeTemplateSpec{ + AWS: v1alpha1.AWS{ + Context: aws.String("context-1"), + InstanceProfile: aws.String("profile-1"), + Tags: map[string]string{ + "keyTag-1": "valueTag-1", + "keyTag-2": "valueTag-2", + }, + LaunchTemplate: v1alpha1.LaunchTemplate{ + MetadataOptions: &v1alpha1.MetadataOptions{ + HTTPEndpoint: aws.String("test-metadata-1"), + }, + BlockDeviceMappings: []*v1alpha1.BlockDeviceMapping{ + { + DeviceName: aws.String("map-device-1"), + }, + { + DeviceName: aws.String("map-device-2"), + }, + }, + }, + }, + UserData: aws.String("userdata-test-1"), + DetailedMonitoring: aws.Bool(false), + } + awsnodetemplate = test.AWSNodeTemplate(awsnodetemplatespec) + }) + DescribeTable("should change hash when static fields are updated", func(awsnodetemplatespec v1alpha1.AWSNodeTemplateSpec) { + expectedHash := awsnodetemplate.Hash() + updatedAWSNodeTemplate := test.AWSNodeTemplate(*awsnodetemplatespec.DeepCopy(), awsnodetemplatespec) + actualHash := updatedAWSNodeTemplate.Hash() + Expect(actualHash).ToNot(Equal(fmt.Sprint(expectedHash))) + }, + 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)}), + ) + It("should not change hash when behavior/dynamic fields are updated", func() { + actualHash := awsnodetemplate.Hash() + + expectedHash, err := hashstructure.Hash(awsnodetemplate.Spec, hashstructure.FormatV2, &hashstructure.HashOptions{ + SlicesAsSets: true, + IgnoreZeroValue: true, + ZeroNil: true, + }) + Expect(err).ToNot(HaveOccurred()) + Expect(actualHash).To(Equal(fmt.Sprint(expectedHash))) + + // Update a behavior/dynamic field + awsnodetemplate.Spec.SubnetSelector = map[string]string{"subnet-test-key": "subnet-test-value"} + awsnodetemplate.Spec.SecurityGroupSelector = map[string]string{"sg-test-key": "sg-test-value"} + awsnodetemplate.Spec.AMIFamily = aws.String("test-family") + awsnodetemplate.Spec.AMISelector = map[string]string{"ami-test-key": "ami-test-value"} + + actualHash = awsnodetemplate.Hash() + Expect(err).ToNot(HaveOccurred()) + Expect(actualHash).To(Equal(fmt.Sprint(expectedHash))) + }) + It("should expect two provisioner with the same spec to have the same provisioner hash", func() { + awsnodetemplateTwo := &v1alpha1.AWSNodeTemplate{ + ObjectMeta: metav1.ObjectMeta{Name: strings.ToLower(randomdata.SillyName())}, + } + awsnodetemplateTwo.Spec = awsnodetemplatespec + + Expect(awsnodetemplate.Hash()).To(Equal(awsnodetemplateTwo.Hash())) + }) + It("should expect fields that are reordered to not produce a new hash", func() { + expectedHash := awsnodetemplate.Hash() + + // Change one static field for 5 provisioners + awsnodetemplatesFieldToChange := []*v1alpha1.AWSNodeTemplate{ + test.AWSNodeTemplate(*awsnodetemplatespec.DeepCopy(), v1alpha1.AWSNodeTemplateSpec{AWS: v1alpha1.AWS{Tags: map[string]string{"keyTag-2": "valueTag-2", "keyTag-1": "valueTag-1"}}}), + test.AWSNodeTemplate(*awsnodetemplatespec.DeepCopy(), v1alpha1.AWSNodeTemplateSpec{AWS: v1alpha1.AWS{LaunchTemplate: v1alpha1.LaunchTemplate{BlockDeviceMappings: []*v1alpha1.BlockDeviceMapping{{DeviceName: aws.String("map-device-2")}, {DeviceName: aws.String("map-device-1")}}}}}), + } + + for _, updatedAWSNodeTemplate := range awsnodetemplatesFieldToChange { + actualHash := updatedAWSNodeTemplate.Hash() + Expect(actualHash).To(Equal(fmt.Sprint(expectedHash))) + } + }) + }) }) diff --git a/pkg/cloudprovider/cloudprovider.go b/pkg/cloudprovider/cloudprovider.go index feab71384b1e..5a12e3ab46cd 100644 --- a/pkg/cloudprovider/cloudprovider.go +++ b/pkg/cloudprovider/cloudprovider.go @@ -101,7 +101,11 @@ func (c *CloudProvider) Create(ctx context.Context, machine *v1alpha5.Machine) ( instanceType, _ := lo.Find(instanceTypes, func(i *cloudprovider.InstanceType) bool { return i.Name == instance.Type }) - return c.instanceToMachine(instance, instanceType), nil + outputMachine := c.instanceToMachine(instance, instanceType) + outputMachine.Annotations = lo.Assign(machine.Annotations, map[string]string{ + v1alpha1.AnnotationNodeTemplateHash: nodeTemplate.Hash(), + }) + return outputMachine, nil } // Link adds a tag to the cloudprovider machine to tell the cloudprovider that it's now owned by a Machine diff --git a/pkg/cloudprovider/suite_test.go b/pkg/cloudprovider/suite_test.go index f0d3896c17d6..8fc6c348ae0d 100644 --- a/pkg/cloudprovider/suite_test.go +++ b/pkg/cloudprovider/suite_test.go @@ -161,6 +161,14 @@ var _ = Describe("CloudProvider", func() { Expect(corecloudproivder.IsInsufficientCapacityError(err)).To(BeTrue()) Expect(cloudProviderMachine).To(BeNil()) }) + It("should return AWSNodetemplate Hash on the machine", func() { + ExpectApplied(ctx, env.Client, provisioner, nodeTemplate, machine) + cloudProviderMachine, err := cloudProvider.Create(ctx, machine) + Expect(err).To(BeNil()) + Expect(cloudProviderMachine).ToNot(BeNil()) + _, ok := cloudProviderMachine.ObjectMeta.Annotations[v1alpha1.AnnotationNodeTemplateHash] + Expect(ok).To(BeTrue()) + }) Context("Defaulting", func() { // Intent here is that if updates occur on the provisioningController, the Provisioner doesn't need to be recreated It("should not set the InstanceProfile with the default if none provided in Provisioner", func() {