From 013cf14ac482e31392fad57138cf99400e9635e3 Mon Sep 17 00:00:00 2001 From: Sebastian Cole Date: Wed, 20 Sep 2023 16:20:08 +1000 Subject: [PATCH] Adding nodeclass test for LicenseSelectors --- pkg/apis/v1beta1/ec2nodeclass.go | 24 +++++++------- pkg/apis/v1beta1/ec2nodeclass_status.go | 30 ++++++++--------- pkg/apis/v1beta1/zz_generated.deepcopy.go | 32 +++++++++++++++++++ pkg/fake/rgapi.go | 7 +++- .../hostresourcegroup/hostresourcegroup.go | 25 ++++++++------- pkg/providers/license/license.go | 3 +- .../placementgroup/placementgroup.go | 7 ++-- pkg/utils/nodeclass/suite_test.go | 7 ++++ 8 files changed, 91 insertions(+), 44 deletions(-) diff --git a/pkg/apis/v1beta1/ec2nodeclass.go b/pkg/apis/v1beta1/ec2nodeclass.go index 1e2436b57fc3..d3c6e1dd60de 100644 --- a/pkg/apis/v1beta1/ec2nodeclass.go +++ b/pkg/apis/v1beta1/ec2nodeclass.go @@ -42,12 +42,12 @@ type EC2NodeClassSpec struct { // LicenseSelectorTerms is a list of LicenseSelectors. The terms are ORed. // +optional LicenseSelectorTerms []LicenseSelectorTerm `json:"licenseSelectorTerms,omitempty" hash:"ignore"` - // HostResourceGroupSelectorTerms is a list of HostResourceGroupSelectors. The terms are ORed. - // +optional - HostResourceGroupSelectorTerms []HostResourceGroupSelectorTerm `json:"hostResourceGroupSelectorTerms,omitempty" hash:"ignore"` - // PlacementGroupSelectorTerms is a list of PlacementGroupSelector. The terms are ORed. - // +optional - PlacementGroupSelectorTerms []PlacementGroupSelectorTerm `json:"placementGroupSelectorTerms,omitempty" hash:"ignore"` + // HostResourceGroupSelectorTerms is a list of HostResourceGroupSelectors. The terms are ORed. + // +optional + HostResourceGroupSelectorTerms []HostResourceGroupSelectorTerm `json:"hostResourceGroupSelectorTerms,omitempty" hash:"ignore"` + // PlacementGroupSelectorTerms is a list of PlacementGroupSelector. The terms are ORed. + // +optional + PlacementGroupSelectorTerms []PlacementGroupSelectorTerm `json:"placementGroupSelectorTerms,omitempty" hash:"ignore"` // UserData to be applied to the provisioned nodes. // It must be in the appropriate format based on the AMIFamily in use. Karpenter will merge certain fields into // this UserData to ensure nodes are being provisioned with the correct configuration. @@ -189,17 +189,17 @@ type LicenseSelectorTerm struct { // HostResourceGroupSelectorTerm defines the selection logic for host Resource groups // that are used to launch nodes. If multiple fields are used for selection, the requirements are ANDed type HostResourceGroupSelectorTerm struct { - // Name of the hrg to be selected - // +optional - Name string `json:"name,omitempty"` + // Name of the hrg to be selected + // +optional + Name string `json:"name,omitempty"` } // PlacementGroupSelectorTerm defines the selection logic for ec2 placement groups // that are used to launch nodes. If multiple fields are used for selection, the requirements are ANDed type PlacementGroupSelectorTerm struct { - // Name of the placement group to be selected - // +optional - Name string `json:"name,omitempty"` + // Name of the placement group to be selected + // +optional + Name string `json:"name,omitempty"` } // MetadataOptions contains parameters for specifying the exposure of the diff --git a/pkg/apis/v1beta1/ec2nodeclass_status.go b/pkg/apis/v1beta1/ec2nodeclass_status.go index dadfeeba3225..15938a3ef27b 100644 --- a/pkg/apis/v1beta1/ec2nodeclass_status.go +++ b/pkg/apis/v1beta1/ec2nodeclass_status.go @@ -51,12 +51,12 @@ type AMI struct { // HostResourceGroup contains the resolved host resource group name and arn for node launch type HostResourceGroup struct { - // Name of the HRG - // +optional - Name string `json:"name,omitempty"` - // Arn of the HRG - // +optional - ARN string `json:"arn,omitempty"` + // Name of the HRG + // +optional + Name string `json:"name,omitempty"` + // Arn of the HRG + // +optional + ARN string `json:"arn,omitempty"` } // EC2NodeClassStatus contains the resolved state of the EC2NodeClass @@ -73,13 +73,13 @@ type EC2NodeClassStatus struct { // cluster under the AMI selectors. // +optional AMIs []AMI `json:"amis,omitempty"` - // Licenses contains the license arns - // +optional - Licenses []string `json:"licenses,omitempty"` - // HostResourceGroups contains the HRG arns - // +optional - HostResourceGroup *HostResourceGroup `json:"hostResourceGroup,omitempty"` - // PlacementGroups contains the ec2 placement group arns - // +optional - PlacementGroups []string `json:"placementGroups,omitempty"` + // Licenses contains the license arns + // +optional + Licenses []string `json:"licenses,omitempty"` + // HostResourceGroups contains the HRG arns + // +optional + HostResourceGroup *HostResourceGroup `json:"hostResourceGroup,omitempty"` + // PlacementGroups contains the ec2 placement group arns + // +optional + PlacementGroups []string `json:"placementGroups,omitempty"` } diff --git a/pkg/apis/v1beta1/zz_generated.deepcopy.go b/pkg/apis/v1beta1/zz_generated.deepcopy.go index d0ae48f58b7f..34ebcc10daf0 100644 --- a/pkg/apis/v1beta1/zz_generated.deepcopy.go +++ b/pkg/apis/v1beta1/zz_generated.deepcopy.go @@ -246,6 +246,11 @@ func (in *EC2NodeClassSpec) DeepCopyInto(out *EC2NodeClassSpec) { *out = make([]HostResourceGroupSelectorTerm, len(*in)) copy(*out, *in) } + if in.PlacementGroupSelectorTerms != nil { + in, out := &in.PlacementGroupSelectorTerms, &out.PlacementGroupSelectorTerms + *out = make([]PlacementGroupSelectorTerm, len(*in)) + copy(*out, *in) + } if in.UserData != nil { in, out := &in.UserData, &out.UserData *out = new(string) @@ -329,6 +334,13 @@ func (in *EC2NodeClassSpec) DeepCopyInto(out *EC2NodeClassSpec) { (*out)[key] = val } } + if in.OriginalPlacementGroupSelector != nil { + in, out := &in.OriginalPlacementGroupSelector, &out.OriginalPlacementGroupSelector + *out = make(map[string]string, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new EC2NodeClassSpec. @@ -371,6 +383,11 @@ func (in *EC2NodeClassStatus) DeepCopyInto(out *EC2NodeClassStatus) { *out = new(HostResourceGroup) **out = **in } + if in.PlacementGroups != nil { + in, out := &in.PlacementGroups, &out.PlacementGroups + *out = make([]string, len(*in)) + copy(*out, *in) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new EC2NodeClassStatus. @@ -463,6 +480,21 @@ func (in *MetadataOptions) DeepCopy() *MetadataOptions { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *PlacementGroupSelectorTerm) DeepCopyInto(out *PlacementGroupSelectorTerm) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new PlacementGroupSelectorTerm. +func (in *PlacementGroupSelectorTerm) DeepCopy() *PlacementGroupSelectorTerm { + if in == nil { + return nil + } + out := new(PlacementGroupSelectorTerm) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *SecurityGroup) DeepCopyInto(out *SecurityGroup) { *out = *in diff --git a/pkg/fake/rgapi.go b/pkg/fake/rgapi.go index 5b31d5f9504c..f0b4141352f9 100644 --- a/pkg/fake/rgapi.go +++ b/pkg/fake/rgapi.go @@ -23,6 +23,11 @@ import ( "github.com/aws/aws-sdk-go/service/resourcegroups/resourcegroupsiface" ) +var ( + TEST_LICENSE_ARN = "arn:aws:license-manager:us-west-2:11111111111:license-configuration:lic-94ba36399bd98eaad808b0ffb1d1604b" + TEST_LICENSE_NAME = "test-license" +) + type ResourceGroupsAPI struct { resourcegroupsiface.ResourceGroupsAPI ResourceGroupsBehaviour @@ -42,7 +47,7 @@ func (r *ResourceGroupsAPI) ListGroupsWithContext(_ aws.Context, input *resource return r.ListGroupsBehavior.Invoke(input, func(input *resourcegroups.ListGroupsInput) (*resourcegroups.ListGroupsOutput, error) { return &resourcegroups.ListGroupsOutput{ GroupIdentifiers: []*resourcegroups.GroupIdentifier{ - {GroupArn: aws.String("arn:aws:license-manager:us-west-2:11111111111:license-configuration:lic-94ba36399bd98eaad808b0ffb1d1604b"), GroupName: aws.String("test-license")}, + {GroupArn: aws.String(TEST_LICENSE_ARN), GroupName: aws.String(TEST_LICENSE_NAME)}, }, }, nil }) diff --git a/pkg/providers/hostresourcegroup/hostresourcegroup.go b/pkg/providers/hostresourcegroup/hostresourcegroup.go index 7230d4dcac06..1e6c0a580fd6 100644 --- a/pkg/providers/hostresourcegroup/hostresourcegroup.go +++ b/pkg/providers/hostresourcegroup/hostresourcegroup.go @@ -16,17 +16,18 @@ package hostresourcegroup import ( "context" + "fmt" "sync" - "fmt" //"github.com/aws/aws-sdk-go/service/resourcegroups" "github.com/aws/aws-sdk-go/service/resourcegroups" "github.com/aws/aws-sdk-go/service/resourcegroups/resourcegroupsiface" - "github.com/aws/karpenter-core/pkg/utils/pretty" - "github.com/aws/karpenter/pkg/apis/v1beta1" "github.com/mitchellh/hashstructure/v2" "github.com/patrickmn/go-cache" "knative.dev/pkg/logging" + + "github.com/aws/karpenter-core/pkg/utils/pretty" + "github.com/aws/karpenter/pkg/apis/v1beta1" ) type Provider struct { @@ -50,10 +51,10 @@ func (p *Provider) Get(ctx context.Context, nodeClass *v1beta1.EC2NodeClass) (*v p.Lock() defer p.Unlock() - selectors := nodeClass.Spec.HostResourceGroupSelectorTerms - if selectors == nil { - return nil, nil - } + selectors := nodeClass.Spec.HostResourceGroupSelectorTerms + if selectors == nil { + return nil, nil + } // Look for a cached result hash, err := hashstructure.Hash(selectors, hashstructure.FormatV2, &hashstructure.HashOptions{SlicesAsSets: true}) @@ -64,14 +65,14 @@ func (p *Provider) Get(ctx context.Context, nodeClass *v1beta1.EC2NodeClass) (*v return cached.(*v1beta1.HostResourceGroup), nil } - var match *v1beta1.HostResourceGroup + var match *v1beta1.HostResourceGroup err = p.resourcegroups.ListGroupsPagesWithContext(ctx, &resourcegroups.ListGroupsInput{}, func(page *resourcegroups.ListGroupsOutput, lastPage bool) bool { for i := range page.GroupIdentifiers { for x := range selectors { if *page.GroupIdentifiers[i].GroupName == selectors[x].Name { - match = &v1beta1.HostResourceGroup{ ARN: *page.GroupIdentifiers[i].GroupArn, Name: *page.GroupIdentifiers[i].GroupName } - p.cache.SetDefault(fmt.Sprint(hash), match) - return false + match = &v1beta1.HostResourceGroup{ARN: *page.GroupIdentifiers[i].GroupArn, Name: *page.GroupIdentifiers[i].GroupName} + p.cache.SetDefault(fmt.Sprint(hash), match) + return false } } } @@ -84,7 +85,7 @@ func (p *Provider) Get(ctx context.Context, nodeClass *v1beta1.EC2NodeClass) (*v } if p.cm.HasChanged(fmt.Sprintf("hostresourcegroups/%t/%s", nodeClass.IsNodeTemplate, nodeClass.Name), match) { logging.FromContext(ctx). - With("host resource group", match). + With("host resource group", match). Debugf("discovered host resource group") } diff --git a/pkg/providers/license/license.go b/pkg/providers/license/license.go index c9343c7a1e0b..7dd8a3a0d855 100644 --- a/pkg/providers/license/license.go +++ b/pkg/providers/license/license.go @@ -26,8 +26,9 @@ import ( "github.com/aws/karpenter/pkg/apis/v1beta1" - "github.com/aws/karpenter-core/pkg/utils/pretty" "knative.dev/pkg/logging" + + "github.com/aws/karpenter-core/pkg/utils/pretty" ) type Provider struct { diff --git a/pkg/providers/placementgroup/placementgroup.go b/pkg/providers/placementgroup/placementgroup.go index f02652dc56d7..cfdb31689293 100644 --- a/pkg/providers/placementgroup/placementgroup.go +++ b/pkg/providers/placementgroup/placementgroup.go @@ -26,8 +26,9 @@ import ( "github.com/aws/karpenter/pkg/apis/v1beta1" - "github.com/aws/karpenter-core/pkg/utils/pretty" "knative.dev/pkg/logging" + + "github.com/aws/karpenter-core/pkg/utils/pretty" ) type Provider struct { @@ -78,14 +79,14 @@ func (p *Provider) Get(ctx context.Context, nodeClass *v1beta1.EC2NodeClass) (*e for x := range selectors { if *output.PlacementGroups[i].GroupName == selectors[x].Name { match = output.PlacementGroups[i] - p.cache.SetDefault(fmt.Sprint(hash), match) + p.cache.SetDefault(fmt.Sprint(hash), match) break } } } if p.cm.HasChanged(fmt.Sprintf("placementgroup/%t/%s", nodeClass.IsNodeTemplate, nodeClass.Name), match) { logging.FromContext(ctx). - With("placement group", match). + With("placement group", match). Debugf("discovered placement group") } diff --git a/pkg/utils/nodeclass/suite_test.go b/pkg/utils/nodeclass/suite_test.go index 7b996b3a0659..77be7a488cf6 100644 --- a/pkg/utils/nodeclass/suite_test.go +++ b/pkg/utils/nodeclass/suite_test.go @@ -434,6 +434,13 @@ var _ = Describe("NodeClassUtils", func() { Expect(convertedNodeTemplate.Status.Subnets).To(Equal(nodeTemplate.Status.Subnets)) Expect(convertedNodeTemplate.Status.AMIs).To(Equal(nodeTemplate.Status.AMIs)) }) + It("should convert a AWSNodeTemplate with LicenseSelectors to a EC2NodeClass and back", func() { + nodeTemplate.Status.Licenses = []string{"test-license"} + nodeTemplate.Spec.LicenseSelector = map[string]string{"name": "test-license"} + convertedNodeTemplate := nodetemplateutil.New(nodeclassutil.New(nodeTemplate)) + Expect(convertedNodeTemplate.Status.Licenses).To(Equal(nodeTemplate.Status.Licenses)) + Expect(convertedNodeTemplate.Spec.LicenseSelector).To(Equal(nodeTemplate.Spec.LicenseSelector)) + }) It("should retrieve a EC2NodeClass with a get call", func() { nodeClass := test.EC2NodeClass() ExpectApplied(ctx, env.Client, nodeClass)