Skip to content

Commit

Permalink
Remove CDL on tags for Selector Terms
Browse files Browse the repository at this point in the history
  • Loading branch information
@engedaam
engedaam committed Dec 5, 2023
1 parent fbdb621 commit 034596c
Show file tree
Hide file tree
Showing 2 changed files with 228 additions and 30 deletions.
71 changes: 51 additions & 20 deletions pkg/utils/nodeclass/nodeclass.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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
}
Expand All @@ -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":
Expand All @@ -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,
})
}
}
}
}
Expand Down Expand Up @@ -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
}
187 changes: 177 additions & 10 deletions pkg/utils/nodeclass/suite_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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",
},
},
Expand All @@ -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",
},
},
Expand All @@ -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",
},
},
Expand All @@ -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",
},
},
Expand All @@ -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",
},
},
Expand All @@ -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",
},
},
Expand All @@ -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",
},
},
Expand All @@ -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))
Expand Down

0 comments on commit 034596c

Please sign in to comment.