From 034596cb14c38c0b86732b88c10abf2cf7ddc534 Mon Sep 17 00:00:00 2001 From: Amanuel Engeda Date: Mon, 4 Dec 2023 17:54:00 -0800 Subject: [PATCH] Remove CDL on tags for Selector Terms --- pkg/utils/nodeclass/nodeclass.go | 71 ++++++++---- pkg/utils/nodeclass/suite_test.go | 187 ++++++++++++++++++++++++++++-- 2 files changed, 228 insertions(+), 30 deletions(-) diff --git a/pkg/utils/nodeclass/nodeclass.go b/pkg/utils/nodeclass/nodeclass.go index d8d34bdf7fca..f35122525d53 100644 --- a/pkg/utils/nodeclass/nodeclass.go +++ b/pkg/utils/nodeclass/nodeclass.go @@ -61,21 +61,23 @@ func NewSubnetSelectorTerms(subnetSelector map[string]string) (terms []v1beta1.S } // Each of these slices needs to be pre-populated with the "0" element so that we can properly generate permutations ids := []string{""} - tags := map[string]string{} + tagSet := []map[string]string{make(map[string]string)} for k, v := range subnetSelector { switch k { case "aws-ids", "aws::ids": ids = lo.Map(strings.Split(v, ","), func(s string, _ int) string { return strings.Trim(s, " ") }) default: - tags[k] = v + tagSet = createSelectorTags(k, v, tagSet) } } // If there are some "special" keys used, we have to represent the old selector as multiple terms for _, id := range ids { - terms = append(terms, v1beta1.SubnetSelectorTerm{ - Tags: tags, - ID: id, - }) + for _, tag := range tagSet { + terms = append(terms, v1beta1.SubnetSelectorTerm{ + Tags: tag, + ID: id, + }) + } } return terms } @@ -86,21 +88,23 @@ func NewSecurityGroupSelectorTerms(securityGroupSelector map[string]string) (ter } // Each of these slices needs to be pre-populated with the "0" element so that we can properly generate permutations ids := []string{""} - tags := map[string]string{} + tagSet := []map[string]string{make(map[string]string)} for k, v := range securityGroupSelector { switch k { case "aws-ids", "aws::ids": ids = lo.Map(strings.Split(v, ","), func(s string, _ int) string { return strings.Trim(s, " ") }) default: - tags[k] = v + tagSet = createSelectorTags(k, v, tagSet) } } // If there are some "special" keys used, we have to represent the old selector as multiple terms for _, id := range ids { - terms = append(terms, v1beta1.SecurityGroupSelectorTerm{ - Tags: tags, - ID: id, - }) + for _, tag := range tagSet { + terms = append(terms, v1beta1.SecurityGroupSelectorTerm{ + Tags: tag, + ID: id, + }) + } } return terms } @@ -113,7 +117,7 @@ func NewAMISelectorTerms(amiSelector map[string]string) (terms []v1beta1.AMISele ids := []string{""} names := []string{""} owners := []string{""} - tags := map[string]string{} + tagSet := []map[string]string{make(map[string]string)} for k, v := range amiSelector { switch k { case "aws-ids", "aws::ids": @@ -123,19 +127,21 @@ func NewAMISelectorTerms(amiSelector map[string]string) (terms []v1beta1.AMISele case "aws::owners": owners = lo.Map(strings.Split(v, ","), func(s string, _ int) string { return strings.Trim(s, " ") }) default: - tags[k] = v + tagSet = createSelectorTags(k, v, tagSet) } } // If there are some "special" keys used, we have to represent the old selector as multiple terms for _, owner := range owners { for _, id := range ids { for _, name := range names { - terms = append(terms, v1beta1.AMISelectorTerm{ - Tags: tags, - ID: id, - Name: name, - Owner: owner, - }) + for _, tag := range tagSet { + terms = append(terms, v1beta1.AMISelectorTerm{ + Tags: tag, + ID: id, + Name: name, + Owner: owner, + }) + } } } } @@ -245,3 +251,28 @@ func PatchStatus(ctx context.Context, c client.Client, stored, nodeClass *v1beta func HashAnnotation(nodeClass *v1beta1.EC2NodeClass) map[string]string { return map[string]string{v1beta1.AnnotationEC2NodeClassHash: nodeClass.Hash()} } + +func createSelectorTags(k string, v string, tagSet []map[string]string) []map[string]string { + cdlValue := lo.Map(strings.Split(v, ","), func(s string, _ int) string { return strings.Trim(s, " ") }) + for _, val := range cdlValue { + for _, tag := range tagSet { + if _, ok := tag[k]; ok { + tempTag := deepCopyMap(tag) + tempTag[k] = val + tagSet = append(tagSet, tempTag) + } else { + tag[k] = val + } + } + } + + return tagSet +} + +func deepCopyMap(m map[string]string) map[string]string { + result := map[string]string{} + for k, v := range m { + result[k] = v + } + return result +} diff --git a/pkg/utils/nodeclass/suite_test.go b/pkg/utils/nodeclass/suite_test.go index 287bbb90bd81..56540e44d2c7 100644 --- a/pkg/utils/nodeclass/suite_test.go +++ b/pkg/utils/nodeclass/suite_test.go @@ -401,7 +401,7 @@ var _ = Describe("NodeClassUtils", func() { "aws::name": "ami-name1,ami-name2", "aws::owners": "self,amazon", "aws::ids": "ami-1234,ami-5678", - "custom-tag": "custom-value", + "custom-tag": "custom-value1,custom-value2", "custom-tag2": "custom-value2", } nodeClass := nodeclassutil.New(nodeTemplate) @@ -419,14 +419,23 @@ var _ = Describe("NodeClassUtils", func() { // Expect AMISelectorTerms to be exactly what we would expect from the filtering above // This should include all permutations of the filters that could be used by this selector mechanism - Expect(nodeClass.Spec.AMISelectorTerms).To(HaveLen(8)) + Expect(nodeClass.Spec.AMISelectorTerms).To(HaveLen(16)) Expect(nodeClass.Spec.AMISelectorTerms).To(ConsistOf( v1beta1.AMISelectorTerm{ Name: "ami-name1", Owner: "self", ID: "ami-1234", Tags: map[string]string{ - "custom-tag": "custom-value", + "custom-tag": "custom-value1", + "custom-tag2": "custom-value2", + }, + }, + v1beta1.AMISelectorTerm{ + Name: "ami-name1", + Owner: "self", + ID: "ami-1234", + Tags: map[string]string{ + "custom-tag": "custom-value2", "custom-tag2": "custom-value2", }, }, @@ -435,7 +444,25 @@ var _ = Describe("NodeClassUtils", func() { Owner: "self", ID: "ami-5678", Tags: map[string]string{ - "custom-tag": "custom-value", + "custom-tag": "custom-value1", + "custom-tag2": "custom-value2", + }, + }, + v1beta1.AMISelectorTerm{ + Name: "ami-name1", + Owner: "self", + ID: "ami-5678", + Tags: map[string]string{ + "custom-tag": "custom-value2", + "custom-tag2": "custom-value2", + }, + }, + v1beta1.AMISelectorTerm{ + Name: "ami-name1", + Owner: "amazon", + ID: "ami-1234", + Tags: map[string]string{ + "custom-tag": "custom-value1", "custom-tag2": "custom-value2", }, }, @@ -444,7 +471,7 @@ var _ = Describe("NodeClassUtils", func() { Owner: "amazon", ID: "ami-1234", Tags: map[string]string{ - "custom-tag": "custom-value", + "custom-tag": "custom-value2", "custom-tag2": "custom-value2", }, }, @@ -453,7 +480,25 @@ var _ = Describe("NodeClassUtils", func() { Owner: "amazon", ID: "ami-5678", Tags: map[string]string{ - "custom-tag": "custom-value", + "custom-tag": "custom-value1", + "custom-tag2": "custom-value2", + }, + }, + v1beta1.AMISelectorTerm{ + Name: "ami-name1", + Owner: "amazon", + ID: "ami-5678", + Tags: map[string]string{ + "custom-tag": "custom-value2", + "custom-tag2": "custom-value2", + }, + }, + v1beta1.AMISelectorTerm{ + Name: "ami-name2", + Owner: "self", + ID: "ami-1234", + Tags: map[string]string{ + "custom-tag": "custom-value1", "custom-tag2": "custom-value2", }, }, @@ -462,7 +507,7 @@ var _ = Describe("NodeClassUtils", func() { Owner: "self", ID: "ami-1234", Tags: map[string]string{ - "custom-tag": "custom-value", + "custom-tag": "custom-value2", "custom-tag2": "custom-value2", }, }, @@ -471,7 +516,25 @@ var _ = Describe("NodeClassUtils", func() { Owner: "self", ID: "ami-5678", Tags: map[string]string{ - "custom-tag": "custom-value", + "custom-tag": "custom-value1", + "custom-tag2": "custom-value2", + }, + }, + v1beta1.AMISelectorTerm{ + Name: "ami-name2", + Owner: "self", + ID: "ami-5678", + Tags: map[string]string{ + "custom-tag": "custom-value2", + "custom-tag2": "custom-value2", + }, + }, + v1beta1.AMISelectorTerm{ + Name: "ami-name2", + Owner: "amazon", + ID: "ami-1234", + Tags: map[string]string{ + "custom-tag": "custom-value1", "custom-tag2": "custom-value2", }, }, @@ -480,7 +543,7 @@ var _ = Describe("NodeClassUtils", func() { Owner: "amazon", ID: "ami-1234", Tags: map[string]string{ - "custom-tag": "custom-value", + "custom-tag": "custom-value2", "custom-tag2": "custom-value2", }, }, @@ -489,10 +552,114 @@ var _ = Describe("NodeClassUtils", func() { Owner: "amazon", ID: "ami-5678", Tags: map[string]string{ - "custom-tag": "custom-value", + "custom-tag": "custom-value1", "custom-tag2": "custom-value2", }, }, + v1beta1.AMISelectorTerm{ + Name: "ami-name2", + Owner: "amazon", + ID: "ami-5678", + Tags: map[string]string{ + "custom-tag": "custom-value2", + "custom-tag2": "custom-value2", + }, + }, + )) + + Expect(nodeClass.Spec.AMIFamily).To(Equal(nodeTemplate.Spec.AMIFamily)) + Expect(nodeClass.Spec.UserData).To(Equal(nodeTemplate.Spec.UserData)) + Expect(nodeClass.Spec.Role).To(BeEmpty()) + Expect(nodeClass.Spec.Tags).To(Equal(nodeTemplate.Spec.Tags)) + ExpectBlockDeviceMappingsEqual(nodeTemplate.Spec.BlockDeviceMappings, nodeClass.Spec.BlockDeviceMappings) + Expect(nodeClass.Spec.DetailedMonitoring).To(Equal(nodeTemplate.Spec.DetailedMonitoring)) + ExpectMetadataOptionsEqual(nodeTemplate.Spec.MetadataOptions, nodeClass.Spec.MetadataOptions) + Expect(nodeClass.Spec.Context).To(Equal(nodeTemplate.Spec.Context)) + Expect(nodeClass.Spec.LaunchTemplateName).To(Equal(nodeTemplate.Spec.LaunchTemplateName)) + Expect(nodeClass.Spec.InstanceProfile).To(Equal(nodeTemplate.Spec.InstanceProfile)) + + ExpectSubnetStatusEqual(nodeTemplate.Status.Subnets, nodeClass.Status.Subnets) + ExpectSecurityGroupStatusEqual(nodeTemplate.Status.SecurityGroups, nodeClass.Status.SecurityGroups) + ExpectAMIStatusEqual(nodeTemplate.Status.AMIs, nodeClass.Status.AMIs) + }) + It("should convert a AWSNodeTemplate to a EC2NodeClass (with comma delineated list on SubnetSelector)", func() { + nodeTemplate.Spec.SubnetSelector = map[string]string{ + "test-key-1": "test-value-1,test-value-2", + "test-key-2": "test-value-1", + } + + nodeClass := nodeclassutil.New(nodeTemplate) + + for k, v := range nodeTemplate.Annotations { + Expect(nodeClass.Annotations).To(HaveKeyWithValue(k, v)) + } + for k, v := range nodeTemplate.Labels { + Expect(nodeClass.Labels).To(HaveKeyWithValue(k, v)) + } + Expect(nodeClass.Spec.SubnetSelectorTerms).To(HaveLen(2)) + Expect(nodeClass.Spec.SubnetSelectorTerms).To(ConsistOf( + v1beta1.SubnetSelectorTerm{ + Tags: map[string]string{ + "test-key-1": "test-value-1", + "test-key-2": "test-value-1", + }, + }, + v1beta1.SubnetSelectorTerm{ + Tags: map[string]string{ + "test-key-1": "test-value-2", + "test-key-2": "test-value-1", + }, + }, + )) + + Expect(nodeClass.Spec.SecurityGroupSelectorTerms).To(HaveLen(1)) + Expect(nodeClass.Spec.SecurityGroupSelectorTerms[0].Tags).To(Equal(nodeTemplate.Spec.SecurityGroupSelector)) + + Expect(nodeClass.Spec.AMIFamily).To(Equal(nodeTemplate.Spec.AMIFamily)) + Expect(nodeClass.Spec.UserData).To(Equal(nodeTemplate.Spec.UserData)) + Expect(nodeClass.Spec.Role).To(BeEmpty()) + Expect(nodeClass.Spec.Tags).To(Equal(nodeTemplate.Spec.Tags)) + ExpectBlockDeviceMappingsEqual(nodeTemplate.Spec.BlockDeviceMappings, nodeClass.Spec.BlockDeviceMappings) + Expect(nodeClass.Spec.DetailedMonitoring).To(Equal(nodeTemplate.Spec.DetailedMonitoring)) + ExpectMetadataOptionsEqual(nodeTemplate.Spec.MetadataOptions, nodeClass.Spec.MetadataOptions) + Expect(nodeClass.Spec.Context).To(Equal(nodeTemplate.Spec.Context)) + Expect(nodeClass.Spec.LaunchTemplateName).To(Equal(nodeTemplate.Spec.LaunchTemplateName)) + Expect(nodeClass.Spec.InstanceProfile).To(Equal(nodeTemplate.Spec.InstanceProfile)) + + ExpectSubnetStatusEqual(nodeTemplate.Status.Subnets, nodeClass.Status.Subnets) + ExpectSecurityGroupStatusEqual(nodeTemplate.Status.SecurityGroups, nodeClass.Status.SecurityGroups) + ExpectAMIStatusEqual(nodeTemplate.Status.AMIs, nodeClass.Status.AMIs) + }) + It("should convert a AWSNodeTemplate to a EC2NodeClass (with comma delineated list on SecurityGroupSelector)", func() { + nodeTemplate.Spec.SecurityGroupSelector = map[string]string{ + "test-key-1": "test-value-1,test-value-2", + "test-key-2": "test-value-1", + } + + nodeClass := nodeclassutil.New(nodeTemplate) + + for k, v := range nodeTemplate.Annotations { + Expect(nodeClass.Annotations).To(HaveKeyWithValue(k, v)) + } + for k, v := range nodeTemplate.Labels { + Expect(nodeClass.Labels).To(HaveKeyWithValue(k, v)) + } + Expect(nodeClass.Spec.SubnetSelectorTerms).To(HaveLen(1)) + Expect(nodeClass.Spec.SubnetSelectorTerms[0].Tags).To(Equal(nodeTemplate.Spec.SubnetSelector)) + Expect(nodeClass.Spec.SecurityGroupSelectorTerms).To(HaveLen(2)) + Expect(nodeClass.Spec.SecurityGroupSelectorTerms).To(ConsistOf( + v1beta1.SecurityGroupSelectorTerm{ + Tags: map[string]string{ + "test-key-1": "test-value-1", + "test-key-2": "test-value-1", + }, + }, + v1beta1.SecurityGroupSelectorTerm{ + Tags: map[string]string{ + "test-key-1": "test-value-2", + "test-key-2": "test-value-1", + }, + }, )) Expect(nodeClass.Spec.AMIFamily).To(Equal(nodeTemplate.Spec.AMIFamily))