Skip to content

Commit

Permalink
Update the AWSNodeTemplate Hash
Browse files Browse the repository at this point in the history
  • Loading branch information
engedaam committed Jul 25, 2023
1 parent 4cecea2 commit 825c846
Show file tree
Hide file tree
Showing 6 changed files with 127 additions and 7 deletions.
15 changes: 14 additions & 1 deletion pkg/apis/v1alpha1/awsnodetemplate.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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"`
Expand All @@ -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 {
Expand Down
10 changes: 5 additions & 5 deletions pkg/apis/v1alpha1/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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"`
Expand All @@ -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
Expand Down
1 change: 1 addition & 0 deletions pkg/apis/v1alpha1/register.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down
94 changes: 94 additions & 0 deletions pkg/apis/v1alpha1/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,18 +16,23 @@ 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"
"knative.dev/pkg/ptr"

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
Expand Down Expand Up @@ -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)))
}
})
})
})
6 changes: 5 additions & 1 deletion pkg/cloudprovider/cloudprovider.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 8 additions & 0 deletions pkg/cloudprovider/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down

0 comments on commit 825c846

Please sign in to comment.