Skip to content

Commit

Permalink
Fix hash volume size quantity value
Browse files Browse the repository at this point in the history
  • Loading branch information
jonathan-innis committed Mar 8, 2024
1 parent 950be07 commit 7d52a31
Show file tree
Hide file tree
Showing 3 changed files with 75 additions and 3 deletions.
2 changes: 1 addition & 1 deletion pkg/apis/v1beta1/ec2nodeclass.go
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,7 @@ type BlockDevice struct {
// +kubebuilder:validation:Pattern:="^((?:[1-9][0-9]{0,3}|[1-4][0-9]{4}|[5][0-8][0-9]{3}|59000)Gi|(?:[1-9][0-9]{0,3}|[1-5][0-9]{4}|[6][0-3][0-9]{3}|64000)G|([1-9]||[1-5][0-7]|58)Ti|([1-9]||[1-5][0-9]|6[0-3]|64)T)$"
// +kubebuilder:validation:XIntOrString
// +optional
VolumeSize *resource.Quantity `json:"volumeSize,omitempty"`
VolumeSize *resource.Quantity `json:"volumeSize,omitempty" hash:"string"`
// VolumeType of the block device.
// For more information, see Amazon EBS volume types (https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/EBSVolumeTypes.html)
// in the Amazon Elastic Compute Cloud User Guide.
Expand Down
42 changes: 41 additions & 1 deletion pkg/apis/v1beta1/ec2nodeclass_hash_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"github.com/aws/aws-sdk-go/aws"
"github.com/imdario/mergo"
"github.com/samber/lo"
"sigs.k8s.io/karpenter/pkg/utils/resources"

"github.com/aws/karpenter-provider-aws/pkg/apis/v1beta1"
"github.com/aws/karpenter-provider-aws/pkg/test"
Expand Down Expand Up @@ -66,7 +67,21 @@ var _ = Describe("Hash", func() {
Entry("UserData Drift", "588756456110800812", v1beta1.EC2NodeClass{Spec: v1beta1.EC2NodeClassSpec{UserData: aws.String("userdata-test-2")}}),
Entry("Tags Drift", "2471764681523766508", v1beta1.EC2NodeClass{Spec: v1beta1.EC2NodeClassSpec{Tags: map[string]string{"keyTag-test-3": "valueTag-test-3"}}}),
Entry("MetadataOptions Drift", "11030161632375731908", v1beta1.EC2NodeClass{Spec: v1beta1.EC2NodeClassSpec{MetadataOptions: &v1beta1.MetadataOptions{HTTPEndpoint: aws.String("test-metadata-2")}}}),
Entry("BlockDeviceMappings Drift", "436753305915039702", v1beta1.EC2NodeClass{Spec: v1beta1.EC2NodeClassSpec{BlockDeviceMappings: []*v1beta1.BlockDeviceMapping{{DeviceName: aws.String("map-device-test-3")}}}}),
Entry("BlockDeviceMappings Drift", "2884646775270747279", v1beta1.EC2NodeClass{Spec: v1beta1.EC2NodeClassSpec{BlockDeviceMappings: []*v1beta1.BlockDeviceMapping{
{
DeviceName: aws.String("map-device-test-3"),
EBS: &v1beta1.BlockDevice{
DeleteOnTermination: aws.Bool(true),
Encrypted: aws.Bool(true),
IOPS: aws.Int64(1000),
KMSKeyID: aws.String("kmskeyid"),
SnapshotID: aws.String("snapshotid"),
Throughput: aws.Int64(1000),
VolumeSize: resources.Quantity("100Gi"),
VolumeType: aws.String("volumetype"),
},
},
}}}),
Entry("Context Drift", "3729470655588343019", v1beta1.EC2NodeClass{Spec: v1beta1.EC2NodeClassSpec{Context: aws.String("context-2")}}),
Entry("DetailedMonitoring Drift", "17892305444040067573", v1beta1.EC2NodeClass{Spec: v1beta1.EC2NodeClassSpec{DetailedMonitoring: aws.Bool(true)}}),
Entry("AMIFamily Drift", "9493798894326942407", v1beta1.EC2NodeClass{Spec: v1beta1.EC2NodeClassSpec{AMIFamily: aws.String(v1beta1.AMIFamilyBottlerocket)}}),
Expand Down Expand Up @@ -105,6 +120,31 @@ var _ = Describe("Hash", func() {
updatedHash := nodeClass.Hash()
Expect(hash).ToNot(Equal(updatedHash))
})
It("should change hash when volumeSize is updated on blockDeviceMapping", func() {
nodeClass.Spec.BlockDeviceMappings = []*v1beta1.BlockDeviceMapping{
{
DeviceName: aws.String("map-device-1"),
EBS: &v1beta1.BlockDevice{
VolumeSize: resources.Quantity("5Gi"),
},
},
{
DeviceName: aws.String("map-device-2"),
EBS: &v1beta1.BlockDevice{
VolumeSize: resources.Quantity("100Gi"),
},
},
}
hash := nodeClass.Hash()
nodeClass.Spec.BlockDeviceMappings[0].EBS.VolumeSize = resources.Quantity("10Gi")
updatedHash := nodeClass.Hash()
Expect(hash).ToNot(Equal(updatedHash))

hash = updatedHash
nodeClass.Spec.BlockDeviceMappings[1].EBS.VolumeSize = resources.Quantity("50Gi")
updatedHash = nodeClass.Hash()
Expect(hash).ToNot(Equal(updatedHash))
})
DescribeTable("should not change hash when slices are re-ordered", func(changes v1beta1.EC2NodeClass) {
hash := nodeClass.Hash()
Expect(mergo.Merge(nodeClass, changes, mergo.WithOverride)).To(Succeed())
Expand Down
34 changes: 33 additions & 1 deletion test/suites/drift/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import (
"k8s.io/apimachinery/pkg/util/intstr"
"k8s.io/apimachinery/pkg/util/sets"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/karpenter/pkg/utils/resources"

awssdk "github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/service/ec2"
Expand Down Expand Up @@ -680,7 +681,7 @@ var _ = Describe("Drift", func() {
{
DeviceName: awssdk.String("/dev/xvda"),
EBS: &v1beta1.BlockDevice{
VolumeSize: resource.NewScaledQuantity(20, resource.Giga),
VolumeSize: resources.Quantity("20Gi"),
VolumeType: awssdk.String("gp3"),
Encrypted: awssdk.Bool(true),
},
Expand Down Expand Up @@ -719,6 +720,37 @@ var _ = Describe("Drift", func() {
env.EventuallyExpectNotFound(pod, node)
env.EventuallyExpectHealthyPodCount(selector, numPods)
})
It("should drift the EC2NodeClass on BlockDeviceMappings volume size update", func() {
nodeClass.Spec.BlockDeviceMappings = []*v1beta1.BlockDeviceMapping{
{
DeviceName: awssdk.String("/dev/xvda"),
EBS: &v1beta1.BlockDevice{
VolumeSize: resources.Quantity("20Gi"),
VolumeType: awssdk.String("gp3"),
Encrypted: awssdk.Bool(true),
},
},
}
env.ExpectCreated(dep, nodeClass, nodePool)
pod := env.EventuallyExpectHealthyPodCount(selector, numPods)[0]
nodeClaim := env.EventuallyExpectCreatedNodeClaimCount("==", 1)[0]
node := env.ExpectCreatedNodeCount("==", 1)[0]

nodeClass.Spec.BlockDeviceMappings[0].EBS.VolumeSize = resources.Quantity("100Gi")
env.ExpectCreatedOrUpdated()

By("validating the drifted status condition has propagated")
Eventually(func(g Gomega) {
g.Expect(env.Client.Get(env, client.ObjectKeyFromObject(nodeClaim), nodeClaim)).To(Succeed())
g.Expect(nodeClaim.StatusConditions().GetCondition(corev1beta1.Drifted)).ToNot(BeNil())
g.Expect(nodeClaim.StatusConditions().GetCondition(corev1beta1.Drifted).IsTrue()).To(BeTrue())
}).Should(Succeed())

delete(pod.Annotations, corev1beta1.DoNotDisruptAnnotationKey)
env.ExpectUpdated(pod)
env.EventuallyExpectNotFound(pod, node)
env.EventuallyExpectHealthyPodCount(selector, numPods)
})
Context("Failure", func() {
It("should not continue to drift if a node never registers", func() {
// launch a new nodeClaim
Expand Down

0 comments on commit 7d52a31

Please sign in to comment.