From 7f08088948768194ce7d6a0855853c886ceff10e Mon Sep 17 00:00:00 2001 From: Sebastian Cole Date: Mon, 4 Sep 2023 14:17:11 +1000 Subject: [PATCH 01/33] Document the api changes --- .../en/preview/concepts/node-templates.md | 67 ++++++++++++++++--- 1 file changed, 56 insertions(+), 11 deletions(-) diff --git a/website/content/en/preview/concepts/node-templates.md b/website/content/en/preview/concepts/node-templates.md index ecab793e0984..e55f19e811ac 100644 --- a/website/content/en/preview/concepts/node-templates.md +++ b/website/content/en/preview/concepts/node-templates.md @@ -24,20 +24,27 @@ kind: AWSNodeTemplate metadata: name: default spec: - subnetSelector: { ... } # required, discovers tagged subnets to attach to instances - securityGroupSelector: { ... } # required, discovers tagged security groups to attach to instances - instanceProfile: "..." # optional, overrides the node's identity from global settings - amiFamily: "..." # optional, resolves a default ami and userdata - amiSelector: { ... } # optional, discovers tagged amis to override the amiFamily's default - userData: "..." # optional, overrides autogenerated userdata with a merge semantic - tags: { ... } # optional, propagates tags to underlying EC2 resources - metadataOptions: { ... } # optional, configures IMDS for the instance - blockDeviceMappings: [ ... ] # optional, configures storage devices for the instance - detailedMonitoring: "..." # optional, configures detailed monitoring for the instance + subnetSelector: { ... } # required, discovers tagged subnets to attach to instances + securityGroupSelector: { ... } # required, discovers tagged security groups to attach to instances + instanceProfile: "..." # optional, overrides the node's identity from global settings + amiFamily: "..." # optional, resolves a default ami and userdata + amiSelector: { ... } # optional, discovers tagged amis to override the amiFamily's default + userData: "..." # optional, overrides autogenerated userdata with a merge semantic + tags: { ... } # optional, propagates tags to underlying EC2 resources + metadataOptions: { ... } # optional, configures IMDS for the instance + blockDeviceMappings: [ ... ] # optional, configures storage devices for the instance + detailedMonitoring: "..." # optional, configures detailed monitoring for the instance + licenseSelector: { ... } #optional, discovers resource group license configurations + hostResourceGroupSelector: { ... } # optional, discovers resource groups to launch instances into + placementGroupSelector: { ... } # optional, discovers resource groups to launch instances into + status: subnets: { ... } # resolved subnets securityGroups: { ... } # resolved security groups amis: { ... } # resolved AMIs + licenseConfigrations: { ... } # resolved licenseConfigurationSpecifications + hostResourceGroups: { ... } # resolved hostResourceGroups + placementGroups: { ... } # resolved ec2 placementGroups ``` Refer to the [Provisioner docs]({{}}) for settings applicable to all providers. To explore various `AWSNodeTemplate` configurations, refer to the examples provided [in the Karpenter Github repository](https://github.com/aws/karpenter/blob/main/examples/provisioner/). @@ -607,6 +614,32 @@ spec: detailedMonitoring: true ``` +## spec.licenseSelector + licenseSelector: { ... } #optional, discovers resource group license configurations + Example: + ``` + licenseSelector: + name: myLicense + tags: + key: value + ``` + +## spec.hostResourceGroupSelector +``` + hostResourceGroupSelector: + name: "r5-1" +``` + +## spec.placementGroupSelector + placementGroupSelector: { ... } # optional, discovers resource groups to launch instances into + ``` + + placementGroupSelector: + name: placementGroup + tags: + key: value + ``` + ## status.subnets `status.subnets` contains the `id` and `zone` of the subnets utilized during node launch. The subnets are sorted by the available IP address count in decreasing order. @@ -686,4 +719,16 @@ status: values: - aws - nvidia -``` \ No newline at end of file +``` + +## status.licenseConfigurations + +contains the arn's of the licenses used to launch instances + +## status.hostResourceGroups + +contains the arns of the host resource groups + +## status.placementGroups + +contains the arns of the ec2 placement groups From 4bf971952a5c3b1f2c365477b3a722cd1e9574a3 Mon Sep 17 00:00:00 2001 From: Sebastian Cole Date: Mon, 4 Sep 2023 14:55:57 +1000 Subject: [PATCH 02/33] Adding design doc --- designs/dedicatedhosts.md | 107 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 107 insertions(+) create mode 100644 designs/dedicatedhosts.md diff --git a/designs/dedicatedhosts.md b/designs/dedicatedhosts.md new file mode 100644 index 000000000000..4322bb03820e --- /dev/null +++ b/designs/dedicatedhosts.md @@ -0,0 +1,107 @@ +# Dedicated host support +## Background + +As described in [3182](https://github.com/aws/karpenter/issues/3182) AWS provides the +ability to launch EC2 instances on [Dedicated Host hardware](https://docs.aws.amazon.com/license-manager/latest/userguide/host-resource-groups.html) +Dedicated hosts are often used to pay for ec2 usage as capital expenditure (CapEx) +rather than an operating expense (OpEx). This is often useful for publicly listed +companies that want to manage their revenue to OpEx ratio. + +AWS allows the allocation of ec2 instances to Dedidcated Host Host Resource Groups (HRG) +through the use of launchtemplates. +Detailed documentation for launchtemplates and host resource groups can be found here: +- https://docs.aws.amazon.com/autoscaling/ec2/userguide/create-launch-template.html#advanced-settings-for-your-launch-template +- https://docs.aws.amazon.com/license-manager/latest/userguide/host-resource-groups.html + +## Solutions + +aws-karpenter already supports a set of [LaunchTemplate configuration](https://github.com/aws/karpenter/blob/main/pkg/apis/v1alpha1/awsnodetemplate.go) +with design document: https://github.com/aws/karpenter/blob/main/designs/aws-launch-templates-v2.md + +1. Implement the minimal fields from https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-ec2-launchtemplate-launchtemplatedata-placement.html +specifically `placement.hostResourceGroupArn` and `licenseConfiguration` + +this would look like: + +``` +apiVersion: karpenter.k8s.aws/v1alpha1 +kind: AWSNodeTemplate +metadata: + name: default +spec: + licenseConfiguration: arn:aws:license-manager:eu-east-1:123456789012:license-configuration:lic-edf7f9e241f5e16f29996c842111f448 # optional, arn of the license configuration + placement: + hostResourceGroupArn: arn:aws:resource-groups:us-east-1:123456789012:group/my-hrg-name #optional, The ARN of the host resource group in which to launch the instances. If you specify a host resource group ARN, omit the Tenancy parameter or set it to host. + +``` + +2. Implement the complete fields from AWS Launch Templates + +eg. +``` +apiVersion: karpenter.k8s.aws/v1alpha1 +kind: AWSNodeTemplate +metadata: + name: default +spec: + licenseConfiguration: arn:aws:license-manager:eu-east-1:123456789012:license-configuration:lic-edf7f9e241f5e16f29996c842111f448 # optional, arn of the license configuration + placement: + affinity: #optional + availabilityZone: #optional + groupId: #optional, something to do with placement groups + groupName: #optional + hostId: #optional, ID of the dedicated host + hostResourceGroupArn: arn:aws:resource-groups:us-east-1:123456789012:group/my-hrg-name #optional, The ARN of the host resource group in which to launch the instances. If you specify a host resource group ARN, omit the Tenancy parameter or set it to host. + paritionNumber: #optional, The number of the partition the instance should launch in. Valid only if the placement group strategy is set to partition. + spreadDomain: #optional, reserved for future use + tenancy: dedicated #optional, The tenancy of the instance. An instance with a tenancy of dedicated runs on single-tenant hardware, one of dedicated | default | host + +``` + +3. Implement a simplified API +AWS Launch templates also include extra fields + +``` +apiVersion: karpenter.k8s.aws/v1alpha1 +kind: AWSNodeTemplate +metadata: + name: default +spec: + dedicatedHostConfig: + licenseConfiguration: arn:aws:license-manager:eu-east-1:123456789012:license-configuration:lic-edf7f9e241f5e16f29996c842111f448 # optional, arn of the license configuration + resourceHostGroup: arn:aws:resource-groups:us-east-1:123456789012:group/my-hrg-name #option, arn of the HRG +``` + +4. Implement Selectors for all relevant fields + +``` +apiVersion: karpenter.k8s.aws/v1alpha1 +kind: AWSNodeTemplate +metadata: + name: default +spec: + licenseSelector: + name: "myLicense" + hostResourceGroupSelector: + name: "myHrg" +``` + +This would require inexpensive or filterable APIs to query for available license configurations and host resource groups. +Today the recommended apis are: +* `aws license-manager list-license-configurations` +* `aws resource-groups list-groups` +* `aws ec2 describe-placement-groups` + + +## Recommendations + +1. Middle ground solution, stick both to the AWS Launch Template api, but implementing the smallest set of configuration practical, +limits the amount of checks required to be performed in Karpenter, eg. setting both hostId and HostResourceGroupArn + +2. Completely copies the AWS api, would allow the full set of possible configurations supported by AWS. As documented in launch-template-options.md also requires the most work to implement and support. + +3. Focuses entirely on the dedicated host feature, ignoring other configuration options. Reduces confusion + +## Decision from Working Group meeting + +Implement selectors for all relevant fields to improve portability of configuration between clusters / regions / accounts. From d91d401b43d4ae84c5ea4b2011683dd59c0fdcc4 Mon Sep 17 00:00:00 2001 From: Sebastian Cole Date: Mon, 4 Sep 2023 16:27:03 +1000 Subject: [PATCH 03/33] Adding fields to API --- pkg/apis/v1alpha1/awsnodetemplate.go | 12 ++++++++++++ pkg/apis/v1alpha1/provider.go | 9 +++++++++ .../content/en/preview/concepts/node-templates.md | 2 -- 3 files changed, 21 insertions(+), 2 deletions(-) diff --git a/pkg/apis/v1alpha1/awsnodetemplate.go b/pkg/apis/v1alpha1/awsnodetemplate.go index 7722bdf8443b..03ba492935d4 100644 --- a/pkg/apis/v1alpha1/awsnodetemplate.go +++ b/pkg/apis/v1alpha1/awsnodetemplate.go @@ -69,6 +69,18 @@ type AWSNodeTemplateStatus struct { // cluster under the AMI selectors. // +optional AMIs []AMI `json:"amis,omitempty"` + // Licenses contains the current License Configuration Specifications + // that currently apply given the LicenseConfiguration selectors. + // +optional + Licenses []string `json:"licenses,omitempty"` + // HostResourceGroups contains the current Host Resource Groups + // that currently apply given the HostResourceGroup selectors. + // +optional + HostResourceGroups []string `json:"hostResourceGroups,omitempty"` + // Licenses contains the current License Configuration Specifications + // that currently apply given the LicenseConfiguration selectors. + // +optional + PlacementGroups []string `json:"placementGroups,omitempty"` } // AWSNodeTemplateSpec is the top level specification for the AWS Karpenter Provider. diff --git a/pkg/apis/v1alpha1/provider.go b/pkg/apis/v1alpha1/provider.go index 0fe2f8071b06..c0e1b56e189c 100644 --- a/pkg/apis/v1alpha1/provider.go +++ b/pkg/apis/v1alpha1/provider.go @@ -41,6 +41,15 @@ type AWS struct { // SecurityGroups specify the names of the security groups. // +optional SecurityGroupSelector map[string]string `json:"securityGroupSelector,omitempty" hash:"ignore"` + // LicenseConfiguration specifies the license configuration specifications to launch instances with + // +optional + LicenseSelector map[string]string `json:"licenseSelector,omitempty" hash:"ignore"` + // HostResourceGroups specific the hostResourceGroupArns to use for ec2 placement + // +optional + HostResourceGroupSelector map[string]string `json:"hostResourceGroup,omitempty" hash:"ignore"` + // PlacementGroup specifies the placement group to use for ec2 placement + // +optional + PlacementGroupSelector map[string]string `json:"placementGroup,omitempty" hash:"ignore"` // Tags to be applied on ec2 resources like instances and launch templates. // +optional Tags map[string]string `json:"tags,omitempty"` diff --git a/website/content/en/preview/concepts/node-templates.md b/website/content/en/preview/concepts/node-templates.md index e55f19e811ac..159ad995c76a 100644 --- a/website/content/en/preview/concepts/node-templates.md +++ b/website/content/en/preview/concepts/node-templates.md @@ -620,8 +620,6 @@ spec: ``` licenseSelector: name: myLicense - tags: - key: value ``` ## spec.hostResourceGroupSelector From 57a7a27b7dfce31273d4f723e919be98356e2648 Mon Sep 17 00:00:00 2001 From: Sebastian Cole Date: Mon, 11 Sep 2023 11:40:01 +1000 Subject: [PATCH 04/33] Establish new license provider --- pkg/operator/operator.go | 4 ++++ pkg/providers/license/license.go | 35 ++++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+) create mode 100644 pkg/providers/license/license.go diff --git a/pkg/operator/operator.go b/pkg/operator/operator.go index 65b355eae75b..73ee09f732fa 100644 --- a/pkg/operator/operator.go +++ b/pkg/operator/operator.go @@ -52,6 +52,7 @@ import ( "github.com/aws/karpenter/pkg/providers/instance" "github.com/aws/karpenter/pkg/providers/instancetype" "github.com/aws/karpenter/pkg/providers/launchtemplate" + "github.com/aws/karpenter/pkg/providers/license" "github.com/aws/karpenter/pkg/providers/pricing" "github.com/aws/karpenter/pkg/providers/securitygroup" "github.com/aws/karpenter/pkg/providers/subnet" @@ -75,6 +76,7 @@ type Operator struct { VersionProvider *version.Provider InstanceTypesProvider *instancetype.Provider InstanceProvider *instance.Provider + LicenseProvider *license.Provider } func NewOperator(ctx context.Context, operator *operator.Operator) (context.Context, *Operator) { @@ -161,6 +163,7 @@ func NewOperator(ctx context.Context, operator *operator.Operator) (context.Cont subnetProvider, launchTemplateProvider, ) + licenseProvider := license.NewProvider(ec2api, cache.New(awscache.DefaultTTL, awscache.DefaultCleanupInterval)) return ctx, &Operator{ Operator: operator, @@ -176,6 +179,7 @@ func NewOperator(ctx context.Context, operator *operator.Operator) (context.Cont PricingProvider: pricingProvider, InstanceTypesProvider: instanceTypeProvider, InstanceProvider: instanceProvider, + LicenseProvider: licenseProvider, } } diff --git a/pkg/providers/license/license.go b/pkg/providers/license/license.go new file mode 100644 index 000000000000..13eef973d25e --- /dev/null +++ b/pkg/providers/license/license.go @@ -0,0 +1,35 @@ +package license + +import ( + "context" + "sync" + + "github.com/aws/aws-sdk-go/service/ec2" + "github.com/aws/aws-sdk-go/service/ec2/ec2iface" + "github.com/patrickmn/go-cache" + + "github.com/aws/karpenter/pkg/apis/v1beta1" + + "github.com/aws/karpenter-core/pkg/utils/pretty" +) + +type Provider struct { + sync.RWMutex + ec2api ec2iface.EC2API + cache *cache.Cache + cm *pretty.ChangeMonitor +} + +func NewProvider(ec2api ec2iface.EC2API, cache *cache.Cache) *Provider { + return &Provider{ + ec2api: ec2api, + cm: pretty.NewChangeMonitor(), + // TODO: Remove cache for v1beta1, utilize resolved subnet from the AWSNodeTemplate.status + // Subnets are sorted on AvailableIpAddressCount, descending order + cache: cache, + } +} + +func (p *Provider) List(ctx context.Context, nodeClass *v1beta1.NodeClass) ([]*ec2.LicenseConfiguration, error) { + return nil, nil +} From 0bda01cc8a4533227dda4e56d757fbcf347db751 Mon Sep 17 00:00:00 2001 From: Sebastian Cole Date: Mon, 11 Sep 2023 12:13:06 +1000 Subject: [PATCH 05/33] More license provider scaffolding --- cmd/controller/main.go | 1 + pkg/apis/v1beta1/ec2nodeclass_status.go | 3 +++ pkg/controllers/controllers.go | 5 +++-- pkg/controllers/nodeclass/controller.go | 30 ++++++++++++++++++++++--- pkg/providers/license/license.go | 11 +++++---- 5 files changed, 39 insertions(+), 11 deletions(-) diff --git a/cmd/controller/main.go b/cmd/controller/main.go index 0146572b9f2c..d07a2e8fd487 100644 --- a/cmd/controller/main.go +++ b/cmd/controller/main.go @@ -66,6 +66,7 @@ func main() { op.SecurityGroupProvider, op.PricingProvider, op.AMIProvider, + op.LicenseProvider, )...). WithWebhooks(ctx, webhooks.NewWebhooks()...). Start(ctx) diff --git a/pkg/apis/v1beta1/ec2nodeclass_status.go b/pkg/apis/v1beta1/ec2nodeclass_status.go index a5a37f50771e..fb0c39ff6bec 100644 --- a/pkg/apis/v1beta1/ec2nodeclass_status.go +++ b/pkg/apis/v1beta1/ec2nodeclass_status.go @@ -63,4 +63,7 @@ type EC2NodeClassStatus struct { // cluster under the AMI selectors. // +optional AMIs []AMI `json:"amis,omitempty"` + // License contains the discovered license configurations + // +optional + Licenses []string `json:"licenses,omitempty"` } diff --git a/pkg/controllers/controllers.go b/pkg/controllers/controllers.go index 4cbdfd707ae8..14cd55e1ccc9 100644 --- a/pkg/controllers/controllers.go +++ b/pkg/controllers/controllers.go @@ -32,6 +32,7 @@ import ( nodeclaimlink "github.com/aws/karpenter/pkg/controllers/nodeclaim/link" "github.com/aws/karpenter/pkg/controllers/nodeclass" "github.com/aws/karpenter/pkg/providers/amifamily" + "github.com/aws/karpenter/pkg/providers/license" "github.com/aws/karpenter/pkg/providers/pricing" "github.com/aws/karpenter/pkg/providers/securitygroup" "github.com/aws/karpenter/pkg/providers/subnet" @@ -42,13 +43,13 @@ import ( func NewControllers(ctx context.Context, sess *session.Session, clk clock.Clock, kubeClient client.Client, recorder events.Recorder, unavailableOfferings *cache.UnavailableOfferings, cloudProvider *cloudprovider.CloudProvider, subnetProvider *subnet.Provider, - securityGroupProvider *securitygroup.Provider, pricingProvider *pricing.Provider, amiProvider *amifamily.Provider) []controller.Controller { + securityGroupProvider *securitygroup.Provider, pricingProvider *pricing.Provider, amiProvider *amifamily.Provider, licenseProvider *license.Provider) []controller.Controller { logging.FromContext(ctx).With("version", project.Version).Debugf("discovered version") linkController := nodeclaimlink.NewController(kubeClient, cloudProvider) controllers := []controller.Controller{ - nodeclass.NewNodeTemplateController(kubeClient, subnetProvider, securityGroupProvider, amiProvider), + nodeclass.NewNodeTemplateController(kubeClient, subnetProvider, securityGroupProvider, amiProvider, licenseProvider), linkController, nodeclaimgarbagecollection.NewController(kubeClient, cloudProvider, linkController), } diff --git a/pkg/controllers/nodeclass/controller.go b/pkg/controllers/nodeclass/controller.go index eebb633ac813..b677fdfffe1a 100644 --- a/pkg/controllers/nodeclass/controller.go +++ b/pkg/controllers/nodeclass/controller.go @@ -38,6 +38,7 @@ import ( "github.com/aws/karpenter/pkg/apis/v1alpha1" "github.com/aws/karpenter/pkg/apis/v1beta1" "github.com/aws/karpenter/pkg/providers/amifamily" + "github.com/aws/karpenter/pkg/providers/license" "github.com/aws/karpenter/pkg/providers/securitygroup" "github.com/aws/karpenter/pkg/providers/subnet" nodeclassutil "github.com/aws/karpenter/pkg/utils/nodeclass" @@ -48,15 +49,17 @@ type Controller struct { subnetProvider *subnet.Provider securityGroupProvider *securitygroup.Provider amiProvider *amifamily.Provider + licenseProvider *license.Provider } func NewController(kubeClient client.Client, subnetProvider *subnet.Provider, - securityGroupProvider *securitygroup.Provider, amiProvider *amifamily.Provider) *Controller { + securityGroupProvider *securitygroup.Provider, amiProvider *amifamily.Provider, licenseProvider *license.Provider) *Controller { return &Controller{ kubeClient: kubeClient, subnetProvider: subnetProvider, securityGroupProvider: securityGroupProvider, amiProvider: amiProvider, + licenseProvider: licenseProvider, } } @@ -152,15 +155,36 @@ func (c *Controller) resolveAMIs(ctx context.Context, nodeClass *v1beta1.EC2Node return nil } +func (c *Controller) resolveLicenses(ctx context.Context, nodeClass *v1beta1.NodeClass) error { + licenses, err := c.licenseProvider.List(ctx, nodeClass) + if err != nil { + return err + } + if len(licenses) == 0 { + nodeClass.Status.Licenses = nil + return fmt.Errorf("no licenses match selector") + } + nodeClass.Status.Licenses = licenses + + return nil + +} + //nolint:revive type NodeClassController struct { *Controller } func NewNodeClassController(kubeClient client.Client, subnetProvider *subnet.Provider, +<<<<<<< HEAD securityGroupProvider *securitygroup.Provider, amiProvider *amifamily.Provider) corecontroller.Controller { return corecontroller.Typed[*v1beta1.EC2NodeClass](kubeClient, &NodeClassController{ Controller: NewController(kubeClient, subnetProvider, securityGroupProvider, amiProvider), +======= + securityGroupProvider *securitygroup.Provider, amiProvider *amifamily.Provider, licenseProvider *license.Provider) corecontroller.Controller { + return corecontroller.Typed[*v1beta1.NodeClass](kubeClient, &NodeClassController{ + Controller: NewController(kubeClient, subnetProvider, securityGroupProvider, amiProvider, licenseProvider), +>>>>>>> 7b9eb759 (More license provider scaffolding) }) } @@ -188,9 +212,9 @@ type NodeTemplateController struct { } func NewNodeTemplateController(kubeClient client.Client, subnetProvider *subnet.Provider, - securityGroupProvider *securitygroup.Provider, amiProvider *amifamily.Provider) corecontroller.Controller { + securityGroupProvider *securitygroup.Provider, amiProvider *amifamily.Provider, licenseProvider *license.Provider) corecontroller.Controller { return corecontroller.Typed[*v1alpha1.AWSNodeTemplate](kubeClient, &NodeTemplateController{ - Controller: NewController(kubeClient, subnetProvider, securityGroupProvider, amiProvider), + Controller: NewController(kubeClient, subnetProvider, securityGroupProvider, amiProvider, licenseProvider), }) } diff --git a/pkg/providers/license/license.go b/pkg/providers/license/license.go index 13eef973d25e..3f700aee961a 100644 --- a/pkg/providers/license/license.go +++ b/pkg/providers/license/license.go @@ -4,7 +4,6 @@ import ( "context" "sync" - "github.com/aws/aws-sdk-go/service/ec2" "github.com/aws/aws-sdk-go/service/ec2/ec2iface" "github.com/patrickmn/go-cache" @@ -15,9 +14,9 @@ import ( type Provider struct { sync.RWMutex - ec2api ec2iface.EC2API - cache *cache.Cache - cm *pretty.ChangeMonitor + ec2api ec2iface.EC2API + cache *cache.Cache + cm *pretty.ChangeMonitor } func NewProvider(ec2api ec2iface.EC2API, cache *cache.Cache) *Provider { @@ -30,6 +29,6 @@ func NewProvider(ec2api ec2iface.EC2API, cache *cache.Cache) *Provider { } } -func (p *Provider) List(ctx context.Context, nodeClass *v1beta1.NodeClass) ([]*ec2.LicenseConfiguration, error) { - return nil, nil +func (p *Provider) List(ctx context.Context, nodeClass *v1beta1.NodeClass) ([]string, error) { + return []string{"beep", "boop", "bop"}, nil } From 77bb9d3a547a6d10f953cb8909211be643f54a90 Mon Sep 17 00:00:00 2001 From: Sebastian Cole Date: Mon, 11 Sep 2023 12:17:36 +1000 Subject: [PATCH 06/33] Generated code --- .../karpenter.k8s.aws_awsnodetemplates.yaml | 36 ++++++++++++++++++ .../karpenter.k8s.aws_ec2nodeclasses.yaml | 5 +++ pkg/apis/settings/zz_generated.deepcopy.go | 1 + pkg/apis/v1alpha1/zz_generated.deepcopy.go | 37 +++++++++++++++++++ pkg/apis/v1alpha5/zz_generated.deepcopy.go | 1 + pkg/apis/v1beta1/zz_generated.deepcopy.go | 6 +++ 6 files changed, 86 insertions(+) diff --git a/pkg/apis/crds/karpenter.k8s.aws_awsnodetemplates.yaml b/pkg/apis/crds/karpenter.k8s.aws_awsnodetemplates.yaml index 20cf10f8b3d2..aeb4936e7093 100644 --- a/pkg/apis/crds/karpenter.k8s.aws_awsnodetemplates.yaml +++ b/pkg/apis/crds/karpenter.k8s.aws_awsnodetemplates.yaml @@ -129,6 +129,12 @@ spec: description: DetailedMonitoring controls if detailed monitoring is enabled for instances that are launched type: boolean + hostResourceGroup: + additionalProperties: + type: string + description: HostResourceGroups specific the hostResourceGroupArns + to use for ec2 placement + type: object instanceProfile: description: InstanceProfile is the AWS identity that instances use. type: string @@ -144,6 +150,12 @@ spec: a custom launch template and is exposed in the Spec as `launchTemplate` for backwards compatibility.' type: string + licenseSelector: + additionalProperties: + type: string + description: LicenseConfiguration specifies the license configuration + specifications to launch instances with + type: object metadataOptions: description: "MetadataOptions for the generated launch template of provisioned nodes. \n This specifies the exposure of the Instance @@ -192,6 +204,12 @@ spec: credentials are not available." type: string type: object + placementGroup: + additionalProperties: + type: string + description: PlacementGroup specifies the placement group to use for + ec2 placement + type: object securityGroupSelector: additionalProperties: type: string @@ -270,6 +288,24 @@ spec: - requirements type: object type: array + hostResourceGroups: + description: HostResourceGroups contains the current Host Resource + Groups that currently apply given the HostResourceGroup selectors. + items: + type: string + type: array + licenses: + description: Licenses contains the current License Configuration Specifications + that currently apply given the LicenseConfiguration selectors. + items: + type: string + type: array + placementGroups: + description: Licenses contains the current License Configuration Specifications + that currently apply given the LicenseConfiguration selectors. + items: + type: string + type: array securityGroups: description: SecurityGroups contains the current Security Groups values that are available to the cluster under the SecurityGroups selectors. diff --git a/pkg/apis/crds/karpenter.k8s.aws_ec2nodeclasses.yaml b/pkg/apis/crds/karpenter.k8s.aws_ec2nodeclasses.yaml index 71fd0962605a..43025e36ebfc 100644 --- a/pkg/apis/crds/karpenter.k8s.aws_ec2nodeclasses.yaml +++ b/pkg/apis/crds/karpenter.k8s.aws_ec2nodeclasses.yaml @@ -340,6 +340,11 @@ spec: - requirements type: object type: array + licenses: + description: License contains the discovered license configurations + items: + type: string + type: array securityGroups: description: SecurityGroups contains the current Security Groups values that are available to the cluster under the SecurityGroups selectors. diff --git a/pkg/apis/settings/zz_generated.deepcopy.go b/pkg/apis/settings/zz_generated.deepcopy.go index 51a3bd930dd9..43eaa173bfe6 100644 --- a/pkg/apis/settings/zz_generated.deepcopy.go +++ b/pkg/apis/settings/zz_generated.deepcopy.go @@ -1,4 +1,5 @@ //go:build !ignore_autogenerated +// +build !ignore_autogenerated /* Licensed under the Apache License, Version 2.0 (the "License"); diff --git a/pkg/apis/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/v1alpha1/zz_generated.deepcopy.go index 14309d1b7389..15b0b9d21ac8 100644 --- a/pkg/apis/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/v1alpha1/zz_generated.deepcopy.go @@ -1,4 +1,5 @@ //go:build !ignore_autogenerated +// +build !ignore_autogenerated /* Licensed under the Apache License, Version 2.0 (the "License"); @@ -78,6 +79,27 @@ func (in *AWS) DeepCopyInto(out *AWS) { (*out)[key] = val } } + if in.LicenseSelector != nil { + in, out := &in.LicenseSelector, &out.LicenseSelector + *out = make(map[string]string, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } + if in.HostResourceGroupSelector != nil { + in, out := &in.HostResourceGroupSelector, &out.HostResourceGroupSelector + *out = make(map[string]string, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } + if in.PlacementGroupSelector != nil { + in, out := &in.PlacementGroupSelector, &out.PlacementGroupSelector + *out = make(map[string]string, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } if in.Tags != nil { in, out := &in.Tags, &out.Tags *out = make(map[string]string, len(*in)) @@ -218,6 +240,21 @@ func (in *AWSNodeTemplateStatus) DeepCopyInto(out *AWSNodeTemplateStatus) { (*in)[i].DeepCopyInto(&(*out)[i]) } } + if in.Licenses != nil { + in, out := &in.Licenses, &out.Licenses + *out = make([]string, len(*in)) + copy(*out, *in) + } + if in.HostResourceGroups != nil { + in, out := &in.HostResourceGroups, &out.HostResourceGroups + *out = make([]string, len(*in)) + copy(*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 AWSNodeTemplateStatus. diff --git a/pkg/apis/v1alpha5/zz_generated.deepcopy.go b/pkg/apis/v1alpha5/zz_generated.deepcopy.go index 722b9872175f..e55c4cb4b353 100644 --- a/pkg/apis/v1alpha5/zz_generated.deepcopy.go +++ b/pkg/apis/v1alpha5/zz_generated.deepcopy.go @@ -1,4 +1,5 @@ //go:build !ignore_autogenerated +// +build !ignore_autogenerated /* Licensed under the Apache License, Version 2.0 (the "License"); diff --git a/pkg/apis/v1beta1/zz_generated.deepcopy.go b/pkg/apis/v1beta1/zz_generated.deepcopy.go index bdc0e52d0092..9a1c13e4c29f 100644 --- a/pkg/apis/v1beta1/zz_generated.deepcopy.go +++ b/pkg/apis/v1beta1/zz_generated.deepcopy.go @@ -1,4 +1,5 @@ //go:build !ignore_autogenerated +// +build !ignore_autogenerated /* Licensed under the Apache License, Version 2.0 (the "License"); @@ -336,6 +337,11 @@ func (in *EC2NodeClassStatus) DeepCopyInto(out *EC2NodeClassStatus) { (*in)[i].DeepCopyInto(&(*out)[i]) } } + if in.Licenses != nil { + in, out := &in.Licenses, &out.Licenses + *out = make([]string, len(*in)) + copy(*out, *in) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new EC2NodeClassStatus. From bf0ac907798dd8d3cbdffebe3c8f3f294696b343 Mon Sep 17 00:00:00 2001 From: Sebastian Cole Date: Tue, 12 Sep 2023 16:33:22 +1000 Subject: [PATCH 07/33] Implement licenses selector --- .../karpenter.k8s.aws_ec2nodeclasses.yaml | 15 +- pkg/apis/v1beta1/ec2nodeclass.go | 16 + pkg/apis/v1beta1/ec2nodeclass_status.go | 2 +- pkg/apis/v1beta1/zz_generated.deepcopy.go | 27 + pkg/controllers/nodeclass/controller.go | 19 +- pkg/controllers/nodeclass/suite_test.go | 699 ++++++++++++++++++ pkg/operator/operator.go | 4 +- pkg/providers/license/license.go | 75 +- pkg/test/environment.go | 5 + pkg/utils/nodeclass/nodeclass.go | 11 + pkg/utils/nodetemplate/nodetemplate.go | 6 + 11 files changed, 857 insertions(+), 22 deletions(-) diff --git a/pkg/apis/crds/karpenter.k8s.aws_ec2nodeclasses.yaml b/pkg/apis/crds/karpenter.k8s.aws_ec2nodeclasses.yaml index 43025e36ebfc..f3d43bb24ce5 100644 --- a/pkg/apis/crds/karpenter.k8s.aws_ec2nodeclasses.yaml +++ b/pkg/apis/crds/karpenter.k8s.aws_ec2nodeclasses.yaml @@ -163,6 +163,19 @@ spec: description: DetailedMonitoring controls if detailed monitoring is enabled for instances that are launched type: boolean + licenseSelectorTerms: + description: LicenseSelectorTerms is a list of LicenseSelectors. The + terms are ORed. + items: + description: LicenseSelectorTerm defines selection logic for a licenseConfigurationSpecification + used to launch nodes If multiple fields are used for selection, + the requirements are ANDed. + properties: + name: + description: Name of the license to be selected. + type: string + type: object + type: array metadataOptions: default: httpEndpoint: enabled @@ -341,7 +354,7 @@ spec: type: object type: array licenses: - description: License contains the discovered license configurations + description: Licenses contains the license arns items: type: string type: array diff --git a/pkg/apis/v1beta1/ec2nodeclass.go b/pkg/apis/v1beta1/ec2nodeclass.go index 74c4ce16e80c..0ae8b39bc659 100644 --- a/pkg/apis/v1beta1/ec2nodeclass.go +++ b/pkg/apis/v1beta1/ec2nodeclass.go @@ -39,6 +39,9 @@ type EC2NodeClassSpec struct { // +kubebuilder:validation:Enum:={AL2,Bottlerocket,Ubuntu,Custom,Windows2019,Windows2022} // +required AMIFamily *string `json:"amiFamily"` + // LicenseSelectorTerms is a list of LicenseSelectors. The terms are ORed. + // +optional + LicenseSelectorTerms []LicenseSelectorTerm `json:"licenseSelectorTerms,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. @@ -102,6 +105,11 @@ type EC2NodeClassSpec struct { // DO NOT USE THIS VALUE when performing business logic in code // +optional OriginalAMISelector map[string]string `json:"-" hash:"ignore"` + // TODO @joinnis: Remove this field when v1alpha5 is unsupported in a future version of Karpenter + // OriginalAMISelector is the original ami selector that was used by the v1alpha5 representation of this API. + // DO NOT USE THIS VALUE when performing business logic in code + // +optional + OriginalLicenseSelector map[string]string `json:"-" hash:"ignore"` } // SubnetSelectorTerm defines selection logic for a subnet used by Karpenter to launch nodes. @@ -154,6 +162,14 @@ type AMISelectorTerm struct { Owner string `json:"owner,omitempty"` } +// LicenseSelectorTerm defines selection logic for a licenseConfigurationSpecification used to launch nodes +// If multiple fields are used for selection, the requirements are ANDed. +type LicenseSelectorTerm struct { + // Name of the license to be selected. + // +optional + Name string `json:"name,omitempty"` +} + // MetadataOptions contains parameters for specifying the exposure of the // Instance Metadata Service to provisioned EC2 nodes. type MetadataOptions struct { diff --git a/pkg/apis/v1beta1/ec2nodeclass_status.go b/pkg/apis/v1beta1/ec2nodeclass_status.go index fb0c39ff6bec..20cbe1d510b6 100644 --- a/pkg/apis/v1beta1/ec2nodeclass_status.go +++ b/pkg/apis/v1beta1/ec2nodeclass_status.go @@ -63,7 +63,7 @@ type EC2NodeClassStatus struct { // cluster under the AMI selectors. // +optional AMIs []AMI `json:"amis,omitempty"` - // License contains the discovered license configurations + // Licenses contains the license arns // +optional Licenses []string `json:"licenses,omitempty"` } diff --git a/pkg/apis/v1beta1/zz_generated.deepcopy.go b/pkg/apis/v1beta1/zz_generated.deepcopy.go index 9a1c13e4c29f..8890dbbfb9c4 100644 --- a/pkg/apis/v1beta1/zz_generated.deepcopy.go +++ b/pkg/apis/v1beta1/zz_generated.deepcopy.go @@ -236,6 +236,11 @@ func (in *EC2NodeClassSpec) DeepCopyInto(out *EC2NodeClassSpec) { *out = new(string) **out = **in } + if in.LicenseSelectorTerms != nil { + in, out := &in.LicenseSelectorTerms, &out.LicenseSelectorTerms + *out = make([]LicenseSelectorTerm, len(*in)) + copy(*out, *in) + } if in.UserData != nil { in, out := &in.UserData, &out.UserData *out = new(string) @@ -305,6 +310,13 @@ func (in *EC2NodeClassSpec) DeepCopyInto(out *EC2NodeClassSpec) { (*out)[key] = val } } + if in.OriginalLicenseSelector != nil { + in, out := &in.OriginalLicenseSelector, &out.OriginalLicenseSelector + *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. @@ -354,6 +366,21 @@ func (in *EC2NodeClassStatus) DeepCopy() *EC2NodeClassStatus { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *LicenseSelectorTerm) DeepCopyInto(out *LicenseSelectorTerm) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new LicenseSelectorTerm. +func (in *LicenseSelectorTerm) DeepCopy() *LicenseSelectorTerm { + if in == nil { + return nil + } + out := new(LicenseSelectorTerm) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *MetadataOptions) DeepCopyInto(out *MetadataOptions) { *out = *in diff --git a/pkg/controllers/nodeclass/controller.go b/pkg/controllers/nodeclass/controller.go index b677fdfffe1a..8af6cae2ed8c 100644 --- a/pkg/controllers/nodeclass/controller.go +++ b/pkg/controllers/nodeclass/controller.go @@ -70,6 +70,7 @@ func (c *Controller) Reconcile(ctx context.Context, nodeClass *v1beta1.EC2NodeCl c.resolveSubnets(ctx, nodeClass), c.resolveSecurityGroups(ctx, nodeClass), c.resolveAMIs(ctx, nodeClass), + c.resolveLicenses(ctx, nodeClass), ) if !equality.Semantic.DeepEqual(stored, nodeClass) { statusCopy := nodeClass.DeepCopy() @@ -156,17 +157,13 @@ func (c *Controller) resolveAMIs(ctx context.Context, nodeClass *v1beta1.EC2Node } func (c *Controller) resolveLicenses(ctx context.Context, nodeClass *v1beta1.NodeClass) error { - licenses, err := c.licenseProvider.List(ctx, nodeClass) - if err != nil { - return err - } - if len(licenses) == 0 { - nodeClass.Status.Licenses = nil - return fmt.Errorf("no licenses match selector") - } - nodeClass.Status.Licenses = licenses - - return nil + licenses, err := c.licenseProvider.List(ctx, nodeClass) + if err != nil { + return err + } + nodeClass.Status.Licenses = licenses + + return nil } diff --git a/pkg/controllers/nodeclass/suite_test.go b/pkg/controllers/nodeclass/suite_test.go index fee20d1148c0..f4aaeb6b6d79 100644 --- a/pkg/controllers/nodeclass/suite_test.go +++ b/pkg/controllers/nodeclass/suite_test.go @@ -71,3 +71,702 @@ var _ = BeforeEach(func() { var _ = AfterEach(func() { ExpectCleanedUp(ctx, env.Client) }) + +var _ = Describe("AWSNodeTemplateController", func() { + Context("Subnet Status", func() { + It("Should update AWSNodeTemplate status for Subnets", func() { + ExpectApplied(ctx, env.Client, nodeTemplate) + ExpectReconcileSucceeded(ctx, controller, client.ObjectKeyFromObject(nodeTemplate)) + nodeTemplate = ExpectExists(ctx, env.Client, nodeTemplate) + Expect(nodeTemplate.Status.Subnets).To(ConsistOf( + v1alpha1.Subnet{ + ID: "subnet-test1", + Zone: "test-zone-1a", + }, + v1alpha1.Subnet{ + ID: "subnet-test2", + Zone: "test-zone-1b", + }, + v1alpha1.Subnet{ + ID: "subnet-test3", + Zone: "test-zone-1c", + }, + )) + }) + It("Should have the correct ordering for the Subnets", func() { + awsEnv.EC2API.DescribeSubnetsOutput.Set(&ec2.DescribeSubnetsOutput{Subnets: []*ec2.Subnet{ + {SubnetId: aws.String("subnet-test1"), AvailabilityZone: aws.String("test-zone-1a"), AvailableIpAddressCount: aws.Int64(20)}, + {SubnetId: aws.String("subnet-test2"), AvailabilityZone: aws.String("test-zone-1b"), AvailableIpAddressCount: aws.Int64(100)}, + {SubnetId: aws.String("subnet-test3"), AvailabilityZone: aws.String("test-zone-1c"), AvailableIpAddressCount: aws.Int64(50)}, + }}) + ExpectApplied(ctx, env.Client, nodeTemplate) + ExpectReconcileSucceeded(ctx, controller, client.ObjectKeyFromObject(nodeTemplate)) + nodeTemplate = ExpectExists(ctx, env.Client, nodeTemplate) + Expect(nodeTemplate.Status.Subnets).To(ConsistOf( + v1alpha1.Subnet{ + ID: "subnet-test2", + Zone: "test-zone-1b", + }, + v1alpha1.Subnet{ + ID: "subnet-test3", + Zone: "test-zone-1c", + }, + v1alpha1.Subnet{ + ID: "subnet-test1", + Zone: "test-zone-1a", + }, + )) + }) + It("Should resolve a valid selectors for Subnet by tags", func() { + nodeTemplate.Spec.SubnetSelector = map[string]string{`Name`: `test-subnet-1,test-subnet-2`} + ExpectApplied(ctx, env.Client, nodeTemplate) + ExpectReconcileSucceeded(ctx, controller, client.ObjectKeyFromObject(nodeTemplate)) + nodeTemplate = ExpectExists(ctx, env.Client, nodeTemplate) + Expect(nodeTemplate.Status.Subnets).To(ConsistOf( + v1alpha1.Subnet{ + ID: "subnet-test1", + Zone: "test-zone-1a", + }, + v1alpha1.Subnet{ + ID: "subnet-test2", + Zone: "test-zone-1b", + }, + )) + }) + It("Should resolve a valid selectors for Subnet by ids", func() { + nodeTemplate.Spec.SubnetSelector = map[string]string{`aws-ids`: `subnet-test1`} + ExpectApplied(ctx, env.Client, nodeTemplate) + ExpectReconcileSucceeded(ctx, controller, client.ObjectKeyFromObject(nodeTemplate)) + nodeTemplate = ExpectExists(ctx, env.Client, nodeTemplate) + Expect(nodeTemplate.Status.Subnets).To(Equal([]v1alpha1.Subnet{ + { + ID: "subnet-test1", + Zone: "test-zone-1a", + }, + })) + }) + It("Should update Subnet status when the Subnet selector gets updated by tags", func() { + ExpectApplied(ctx, env.Client, nodeTemplate) + ExpectReconcileSucceeded(ctx, controller, client.ObjectKeyFromObject(nodeTemplate)) + nodeTemplate = ExpectExists(ctx, env.Client, nodeTemplate) + Expect(nodeTemplate.Status.Subnets).To(ConsistOf( + v1alpha1.Subnet{ + ID: "subnet-test1", + Zone: "test-zone-1a", + }, + v1alpha1.Subnet{ + ID: "subnet-test2", + Zone: "test-zone-1b", + }, + v1alpha1.Subnet{ + ID: "subnet-test3", + Zone: "test-zone-1c", + }, + )) + + nodeTemplate.Spec.SubnetSelector = map[string]string{`Name`: `test-subnet-1,test-subnet-2`} + ExpectApplied(ctx, env.Client, nodeTemplate) + ExpectReconcileSucceeded(ctx, controller, client.ObjectKeyFromObject(nodeTemplate)) + nodeTemplate = ExpectExists(ctx, env.Client, nodeTemplate) + Expect(nodeTemplate.Status.Subnets).To(ConsistOf( + v1alpha1.Subnet{ + ID: "subnet-test1", + Zone: "test-zone-1a", + }, + v1alpha1.Subnet{ + ID: "subnet-test2", + Zone: "test-zone-1b", + }, + )) + }) + It("Should update Subnet status when the Subnet selector gets updated by ids", func() { + ExpectApplied(ctx, env.Client, nodeTemplate) + ExpectReconcileSucceeded(ctx, controller, client.ObjectKeyFromObject(nodeTemplate)) + nodeTemplate = ExpectExists(ctx, env.Client, nodeTemplate) + Expect(nodeTemplate.Status.Subnets).To(ConsistOf( + v1alpha1.Subnet{ + ID: "subnet-test1", + Zone: "test-zone-1a", + }, + v1alpha1.Subnet{ + ID: "subnet-test2", + Zone: "test-zone-1b", + }, + v1alpha1.Subnet{ + ID: "subnet-test3", + Zone: "test-zone-1c", + }, + )) + + nodeTemplate.Spec.SubnetSelector = map[string]string{`aws-ids`: `subnet-test1`} + ExpectApplied(ctx, env.Client, nodeTemplate) + ExpectReconcileSucceeded(ctx, controller, client.ObjectKeyFromObject(nodeTemplate)) + nodeTemplate = ExpectExists(ctx, env.Client, nodeTemplate) + Expect(nodeTemplate.Status.Subnets).To(ConsistOf( + v1alpha1.Subnet{ + ID: "subnet-test1", + Zone: "test-zone-1a", + }, + )) + }) + It("Should not resolve a invalid selectors for Subnet", func() { + nodeTemplate.Spec.SubnetSelector = map[string]string{`foo`: `invalid`} + ExpectApplied(ctx, env.Client, nodeTemplate) + ExpectReconcileFailed(ctx, controller, client.ObjectKeyFromObject(nodeTemplate)) + nodeTemplate = ExpectExists(ctx, env.Client, nodeTemplate) + Expect(nodeTemplate.Status.Subnets).To(BeNil()) + }) + It("Should not resolve a invalid selectors for an updated Subnet selectors", func() { + ExpectApplied(ctx, env.Client, nodeTemplate) + ExpectReconcileSucceeded(ctx, controller, client.ObjectKeyFromObject(nodeTemplate)) + nodeTemplate = ExpectExists(ctx, env.Client, nodeTemplate) + Expect(nodeTemplate.Status.Subnets).To(ConsistOf( + v1alpha1.Subnet{ + ID: "subnet-test1", + Zone: "test-zone-1a", + }, + v1alpha1.Subnet{ + ID: "subnet-test2", + Zone: "test-zone-1b", + }, + v1alpha1.Subnet{ + ID: "subnet-test3", + Zone: "test-zone-1c", + }, + )) + + nodeTemplate.Spec.SubnetSelector = map[string]string{`foo`: `invalid`} + ExpectApplied(ctx, env.Client, nodeTemplate) + ExpectReconcileFailed(ctx, controller, client.ObjectKeyFromObject(nodeTemplate)) + nodeTemplate = ExpectExists(ctx, env.Client, nodeTemplate) + Expect(nodeTemplate.Status.Subnets).To(BeNil()) + }) + }) + Context("Security Groups Status", func() { + It("Should expect no errors when security groups are not in the AWSNodeTemplate", func() { + // TODO: Remove test for v1beta1, as security groups will be required + nodeTemplate.Spec.SecurityGroupSelector = nil + ExpectApplied(ctx, env.Client, nodeTemplate) + ExpectReconcileSucceeded(ctx, controller, client.ObjectKeyFromObject(nodeTemplate)) + Expect(nodeTemplate.Status.SecurityGroups).To(BeNil()) + }) + It("Should update AWSNodeTemplate status for Security Groups", func() { + ExpectApplied(ctx, env.Client, nodeTemplate) + ExpectReconcileSucceeded(ctx, controller, client.ObjectKeyFromObject(nodeTemplate)) + nodeTemplate = ExpectExists(ctx, env.Client, nodeTemplate) + Expect(nodeTemplate.Status.SecurityGroups).To(ConsistOf( + v1alpha1.SecurityGroup{ + ID: "sg-test1", + Name: "securityGroup-test1", + }, + v1alpha1.SecurityGroup{ + ID: "sg-test2", + Name: "securityGroup-test2", + }, + v1alpha1.SecurityGroup{ + ID: "sg-test3", + Name: "securityGroup-test3", + }, + )) + }) + It("Should resolve a valid selectors for Security Groups by tags", func() { + nodeTemplate.Spec.SecurityGroupSelector = map[string]string{`Name`: `test-security-group-1,test-security-group-2`} + ExpectApplied(ctx, env.Client, nodeTemplate) + ExpectReconcileSucceeded(ctx, controller, client.ObjectKeyFromObject(nodeTemplate)) + nodeTemplate = ExpectExists(ctx, env.Client, nodeTemplate) + Expect(nodeTemplate.Status.SecurityGroups).To(ConsistOf( + v1alpha1.SecurityGroup{ + ID: "sg-test1", + Name: "securityGroup-test1", + }, + v1alpha1.SecurityGroup{ + ID: "sg-test2", + Name: "securityGroup-test2", + }, + )) + }) + It("Should resolve a valid selectors for Security Groups by ids", func() { + nodeTemplate.Spec.SecurityGroupSelector = map[string]string{`aws-ids`: `sg-test1`} + ExpectApplied(ctx, env.Client, nodeTemplate) + ExpectReconcileSucceeded(ctx, controller, client.ObjectKeyFromObject(nodeTemplate)) + nodeTemplate = ExpectExists(ctx, env.Client, nodeTemplate) + Expect(nodeTemplate.Status.SecurityGroups).To(ConsistOf( + v1alpha1.SecurityGroup{ + ID: "sg-test1", + Name: "securityGroup-test1", + }, + )) + }) + It("Should update Security Groups status when the Security Groups selector gets updated by tags", func() { + ExpectApplied(ctx, env.Client, nodeTemplate) + ExpectReconcileSucceeded(ctx, controller, client.ObjectKeyFromObject(nodeTemplate)) + nodeTemplate = ExpectExists(ctx, env.Client, nodeTemplate) + Expect(nodeTemplate.Status.SecurityGroups).To(ConsistOf( + v1alpha1.SecurityGroup{ + ID: "sg-test1", + Name: "securityGroup-test1", + }, + v1alpha1.SecurityGroup{ + ID: "sg-test2", + Name: "securityGroup-test2", + }, + v1alpha1.SecurityGroup{ + ID: "sg-test3", + Name: "securityGroup-test3", + }, + )) + + nodeTemplate.Spec.SecurityGroupSelector = map[string]string{`Name`: `test-security-group-1,test-security-group-2`} + ExpectApplied(ctx, env.Client, nodeTemplate) + ExpectReconcileSucceeded(ctx, controller, client.ObjectKeyFromObject(nodeTemplate)) + nodeTemplate = ExpectExists(ctx, env.Client, nodeTemplate) + Expect(nodeTemplate.Status.SecurityGroups).To(ConsistOf( + v1alpha1.SecurityGroup{ + ID: "sg-test1", + Name: "securityGroup-test1", + }, + v1alpha1.SecurityGroup{ + ID: "sg-test2", + Name: "securityGroup-test2", + }, + )) + }) + It("Should update Security Groups status when the Security Groups selector gets updated by ids", func() { + ExpectApplied(ctx, env.Client, nodeTemplate) + ExpectReconcileSucceeded(ctx, controller, client.ObjectKeyFromObject(nodeTemplate)) + nodeTemplate = ExpectExists(ctx, env.Client, nodeTemplate) + Expect(nodeTemplate.Status.SecurityGroups).To(ConsistOf( + v1alpha1.SecurityGroup{ + ID: "sg-test1", + Name: "securityGroup-test1", + }, + v1alpha1.SecurityGroup{ + ID: "sg-test2", + Name: "securityGroup-test2", + }, + v1alpha1.SecurityGroup{ + ID: "sg-test3", + Name: "securityGroup-test3", + }, + )) + + nodeTemplate.Spec.SecurityGroupSelector = map[string]string{`aws-ids`: `sg-test1`} + ExpectApplied(ctx, env.Client, nodeTemplate) + ExpectReconcileSucceeded(ctx, controller, client.ObjectKeyFromObject(nodeTemplate)) + nodeTemplate = ExpectExists(ctx, env.Client, nodeTemplate) + Expect(nodeTemplate.Status.SecurityGroups).To(ConsistOf( + v1alpha1.SecurityGroup{ + ID: "sg-test1", + Name: "securityGroup-test1", + }, + )) + }) + It("Should not resolve a invalid selectors for Security Groups", func() { + nodeTemplate.Spec.SecurityGroupSelector = map[string]string{`foo`: `invalid`} + ExpectApplied(ctx, env.Client, nodeTemplate) + ExpectReconcileFailed(ctx, controller, client.ObjectKeyFromObject(nodeTemplate)) + nodeTemplate = ExpectExists(ctx, env.Client, nodeTemplate) + Expect(nodeTemplate.Status.SecurityGroups).To(BeNil()) + }) + It("Should not resolve a invalid selectors for an updated Security Groups selector", func() { + ExpectApplied(ctx, env.Client, nodeTemplate) + ExpectReconcileSucceeded(ctx, controller, client.ObjectKeyFromObject(nodeTemplate)) + nodeTemplate = ExpectExists(ctx, env.Client, nodeTemplate) + Expect(nodeTemplate.Status.SecurityGroups).To(ConsistOf( + v1alpha1.SecurityGroup{ + ID: "sg-test1", + Name: "securityGroup-test1", + }, + v1alpha1.SecurityGroup{ + ID: "sg-test2", + Name: "securityGroup-test2", + }, + v1alpha1.SecurityGroup{ + ID: "sg-test3", + Name: "securityGroup-test3", + }, + )) + + nodeTemplate.Spec.SecurityGroupSelector = map[string]string{`foo`: `invalid`} + ExpectApplied(ctx, env.Client, nodeTemplate) + ExpectReconcileFailed(ctx, controller, client.ObjectKeyFromObject(nodeTemplate)) + nodeTemplate = ExpectExists(ctx, env.Client, nodeTemplate) + Expect(nodeTemplate.Status.SecurityGroups).To(BeNil()) + }) + }) + Context("AMI Status", func() { + BeforeEach(func() { + awsEnv.EC2API.DescribeImagesOutput.Set(&ec2.DescribeImagesOutput{ + Images: []*ec2.Image{ + { + Name: aws.String("test-ami-1"), + ImageId: aws.String("ami-test1"), + CreationDate: aws.String(time.Now().Format(time.RFC3339)), + Architecture: aws.String("x86_64"), + Tags: []*ec2.Tag{ + {Key: aws.String("Name"), Value: aws.String("test-ami-1")}, + {Key: aws.String("foo"), Value: aws.String("bar")}, + }, + }, + { + Name: aws.String("test-ami-2"), + ImageId: aws.String("ami-test2"), + CreationDate: aws.String(time.Now().Add(time.Minute).Format(time.RFC3339)), + Architecture: aws.String("x86_64"), + Tags: []*ec2.Tag{ + {Key: aws.String("Name"), Value: aws.String("test-ami-2")}, + {Key: aws.String("foo"), Value: aws.String("bar")}, + }, + }, + { + Name: aws.String("test-ami-3"), + ImageId: aws.String("ami-test3"), + CreationDate: aws.String(time.Now().Add(2 * time.Minute).Format(time.RFC3339)), + Architecture: aws.String("x86_64"), + Tags: []*ec2.Tag{ + {Key: aws.String("Name"), Value: aws.String("test-ami-3")}, + {Key: aws.String("foo"), Value: aws.String("bar")}, + }, + }, + }, + }) + }) + It("should resolve amiSelector AMIs and requirements into status", func() { + version := lo.Must(awsEnv.VersionProvider.Get(ctx)) + + awsEnv.SSMAPI.Parameters = map[string]string{ + fmt.Sprintf("/aws/service/eks/optimized-ami/%s/amazon-linux-2/recommended/image_id", version): "ami-id-123", + fmt.Sprintf("/aws/service/eks/optimized-ami/%s/amazon-linux-2-gpu/recommended/image_id", version): "ami-id-456", + fmt.Sprintf("/aws/service/eks/optimized-ami/%s/amazon-linux-2%s/recommended/image_id", version, fmt.Sprintf("-%s", v1alpha5.ArchitectureArm64)): "ami-id-789", + } + + awsEnv.EC2API.DescribeImagesOutput.Set(&ec2.DescribeImagesOutput{ + Images: []*ec2.Image{ + { + Name: aws.String("test-ami-1"), + ImageId: aws.String("ami-id-123"), + CreationDate: aws.String(time.Now().Format(time.RFC3339)), + Architecture: aws.String("x86_64"), + Tags: []*ec2.Tag{ + {Key: aws.String("Name"), Value: aws.String("test-ami-1")}, + {Key: aws.String("foo"), Value: aws.String("bar")}, + }, + }, + { + Name: aws.String("test-ami-2"), + ImageId: aws.String("ami-id-456"), + CreationDate: aws.String(time.Now().Add(time.Minute).Format(time.RFC3339)), + Architecture: aws.String("x86_64"), + Tags: []*ec2.Tag{ + {Key: aws.String("Name"), Value: aws.String("test-ami-2")}, + {Key: aws.String("foo"), Value: aws.String("bar")}, + }, + }, + { + Name: aws.String("test-ami-3"), + ImageId: aws.String("ami-id-789"), + CreationDate: aws.String(time.Now().Add(2 * time.Minute).Format(time.RFC3339)), + Architecture: aws.String("x86_64"), + Tags: []*ec2.Tag{ + {Key: aws.String("Name"), Value: aws.String("test-ami-3")}, + {Key: aws.String("foo"), Value: aws.String("bar")}, + }, + }, + }, + }) + nodeTemplate.Spec.AMISelector = nil + ExpectApplied(ctx, env.Client, nodeTemplate) + ExpectReconcileSucceeded(ctx, controller, client.ObjectKeyFromObject(nodeTemplate)) + nodeTemplate = ExpectExists(ctx, env.Client, nodeTemplate) + ExpectConsistOfAMIs([]v1alpha1.AMI{ + { + Name: "test-ami-1", + ID: "ami-id-123", + Requirements: []v1.NodeSelectorRequirement{ + { + Key: v1.LabelArchStable, + Operator: v1.NodeSelectorOpIn, + Values: []string{v1alpha5.ArchitectureAmd64}, + }, + { + Key: v1alpha1.LabelInstanceGPUCount, + Operator: v1.NodeSelectorOpDoesNotExist, + }, + { + Key: v1alpha1.LabelInstanceAcceleratorCount, + Operator: v1.NodeSelectorOpDoesNotExist, + }, + }, + }, + { + Name: "test-ami-3", + ID: "ami-id-789", + Requirements: []v1.NodeSelectorRequirement{ + { + Key: v1.LabelArchStable, + Operator: v1.NodeSelectorOpIn, + Values: []string{v1alpha5.ArchitectureArm64}, + }, + { + Key: v1alpha1.LabelInstanceGPUCount, + Operator: v1.NodeSelectorOpDoesNotExist, + }, + { + Key: v1alpha1.LabelInstanceAcceleratorCount, + Operator: v1.NodeSelectorOpDoesNotExist, + }, + }, + }, + { + Name: "test-ami-2", + ID: "ami-id-456", + Requirements: []v1.NodeSelectorRequirement{ + { + Key: v1.LabelArchStable, + Operator: v1.NodeSelectorOpIn, + Values: []string{v1alpha5.ArchitectureAmd64}, + }, + { + Key: v1alpha1.LabelInstanceGPUCount, + Operator: v1.NodeSelectorOpExists, + }, + }, + }, + { + Name: "test-ami-2", + ID: "ami-id-456", + Requirements: []v1.NodeSelectorRequirement{ + { + Key: v1.LabelArchStable, + Operator: v1.NodeSelectorOpIn, + Values: []string{v1alpha5.ArchitectureAmd64}, + }, + { + Key: v1alpha1.LabelInstanceAcceleratorCount, + Operator: v1.NodeSelectorOpExists, + }, + }, + }, + }, nodeTemplate.Status.AMIs) + }) + It("should resolve amiSelector AMis and requirements into status when all SSM aliases don't resolve", func() { + version := lo.Must(awsEnv.VersionProvider.Get(ctx)) + // This parameter set doesn't include any of the Nvidia AMIs + awsEnv.SSMAPI.Parameters = map[string]string{ + fmt.Sprintf("/aws/service/bottlerocket/aws-k8s-%s/x86_64/latest/image_id", version): "ami-id-123", + fmt.Sprintf("/aws/service/bottlerocket/aws-k8s-%s/arm64/latest/image_id", version): "ami-id-456", + } + nodeTemplate.Spec.AMIFamily = &v1alpha1.AMIFamilyBottlerocket + nodeTemplate.Spec.AMISelector = nil + awsEnv.EC2API.DescribeImagesOutput.Set(&ec2.DescribeImagesOutput{ + Images: []*ec2.Image{ + { + Name: aws.String("test-ami-1"), + ImageId: aws.String("ami-id-123"), + CreationDate: aws.String(time.Now().Format(time.RFC3339)), + Architecture: aws.String("x86_64"), + Tags: []*ec2.Tag{ + {Key: aws.String("Name"), Value: aws.String("test-ami-1")}, + {Key: aws.String("foo"), Value: aws.String("bar")}, + }, + }, + { + Name: aws.String("test-ami-2"), + ImageId: aws.String("ami-id-456"), + CreationDate: aws.String(time.Now().Add(time.Minute).Format(time.RFC3339)), + Architecture: aws.String("arm64"), + Tags: []*ec2.Tag{ + {Key: aws.String("Name"), Value: aws.String("test-ami-2")}, + {Key: aws.String("foo"), Value: aws.String("bar")}, + }, + }, + }, + }) + ExpectApplied(ctx, env.Client, nodeTemplate) + ExpectReconcileSucceeded(ctx, controller, client.ObjectKeyFromObject(nodeTemplate)) + nodeTemplate = ExpectExists(ctx, env.Client, nodeTemplate) + + ExpectConsistOfAMIs([]v1alpha1.AMI{ + { + Name: "test-ami-1", + ID: "ami-id-123", + Requirements: []v1.NodeSelectorRequirement{ + { + Key: v1.LabelArchStable, + Operator: v1.NodeSelectorOpIn, + Values: []string{v1alpha5.ArchitectureAmd64}, + }, + { + Key: v1alpha1.LabelInstanceGPUCount, + Operator: v1.NodeSelectorOpDoesNotExist, + }, + { + Key: v1alpha1.LabelInstanceAcceleratorCount, + Operator: v1.NodeSelectorOpDoesNotExist, + }, + }, + }, + { + Name: "test-ami-2", + ID: "ami-id-456", + Requirements: []v1.NodeSelectorRequirement{ + { + Key: v1.LabelArchStable, + Operator: v1.NodeSelectorOpIn, + Values: []string{v1alpha5.ArchitectureArm64}, + }, + { + Key: v1alpha1.LabelInstanceGPUCount, + Operator: v1.NodeSelectorOpDoesNotExist, + }, + { + Key: v1alpha1.LabelInstanceAcceleratorCount, + Operator: v1.NodeSelectorOpDoesNotExist, + }, + }, + }, + }, nodeTemplate.Status.AMIs) + }) + It("Should resolve a valid AMI selector", func() { + ExpectApplied(ctx, env.Client, nodeTemplate) + ExpectReconcileSucceeded(ctx, controller, client.ObjectKeyFromObject(nodeTemplate)) + nodeTemplate = ExpectExists(ctx, env.Client, nodeTemplate) + Expect(nodeTemplate.Status.AMIs).To(ContainElements( + []v1alpha1.AMI{ + { + Name: "test-ami-3", + ID: "ami-test3", + Requirements: []v1.NodeSelectorRequirement{ + { + Key: "kubernetes.io/arch", + Operator: "In", + Values: []string{ + "amd64", + }, + }, + }, + }, + }, + )) + }) + It("should resolve amiSelector AMIs that have well-known tags as AMI requirements into status", func() { + awsEnv.EC2API.DescribeImagesOutput.Set(&ec2.DescribeImagesOutput{ + Images: []*ec2.Image{ + { + Name: aws.String("test-ami-4"), + ImageId: aws.String("ami-test4"), + CreationDate: aws.String(time.Now().Add(2 * time.Minute).Format(time.RFC3339)), + Architecture: aws.String("x86_64"), + Tags: []*ec2.Tag{ + {Key: aws.String("Name"), Value: aws.String("test-ami-3")}, + {Key: aws.String("foo"), Value: aws.String("bar")}, + {Key: aws.String("kubernetes.io/os"), Value: aws.String("test-requirement-1")}, + }, + }, + }, + }) + ExpectApplied(ctx, env.Client, nodeTemplate) + ExpectReconcileSucceeded(ctx, controller, client.ObjectKeyFromObject(nodeTemplate)) + nodeTemplate = ExpectExists(ctx, env.Client, nodeTemplate) + + ExpectConsistOfAMIs([]v1alpha1.AMI{ + { + Name: "test-ami-4", + ID: "ami-test4", + Requirements: []v1.NodeSelectorRequirement{ + { + Key: "kubernetes.io/os", + Operator: "In", + Values: []string{ + "test-requirement-1", + }, + }, + { + Key: "kubernetes.io/arch", + Operator: "In", + Values: []string{ + "amd64", + }, + }, + }, + }, + }, nodeTemplate.Status.AMIs) + }) + }) + Context("License Status", func() { + It("Should resolve a valid License selector", func() { + ExpectApplied(ctx, env.Client, nodeTemplate) + ExpectReconcileSucceeded(ctx, controller, client.ObjectKeyFromObject(nodeTemplate)) + nodeTemplate = ExpectExists(ctx, env.Client, nodeTemplate) + }) + }) + Context("AWSNodeTemplate Static Drift Hash", func() { + DescribeTable("should update the static drift hash when nodeTemplate static field is updated", func(awsnodetemplatespec v1alpha1.AWSNodeTemplateSpec) { + updatedAWSNodeTemplate := test.AWSNodeTemplate(*nodeTemplate.Spec.DeepCopy(), awsnodetemplatespec) + updatedAWSNodeTemplate.ObjectMeta = nodeTemplate.ObjectMeta + updatedAWSNodeTemplate.Annotations = map[string]string{v1alpha1.AnnotationNodeTemplateHash: updatedAWSNodeTemplate.Hash()} + + ExpectApplied(ctx, env.Client, nodeTemplate) + ExpectReconcileSucceeded(ctx, controller, client.ObjectKeyFromObject(nodeTemplate)) + nodeTemplate = ExpectExists(ctx, env.Client, nodeTemplate) + + expectedHash := nodeTemplate.Hash() + Expect(nodeTemplate.ObjectMeta.Annotations[v1alpha1.AnnotationNodeTemplateHash]).To(Equal(expectedHash)) + + ExpectApplied(ctx, env.Client, updatedAWSNodeTemplate) + ExpectReconcileSucceeded(ctx, controller, client.ObjectKeyFromObject(nodeTemplate)) + nodeTemplate = ExpectExists(ctx, env.Client, nodeTemplate) + + expectedHashTwo := nodeTemplate.Hash() + Expect(expectedHash).ToNot(Equal(expectedHashTwo)) + Expect(nodeTemplate.ObjectMeta.Annotations[v1alpha1.AnnotationNodeTemplateHash]).To(Equal(expectedHashTwo)) + }, + 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)}), + Entry("AMIFamily Drift", v1alpha1.AWSNodeTemplateSpec{AWS: v1alpha1.AWS{AMIFamily: aws.String(v1alpha1.AMIFamilyBottlerocket)}}), + ) + It("should not update the static drift hash when nodeTemplate dynamic field is updated", func() { + ExpectApplied(ctx, env.Client, nodeTemplate) + ExpectReconcileSucceeded(ctx, controller, client.ObjectKeyFromObject(nodeTemplate)) + nodeTemplate = ExpectExists(ctx, env.Client, nodeTemplate) + + expectedHash := nodeTemplate.Hash() + Expect(nodeTemplate.ObjectMeta.Annotations[v1alpha1.AnnotationNodeTemplateHash]).To(Equal(expectedHash)) + + nodeTemplate.Spec.SubnetSelector = map[string]string{"aws-ids": "subnet-test1"} + nodeTemplate.Spec.SecurityGroupSelector = map[string]string{"aws-ids": "sg-test1"} + nodeTemplate.Spec.AMISelector = map[string]string{"ami-test-key": "ami-test-value"} + + ExpectApplied(ctx, env.Client, nodeTemplate) + ExpectReconcileSucceeded(ctx, controller, client.ObjectKeyFromObject(nodeTemplate)) + nodeTemplate = ExpectExists(ctx, env.Client, nodeTemplate) + + Expect(nodeTemplate.ObjectMeta.Annotations[v1alpha1.AnnotationNodeTemplateHash]).To(Equal(expectedHash)) + }) + It("should maintain the same hash, before and after the NodeClass conversion", func() { + hash := nodeTemplate.Hash() + nodeClass := nodeclassutil.New(nodeTemplate) + convertedHash := nodeclassutil.HashAnnotation(nodeClass) + Expect(convertedHash).To(HaveKeyWithValue(v1alpha1.AnnotationNodeTemplateHash, hash)) + }) + }) +}) + +func ExpectConsistOfAMIs(expected, actual []v1alpha1.AMI) { + GinkgoHelper() + Expect(actual).To(HaveLen(len(expected))) + + for _, list := range [][]v1alpha1.AMI{expected, actual} { + for _, elem := range list { + sort.Slice(elem.Requirements, func(i, j int) bool { + return elem.Requirements[i].Key < elem.Requirements[j].Key + }) + } + } + Expect(actual).To(ConsistOf(lo.Map(expected, func(a v1alpha1.AMI, _ int) interface{} { return a })...)) +} diff --git a/pkg/operator/operator.go b/pkg/operator/operator.go index 73ee09f732fa..241df89fd468 100644 --- a/pkg/operator/operator.go +++ b/pkg/operator/operator.go @@ -30,6 +30,7 @@ import ( "github.com/aws/aws-sdk-go/aws/endpoints" "github.com/aws/aws-sdk-go/aws/request" "github.com/aws/aws-sdk-go/aws/session" + "github.com/aws/aws-sdk-go/service/licensemanager" "github.com/aws/aws-sdk-go/service/ec2" "github.com/aws/aws-sdk-go/service/ec2/ec2iface" "github.com/aws/aws-sdk-go/service/eks" @@ -163,7 +164,8 @@ func NewOperator(ctx context.Context, operator *operator.Operator) (context.Cont subnetProvider, launchTemplateProvider, ) - licenseProvider := license.NewProvider(ec2api, cache.New(awscache.DefaultTTL, awscache.DefaultCleanupInterval)) + licenseProvider := license.NewProvider(licensemanager.New(sess) , cache.New(awscache.DefaultTTL, awscache.DefaultCleanupInterval)) + return ctx, &Operator{ Operator: operator, diff --git a/pkg/providers/license/license.go b/pkg/providers/license/license.go index 3f700aee961a..b1c8f2fbda57 100644 --- a/pkg/providers/license/license.go +++ b/pkg/providers/license/license.go @@ -1,28 +1,47 @@ +/* +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + package license import ( "context" + "fmt" "sync" - "github.com/aws/aws-sdk-go/service/ec2/ec2iface" + "github.com/aws/aws-sdk-go/service/licensemanager" + "github.com/aws/aws-sdk-go/service/licensemanager/licensemanageriface" + "github.com/mitchellh/hashstructure/v2" "github.com/patrickmn/go-cache" "github.com/aws/karpenter/pkg/apis/v1beta1" "github.com/aws/karpenter-core/pkg/utils/pretty" + "github.com/samber/lo" + "knative.dev/pkg/logging" ) type Provider struct { sync.RWMutex - ec2api ec2iface.EC2API - cache *cache.Cache - cm *pretty.ChangeMonitor + licensemanager licensemanageriface.LicenseManagerAPI + cache *cache.Cache + cm *pretty.ChangeMonitor } -func NewProvider(ec2api ec2iface.EC2API, cache *cache.Cache) *Provider { +func NewProvider(lmapi licensemanageriface.LicenseManagerAPI, cache *cache.Cache) *Provider { return &Provider{ - ec2api: ec2api, - cm: pretty.NewChangeMonitor(), + licensemanager: lmapi, + cm: pretty.NewChangeMonitor(), // TODO: Remove cache for v1beta1, utilize resolved subnet from the AWSNodeTemplate.status // Subnets are sorted on AvailableIpAddressCount, descending order cache: cache, @@ -30,5 +49,45 @@ func NewProvider(ec2api ec2iface.EC2API, cache *cache.Cache) *Provider { } func (p *Provider) List(ctx context.Context, nodeClass *v1beta1.NodeClass) ([]string, error) { - return []string{"beep", "boop", "bop"}, nil + p.Lock() + defer p.Unlock() + + // Get selectors from the nodeClass, exit if no selectors defined + selectors := nodeClass.Spec.LicenseSelectorTerms + if selectors == nil { + logging.FromContext(ctx). + Debugf("no selectors present") + return nil, nil + } + + // Look for a cached result + hash, err := hashstructure.Hash(selectors, hashstructure.FormatV2, &hashstructure.HashOptions{SlicesAsSets: true}) + if err != nil { + return nil, err + } + if cached, ok := p.cache.Get(fmt.Sprint(hash)); ok { + return cached.([]string), nil + } + + licenses := []string{} + // Look up all License Configurations + output, err := p.licensemanager.ListLicenseConfigurationsWithContext(ctx, &licensemanager.ListLicenseConfigurationsInput{}) + if err != nil { + return nil, err + } + for i := range output.LicenseConfigurations { + // filter results to only include those that match at least 1 selector + for x := range selectors { + if *output.LicenseConfigurations[i].Name == selectors[x].Name { + licenses = append(licenses, *output.LicenseConfigurations[i].LicenseConfigurationArn) + } + } + } + + if p.cm.HasChanged(fmt.Sprintf("license/%t/%s", nodeClass.IsNodeTemplate, nodeClass.Name), licenses) { + logging.FromContext(ctx). + With("licenseProvider", lo.Map(licenses, func(s string, _ int) string { return s })). + Debugf("discovered license configuration") + } + return licenses, nil } diff --git a/pkg/test/environment.go b/pkg/test/environment.go index 34af278641ba..d4bc98e67e33 100644 --- a/pkg/test/environment.go +++ b/pkg/test/environment.go @@ -28,6 +28,7 @@ import ( "github.com/aws/karpenter/pkg/providers/instance" "github.com/aws/karpenter/pkg/providers/instancetype" "github.com/aws/karpenter/pkg/providers/launchtemplate" + "github.com/aws/karpenter/pkg/providers/license" "github.com/aws/karpenter/pkg/providers/pricing" "github.com/aws/karpenter/pkg/providers/securitygroup" "github.com/aws/karpenter/pkg/providers/subnet" @@ -63,6 +64,7 @@ type Environment struct { AMIResolver *amifamily.Resolver VersionProvider *version.Provider LaunchTemplateProvider *launchtemplate.Provider + LicenseProvider *license.Provider } func NewEnvironment(ctx context.Context, env *coretest.Environment) *Environment { @@ -78,6 +80,7 @@ func NewEnvironment(ctx context.Context, env *coretest.Environment) *Environment launchTemplateCache := cache.New(awscache.DefaultTTL, awscache.DefaultCleanupInterval) subnetCache := cache.New(awscache.DefaultTTL, awscache.DefaultCleanupInterval) securityGroupCache := cache.New(awscache.DefaultTTL, awscache.DefaultCleanupInterval) + licenseCache := cache.New(awscache.DefaultTTL, awscache.DefaultCleanupInterval) fakePricingAPI := &fake.PricingAPI{} // Providers @@ -110,6 +113,7 @@ func NewEnvironment(ctx context.Context, env *coretest.Environment) *Environment subnetProvider, launchTemplateProvider, ) + licenseProvider := license.NewProvider(ec2api, licenseCache) return &Environment{ EC2API: ec2api, @@ -133,6 +137,7 @@ func NewEnvironment(ctx context.Context, env *coretest.Environment) *Environment AMIResolver: amiResolver, VersionProvider: versionProvider, LaunchTemplateProvider: launchTemplateProvider, + LicenseProvider: licenseProvider, } } diff --git a/pkg/utils/nodeclass/nodeclass.go b/pkg/utils/nodeclass/nodeclass.go index 1f818689beab..440fc9bdcee4 100644 --- a/pkg/utils/nodeclass/nodeclass.go +++ b/pkg/utils/nodeclass/nodeclass.go @@ -44,6 +44,8 @@ func New(nodeTemplate *v1alpha1.AWSNodeTemplate) *v1beta1.EC2NodeClass { AMISelectorTerms: NewAMISelectorTerms(nodeTemplate.Spec.AMISelector), OriginalAMISelector: nodeTemplate.Spec.AMISelector, AMIFamily: nodeTemplate.Spec.AMIFamily, + LicenseSelectorTerms: NewLicenseSelectorTerms(nodeTemplate.Spec.LicenseSelector), + OriginalLicenseSelector: nodeTemplate.Spec.LicenseSelector, UserData: nodeTemplate.Spec.UserData, Tags: nodeTemplate.Spec.Tags, BlockDeviceMappings: NewBlockDeviceMappings(nodeTemplate.Spec.BlockDeviceMappings), @@ -148,6 +150,15 @@ func NewAMISelectorTerms(amiSelector map[string]string) (terms []v1beta1.AMISele } return terms } +func NewLicenseSelectorTerms(selectors map[string]string) (terms []v1beta1.LicenseSelectorTerm) { + if len(selectors) == 0 { + return nil + } + + return []v1beta1.LicenseSelectorTerm{ + {Name: selectors["name"]}, + } +} func NewBlockDeviceMappings(bdms []*v1alpha1.BlockDeviceMapping) []*v1beta1.BlockDeviceMapping { if bdms == nil { diff --git a/pkg/utils/nodetemplate/nodetemplate.go b/pkg/utils/nodetemplate/nodetemplate.go index 3112a04b51e9..a1708fdf317f 100644 --- a/pkg/utils/nodetemplate/nodetemplate.go +++ b/pkg/utils/nodetemplate/nodetemplate.go @@ -33,6 +33,7 @@ func New(nodeClass *v1beta1.EC2NodeClass) *v1alpha1.AWSNodeTemplate { InstanceProfile: nodeClass.Spec.InstanceProfile, SubnetSelector: nodeClass.Spec.OriginalSubnetSelector, SecurityGroupSelector: nodeClass.Spec.OriginalSecurityGroupSelector, + LicenseSelector: nodeClass.Spec.OriginalLicenseSelector, Tags: nodeClass.Spec.Tags, LaunchTemplate: v1alpha1.LaunchTemplate{ LaunchTemplateName: nodeClass.Spec.LaunchTemplateName, @@ -47,6 +48,7 @@ func New(nodeClass *v1beta1.EC2NodeClass) *v1alpha1.AWSNodeTemplate { Subnets: NewSubnets(nodeClass.Status.Subnets), SecurityGroups: NewSecurityGroups(nodeClass.Status.SecurityGroups), AMIs: NewAMIs(nodeClass.Status.AMIs), + Licenses: NewLicenses(nodeClass.Status.Licenses), }, } } @@ -134,3 +136,7 @@ func NewAMIs(amis []v1beta1.AMI) []v1alpha1.AMI { } }) } + +func NewLicenses(licenses []string) []string { + return licenses +} From c050b875820df75d2f705a57285014e92cdcbd9e Mon Sep 17 00:00:00 2001 From: Sebastian Cole Date: Wed, 13 Sep 2023 13:47:04 +1000 Subject: [PATCH 08/33] Add license provider into launchtemplate creation --- pkg/operator/operator.go | 5 +++-- pkg/providers/amifamily/resolver.go | 15 ++++++++++++--- .../launchtemplate/launchtemplate.go | 19 ++++++++++++++++++- pkg/test/environment.go | 4 ++-- 4 files changed, 35 insertions(+), 8 deletions(-) diff --git a/pkg/operator/operator.go b/pkg/operator/operator.go index 241df89fd468..596922cb62b4 100644 --- a/pkg/operator/operator.go +++ b/pkg/operator/operator.go @@ -134,7 +134,8 @@ func NewOperator(ctx context.Context, operator *operator.Operator) (context.Cont ) versionProvider := version.NewProvider(operator.KubernetesInterface, cache.New(awscache.DefaultTTL, awscache.DefaultCleanupInterval)) amiProvider := amifamily.NewProvider(versionProvider, ssm.New(sess), ec2api, cache.New(awscache.DefaultTTL, awscache.DefaultCleanupInterval)) - amiResolver := amifamily.New(amiProvider) + licenseProvider := license.NewProvider(licensemanager.New(sess) , cache.New(awscache.DefaultTTL, awscache.DefaultCleanupInterval)) + amiResolver := amifamily.New(amiProvider, licenseProvider) launchTemplateProvider := launchtemplate.NewProvider( ctx, cache.New(awscache.DefaultTTL, awscache.DefaultCleanupInterval), @@ -142,6 +143,7 @@ func NewOperator(ctx context.Context, operator *operator.Operator) (context.Cont amiResolver, securityGroupProvider, subnetProvider, + licenseProvider, lo.Must(getCABundle(ctx, operator.GetConfig())), operator.Elected(), kubeDNSIP, @@ -164,7 +166,6 @@ func NewOperator(ctx context.Context, operator *operator.Operator) (context.Cont subnetProvider, launchTemplateProvider, ) - licenseProvider := license.NewProvider(licensemanager.New(sess) , cache.New(awscache.DefaultTTL, awscache.DefaultCleanupInterval)) return ctx, &Operator{ diff --git a/pkg/providers/amifamily/resolver.go b/pkg/providers/amifamily/resolver.go index 135ae96fa1fc..7b7bf631c0ac 100644 --- a/pkg/providers/amifamily/resolver.go +++ b/pkg/providers/amifamily/resolver.go @@ -29,6 +29,7 @@ import ( corev1beta1 "github.com/aws/karpenter-core/pkg/apis/v1beta1" "github.com/aws/karpenter/pkg/apis/v1beta1" "github.com/aws/karpenter/pkg/providers/amifamily/bootstrap" + "github.com/aws/karpenter/pkg/providers/license" "github.com/aws/karpenter-core/pkg/cloudprovider" "github.com/aws/karpenter-core/pkg/scheduling" @@ -42,7 +43,8 @@ var DefaultEBS = v1beta1.BlockDevice{ // Resolver is able to fill-in dynamic launch template parameters type Resolver struct { - amiProvider *Provider + amiProvider *Provider + licenseProvider *license.Provider } // Options define the static launch template parameters @@ -69,6 +71,7 @@ type LaunchTemplate struct { AMIID string InstanceTypes []*cloudprovider.InstanceType `hash:"ignore"` DetailedMonitoring bool + Licenses []string } // AMIFamily can be implemented to override the default logic for generating dynamic launch template parameters @@ -107,9 +110,10 @@ func (d DefaultFamily) FeatureFlags() FeatureFlags { } // New constructs a new launch template Resolver -func New(amiProvider *Provider) *Resolver { +func New(amiProvider *Provider, licenseProvider *license.Provider) *Resolver { return &Resolver{ - amiProvider: amiProvider, + amiProvider: amiProvider, + licenseProvider: licenseProvider, } } @@ -128,6 +132,10 @@ func (r Resolver) Resolve(ctx context.Context, nodeClass *v1beta1.EC2NodeClass, if len(mappedAMIs) == 0 { return nil, fmt.Errorf("no instance types satisfy requirements of amis %v", amis) } + licenses, err := r.licenseProvider.List(ctx, nodeClass) + if err != nil { + return nil, err + } var resolvedTemplates []*LaunchTemplate for amiID, instanceTypes := range mappedAMIs { maxPodsToInstanceTypes := lo.GroupBy(instanceTypes, func(instanceType *cloudprovider.InstanceType) int { @@ -161,6 +169,7 @@ func (r Resolver) Resolve(ctx context.Context, nodeClass *v1beta1.EC2NodeClass, DetailedMonitoring: aws.BoolValue(nodeClass.Spec.DetailedMonitoring), AMIID: amiID, InstanceTypes: instanceTypes, + Licenses: licenses, } if len(resolved.BlockDeviceMappings) == 0 { resolved.BlockDeviceMappings = amiFamily.DefaultBlockDeviceMappings() diff --git a/pkg/providers/launchtemplate/launchtemplate.go b/pkg/providers/launchtemplate/launchtemplate.go index 4341fdcc02ff..717e1cc59d09 100644 --- a/pkg/providers/launchtemplate/launchtemplate.go +++ b/pkg/providers/launchtemplate/launchtemplate.go @@ -40,6 +40,7 @@ import ( "github.com/aws/karpenter/pkg/apis/v1beta1" awserrors "github.com/aws/karpenter/pkg/errors" "github.com/aws/karpenter/pkg/providers/amifamily" + "github.com/aws/karpenter/pkg/providers/license" "github.com/aws/karpenter/pkg/providers/securitygroup" "github.com/aws/karpenter/pkg/providers/subnet" "github.com/aws/karpenter/pkg/utils" @@ -59,6 +60,7 @@ type Provider struct { amiFamily *amifamily.Resolver securityGroupProvider *securitygroup.Provider subnetProvider *subnet.Provider + licenseProvider *license.Provider cache *cache.Cache caBundle *string cm *pretty.ChangeMonitor @@ -66,12 +68,13 @@ type Provider struct { ClusterEndpoint string } -func NewProvider(ctx context.Context, cache *cache.Cache, ec2api ec2iface.EC2API, amiFamily *amifamily.Resolver, securityGroupProvider *securitygroup.Provider, subnetProvider *subnet.Provider, caBundle *string, startAsync <-chan struct{}, kubeDNSIP net.IP, clusterEndpoint string) *Provider { +func NewProvider(ctx context.Context, cache *cache.Cache, ec2api ec2iface.EC2API, amiFamily *amifamily.Resolver, securityGroupProvider *securitygroup.Provider, subnetProvider *subnet.Provider, licenseProvider *license.Provider, caBundle *string, startAsync <-chan struct{}, kubeDNSIP net.IP, clusterEndpoint string) *Provider { l := &Provider{ ec2api: ec2api, amiFamily: amiFamily, securityGroupProvider: securityGroupProvider, subnetProvider: subnetProvider, + licenseProvider: licenseProvider, cache: cache, caBundle: caBundle, cm: pretty.NewChangeMonitor(), @@ -247,6 +250,7 @@ func (p *Provider) createLaunchTemplate(ctx context.Context, options *amifamily. TagSpecifications: []*ec2.LaunchTemplateTagSpecificationRequest{ {ResourceType: aws.String(ec2.ResourceTypeNetworkInterface), Tags: utils.MergeTags(options.Tags)}, }, + LicenseSpecifications: generateLicenseSpecification(options.Licenses), }, TagSpecifications: []*ec2.TagSpecification{ { @@ -262,6 +266,19 @@ func (p *Provider) createLaunchTemplate(ctx context.Context, options *amifamily. return output.LaunchTemplate, nil } +func generateLicenseSpecification(licenses []string) []*ec2.LaunchTemplateLicenseConfigurationRequest { + result := []*ec2.LaunchTemplateLicenseConfigurationRequest{} + if len(licenses) == 0 { + return nil + } + for i := range licenses { + result = append(result, &ec2.LaunchTemplateLicenseConfigurationRequest{ LicenseConfigurationArn: aws.String(licenses[i])}) + + } + return result + +} + // generateNetworkInterface generates a network interface for the launch template. // If all referenced subnets do not assign public IPv4 addresses to EC2 instances therein, we explicitly set // AssociatePublicIpAddress to 'false' in the Launch Template, generated based on this configuration struct. diff --git a/pkg/test/environment.go b/pkg/test/environment.go index d4bc98e67e33..b81dab375e77 100644 --- a/pkg/test/environment.go +++ b/pkg/test/environment.go @@ -89,7 +89,8 @@ func NewEnvironment(ctx context.Context, env *coretest.Environment) *Environment securityGroupProvider := securitygroup.NewProvider(ec2api, securityGroupCache) versionProvider := version.NewProvider(env.KubernetesInterface, kubernetesVersionCache) amiProvider := amifamily.NewProvider(versionProvider, ssmapi, ec2api, ec2Cache) - amiResolver := amifamily.New(amiProvider) + licenseProvider := license.NewProvider(ec2api, licenseCache) + amiResolver := amifamily.New(amiProvider, licenseProvider) instanceTypesProvider := instancetype.NewProvider("", instanceTypeCache, ec2api, subnetProvider, unavailableOfferingsCache, pricingProvider) launchTemplateProvider := launchtemplate.NewProvider( @@ -113,7 +114,6 @@ func NewEnvironment(ctx context.Context, env *coretest.Environment) *Environment subnetProvider, launchTemplateProvider, ) - licenseProvider := license.NewProvider(ec2api, licenseCache) return &Environment{ EC2API: ec2api, From 1e6f05d30f73605092847eadaf7db39a9c9a4b5b Mon Sep 17 00:00:00 2001 From: Sebastian Cole Date: Thu, 14 Sep 2023 13:40:38 +1000 Subject: [PATCH 09/33] Add api for HostResourceGroups selection --- .../karpenter.k8s.aws_ec2nodeclasses.yaml | 24 +++++++++++++ pkg/apis/v1beta1/ec2nodeclass.go | 11 ++++++ pkg/apis/v1beta1/ec2nodeclass_status.go | 13 +++++++ pkg/apis/v1beta1/zz_generated.deepcopy.go | 36 +++++++++++++++++++ 4 files changed, 84 insertions(+) diff --git a/pkg/apis/crds/karpenter.k8s.aws_ec2nodeclasses.yaml b/pkg/apis/crds/karpenter.k8s.aws_ec2nodeclasses.yaml index f3d43bb24ce5..8d002d11f212 100644 --- a/pkg/apis/crds/karpenter.k8s.aws_ec2nodeclasses.yaml +++ b/pkg/apis/crds/karpenter.k8s.aws_ec2nodeclasses.yaml @@ -163,6 +163,20 @@ spec: description: DetailedMonitoring controls if detailed monitoring is enabled for instances that are launched type: boolean + hostResourceGroupSelectorTerms: + description: HostResourceGroupSelectorTerms is a list of HostResourceGroupSelectors. + The terms are ORed. + items: + description: 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 + properties: + name: + description: Name of the hrg to be selected + type: string + type: object + type: array licenseSelectorTerms: description: LicenseSelectorTerms is a list of LicenseSelectors. The terms are ORed. @@ -353,6 +367,16 @@ spec: - requirements type: object type: array + hostResourceGroup: + description: HostResourceGroups contains the HRG arns + properties: + arn: + description: Arn of the HRG + type: string + name: + description: Name of the HRG + type: string + type: object licenses: description: Licenses contains the license arns items: diff --git a/pkg/apis/v1beta1/ec2nodeclass.go b/pkg/apis/v1beta1/ec2nodeclass.go index 0ae8b39bc659..efab7def5820 100644 --- a/pkg/apis/v1beta1/ec2nodeclass.go +++ b/pkg/apis/v1beta1/ec2nodeclass.go @@ -42,6 +42,9 @@ 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"` // 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. @@ -170,6 +173,14 @@ type LicenseSelectorTerm struct { Name string `json:"name,omitempty"` } +// 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"` +} + // MetadataOptions contains parameters for specifying the exposure of the // Instance Metadata Service to provisioned EC2 nodes. type MetadataOptions struct { diff --git a/pkg/apis/v1beta1/ec2nodeclass_status.go b/pkg/apis/v1beta1/ec2nodeclass_status.go index 20cbe1d510b6..f0d3e87758f7 100644 --- a/pkg/apis/v1beta1/ec2nodeclass_status.go +++ b/pkg/apis/v1beta1/ec2nodeclass_status.go @@ -49,6 +49,16 @@ type AMI struct { Requirements []v1.NodeSelectorRequirement `json:"requirements"` } +// 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"` +} + // EC2NodeClassStatus contains the resolved state of the EC2NodeClass type EC2NodeClassStatus struct { // Subnets contains the current Subnet values that are available to the @@ -66,4 +76,7 @@ type EC2NodeClassStatus struct { // Licenses contains the license arns // +optional Licenses []string `json:"licenses,omitempty"` + // HostResourceGroups contains the HRG arns + // +optional + HostResourceGroup HostResourceGroup `json:"hostResourceGroup,omitempty"` } diff --git a/pkg/apis/v1beta1/zz_generated.deepcopy.go b/pkg/apis/v1beta1/zz_generated.deepcopy.go index 8890dbbfb9c4..8a025e9fc187 100644 --- a/pkg/apis/v1beta1/zz_generated.deepcopy.go +++ b/pkg/apis/v1beta1/zz_generated.deepcopy.go @@ -241,6 +241,11 @@ func (in *EC2NodeClassSpec) DeepCopyInto(out *EC2NodeClassSpec) { *out = make([]LicenseSelectorTerm, len(*in)) copy(*out, *in) } + if in.HostResourceGroupSelectorTerms != nil { + in, out := &in.HostResourceGroupSelectorTerms, &out.HostResourceGroupSelectorTerms + *out = make([]HostResourceGroupSelectorTerm, len(*in)) + copy(*out, *in) + } if in.UserData != nil { in, out := &in.UserData, &out.UserData *out = new(string) @@ -354,6 +359,7 @@ func (in *EC2NodeClassStatus) DeepCopyInto(out *EC2NodeClassStatus) { *out = make([]string, len(*in)) copy(*out, *in) } + out.HostResourceGroup = in.HostResourceGroup } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new EC2NodeClassStatus. @@ -366,6 +372,36 @@ func (in *EC2NodeClassStatus) DeepCopy() *EC2NodeClassStatus { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *HostResourceGroup) DeepCopyInto(out *HostResourceGroup) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new HostResourceGroup. +func (in *HostResourceGroup) DeepCopy() *HostResourceGroup { + if in == nil { + return nil + } + out := new(HostResourceGroup) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *HostResourceGroupSelectorTerm) DeepCopyInto(out *HostResourceGroupSelectorTerm) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new HostResourceGroupSelectorTerm. +func (in *HostResourceGroupSelectorTerm) DeepCopy() *HostResourceGroupSelectorTerm { + if in == nil { + return nil + } + out := new(HostResourceGroupSelectorTerm) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *LicenseSelectorTerm) DeepCopyInto(out *LicenseSelectorTerm) { *out = *in From 1410dcea5192814df4a0d5627d2bbc3bedc9ad1f Mon Sep 17 00:00:00 2001 From: Sebastian Cole Date: Thu, 14 Sep 2023 15:55:02 +1000 Subject: [PATCH 10/33] Plumbing for host resource group provider into launchtemplates --- cmd/controller/main.go | 1 + .../karpenter.k8s.aws_awsnodetemplates.yaml | 4 +- pkg/apis/v1alpha1/provider.go | 4 +- pkg/apis/v1beta1/ec2nodeclass.go | 7 ++- pkg/apis/v1beta1/ec2nodeclass_status.go | 2 +- pkg/apis/v1beta1/zz_generated.deepcopy.go | 13 +++- pkg/controllers/controllers.go | 5 +- pkg/controllers/nodeclass/controller.go | 51 +++++++++------- pkg/operator/operator.go | 14 +++-- pkg/providers/amifamily/resolver.go | 32 ++++++++-- .../launchtemplate/launchtemplate.go | 11 ++++ pkg/utils/nodeclass/nodeclass.go | 60 ++++++++++++------- pkg/utils/nodetemplate/nodetemplate.go | 28 +++++---- 13 files changed, 160 insertions(+), 72 deletions(-) diff --git a/cmd/controller/main.go b/cmd/controller/main.go index d07a2e8fd487..d3f14a078b7a 100644 --- a/cmd/controller/main.go +++ b/cmd/controller/main.go @@ -67,6 +67,7 @@ func main() { op.PricingProvider, op.AMIProvider, op.LicenseProvider, + op.HostResourceGroupProvider, )...). WithWebhooks(ctx, webhooks.NewWebhooks()...). Start(ctx) diff --git a/pkg/apis/crds/karpenter.k8s.aws_awsnodetemplates.yaml b/pkg/apis/crds/karpenter.k8s.aws_awsnodetemplates.yaml index aeb4936e7093..a57cc4bbcf10 100644 --- a/pkg/apis/crds/karpenter.k8s.aws_awsnodetemplates.yaml +++ b/pkg/apis/crds/karpenter.k8s.aws_awsnodetemplates.yaml @@ -129,7 +129,7 @@ spec: description: DetailedMonitoring controls if detailed monitoring is enabled for instances that are launched type: boolean - hostResourceGroup: + hostResourceGroupSelector: additionalProperties: type: string description: HostResourceGroups specific the hostResourceGroupArns @@ -204,7 +204,7 @@ spec: credentials are not available." type: string type: object - placementGroup: + placementGroupSelector: additionalProperties: type: string description: PlacementGroup specifies the placement group to use for diff --git a/pkg/apis/v1alpha1/provider.go b/pkg/apis/v1alpha1/provider.go index c0e1b56e189c..a0ac32f06b65 100644 --- a/pkg/apis/v1alpha1/provider.go +++ b/pkg/apis/v1alpha1/provider.go @@ -46,10 +46,10 @@ type AWS struct { LicenseSelector map[string]string `json:"licenseSelector,omitempty" hash:"ignore"` // HostResourceGroups specific the hostResourceGroupArns to use for ec2 placement // +optional - HostResourceGroupSelector map[string]string `json:"hostResourceGroup,omitempty" hash:"ignore"` + HostResourceGroupSelector map[string]string `json:"hostResourceGroupSelector,omitempty" hash:"ignore"` // PlacementGroup specifies the placement group to use for ec2 placement // +optional - PlacementGroupSelector map[string]string `json:"placementGroup,omitempty" hash:"ignore"` + PlacementGroupSelector map[string]string `json:"placementGroupSelector,omitempty" hash:"ignore"` // Tags to be applied on ec2 resources like instances and launch templates. // +optional Tags map[string]string `json:"tags,omitempty"` diff --git a/pkg/apis/v1beta1/ec2nodeclass.go b/pkg/apis/v1beta1/ec2nodeclass.go index efab7def5820..2998ac73125f 100644 --- a/pkg/apis/v1beta1/ec2nodeclass.go +++ b/pkg/apis/v1beta1/ec2nodeclass.go @@ -109,10 +109,15 @@ type EC2NodeClassSpec struct { // +optional OriginalAMISelector map[string]string `json:"-" hash:"ignore"` // TODO @joinnis: Remove this field when v1alpha5 is unsupported in a future version of Karpenter - // OriginalAMISelector is the original ami selector that was used by the v1alpha5 representation of this API. + // OriginalLicenseSelector is the original license selector that was used by the v1alpha5 representation of this API. // DO NOT USE THIS VALUE when performing business logic in code // +optional OriginalLicenseSelector map[string]string `json:"-" hash:"ignore"` + // TODO @joinnis: Remove this field when v1alpha5 is unsupported in a future version of Karpenter + // OriginalHostResourceGroupSelector is the original hrg selector that was used by the v1alpha5 representation of this API. + // DO NOT USE THIS VALUE when performing business logic in code + // +optional + OriginalHostResourceGroupSelector map[string]string `json:"-" hash:"ignore"` } // SubnetSelectorTerm defines selection logic for a subnet used by Karpenter to launch nodes. diff --git a/pkg/apis/v1beta1/ec2nodeclass_status.go b/pkg/apis/v1beta1/ec2nodeclass_status.go index f0d3e87758f7..9f0fb4fc07ab 100644 --- a/pkg/apis/v1beta1/ec2nodeclass_status.go +++ b/pkg/apis/v1beta1/ec2nodeclass_status.go @@ -78,5 +78,5 @@ type EC2NodeClassStatus struct { Licenses []string `json:"licenses,omitempty"` // HostResourceGroups contains the HRG arns // +optional - HostResourceGroup HostResourceGroup `json:"hostResourceGroup,omitempty"` + HostResourceGroup *HostResourceGroup `json:"hostResourceGroup,omitempty"` } diff --git a/pkg/apis/v1beta1/zz_generated.deepcopy.go b/pkg/apis/v1beta1/zz_generated.deepcopy.go index 8a025e9fc187..d0ae48f58b7f 100644 --- a/pkg/apis/v1beta1/zz_generated.deepcopy.go +++ b/pkg/apis/v1beta1/zz_generated.deepcopy.go @@ -322,6 +322,13 @@ func (in *EC2NodeClassSpec) DeepCopyInto(out *EC2NodeClassSpec) { (*out)[key] = val } } + if in.OriginalHostResourceGroupSelector != nil { + in, out := &in.OriginalHostResourceGroupSelector, &out.OriginalHostResourceGroupSelector + *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. @@ -359,7 +366,11 @@ func (in *EC2NodeClassStatus) DeepCopyInto(out *EC2NodeClassStatus) { *out = make([]string, len(*in)) copy(*out, *in) } - out.HostResourceGroup = in.HostResourceGroup + if in.HostResourceGroup != nil { + in, out := &in.HostResourceGroup, &out.HostResourceGroup + *out = new(HostResourceGroup) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new EC2NodeClassStatus. diff --git a/pkg/controllers/controllers.go b/pkg/controllers/controllers.go index 14cd55e1ccc9..34bd78a61010 100644 --- a/pkg/controllers/controllers.go +++ b/pkg/controllers/controllers.go @@ -32,6 +32,7 @@ import ( nodeclaimlink "github.com/aws/karpenter/pkg/controllers/nodeclaim/link" "github.com/aws/karpenter/pkg/controllers/nodeclass" "github.com/aws/karpenter/pkg/providers/amifamily" + "github.com/aws/karpenter/pkg/providers/hostresourcegroup" "github.com/aws/karpenter/pkg/providers/license" "github.com/aws/karpenter/pkg/providers/pricing" "github.com/aws/karpenter/pkg/providers/securitygroup" @@ -43,13 +44,13 @@ import ( func NewControllers(ctx context.Context, sess *session.Session, clk clock.Clock, kubeClient client.Client, recorder events.Recorder, unavailableOfferings *cache.UnavailableOfferings, cloudProvider *cloudprovider.CloudProvider, subnetProvider *subnet.Provider, - securityGroupProvider *securitygroup.Provider, pricingProvider *pricing.Provider, amiProvider *amifamily.Provider, licenseProvider *license.Provider) []controller.Controller { + securityGroupProvider *securitygroup.Provider, pricingProvider *pricing.Provider, amiProvider *amifamily.Provider, licenseProvider *license.Provider, hostResourceGroupProvider *hostresourcegroup.Provider) []controller.Controller { logging.FromContext(ctx).With("version", project.Version).Debugf("discovered version") linkController := nodeclaimlink.NewController(kubeClient, cloudProvider) controllers := []controller.Controller{ - nodeclass.NewNodeTemplateController(kubeClient, subnetProvider, securityGroupProvider, amiProvider, licenseProvider), + nodeclass.NewNodeTemplateController(kubeClient, subnetProvider, securityGroupProvider, amiProvider, licenseProvider, hostResourceGroupProvider), linkController, nodeclaimgarbagecollection.NewController(kubeClient, cloudProvider, linkController), } diff --git a/pkg/controllers/nodeclass/controller.go b/pkg/controllers/nodeclass/controller.go index 8af6cae2ed8c..e29bfd08cd35 100644 --- a/pkg/controllers/nodeclass/controller.go +++ b/pkg/controllers/nodeclass/controller.go @@ -38,6 +38,7 @@ import ( "github.com/aws/karpenter/pkg/apis/v1alpha1" "github.com/aws/karpenter/pkg/apis/v1beta1" "github.com/aws/karpenter/pkg/providers/amifamily" + "github.com/aws/karpenter/pkg/providers/hostresourcegroup" "github.com/aws/karpenter/pkg/providers/license" "github.com/aws/karpenter/pkg/providers/securitygroup" "github.com/aws/karpenter/pkg/providers/subnet" @@ -45,21 +46,23 @@ import ( ) type Controller struct { - kubeClient client.Client - subnetProvider *subnet.Provider - securityGroupProvider *securitygroup.Provider - amiProvider *amifamily.Provider - licenseProvider *license.Provider + kubeClient client.Client + subnetProvider *subnet.Provider + securityGroupProvider *securitygroup.Provider + amiProvider *amifamily.Provider + licenseProvider *license.Provider + hostResourceGroupProvider *hostresourcegroup.Provider } func NewController(kubeClient client.Client, subnetProvider *subnet.Provider, - securityGroupProvider *securitygroup.Provider, amiProvider *amifamily.Provider, licenseProvider *license.Provider) *Controller { + securityGroupProvider *securitygroup.Provider, amiProvider *amifamily.Provider, licenseProvider *license.Provider, hostresourcegroupProvider *hostresourcegroup.Provider) *Controller { return &Controller{ - kubeClient: kubeClient, - subnetProvider: subnetProvider, - securityGroupProvider: securityGroupProvider, - amiProvider: amiProvider, - licenseProvider: licenseProvider, + kubeClient: kubeClient, + subnetProvider: subnetProvider, + securityGroupProvider: securityGroupProvider, + amiProvider: amiProvider, + licenseProvider: licenseProvider, + hostResourceGroupProvider: hostresourcegroupProvider, } } @@ -71,6 +74,7 @@ func (c *Controller) Reconcile(ctx context.Context, nodeClass *v1beta1.EC2NodeCl c.resolveSecurityGroups(ctx, nodeClass), c.resolveAMIs(ctx, nodeClass), c.resolveLicenses(ctx, nodeClass), + c.resolveHostResourceGroups(ctx, nodeClass), ) if !equality.Semantic.DeepEqual(stored, nodeClass) { statusCopy := nodeClass.DeepCopy() @@ -167,21 +171,26 @@ func (c *Controller) resolveLicenses(ctx context.Context, nodeClass *v1beta1.Nod } +func (c *Controller) resolveHostResourceGroups(ctx context.Context, nodeClass *v1beta1.NodeClass) error { + result , err := c.hostResourceGroupProvider.Get(ctx, nodeClass) + if err != nil { + return err + } + + nodeClass.Status.HostResourceGroup = result + + return nil +} + //nolint:revive type NodeClassController struct { *Controller } func NewNodeClassController(kubeClient client.Client, subnetProvider *subnet.Provider, -<<<<<<< HEAD - securityGroupProvider *securitygroup.Provider, amiProvider *amifamily.Provider) corecontroller.Controller { + securityGroupProvider *securitygroup.Provider, amiProvider *amifamily.Provider, licenseProvider *license.Provider, hostresourcegroupProvider *hostresourcegroup.Provider) corecontroller.Controller { return corecontroller.Typed[*v1beta1.EC2NodeClass](kubeClient, &NodeClassController{ - Controller: NewController(kubeClient, subnetProvider, securityGroupProvider, amiProvider), -======= - securityGroupProvider *securitygroup.Provider, amiProvider *amifamily.Provider, licenseProvider *license.Provider) corecontroller.Controller { - return corecontroller.Typed[*v1beta1.NodeClass](kubeClient, &NodeClassController{ - Controller: NewController(kubeClient, subnetProvider, securityGroupProvider, amiProvider, licenseProvider), ->>>>>>> 7b9eb759 (More license provider scaffolding) + Controller: NewController(kubeClient, subnetProvider, securityGroupProvider, amiProvider, licenseProvider, hostresourcegroupProvider), }) } @@ -209,9 +218,9 @@ type NodeTemplateController struct { } func NewNodeTemplateController(kubeClient client.Client, subnetProvider *subnet.Provider, - securityGroupProvider *securitygroup.Provider, amiProvider *amifamily.Provider, licenseProvider *license.Provider) corecontroller.Controller { + securityGroupProvider *securitygroup.Provider, amiProvider *amifamily.Provider, licenseProvider *license.Provider, hostresourcegroupProvider *hostresourcegroup.Provider) corecontroller.Controller { return corecontroller.Typed[*v1alpha1.AWSNodeTemplate](kubeClient, &NodeTemplateController{ - Controller: NewController(kubeClient, subnetProvider, securityGroupProvider, amiProvider, licenseProvider), + Controller: NewController(kubeClient, subnetProvider, securityGroupProvider, amiProvider, licenseProvider, hostresourcegroupProvider), }) } diff --git a/pkg/operator/operator.go b/pkg/operator/operator.go index 596922cb62b4..a411bc253f28 100644 --- a/pkg/operator/operator.go +++ b/pkg/operator/operator.go @@ -30,11 +30,12 @@ import ( "github.com/aws/aws-sdk-go/aws/endpoints" "github.com/aws/aws-sdk-go/aws/request" "github.com/aws/aws-sdk-go/aws/session" - "github.com/aws/aws-sdk-go/service/licensemanager" "github.com/aws/aws-sdk-go/service/ec2" "github.com/aws/aws-sdk-go/service/ec2/ec2iface" "github.com/aws/aws-sdk-go/service/eks" "github.com/aws/aws-sdk-go/service/eks/eksiface" + "github.com/aws/aws-sdk-go/service/licensemanager" + "github.com/aws/aws-sdk-go/service/resourcegroups" "github.com/aws/aws-sdk-go/service/ssm" "github.com/patrickmn/go-cache" @@ -50,6 +51,7 @@ import ( "github.com/aws/karpenter/pkg/apis/settings" awscache "github.com/aws/karpenter/pkg/cache" "github.com/aws/karpenter/pkg/providers/amifamily" + "github.com/aws/karpenter/pkg/providers/hostresourcegroup" "github.com/aws/karpenter/pkg/providers/instance" "github.com/aws/karpenter/pkg/providers/instancetype" "github.com/aws/karpenter/pkg/providers/launchtemplate" @@ -78,6 +80,7 @@ type Operator struct { InstanceTypesProvider *instancetype.Provider InstanceProvider *instance.Provider LicenseProvider *license.Provider + HostResourceGroupProvider *hostresourcegroup.Provider } func NewOperator(ctx context.Context, operator *operator.Operator) (context.Context, *Operator) { @@ -134,8 +137,9 @@ func NewOperator(ctx context.Context, operator *operator.Operator) (context.Cont ) versionProvider := version.NewProvider(operator.KubernetesInterface, cache.New(awscache.DefaultTTL, awscache.DefaultCleanupInterval)) amiProvider := amifamily.NewProvider(versionProvider, ssm.New(sess), ec2api, cache.New(awscache.DefaultTTL, awscache.DefaultCleanupInterval)) - licenseProvider := license.NewProvider(licensemanager.New(sess) , cache.New(awscache.DefaultTTL, awscache.DefaultCleanupInterval)) - amiResolver := amifamily.New(amiProvider, licenseProvider) + licenseProvider := license.NewProvider(licensemanager.New(sess), cache.New(awscache.DefaultTTL, awscache.DefaultCleanupInterval)) + hostresourcegroupProvider := hostresourcegroup.NewProvider(resourcegroups.New(sess), cache.New(awscache.DefaultTTL, awscache.DefaultCleanupInterval)) + amiResolver := amifamily.New(amiProvider, licenseProvider, hostresourcegroupProvider) launchTemplateProvider := launchtemplate.NewProvider( ctx, cache.New(awscache.DefaultTTL, awscache.DefaultCleanupInterval), @@ -143,7 +147,7 @@ func NewOperator(ctx context.Context, operator *operator.Operator) (context.Cont amiResolver, securityGroupProvider, subnetProvider, - licenseProvider, + licenseProvider, lo.Must(getCABundle(ctx, operator.GetConfig())), operator.Elected(), kubeDNSIP, @@ -166,7 +170,6 @@ func NewOperator(ctx context.Context, operator *operator.Operator) (context.Cont subnetProvider, launchTemplateProvider, ) - return ctx, &Operator{ Operator: operator, @@ -183,6 +186,7 @@ func NewOperator(ctx context.Context, operator *operator.Operator) (context.Cont InstanceTypesProvider: instanceTypeProvider, InstanceProvider: instanceProvider, LicenseProvider: licenseProvider, + HostResourceGroupProvider: hostresourcegroupProvider, } } diff --git a/pkg/providers/amifamily/resolver.go b/pkg/providers/amifamily/resolver.go index 7b7bf631c0ac..b437b4de2496 100644 --- a/pkg/providers/amifamily/resolver.go +++ b/pkg/providers/amifamily/resolver.go @@ -29,6 +29,7 @@ import ( corev1beta1 "github.com/aws/karpenter-core/pkg/apis/v1beta1" "github.com/aws/karpenter/pkg/apis/v1beta1" "github.com/aws/karpenter/pkg/providers/amifamily/bootstrap" + "github.com/aws/karpenter/pkg/providers/hostresourcegroup" "github.com/aws/karpenter/pkg/providers/license" "github.com/aws/karpenter-core/pkg/cloudprovider" @@ -43,8 +44,9 @@ var DefaultEBS = v1beta1.BlockDevice{ // Resolver is able to fill-in dynamic launch template parameters type Resolver struct { - amiProvider *Provider - licenseProvider *license.Provider + amiProvider *Provider + licenseProvider *license.Provider + hostResourceGroupProvider *hostresourcegroup.Provider } // Options define the static launch template parameters @@ -72,6 +74,12 @@ type LaunchTemplate struct { InstanceTypes []*cloudprovider.InstanceType `hash:"ignore"` DetailedMonitoring bool Licenses []string + Placement *Placement +} + +// Placement holds the dynamically generated launch template placement parameters +type Placement struct { + HostResourceGroup string } // AMIFamily can be implemented to override the default logic for generating dynamic launch template parameters @@ -110,10 +118,11 @@ func (d DefaultFamily) FeatureFlags() FeatureFlags { } // New constructs a new launch template Resolver -func New(amiProvider *Provider, licenseProvider *license.Provider) *Resolver { +func New(amiProvider *Provider, licenseProvider *license.Provider, hostResourceGroupProvider *hostresourcegroup.Provider) *Resolver { return &Resolver{ - amiProvider: amiProvider, - licenseProvider: licenseProvider, + amiProvider: amiProvider, + licenseProvider: licenseProvider, + hostResourceGroupProvider: hostResourceGroupProvider, } } @@ -136,6 +145,16 @@ func (r Resolver) Resolve(ctx context.Context, nodeClass *v1beta1.EC2NodeClass, if err != nil { return nil, err } + var placement *Placement + hrg, err := r.hostResourceGroupProvider.Get(ctx, nodeClass) + if err != nil { + return nil, err + } + if hrg == nil { + placement = nil + } else { + placement = &Placement{HostResourceGroup: hrg.ARN} + } var resolvedTemplates []*LaunchTemplate for amiID, instanceTypes := range mappedAMIs { maxPodsToInstanceTypes := lo.GroupBy(instanceTypes, func(instanceType *cloudprovider.InstanceType) int { @@ -169,7 +188,8 @@ func (r Resolver) Resolve(ctx context.Context, nodeClass *v1beta1.EC2NodeClass, DetailedMonitoring: aws.BoolValue(nodeClass.Spec.DetailedMonitoring), AMIID: amiID, InstanceTypes: instanceTypes, - Licenses: licenses, + Licenses: licenses, + Placement: placement, } if len(resolved.BlockDeviceMappings) == 0 { resolved.BlockDeviceMappings = amiFamily.DefaultBlockDeviceMappings() diff --git a/pkg/providers/launchtemplate/launchtemplate.go b/pkg/providers/launchtemplate/launchtemplate.go index 717e1cc59d09..060c4b39f6c3 100644 --- a/pkg/providers/launchtemplate/launchtemplate.go +++ b/pkg/providers/launchtemplate/launchtemplate.go @@ -251,6 +251,7 @@ func (p *Provider) createLaunchTemplate(ctx context.Context, options *amifamily. {ResourceType: aws.String(ec2.ResourceTypeNetworkInterface), Tags: utils.MergeTags(options.Tags)}, }, LicenseSpecifications: generateLicenseSpecification(options.Licenses), + Placement: generatePlacement(options.Placement), }, TagSpecifications: []*ec2.TagSpecification{ { @@ -266,6 +267,16 @@ func (p *Provider) createLaunchTemplate(ctx context.Context, options *amifamily. return output.LaunchTemplate, nil } +func generatePlacement(placement *amifamily.Placement) *ec2.LaunchTemplatePlacementRequest { + if placement == nil { + return nil + } + return &ec2.LaunchTemplatePlacementRequest{ + HostResourceGroupArn: aws.String(placement.HostResourceGroup), + } + +} + func generateLicenseSpecification(licenses []string) []*ec2.LaunchTemplateLicenseConfigurationRequest { result := []*ec2.LaunchTemplateLicenseConfigurationRequest{} if len(licenses) == 0 { diff --git a/pkg/utils/nodeclass/nodeclass.go b/pkg/utils/nodeclass/nodeclass.go index 440fc9bdcee4..0b6840d0853b 100644 --- a/pkg/utils/nodeclass/nodeclass.go +++ b/pkg/utils/nodeclass/nodeclass.go @@ -37,33 +37,53 @@ func New(nodeTemplate *v1alpha1.AWSNodeTemplate) *v1beta1.EC2NodeClass { TypeMeta: nodeTemplate.TypeMeta, ObjectMeta: nodeTemplate.ObjectMeta, Spec: v1beta1.EC2NodeClassSpec{ - SubnetSelectorTerms: NewSubnetSelectorTerms(nodeTemplate.Spec.SubnetSelector), - OriginalSubnetSelector: nodeTemplate.Spec.SubnetSelector, - SecurityGroupSelectorTerms: NewSecurityGroupSelectorTerms(nodeTemplate.Spec.SecurityGroupSelector), - OriginalSecurityGroupSelector: nodeTemplate.Spec.SecurityGroupSelector, - AMISelectorTerms: NewAMISelectorTerms(nodeTemplate.Spec.AMISelector), - OriginalAMISelector: nodeTemplate.Spec.AMISelector, - AMIFamily: nodeTemplate.Spec.AMIFamily, - LicenseSelectorTerms: NewLicenseSelectorTerms(nodeTemplate.Spec.LicenseSelector), - OriginalLicenseSelector: nodeTemplate.Spec.LicenseSelector, - UserData: nodeTemplate.Spec.UserData, - Tags: nodeTemplate.Spec.Tags, - BlockDeviceMappings: NewBlockDeviceMappings(nodeTemplate.Spec.BlockDeviceMappings), - DetailedMonitoring: nodeTemplate.Spec.DetailedMonitoring, - MetadataOptions: NewMetadataOptions(nodeTemplate.Spec.MetadataOptions), - Context: nodeTemplate.Spec.Context, - LaunchTemplateName: nodeTemplate.Spec.LaunchTemplateName, - InstanceProfile: nodeTemplate.Spec.InstanceProfile, + SubnetSelectorTerms: NewSubnetSelectorTerms(nodeTemplate.Spec.SubnetSelector), + OriginalSubnetSelector: nodeTemplate.Spec.SubnetSelector, + SecurityGroupSelectorTerms: NewSecurityGroupSelectorTerms(nodeTemplate.Spec.SecurityGroupSelector), + OriginalSecurityGroupSelector: nodeTemplate.Spec.SecurityGroupSelector, + AMISelectorTerms: NewAMISelectorTerms(nodeTemplate.Spec.AMISelector), + OriginalAMISelector: nodeTemplate.Spec.AMISelector, + AMIFamily: nodeTemplate.Spec.AMIFamily, + LicenseSelectorTerms: NewLicenseSelectorTerms(nodeTemplate.Spec.LicenseSelector), + OriginalLicenseSelector: nodeTemplate.Spec.LicenseSelector, + UserData: nodeTemplate.Spec.UserData, + Tags: nodeTemplate.Spec.Tags, + BlockDeviceMappings: NewBlockDeviceMappings(nodeTemplate.Spec.BlockDeviceMappings), + DetailedMonitoring: nodeTemplate.Spec.DetailedMonitoring, + MetadataOptions: NewMetadataOptions(nodeTemplate.Spec.MetadataOptions), + Context: nodeTemplate.Spec.Context, + LaunchTemplateName: nodeTemplate.Spec.LaunchTemplateName, + InstanceProfile: nodeTemplate.Spec.InstanceProfile, + OriginalHostResourceGroupSelector: nodeTemplate.Spec.HostResourceGroupSelector, + HostResourceGroupSelectorTerms: NewHostResourceSelectorTerms(nodeTemplate.Spec.HostResourceGroupSelector), }, Status: v1beta1.EC2NodeClassStatus{ - Subnets: NewSubnets(nodeTemplate.Status.Subnets), - SecurityGroups: NewSecurityGroups(nodeTemplate.Status.SecurityGroups), - AMIs: NewAMIs(nodeTemplate.Status.AMIs), + Subnets: NewSubnets(nodeTemplate.Status.Subnets), + SecurityGroups: NewSecurityGroups(nodeTemplate.Status.SecurityGroups), + AMIs: NewAMIs(nodeTemplate.Status.AMIs), + Licenses: nodeTemplate.Status.Licenses, + HostResourceGroup: NewHostResourceGroup(nodeTemplate.Status.HostResourceGroups), }, IsNodeTemplate: true, } } +func NewHostResourceSelectorTerms(hrgSelector map[string]string) (terms []v1beta1.HostResourceGroupSelectorTerm) { + if len(hrgSelector) == 0 { + return nil + } + return []v1beta1.HostResourceGroupSelectorTerm{ + {Name: hrgSelector["name"]}, + } +} + +func NewHostResourceGroup(hrgs []string) *v1beta1.HostResourceGroup { + if len(hrgs) == 0 { + return nil + } + return &v1beta1.HostResourceGroup{ARN: hrgs[0]} +} + func NewSubnetSelectorTerms(subnetSelector map[string]string) (terms []v1beta1.SubnetSelectorTerm) { if len(subnetSelector) == 0 { return nil diff --git a/pkg/utils/nodetemplate/nodetemplate.go b/pkg/utils/nodetemplate/nodetemplate.go index a1708fdf317f..4d0dbe7c8ba8 100644 --- a/pkg/utils/nodetemplate/nodetemplate.go +++ b/pkg/utils/nodetemplate/nodetemplate.go @@ -28,13 +28,14 @@ func New(nodeClass *v1beta1.EC2NodeClass) *v1alpha1.AWSNodeTemplate { Spec: v1alpha1.AWSNodeTemplateSpec{ UserData: nodeClass.Spec.UserData, AWS: v1alpha1.AWS{ - AMIFamily: nodeClass.Spec.AMIFamily, - Context: nodeClass.Spec.Context, - InstanceProfile: nodeClass.Spec.InstanceProfile, - SubnetSelector: nodeClass.Spec.OriginalSubnetSelector, - SecurityGroupSelector: nodeClass.Spec.OriginalSecurityGroupSelector, - LicenseSelector: nodeClass.Spec.OriginalLicenseSelector, - Tags: nodeClass.Spec.Tags, + AMIFamily: nodeClass.Spec.AMIFamily, + Context: nodeClass.Spec.Context, + InstanceProfile: nodeClass.Spec.InstanceProfile, + SubnetSelector: nodeClass.Spec.OriginalSubnetSelector, + SecurityGroupSelector: nodeClass.Spec.OriginalSecurityGroupSelector, + LicenseSelector: nodeClass.Spec.OriginalLicenseSelector, + HostResourceGroupSelector: nodeClass.Spec.OriginalHostResourceGroupSelector, + Tags: nodeClass.Spec.Tags, LaunchTemplate: v1alpha1.LaunchTemplate{ LaunchTemplateName: nodeClass.Spec.LaunchTemplateName, MetadataOptions: NewMetadataOptions(nodeClass.Spec.MetadataOptions), @@ -45,14 +46,19 @@ func New(nodeClass *v1beta1.EC2NodeClass) *v1alpha1.AWSNodeTemplate { DetailedMonitoring: nodeClass.Spec.DetailedMonitoring, }, Status: v1alpha1.AWSNodeTemplateStatus{ - Subnets: NewSubnets(nodeClass.Status.Subnets), - SecurityGroups: NewSecurityGroups(nodeClass.Status.SecurityGroups), - AMIs: NewAMIs(nodeClass.Status.AMIs), - Licenses: NewLicenses(nodeClass.Status.Licenses), + Subnets: NewSubnets(nodeClass.Status.Subnets), + SecurityGroups: NewSecurityGroups(nodeClass.Status.SecurityGroups), + AMIs: NewAMIs(nodeClass.Status.AMIs), + Licenses: NewLicenses(nodeClass.Status.Licenses), + HostResourceGroups: NewHostResourceGroups(nodeClass.Status.HostResourceGroup), }, } } +func NewHostResourceGroups(hrg *v1beta1.HostResourceGroup) []string { + return []string{hrg.ARN} +} + func NewBlockDeviceMappings(bdms []*v1beta1.BlockDeviceMapping) []*v1alpha1.BlockDeviceMapping { if bdms == nil { return nil From cd8035b9541b5ba5322ca5182aff7abb398e1005 Mon Sep 17 00:00:00 2001 From: Sebastian Cole Date: Thu, 14 Sep 2023 16:56:34 +1000 Subject: [PATCH 11/33] Add missing provider --- .../hostresourcegroup/hostresourcegroup.go | 77 +++++++++++++++++++ pkg/utils/nodetemplate/nodetemplate.go | 3 + 2 files changed, 80 insertions(+) create mode 100644 pkg/providers/hostresourcegroup/hostresourcegroup.go diff --git a/pkg/providers/hostresourcegroup/hostresourcegroup.go b/pkg/providers/hostresourcegroup/hostresourcegroup.go new file mode 100644 index 000000000000..ae5275bc4840 --- /dev/null +++ b/pkg/providers/hostresourcegroup/hostresourcegroup.go @@ -0,0 +1,77 @@ +package hostresourcegroup + +import ( + "context" + "sync" + + //"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/patrickmn/go-cache" + "knative.dev/pkg/logging" +) + +type Provider struct { + sync.RWMutex + resourcegroups resourcegroupsiface.ResourceGroupsAPI + cache *cache.Cache + cm *pretty.ChangeMonitor +} + +func NewProvider(rgapi resourcegroupsiface.ResourceGroupsAPI, cache *cache.Cache) *Provider { + return &Provider{ + resourcegroups: rgapi, + cm: pretty.NewChangeMonitor(), + // TODO: Remove cache for v1beta1, utilize resolved subnet from the AWSNodeTemplate.status + // Subnets are sorted on AvailableIpAddressCount, descending order + cache: cache, + } +} + +func (p *Provider) Get(ctx context.Context, nodeClass *v1beta1.NodeClass) (*v1beta1.HostResourceGroup, error) { + p.Lock() + defer p.Unlock() + var nextToken *string = nil + var groups []*resourcegroups.GroupIdentifier + for { + resp, err := p.resourcegroups.ListGroupsWithContext(ctx, &resourcegroups.ListGroupsInput{ NextToken: nextToken}) + if err != nil { + return nil, err + } + nextToken = resp.NextToken + for i := range resp.GroupIdentifiers { + groups = append(groups, resp.GroupIdentifiers[i]) + } + if nextToken == nil { + break + } + } + + // filter resp to only include those that match the hostResourceGroupSelector Name + for i := range groups { + group := groups[i] + for x := range nodeClass.Spec.HostResourceGroupSelectorTerms { + selector := nodeClass.Spec.HostResourceGroupSelectorTerms[x] + logging.FromContext(ctx). + With("group", group). + With("selector", selector). + Debugf("checking for host resource group match") + if *group.GroupName == selector.Name { + match := &v1beta1.HostResourceGroup{ARN: *group.GroupArn, Name: *group.GroupName} + logging.FromContext(ctx). + With("Matched hrg", match). + Debugf("discovered host resource group configuration") + + return match, nil + } + } + } + logging.FromContext(ctx). + With("groups", groups). + Debugf("No hrg matched") + + // No matching groups + return nil, nil +} diff --git a/pkg/utils/nodetemplate/nodetemplate.go b/pkg/utils/nodetemplate/nodetemplate.go index 4d0dbe7c8ba8..7fc38850e530 100644 --- a/pkg/utils/nodetemplate/nodetemplate.go +++ b/pkg/utils/nodetemplate/nodetemplate.go @@ -56,6 +56,9 @@ func New(nodeClass *v1beta1.EC2NodeClass) *v1alpha1.AWSNodeTemplate { } func NewHostResourceGroups(hrg *v1beta1.HostResourceGroup) []string { + if hrg == nil { + return nil + } return []string{hrg.ARN} } From fbcbab97522f57d464c2bf79889d5ef921cfe230 Mon Sep 17 00:00:00 2001 From: Sebastian Cole Date: Fri, 15 Sep 2023 10:43:13 +1000 Subject: [PATCH 12/33] handling v1beta1.NodeClass -> EC2NodeClass --- pkg/controllers/nodeclass/controller.go | 4 ++-- pkg/providers/hostresourcegroup/hostresourcegroup.go | 2 +- pkg/providers/license/license.go | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/controllers/nodeclass/controller.go b/pkg/controllers/nodeclass/controller.go index e29bfd08cd35..aeb214810d4b 100644 --- a/pkg/controllers/nodeclass/controller.go +++ b/pkg/controllers/nodeclass/controller.go @@ -160,7 +160,7 @@ func (c *Controller) resolveAMIs(ctx context.Context, nodeClass *v1beta1.EC2Node return nil } -func (c *Controller) resolveLicenses(ctx context.Context, nodeClass *v1beta1.NodeClass) error { +func (c *Controller) resolveLicenses(ctx context.Context, nodeClass *v1beta1.EC2NodeClass) error { licenses, err := c.licenseProvider.List(ctx, nodeClass) if err != nil { return err @@ -171,7 +171,7 @@ func (c *Controller) resolveLicenses(ctx context.Context, nodeClass *v1beta1.Nod } -func (c *Controller) resolveHostResourceGroups(ctx context.Context, nodeClass *v1beta1.NodeClass) error { +func (c *Controller) resolveHostResourceGroups(ctx context.Context, nodeClass *v1beta1.EC2NodeClass) error { result , err := c.hostResourceGroupProvider.Get(ctx, nodeClass) if err != nil { return err diff --git a/pkg/providers/hostresourcegroup/hostresourcegroup.go b/pkg/providers/hostresourcegroup/hostresourcegroup.go index ae5275bc4840..39f2239220dd 100644 --- a/pkg/providers/hostresourcegroup/hostresourcegroup.go +++ b/pkg/providers/hostresourcegroup/hostresourcegroup.go @@ -30,7 +30,7 @@ func NewProvider(rgapi resourcegroupsiface.ResourceGroupsAPI, cache *cache.Cache } } -func (p *Provider) Get(ctx context.Context, nodeClass *v1beta1.NodeClass) (*v1beta1.HostResourceGroup, error) { +func (p *Provider) Get(ctx context.Context, nodeClass *v1beta1.EC2NodeClass) (*v1beta1.HostResourceGroup, error) { p.Lock() defer p.Unlock() var nextToken *string = nil diff --git a/pkg/providers/license/license.go b/pkg/providers/license/license.go index b1c8f2fbda57..d1feb09e1ef6 100644 --- a/pkg/providers/license/license.go +++ b/pkg/providers/license/license.go @@ -48,7 +48,7 @@ func NewProvider(lmapi licensemanageriface.LicenseManagerAPI, cache *cache.Cache } } -func (p *Provider) List(ctx context.Context, nodeClass *v1beta1.NodeClass) ([]string, error) { +func (p *Provider) List(ctx context.Context, nodeClass *v1beta1.EC2NodeClass) ([]string, error) { p.Lock() defer p.Unlock() From a66c663018c2096d4d2ad5bc5e498cd89dbed01b Mon Sep 17 00:00:00 2001 From: Sebastian Cole Date: Mon, 18 Sep 2023 16:35:56 +1000 Subject: [PATCH 13/33] Add provider for placement groups --- cmd/controller/main.go | 5 +- .../karpenter.k8s.aws_ec2nodeclasses.yaml | 18 ++++ pkg/apis/v1alpha1/awsnodetemplate.go | 24 ++--- pkg/apis/v1beta1/ec2nodeclass.go | 16 ++++ pkg/apis/v1beta1/ec2nodeclass_status.go | 3 + pkg/controllers/controllers.go | 6 +- pkg/controllers/nodeclass/controller.go | 48 +++++++--- pkg/operator/operator.go | 10 +- pkg/providers/amifamily/resolver.go | 40 +++++--- .../hostresourcegroup/hostresourcegroup.go | 17 ++++ pkg/providers/license/license.go | 7 +- .../placementgroup/placementgroup.go | 94 +++++++++++++++++++ pkg/utils/nodeclass/nodeclass.go | 11 +++ 13 files changed, 254 insertions(+), 45 deletions(-) create mode 100644 pkg/providers/placementgroup/placementgroup.go diff --git a/cmd/controller/main.go b/cmd/controller/main.go index d3f14a078b7a..f437a0528868 100644 --- a/cmd/controller/main.go +++ b/cmd/controller/main.go @@ -66,8 +66,9 @@ func main() { op.SecurityGroupProvider, op.PricingProvider, op.AMIProvider, - op.LicenseProvider, - op.HostResourceGroupProvider, + op.LicenseProvider, + op.HostResourceGroupProvider, + op.PlacementGroupProvider, )...). WithWebhooks(ctx, webhooks.NewWebhooks()...). Start(ctx) diff --git a/pkg/apis/crds/karpenter.k8s.aws_ec2nodeclasses.yaml b/pkg/apis/crds/karpenter.k8s.aws_ec2nodeclasses.yaml index 8d002d11f212..c88ceb5489ba 100644 --- a/pkg/apis/crds/karpenter.k8s.aws_ec2nodeclasses.yaml +++ b/pkg/apis/crds/karpenter.k8s.aws_ec2nodeclasses.yaml @@ -247,6 +247,19 @@ spec: credentials are not available." type: string type: object + placementGroupSelectorTerms: + description: PlacementGroupSelectorTerms is a list of PlacementGroupSelector. + The terms are ORed. + items: + description: 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 + properties: + name: + description: Name of the placement group to be selected + type: string + type: object + type: array role: description: Role is the AWS identity that nodes use. type: string @@ -382,6 +395,11 @@ spec: items: type: string type: array + placementGroups: + description: PlacementGroups contains the ec2 placement group arns + items: + type: string + type: array securityGroups: description: SecurityGroups contains the current Security Groups values that are available to the cluster under the SecurityGroups selectors. diff --git a/pkg/apis/v1alpha1/awsnodetemplate.go b/pkg/apis/v1alpha1/awsnodetemplate.go index 03ba492935d4..1d07ffc40077 100644 --- a/pkg/apis/v1alpha1/awsnodetemplate.go +++ b/pkg/apis/v1alpha1/awsnodetemplate.go @@ -69,18 +69,18 @@ type AWSNodeTemplateStatus struct { // cluster under the AMI selectors. // +optional AMIs []AMI `json:"amis,omitempty"` - // Licenses contains the current License Configuration Specifications - // that currently apply given the LicenseConfiguration selectors. - // +optional - Licenses []string `json:"licenses,omitempty"` - // HostResourceGroups contains the current Host Resource Groups - // that currently apply given the HostResourceGroup selectors. - // +optional - HostResourceGroups []string `json:"hostResourceGroups,omitempty"` - // Licenses contains the current License Configuration Specifications - // that currently apply given the LicenseConfiguration selectors. - // +optional - PlacementGroups []string `json:"placementGroups,omitempty"` + // Licenses contains the current License Configuration Specifications + // that currently apply given the LicenseConfiguration selectors. + // +optional + Licenses []string `json:"licenses,omitempty"` + // HostResourceGroups contains the current Host Resource Groups + // that currently apply given the HostResourceGroup selectors. + // +optional + HostResourceGroups []string `json:"hostResourceGroups,omitempty"` + // Licenses contains the current License Configuration Specifications + // that currently apply given the LicenseConfiguration selectors. + // +optional + PlacementGroups []string `json:"placementGroups,omitempty"` } // AWSNodeTemplateSpec is the top level specification for the AWS Karpenter Provider. diff --git a/pkg/apis/v1beta1/ec2nodeclass.go b/pkg/apis/v1beta1/ec2nodeclass.go index 2998ac73125f..799446b8966c 100644 --- a/pkg/apis/v1beta1/ec2nodeclass.go +++ b/pkg/apis/v1beta1/ec2nodeclass.go @@ -45,6 +45,9 @@ type EC2NodeClassSpec struct { // 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. @@ -118,6 +121,11 @@ type EC2NodeClassSpec struct { // DO NOT USE THIS VALUE when performing business logic in code // +optional OriginalHostResourceGroupSelector map[string]string `json:"-" hash:"ignore"` + // TODO @joinnis: Remove this field when v1alpha5 is unsupported in a future version of Karpenter + // OriginalPlacementGroupSelector is the original placement group selector that was used by the v1alpha5 representation of this API. + // DO NOT USE THIS VALUE when performing business logic in code + // +optional + OriginalPlacementGroupSelector map[string]string `json:"-" hash:"ignore"` } // SubnetSelectorTerm defines selection logic for a subnet used by Karpenter to launch nodes. @@ -186,6 +194,14 @@ type HostResourceGroupSelectorTerm struct { 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"` +} + // MetadataOptions contains parameters for specifying the exposure of the // Instance Metadata Service to provisioned EC2 nodes. type MetadataOptions struct { diff --git a/pkg/apis/v1beta1/ec2nodeclass_status.go b/pkg/apis/v1beta1/ec2nodeclass_status.go index 9f0fb4fc07ab..dadfeeba3225 100644 --- a/pkg/apis/v1beta1/ec2nodeclass_status.go +++ b/pkg/apis/v1beta1/ec2nodeclass_status.go @@ -79,4 +79,7 @@ type EC2NodeClassStatus struct { // 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/controllers/controllers.go b/pkg/controllers/controllers.go index 34bd78a61010..78354d4ab2d2 100644 --- a/pkg/controllers/controllers.go +++ b/pkg/controllers/controllers.go @@ -34,6 +34,7 @@ import ( "github.com/aws/karpenter/pkg/providers/amifamily" "github.com/aws/karpenter/pkg/providers/hostresourcegroup" "github.com/aws/karpenter/pkg/providers/license" + "github.com/aws/karpenter/pkg/providers/placementgroup" "github.com/aws/karpenter/pkg/providers/pricing" "github.com/aws/karpenter/pkg/providers/securitygroup" "github.com/aws/karpenter/pkg/providers/subnet" @@ -44,13 +45,14 @@ import ( func NewControllers(ctx context.Context, sess *session.Session, clk clock.Clock, kubeClient client.Client, recorder events.Recorder, unavailableOfferings *cache.UnavailableOfferings, cloudProvider *cloudprovider.CloudProvider, subnetProvider *subnet.Provider, - securityGroupProvider *securitygroup.Provider, pricingProvider *pricing.Provider, amiProvider *amifamily.Provider, licenseProvider *license.Provider, hostResourceGroupProvider *hostresourcegroup.Provider) []controller.Controller { + securityGroupProvider *securitygroup.Provider, pricingProvider *pricing.Provider, amiProvider *amifamily.Provider, + licenseProvider *license.Provider, hostResourceGroupProvider *hostresourcegroup.Provider, placementGroupProvider *placementgroup.Provider) []controller.Controller { logging.FromContext(ctx).With("version", project.Version).Debugf("discovered version") linkController := nodeclaimlink.NewController(kubeClient, cloudProvider) controllers := []controller.Controller{ - nodeclass.NewNodeTemplateController(kubeClient, subnetProvider, securityGroupProvider, amiProvider, licenseProvider, hostResourceGroupProvider), + nodeclass.NewNodeTemplateController(kubeClient, subnetProvider, securityGroupProvider, amiProvider, licenseProvider, hostResourceGroupProvider, placementGroupProvider), linkController, nodeclaimgarbagecollection.NewController(kubeClient, cloudProvider, linkController), } diff --git a/pkg/controllers/nodeclass/controller.go b/pkg/controllers/nodeclass/controller.go index aeb214810d4b..c5be9a030912 100644 --- a/pkg/controllers/nodeclass/controller.go +++ b/pkg/controllers/nodeclass/controller.go @@ -40,6 +40,7 @@ import ( "github.com/aws/karpenter/pkg/providers/amifamily" "github.com/aws/karpenter/pkg/providers/hostresourcegroup" "github.com/aws/karpenter/pkg/providers/license" + "github.com/aws/karpenter/pkg/providers/placementgroup" "github.com/aws/karpenter/pkg/providers/securitygroup" "github.com/aws/karpenter/pkg/providers/subnet" nodeclassutil "github.com/aws/karpenter/pkg/utils/nodeclass" @@ -52,10 +53,13 @@ type Controller struct { amiProvider *amifamily.Provider licenseProvider *license.Provider hostResourceGroupProvider *hostresourcegroup.Provider + placementGroupProvider *placementgroup.Provider } func NewController(kubeClient client.Client, subnetProvider *subnet.Provider, - securityGroupProvider *securitygroup.Provider, amiProvider *amifamily.Provider, licenseProvider *license.Provider, hostresourcegroupProvider *hostresourcegroup.Provider) *Controller { + securityGroupProvider *securitygroup.Provider, amiProvider *amifamily.Provider, + licenseProvider *license.Provider, hostresourcegroupProvider *hostresourcegroup.Provider, + placementGroupProvider *placementgroup.Provider) *Controller { return &Controller{ kubeClient: kubeClient, subnetProvider: subnetProvider, @@ -63,6 +67,7 @@ func NewController(kubeClient client.Client, subnetProvider *subnet.Provider, amiProvider: amiProvider, licenseProvider: licenseProvider, hostResourceGroupProvider: hostresourcegroupProvider, + placementGroupProvider: placementGroupProvider, } } @@ -75,6 +80,7 @@ func (c *Controller) Reconcile(ctx context.Context, nodeClass *v1beta1.EC2NodeCl c.resolveAMIs(ctx, nodeClass), c.resolveLicenses(ctx, nodeClass), c.resolveHostResourceGroups(ctx, nodeClass), + c.resolvePlacementGroups(ctx, nodeClass), ) if !equality.Semantic.DeepEqual(stored, nodeClass) { statusCopy := nodeClass.DeepCopy() @@ -161,9 +167,10 @@ func (c *Controller) resolveAMIs(ctx context.Context, nodeClass *v1beta1.EC2Node } func (c *Controller) resolveLicenses(ctx context.Context, nodeClass *v1beta1.EC2NodeClass) error { - licenses, err := c.licenseProvider.List(ctx, nodeClass) + licenses, err := c.licenseProvider.Get(ctx, nodeClass) if err != nil { - return err + // Errors from license provider should not interrupt the process + return nil } nodeClass.Status.Licenses = licenses @@ -172,25 +179,40 @@ func (c *Controller) resolveLicenses(ctx context.Context, nodeClass *v1beta1.EC2 } func (c *Controller) resolveHostResourceGroups(ctx context.Context, nodeClass *v1beta1.EC2NodeClass) error { - result , err := c.hostResourceGroupProvider.Get(ctx, nodeClass) - if err != nil { - return err - } + result, err := c.hostResourceGroupProvider.Get(ctx, nodeClass) + if err != nil { + // Errors from host resource group provider should not interrupt the process + return nil + } - nodeClass.Status.HostResourceGroup = result + nodeClass.Status.HostResourceGroup = result return nil } +func (c *Controller) resolvePlacementGroups(ctx context.Context, nodeClass *v1beta1.EC2NodeClass) error { + result, err := c.placementGroupProvider.Get(ctx, nodeClass) + if err != nil { + // Errors from placement group provider should not interrupt the process + return nil + } + if result != nil { + nodeClass.Status.PlacementGroups = append(nodeClass.Status.PlacementGroups, *result.GroupArn) + } + return nil +} + //nolint:revive type NodeClassController struct { *Controller } func NewNodeClassController(kubeClient client.Client, subnetProvider *subnet.Provider, - securityGroupProvider *securitygroup.Provider, amiProvider *amifamily.Provider, licenseProvider *license.Provider, hostresourcegroupProvider *hostresourcegroup.Provider) corecontroller.Controller { + securityGroupProvider *securitygroup.Provider, amiProvider *amifamily.Provider, + licenseProvider *license.Provider, hostresourcegroupProvider *hostresourcegroup.Provider, + placementProvider *placementgroup.Provider) corecontroller.Controller { return corecontroller.Typed[*v1beta1.EC2NodeClass](kubeClient, &NodeClassController{ - Controller: NewController(kubeClient, subnetProvider, securityGroupProvider, amiProvider, licenseProvider, hostresourcegroupProvider), + Controller: NewController(kubeClient, subnetProvider, securityGroupProvider, amiProvider, licenseProvider, hostresourcegroupProvider, placementProvider), }) } @@ -218,9 +240,11 @@ type NodeTemplateController struct { } func NewNodeTemplateController(kubeClient client.Client, subnetProvider *subnet.Provider, - securityGroupProvider *securitygroup.Provider, amiProvider *amifamily.Provider, licenseProvider *license.Provider, hostresourcegroupProvider *hostresourcegroup.Provider) corecontroller.Controller { + securityGroupProvider *securitygroup.Provider, amiProvider *amifamily.Provider, + licenseProvider *license.Provider, hostresourcegroupProvider *hostresourcegroup.Provider, + placementProvider *placementgroup.Provider) corecontroller.Controller { return corecontroller.Typed[*v1alpha1.AWSNodeTemplate](kubeClient, &NodeTemplateController{ - Controller: NewController(kubeClient, subnetProvider, securityGroupProvider, amiProvider, licenseProvider, hostresourcegroupProvider), + Controller: NewController(kubeClient, subnetProvider, securityGroupProvider, amiProvider, licenseProvider, hostresourcegroupProvider, placementProvider), }) } diff --git a/pkg/operator/operator.go b/pkg/operator/operator.go index a411bc253f28..856aee1bb659 100644 --- a/pkg/operator/operator.go +++ b/pkg/operator/operator.go @@ -56,6 +56,7 @@ import ( "github.com/aws/karpenter/pkg/providers/instancetype" "github.com/aws/karpenter/pkg/providers/launchtemplate" "github.com/aws/karpenter/pkg/providers/license" + "github.com/aws/karpenter/pkg/providers/placementgroup" "github.com/aws/karpenter/pkg/providers/pricing" "github.com/aws/karpenter/pkg/providers/securitygroup" "github.com/aws/karpenter/pkg/providers/subnet" @@ -81,6 +82,7 @@ type Operator struct { InstanceProvider *instance.Provider LicenseProvider *license.Provider HostResourceGroupProvider *hostresourcegroup.Provider + PlacementGroupProvider *placementgroup.Provider } func NewOperator(ctx context.Context, operator *operator.Operator) (context.Context, *Operator) { @@ -138,8 +140,9 @@ func NewOperator(ctx context.Context, operator *operator.Operator) (context.Cont versionProvider := version.NewProvider(operator.KubernetesInterface, cache.New(awscache.DefaultTTL, awscache.DefaultCleanupInterval)) amiProvider := amifamily.NewProvider(versionProvider, ssm.New(sess), ec2api, cache.New(awscache.DefaultTTL, awscache.DefaultCleanupInterval)) licenseProvider := license.NewProvider(licensemanager.New(sess), cache.New(awscache.DefaultTTL, awscache.DefaultCleanupInterval)) - hostresourcegroupProvider := hostresourcegroup.NewProvider(resourcegroups.New(sess), cache.New(awscache.DefaultTTL, awscache.DefaultCleanupInterval)) - amiResolver := amifamily.New(amiProvider, licenseProvider, hostresourcegroupProvider) + hostresourcegroupProvider := hostresourcegroup.NewProvider(resourcegroups.New(sess), cache.New(awscache.DefaultTTL, awscache.DefaultCleanupInterval)) + placementGroupProvider := placementgroup.NewProvider(ec2api, cache.New(awscache.DefaultTTL, awscache.DefaultCleanupInterval)) + amiResolver := amifamily.New(amiProvider, licenseProvider, hostresourcegroupProvider, placementGroupProvider) launchTemplateProvider := launchtemplate.NewProvider( ctx, cache.New(awscache.DefaultTTL, awscache.DefaultCleanupInterval), @@ -186,7 +189,8 @@ func NewOperator(ctx context.Context, operator *operator.Operator) (context.Cont InstanceTypesProvider: instanceTypeProvider, InstanceProvider: instanceProvider, LicenseProvider: licenseProvider, - HostResourceGroupProvider: hostresourcegroupProvider, + HostResourceGroupProvider: hostresourcegroupProvider, + PlacementGroupProvider: placementGroupProvider, } } diff --git a/pkg/providers/amifamily/resolver.go b/pkg/providers/amifamily/resolver.go index b437b4de2496..f1d4712db5c0 100644 --- a/pkg/providers/amifamily/resolver.go +++ b/pkg/providers/amifamily/resolver.go @@ -31,6 +31,7 @@ import ( "github.com/aws/karpenter/pkg/providers/amifamily/bootstrap" "github.com/aws/karpenter/pkg/providers/hostresourcegroup" "github.com/aws/karpenter/pkg/providers/license" + "github.com/aws/karpenter/pkg/providers/placementgroup" "github.com/aws/karpenter-core/pkg/cloudprovider" "github.com/aws/karpenter-core/pkg/scheduling" @@ -47,6 +48,7 @@ type Resolver struct { amiProvider *Provider licenseProvider *license.Provider hostResourceGroupProvider *hostresourcegroup.Provider + placementGroupProvider *placementgroup.Provider } // Options define the static launch template parameters @@ -80,6 +82,7 @@ type LaunchTemplate struct { // Placement holds the dynamically generated launch template placement parameters type Placement struct { HostResourceGroup string + PlacementGroup string } // AMIFamily can be implemented to override the default logic for generating dynamic launch template parameters @@ -118,11 +121,12 @@ func (d DefaultFamily) FeatureFlags() FeatureFlags { } // New constructs a new launch template Resolver -func New(amiProvider *Provider, licenseProvider *license.Provider, hostResourceGroupProvider *hostresourcegroup.Provider) *Resolver { +func New(amiProvider *Provider, licenseProvider *license.Provider, hostResourceGroupProvider *hostresourcegroup.Provider, placementGroupProvider *placementgroup.Provider) *Resolver { return &Resolver{ amiProvider: amiProvider, licenseProvider: licenseProvider, hostResourceGroupProvider: hostResourceGroupProvider, + placementGroupProvider: placementGroupProvider, } } @@ -141,19 +145,13 @@ func (r Resolver) Resolve(ctx context.Context, nodeClass *v1beta1.EC2NodeClass, if len(mappedAMIs) == 0 { return nil, fmt.Errorf("no instance types satisfy requirements of amis %v", amis) } - licenses, err := r.licenseProvider.List(ctx, nodeClass) + licenses, err := r.licenseProvider.Get(ctx, nodeClass) if err != nil { return nil, err } - var placement *Placement - hrg, err := r.hostResourceGroupProvider.Get(ctx, nodeClass) - if err != nil { - return nil, err - } - if hrg == nil { - placement = nil - } else { - placement = &Placement{HostResourceGroup: hrg.ARN} + placement, err := r.resolvePlacement(ctx, nodeClass) + if err != nil { + return nil, err } var resolvedTemplates []*LaunchTemplate for amiID, instanceTypes := range mappedAMIs { @@ -203,6 +201,26 @@ func (r Resolver) Resolve(ctx context.Context, nodeClass *v1beta1.EC2NodeClass, return resolvedTemplates, nil } +func (r Resolver) resolvePlacement(ctx context.Context, nodeClass *v1beta1.EC2NodeClass) (*Placement, error) { + placement := &Placement{} + hrg, err := r.hostResourceGroupProvider.Get(ctx, nodeClass) + if err != nil { + return nil, err + } + pg, err := r.placementGroupProvider.Get(ctx, nodeClass) + if err != nil { + return nil, err + } + if pg == nil && hrg == nil { + return nil, nil + } else { + placement.HostResourceGroup = hrg.Name + placement.PlacementGroup = *pg.GroupName + + } + return placement, nil +} + func GetAMIFamily(amiFamily *string, options *Options) AMIFamily { switch aws.StringValue(amiFamily) { case v1beta1.AMIFamilyBottlerocket: diff --git a/pkg/providers/hostresourcegroup/hostresourcegroup.go b/pkg/providers/hostresourcegroup/hostresourcegroup.go index 39f2239220dd..27540079d280 100644 --- a/pkg/providers/hostresourcegroup/hostresourcegroup.go +++ b/pkg/providers/hostresourcegroup/hostresourcegroup.go @@ -1,3 +1,17 @@ +/* +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + package hostresourcegroup import ( @@ -38,6 +52,9 @@ func (p *Provider) Get(ctx context.Context, nodeClass *v1beta1.EC2NodeClass) (*v for { resp, err := p.resourcegroups.ListGroupsWithContext(ctx, &resourcegroups.ListGroupsInput{ NextToken: nextToken}) if err != nil { + logging.FromContext(ctx). + With("aws error", err). + Debugf("Error from resourcegroup:listgroups") return nil, err } nextToken = resp.NextToken diff --git a/pkg/providers/license/license.go b/pkg/providers/license/license.go index d1feb09e1ef6..2e922570cb8a 100644 --- a/pkg/providers/license/license.go +++ b/pkg/providers/license/license.go @@ -48,15 +48,13 @@ func NewProvider(lmapi licensemanageriface.LicenseManagerAPI, cache *cache.Cache } } -func (p *Provider) List(ctx context.Context, nodeClass *v1beta1.EC2NodeClass) ([]string, error) { +func (p *Provider) Get(ctx context.Context, nodeClass *v1beta1.EC2NodeClass) ([]string, error) { p.Lock() defer p.Unlock() // Get selectors from the nodeClass, exit if no selectors defined selectors := nodeClass.Spec.LicenseSelectorTerms if selectors == nil { - logging.FromContext(ctx). - Debugf("no selectors present") return nil, nil } @@ -73,6 +71,9 @@ func (p *Provider) List(ctx context.Context, nodeClass *v1beta1.EC2NodeClass) ([ // Look up all License Configurations output, err := p.licensemanager.ListLicenseConfigurationsWithContext(ctx, &licensemanager.ListLicenseConfigurationsInput{}) if err != nil { + logging.FromContext(ctx). + With("api error", err). + Debugf("Error from licensemanager:ListLicenseConfigurations") return nil, err } for i := range output.LicenseConfigurations { diff --git a/pkg/providers/placementgroup/placementgroup.go b/pkg/providers/placementgroup/placementgroup.go new file mode 100644 index 000000000000..9907733768c4 --- /dev/null +++ b/pkg/providers/placementgroup/placementgroup.go @@ -0,0 +1,94 @@ +/* +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package placementgroup + +import ( + "context" + "fmt" + "sync" + + "github.com/aws/aws-sdk-go/service/ec2" + "github.com/aws/aws-sdk-go/service/ec2/ec2iface" + "github.com/mitchellh/hashstructure/v2" + "github.com/patrickmn/go-cache" + + "github.com/aws/karpenter/pkg/apis/v1beta1" + + "github.com/aws/karpenter-core/pkg/utils/pretty" + "github.com/samber/lo" + "knative.dev/pkg/logging" +) + +type Provider struct { + sync.RWMutex + ec2api ec2iface.EC2API + cache *cache.Cache + cm *pretty.ChangeMonitor +} + +func NewProvider(ec2api ec2iface.EC2API, cache *cache.Cache) *Provider { + return &Provider{ + ec2api: ec2api, + cm: pretty.NewChangeMonitor(), + // TODO: Remove cache for v1beta1, utilize resolved subnet from the AWSNodeTemplate.status + // Subnets are sorted on AvailableIpAddressCount, descending order + cache: cache, + } +} + +func (p *Provider) Get(ctx context.Context, nodeClass *v1beta1.EC2NodeClass) (*ec2.PlacementGroup, error) { + p.Lock() + defer p.Unlock() + + // Get selectors from the nodeClass, exit if no selectors defined + selectors := nodeClass.Spec.LicenseSelectorTerms + if selectors == nil { + return nil, nil + } + + // Look for a cached result + hash, err := hashstructure.Hash(selectors, hashstructure.FormatV2, &hashstructure.HashOptions{SlicesAsSets: true}) + if err != nil { + return nil, err + } + if cached, ok := p.cache.Get(fmt.Sprint(hash)); ok { + return cached.(*ec2.PlacementGroup), nil + } + + groups := []*ec2.PlacementGroup{} + // Look up all License Configurations + output, err := p.ec2api.DescribePlacementGroupsWithContext(ctx, &ec2.DescribePlacementGroupsInput{}) + if err != nil { + logging.FromContext(ctx). + With("aws error", err). + Debugf("Error from ec2:describeplacementgroups") + return nil, err + } + for i := range output.PlacementGroups { + // filter results to only include those that match at least 1 selector + for x := range selectors { + if *output.PlacementGroups[i].GroupName == selectors[x].Name { + groups = append(groups, output.PlacementGroups[i]) + } + } + } + + if p.cm.HasChanged(fmt.Sprintf("placementGroups/%t/%s", nodeClass.IsNodeTemplate, nodeClass.Name), groups) { + logging.FromContext(ctx). + With("placementGroups", lo.Map(groups, func(g *ec2.PlacementGroup, _ int) string { return *g.GroupArn })). + Debugf("discovered placement groups") + } + return groups[0], nil +} diff --git a/pkg/utils/nodeclass/nodeclass.go b/pkg/utils/nodeclass/nodeclass.go index 0b6840d0853b..30113ed6ce29 100644 --- a/pkg/utils/nodeclass/nodeclass.go +++ b/pkg/utils/nodeclass/nodeclass.go @@ -46,6 +46,8 @@ func New(nodeTemplate *v1alpha1.AWSNodeTemplate) *v1beta1.EC2NodeClass { AMIFamily: nodeTemplate.Spec.AMIFamily, LicenseSelectorTerms: NewLicenseSelectorTerms(nodeTemplate.Spec.LicenseSelector), OriginalLicenseSelector: nodeTemplate.Spec.LicenseSelector, + OriginalPlacementGroupSelector: nodeTemplate.Spec.PlacementGroupSelector, + PlacementGroupSelectorTerms: NewPlacementGroupSelectorTerms(nodeTemplate.Spec.PlacementGroupSelector), UserData: nodeTemplate.Spec.UserData, Tags: nodeTemplate.Spec.Tags, BlockDeviceMappings: NewBlockDeviceMappings(nodeTemplate.Spec.BlockDeviceMappings), @@ -68,6 +70,15 @@ func New(nodeTemplate *v1alpha1.AWSNodeTemplate) *v1beta1.EC2NodeClass { } } +func NewPlacementGroupSelectorTerms(placementGroupSelector map[string]string) (terms []v1beta1.PlacementGroupSelectorTerm) { + if len(placementGroupSelector) == 0 { + return nil + } + return []v1beta1.PlacementGroupSelectorTerm{ + {Name: placementGroupSelector["name"]}, + } +} + func NewHostResourceSelectorTerms(hrgSelector map[string]string) (terms []v1beta1.HostResourceGroupSelectorTerm) { if len(hrgSelector) == 0 { return nil From f7fff5ad44744da8f5f392a12c3c676be43adcd1 Mon Sep 17 00:00:00 2001 From: Sebastian Cole Date: Tue, 19 Sep 2023 14:28:16 +1000 Subject: [PATCH 14/33] Fixes to api documentation and testing environment --- .../karpenter.k8s.aws_awsnodetemplates.yaml | 14 ++--- pkg/apis/v1alpha1/awsnodetemplate.go | 4 +- pkg/apis/v1alpha1/provider.go | 6 +-- pkg/fake/lmapi.go | 51 +++++++++++++++++++ pkg/fake/rgapi.go | 50 ++++++++++++++++++ .../launchtemplate/launchtemplate.go | 39 +++++++------- .../placementgroup/placementgroup.go | 17 ++++--- pkg/test/environment.go | 49 ++++++++++++------ 8 files changed, 175 insertions(+), 55 deletions(-) create mode 100644 pkg/fake/lmapi.go create mode 100644 pkg/fake/rgapi.go diff --git a/pkg/apis/crds/karpenter.k8s.aws_awsnodetemplates.yaml b/pkg/apis/crds/karpenter.k8s.aws_awsnodetemplates.yaml index a57cc4bbcf10..ba5a1c4746f3 100644 --- a/pkg/apis/crds/karpenter.k8s.aws_awsnodetemplates.yaml +++ b/pkg/apis/crds/karpenter.k8s.aws_awsnodetemplates.yaml @@ -132,7 +132,7 @@ spec: hostResourceGroupSelector: additionalProperties: type: string - description: HostResourceGroups specific the hostResourceGroupArns + description: HostResourceGroups specifies the hostResourceGroup names to use for ec2 placement type: object instanceProfile: @@ -153,8 +153,8 @@ spec: licenseSelector: additionalProperties: type: string - description: LicenseConfiguration specifies the license configuration - specifications to launch instances with + description: LicenseSelector specifies the license configuration specifications + to launch instances with type: object metadataOptions: description: "MetadataOptions for the generated launch template of @@ -207,8 +207,8 @@ spec: placementGroupSelector: additionalProperties: type: string - description: PlacementGroup specifies the placement group to use for - ec2 placement + description: PlacementGroup specifies the placement group names to + use for ec2 placement type: object securityGroupSelector: additionalProperties: @@ -301,8 +301,8 @@ spec: type: string type: array placementGroups: - description: Licenses contains the current License Configuration Specifications - that currently apply given the LicenseConfiguration selectors. + description: PlacementGroups contains the current placement groups + that currently apply given the PlacementGroup selectors. items: type: string type: array diff --git a/pkg/apis/v1alpha1/awsnodetemplate.go b/pkg/apis/v1alpha1/awsnodetemplate.go index 1d07ffc40077..744cbb6182ff 100644 --- a/pkg/apis/v1alpha1/awsnodetemplate.go +++ b/pkg/apis/v1alpha1/awsnodetemplate.go @@ -77,8 +77,8 @@ type AWSNodeTemplateStatus struct { // that currently apply given the HostResourceGroup selectors. // +optional HostResourceGroups []string `json:"hostResourceGroups,omitempty"` - // Licenses contains the current License Configuration Specifications - // that currently apply given the LicenseConfiguration selectors. + // PlacementGroups contains the current placement groups + // that currently apply given the PlacementGroup selectors. // +optional PlacementGroups []string `json:"placementGroups,omitempty"` } diff --git a/pkg/apis/v1alpha1/provider.go b/pkg/apis/v1alpha1/provider.go index a0ac32f06b65..c72f0a9c88d8 100644 --- a/pkg/apis/v1alpha1/provider.go +++ b/pkg/apis/v1alpha1/provider.go @@ -41,13 +41,13 @@ type AWS struct { // SecurityGroups specify the names of the security groups. // +optional SecurityGroupSelector map[string]string `json:"securityGroupSelector,omitempty" hash:"ignore"` - // LicenseConfiguration specifies the license configuration specifications to launch instances with + // LicenseSelector specifies the license configuration specifications to launch instances with // +optional LicenseSelector map[string]string `json:"licenseSelector,omitempty" hash:"ignore"` - // HostResourceGroups specific the hostResourceGroupArns to use for ec2 placement + // HostResourceGroups specifies the hostResourceGroup names to use for ec2 placement // +optional HostResourceGroupSelector map[string]string `json:"hostResourceGroupSelector,omitempty" hash:"ignore"` - // PlacementGroup specifies the placement group to use for ec2 placement + // PlacementGroup specifies the placement group names to use for ec2 placement // +optional PlacementGroupSelector map[string]string `json:"placementGroupSelector,omitempty" hash:"ignore"` // Tags to be applied on ec2 resources like instances and launch templates. diff --git a/pkg/fake/lmapi.go b/pkg/fake/lmapi.go new file mode 100644 index 000000000000..96277bc1662b --- /dev/null +++ b/pkg/fake/lmapi.go @@ -0,0 +1,51 @@ +/* +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package fake + +import ( + "errors" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/aws/request" + "github.com/aws/aws-sdk-go/service/licensemanager" + "github.com/aws/aws-sdk-go/service/licensemanager/licensemanageriface" +) + +type LicenseManagerAPI struct { + licensemanageriface.LicenseManagerAPI + LicenseManagerBehaviour +} +type LicenseManagerBehaviour struct { + NextError AtomicError + ListLicenseConfigurationsOutput AtomicPtr[licensemanager.ListLicenseConfigurationsOutput] +} + +func (l *LicenseManagerAPI) Reset() { + l.NextError.Reset() + l.ListLicenseConfigurationsOutput.Reset() +} + +func (l *LicenseManagerAPI) ListLicenseConfigurations(_ aws.Context, _ *licensemanager.ListLicenseConfigurationsInput, fn func(*licensemanager.ListLicenseConfigurationsOutput, bool) bool, _ ...request.Option) error { + if !l.NextError.IsNil() { + return l.NextError.Get() + } + if !l.ListLicenseConfigurationsOutput.IsNil() { + fn(l.ListLicenseConfigurationsOutput.Clone(), false) + return nil + } + // fail if the test doesn't provide specific data which causes our pricing provider to use its static price list + return errors.New("no license data provided") +} + diff --git a/pkg/fake/rgapi.go b/pkg/fake/rgapi.go new file mode 100644 index 000000000000..97bd811c2d62 --- /dev/null +++ b/pkg/fake/rgapi.go @@ -0,0 +1,50 @@ +/* +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package fake + +import ( + "errors" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/aws/request" + "github.com/aws/aws-sdk-go/service/resourcegroups" + "github.com/aws/aws-sdk-go/service/resourcegroups/resourcegroupsiface" +) + +type ResourceGroupsAPI struct { + resourcegroupsiface.ResourceGroupsAPI + ResourceGroupsBehaviour +} +type ResourceGroupsBehaviour struct { + NextError AtomicError + ListGroupsOutput AtomicPtr[resourcegroups.ListGroupsOutput] +} + +func (r *ResourceGroupsAPI) Reset() { + r.NextError.Reset() + r.ListGroupsOutput.Reset() +} + +func (r *ResourceGroupsAPI) ListGroups(_ aws.Context, _ *resourcegroups.ListGroupsInput, fn func(*resourcegroups.ListGroupsOutput, bool) bool, _ ...request.Option) error { + if !r.NextError.IsNil() { + return r.NextError.Get() + } + if !r.ListGroupsOutput.IsNil() { + fn(r.ListGroupsOutput.Clone(), false) + return nil + } + // fail if the test doesn't provide specific data which causes our pricing provider to use its static price list + return errors.New("no resource groups provided") +} diff --git a/pkg/providers/launchtemplate/launchtemplate.go b/pkg/providers/launchtemplate/launchtemplate.go index 060c4b39f6c3..1b666576f651 100644 --- a/pkg/providers/launchtemplate/launchtemplate.go +++ b/pkg/providers/launchtemplate/launchtemplate.go @@ -40,7 +40,6 @@ import ( "github.com/aws/karpenter/pkg/apis/v1beta1" awserrors "github.com/aws/karpenter/pkg/errors" "github.com/aws/karpenter/pkg/providers/amifamily" - "github.com/aws/karpenter/pkg/providers/license" "github.com/aws/karpenter/pkg/providers/securitygroup" "github.com/aws/karpenter/pkg/providers/subnet" "github.com/aws/karpenter/pkg/utils" @@ -60,7 +59,6 @@ type Provider struct { amiFamily *amifamily.Resolver securityGroupProvider *securitygroup.Provider subnetProvider *subnet.Provider - licenseProvider *license.Provider cache *cache.Cache caBundle *string cm *pretty.ChangeMonitor @@ -68,13 +66,14 @@ type Provider struct { ClusterEndpoint string } -func NewProvider(ctx context.Context, cache *cache.Cache, ec2api ec2iface.EC2API, amiFamily *amifamily.Resolver, securityGroupProvider *securitygroup.Provider, subnetProvider *subnet.Provider, licenseProvider *license.Provider, caBundle *string, startAsync <-chan struct{}, kubeDNSIP net.IP, clusterEndpoint string) *Provider { +func NewProvider(ctx context.Context, cache *cache.Cache, ec2api ec2iface.EC2API, + amiFamily *amifamily.Resolver, securityGroupProvider *securitygroup.Provider, + subnetProvider *subnet.Provider, caBundle *string, startAsync <-chan struct{}, kubeDNSIP net.IP, clusterEndpoint string) *Provider { l := &Provider{ ec2api: ec2api, amiFamily: amiFamily, securityGroupProvider: securityGroupProvider, subnetProvider: subnetProvider, - licenseProvider: licenseProvider, cache: cache, caBundle: caBundle, cm: pretty.NewChangeMonitor(), @@ -250,8 +249,8 @@ func (p *Provider) createLaunchTemplate(ctx context.Context, options *amifamily. TagSpecifications: []*ec2.LaunchTemplateTagSpecificationRequest{ {ResourceType: aws.String(ec2.ResourceTypeNetworkInterface), Tags: utils.MergeTags(options.Tags)}, }, - LicenseSpecifications: generateLicenseSpecification(options.Licenses), - Placement: generatePlacement(options.Placement), + LicenseSpecifications: generateLicenseSpecification(options.Licenses), + Placement: generatePlacement(options.Placement), }, TagSpecifications: []*ec2.TagSpecification{ { @@ -268,25 +267,25 @@ func (p *Provider) createLaunchTemplate(ctx context.Context, options *amifamily. } func generatePlacement(placement *amifamily.Placement) *ec2.LaunchTemplatePlacementRequest { - if placement == nil { - return nil - } - return &ec2.LaunchTemplatePlacementRequest{ - HostResourceGroupArn: aws.String(placement.HostResourceGroup), - } + if placement == nil { + return nil + } + return &ec2.LaunchTemplatePlacementRequest{ + HostResourceGroupArn: aws.String(placement.HostResourceGroup), + } } func generateLicenseSpecification(licenses []string) []*ec2.LaunchTemplateLicenseConfigurationRequest { - result := []*ec2.LaunchTemplateLicenseConfigurationRequest{} - if len(licenses) == 0 { - return nil - } - for i := range licenses { - result = append(result, &ec2.LaunchTemplateLicenseConfigurationRequest{ LicenseConfigurationArn: aws.String(licenses[i])}) + result := []*ec2.LaunchTemplateLicenseConfigurationRequest{} + if len(licenses) == 0 { + return nil + } + for i := range licenses { + result = append(result, &ec2.LaunchTemplateLicenseConfigurationRequest{LicenseConfigurationArn: aws.String(licenses[i])}) - } - return result + } + return result } diff --git a/pkg/providers/placementgroup/placementgroup.go b/pkg/providers/placementgroup/placementgroup.go index 9907733768c4..5f2cbbebc868 100644 --- a/pkg/providers/placementgroup/placementgroup.go +++ b/pkg/providers/placementgroup/placementgroup.go @@ -27,7 +27,6 @@ import ( "github.com/aws/karpenter/pkg/apis/v1beta1" "github.com/aws/karpenter-core/pkg/utils/pretty" - "github.com/samber/lo" "knative.dev/pkg/logging" ) @@ -67,28 +66,30 @@ func (p *Provider) Get(ctx context.Context, nodeClass *v1beta1.EC2NodeClass) (*e return cached.(*ec2.PlacementGroup), nil } - groups := []*ec2.PlacementGroup{} + var group *ec2.PlacementGroup // Look up all License Configurations output, err := p.ec2api.DescribePlacementGroupsWithContext(ctx, &ec2.DescribePlacementGroupsInput{}) if err != nil { logging.FromContext(ctx). - With("aws error", err). - Debugf("Error from ec2:describeplacementgroups") + With("aws error", err). + Debugf("Error from ec2:describeplacementgroups") return nil, err } for i := range output.PlacementGroups { // filter results to only include those that match at least 1 selector for x := range selectors { if *output.PlacementGroups[i].GroupName == selectors[x].Name { - groups = append(groups, output.PlacementGroups[i]) + group = output.PlacementGroups[i] + break } } } - if p.cm.HasChanged(fmt.Sprintf("placementGroups/%t/%s", nodeClass.IsNodeTemplate, nodeClass.Name), groups) { + if p.cm.HasChanged(fmt.Sprintf("placementGroups/%t/%s", nodeClass.IsNodeTemplate, nodeClass.Name), group) { logging.FromContext(ctx). - With("placementGroups", lo.Map(groups, func(g *ec2.PlacementGroup, _ int) string { return *g.GroupArn })). + With("placementGroup", group). Debugf("discovered placement groups") } - return groups[0], nil + + return group, nil } diff --git a/pkg/test/environment.go b/pkg/test/environment.go index b81dab375e77..64d43d0ed32a 100644 --- a/pkg/test/environment.go +++ b/pkg/test/environment.go @@ -25,10 +25,12 @@ import ( awscache "github.com/aws/karpenter/pkg/cache" "github.com/aws/karpenter/pkg/fake" "github.com/aws/karpenter/pkg/providers/amifamily" + "github.com/aws/karpenter/pkg/providers/hostresourcegroup" "github.com/aws/karpenter/pkg/providers/instance" "github.com/aws/karpenter/pkg/providers/instancetype" "github.com/aws/karpenter/pkg/providers/launchtemplate" "github.com/aws/karpenter/pkg/providers/license" + "github.com/aws/karpenter/pkg/providers/placementgroup" "github.com/aws/karpenter/pkg/providers/pricing" "github.com/aws/karpenter/pkg/providers/securitygroup" "github.com/aws/karpenter/pkg/providers/subnet" @@ -41,9 +43,11 @@ import ( type Environment struct { // API - EC2API *fake.EC2API - SSMAPI *fake.SSMAPI - PricingAPI *fake.PricingAPI + EC2API *fake.EC2API + SSMAPI *fake.SSMAPI + PricingAPI *fake.PricingAPI + LicenseManagerAPI *fake.LicenseManagerAPI + ResourceGroupsAPI *fake.ResourceGroupsAPI // Cache EC2Cache *cache.Cache @@ -53,24 +57,31 @@ type Environment struct { LaunchTemplateCache *cache.Cache SubnetCache *cache.Cache SecurityGroupCache *cache.Cache + LicenseCache *cache.Cache + HostResourceGroupCache *cache.Cache + PlacementGroupCache *cache.Cache // Providers - InstanceTypesProvider *instancetype.Provider - InstanceProvider *instance.Provider - SubnetProvider *subnet.Provider - SecurityGroupProvider *securitygroup.Provider - PricingProvider *pricing.Provider - AMIProvider *amifamily.Provider - AMIResolver *amifamily.Resolver - VersionProvider *version.Provider - LaunchTemplateProvider *launchtemplate.Provider - LicenseProvider *license.Provider + InstanceTypesProvider *instancetype.Provider + InstanceProvider *instance.Provider + SubnetProvider *subnet.Provider + SecurityGroupProvider *securitygroup.Provider + PricingProvider *pricing.Provider + AMIProvider *amifamily.Provider + AMIResolver *amifamily.Resolver + VersionProvider *version.Provider + LaunchTemplateProvider *launchtemplate.Provider + LicenseProvider *license.Provider + HostResourceGroupProvider *hostresourcegroup.Provider + PlacementGroupProvider *placementgroup.Provider } func NewEnvironment(ctx context.Context, env *coretest.Environment) *Environment { // API ec2api := &fake.EC2API{} ssmapi := &fake.SSMAPI{} + lmapi := &fake.LicenseManagerAPI{} + rgapi := &fake.ResourceGroupsAPI{} // cache ec2Cache := cache.New(awscache.DefaultTTL, awscache.DefaultCleanupInterval) @@ -81,6 +92,8 @@ func NewEnvironment(ctx context.Context, env *coretest.Environment) *Environment subnetCache := cache.New(awscache.DefaultTTL, awscache.DefaultCleanupInterval) securityGroupCache := cache.New(awscache.DefaultTTL, awscache.DefaultCleanupInterval) licenseCache := cache.New(awscache.DefaultTTL, awscache.DefaultCleanupInterval) + hostResourceGroupsCache := cache.New(awscache.DefaultTTL, awscache.DefaultCleanupInterval) + placementGroupCache := cache.New(awscache.DefaultTTL, awscache.DefaultCleanupInterval) fakePricingAPI := &fake.PricingAPI{} // Providers @@ -89,8 +102,11 @@ func NewEnvironment(ctx context.Context, env *coretest.Environment) *Environment securityGroupProvider := securitygroup.NewProvider(ec2api, securityGroupCache) versionProvider := version.NewProvider(env.KubernetesInterface, kubernetesVersionCache) amiProvider := amifamily.NewProvider(versionProvider, ssmapi, ec2api, ec2Cache) - licenseProvider := license.NewProvider(ec2api, licenseCache) - amiResolver := amifamily.New(amiProvider, licenseProvider) + licenseProvider := license.NewProvider(lmapi.LicenseManagerAPI, licenseCache) + hostResourceGroupProvider := hostresourcegroup.NewProvider(rgapi, hostResourceGroupsCache) + placementGroupProvider := placementgroup.NewProvider(ec2api, placementGroupCache) + + amiResolver := amifamily.New(amiProvider, licenseProvider, hostResourceGroupProvider, placementGroupProvider) instanceTypesProvider := instancetype.NewProvider("", instanceTypeCache, ec2api, subnetProvider, unavailableOfferingsCache, pricingProvider) launchTemplateProvider := launchtemplate.NewProvider( @@ -127,6 +143,7 @@ func NewEnvironment(ctx context.Context, env *coretest.Environment) *Environment SubnetCache: subnetCache, SecurityGroupCache: securityGroupCache, UnavailableOfferingsCache: unavailableOfferingsCache, + LicenseCache: licenseCache, InstanceTypesProvider: instanceTypesProvider, InstanceProvider: instanceProvider, @@ -144,6 +161,7 @@ func NewEnvironment(ctx context.Context, env *coretest.Environment) *Environment func (env *Environment) Reset() { env.EC2API.Reset() env.SSMAPI.Reset() + env.LicenseManagerAPI.Reset() env.PricingAPI.Reset() env.PricingProvider.Reset() @@ -154,6 +172,7 @@ func (env *Environment) Reset() { env.LaunchTemplateCache.Flush() env.SubnetCache.Flush() env.SecurityGroupCache.Flush() + env.LicenseCache.Flush() mfs, err := crmetrics.Registry.Gather() if err != nil { From 44fdeef211ff86eff95d9f5bb3285703142cd98a Mon Sep 17 00:00:00 2001 From: Sebastian Cole Date: Tue, 19 Sep 2023 16:36:21 +1000 Subject: [PATCH 15/33] Refactor host resource group --- .../hostresourcegroup/hostresourcegroup.go | 56 ++++++------------- .../launchtemplate/launchtemplate.go | 2 +- pkg/providers/license/license.go | 4 +- 3 files changed, 18 insertions(+), 44 deletions(-) diff --git a/pkg/providers/hostresourcegroup/hostresourcegroup.go b/pkg/providers/hostresourcegroup/hostresourcegroup.go index 27540079d280..d4c22b275d90 100644 --- a/pkg/providers/hostresourcegroup/hostresourcegroup.go +++ b/pkg/providers/hostresourcegroup/hostresourcegroup.go @@ -47,48 +47,24 @@ func NewProvider(rgapi resourcegroupsiface.ResourceGroupsAPI, cache *cache.Cache func (p *Provider) Get(ctx context.Context, nodeClass *v1beta1.EC2NodeClass) (*v1beta1.HostResourceGroup, error) { p.Lock() defer p.Unlock() - var nextToken *string = nil - var groups []*resourcegroups.GroupIdentifier - for { - resp, err := p.resourcegroups.ListGroupsWithContext(ctx, &resourcegroups.ListGroupsInput{ NextToken: nextToken}) - if err != nil { - logging.FromContext(ctx). - With("aws error", err). - Debugf("Error from resourcegroup:listgroups") - return nil, err - } - nextToken = resp.NextToken - for i := range resp.GroupIdentifiers { - groups = append(groups, resp.GroupIdentifiers[i]) - } - if nextToken == nil { - break - } - } - - // filter resp to only include those that match the hostResourceGroupSelector Name - for i := range groups { - group := groups[i] - for x := range nodeClass.Spec.HostResourceGroupSelectorTerms { - selector := nodeClass.Spec.HostResourceGroupSelectorTerms[x] - logging.FromContext(ctx). - With("group", group). - With("selector", selector). - Debugf("checking for host resource group match") - if *group.GroupName == selector.Name { - match := &v1beta1.HostResourceGroup{ARN: *group.GroupArn, Name: *group.GroupName} - logging.FromContext(ctx). - With("Matched hrg", match). - Debugf("discovered host resource group configuration") - - return match, nil + 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 nodeClass.Spec.HostResourceGroupSelectorTerms { + selector := nodeClass.Spec.HostResourceGroupSelectorTerms[x] + if *page.GroupIdentifiers[i].GroupName == selector.Name { + match = &v1beta1.HostResourceGroup{ ARN: *page.GroupIdentifiers[i].GroupArn, Name: *page.GroupIdentifiers[i].GroupName } + return false + } } } + return lastPage + }) + + if err != nil { + logging.FromContext(ctx).Errorf("discovery resource groups, %w", err) + return nil, err } - logging.FromContext(ctx). - With("groups", groups). - Debugf("No hrg matched") - // No matching groups - return nil, nil + return match, nil } diff --git a/pkg/providers/launchtemplate/launchtemplate.go b/pkg/providers/launchtemplate/launchtemplate.go index 1b666576f651..3a692a506d82 100644 --- a/pkg/providers/launchtemplate/launchtemplate.go +++ b/pkg/providers/launchtemplate/launchtemplate.go @@ -277,7 +277,7 @@ func generatePlacement(placement *amifamily.Placement) *ec2.LaunchTemplatePlacem } func generateLicenseSpecification(licenses []string) []*ec2.LaunchTemplateLicenseConfigurationRequest { - result := []*ec2.LaunchTemplateLicenseConfigurationRequest{} + var result []*ec2.LaunchTemplateLicenseConfigurationRequest if len(licenses) == 0 { return nil } diff --git a/pkg/providers/license/license.go b/pkg/providers/license/license.go index 2e922570cb8a..50254b768905 100644 --- a/pkg/providers/license/license.go +++ b/pkg/providers/license/license.go @@ -71,9 +71,7 @@ func (p *Provider) Get(ctx context.Context, nodeClass *v1beta1.EC2NodeClass) ([] // Look up all License Configurations output, err := p.licensemanager.ListLicenseConfigurationsWithContext(ctx, &licensemanager.ListLicenseConfigurationsInput{}) if err != nil { - logging.FromContext(ctx). - With("api error", err). - Debugf("Error from licensemanager:ListLicenseConfigurations") + logging.FromContext(ctx).Errorf("listing license configurations %w", err) return nil, err } for i := range output.LicenseConfigurations { From 4381e8d77748b0a8bbf20cdcefe5d46bbcd2dafb Mon Sep 17 00:00:00 2001 From: Sebastian Cole Date: Tue, 19 Sep 2023 17:09:24 +1000 Subject: [PATCH 16/33] Addressing PR feedback for providers --- .../hostresourcegroup/hostresourcegroup.go | 30 ++++++++++++++++--- pkg/providers/license/license.go | 7 +++-- .../placementgroup/placementgroup.go | 22 +++++++------- 3 files changed, 40 insertions(+), 19 deletions(-) diff --git a/pkg/providers/hostresourcegroup/hostresourcegroup.go b/pkg/providers/hostresourcegroup/hostresourcegroup.go index d4c22b275d90..7230d4dcac06 100644 --- a/pkg/providers/hostresourcegroup/hostresourcegroup.go +++ b/pkg/providers/hostresourcegroup/hostresourcegroup.go @@ -17,12 +17,14 @@ package hostresourcegroup import ( "context" "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" ) @@ -47,13 +49,28 @@ func NewProvider(rgapi resourcegroupsiface.ResourceGroupsAPI, cache *cache.Cache func (p *Provider) Get(ctx context.Context, nodeClass *v1beta1.EC2NodeClass) (*v1beta1.HostResourceGroup, error) { p.Lock() defer p.Unlock() + + 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}) + if err != nil { + return nil, err + } + if cached, ok := p.cache.Get(fmt.Sprint(hash)); ok { + return cached.(*v1beta1.HostResourceGroup), nil + } + var match *v1beta1.HostResourceGroup - err := p.resourcegroups.ListGroupsPagesWithContext(ctx, &resourcegroups.ListGroupsInput{}, func(page *resourcegroups.ListGroupsOutput, lastPage bool) bool { + err = p.resourcegroups.ListGroupsPagesWithContext(ctx, &resourcegroups.ListGroupsInput{}, func(page *resourcegroups.ListGroupsOutput, lastPage bool) bool { for i := range page.GroupIdentifiers { - for x := range nodeClass.Spec.HostResourceGroupSelectorTerms { - selector := nodeClass.Spec.HostResourceGroupSelectorTerms[x] - if *page.GroupIdentifiers[i].GroupName == selector.Name { + 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 } } @@ -65,6 +82,11 @@ func (p *Provider) Get(ctx context.Context, nodeClass *v1beta1.EC2NodeClass) (*v logging.FromContext(ctx).Errorf("discovery resource groups, %w", err) return nil, err } + if p.cm.HasChanged(fmt.Sprintf("hostresourcegroups/%t/%s", nodeClass.IsNodeTemplate, nodeClass.Name), match) { + logging.FromContext(ctx). + With("host resource group", match). + Debugf("discovered host resource group") + } return match, nil } diff --git a/pkg/providers/license/license.go b/pkg/providers/license/license.go index 50254b768905..c4bec0c6b966 100644 --- a/pkg/providers/license/license.go +++ b/pkg/providers/license/license.go @@ -67,7 +67,7 @@ func (p *Provider) Get(ctx context.Context, nodeClass *v1beta1.EC2NodeClass) ([] return cached.([]string), nil } - licenses := []string{} + var licenses []string // Look up all License Configurations output, err := p.licensemanager.ListLicenseConfigurationsWithContext(ctx, &licensemanager.ListLicenseConfigurationsInput{}) if err != nil { @@ -75,17 +75,18 @@ func (p *Provider) Get(ctx context.Context, nodeClass *v1beta1.EC2NodeClass) ([] return nil, err } for i := range output.LicenseConfigurations { - // filter results to only include those that match at least 1 selector + // filter results to only include those that match at least 1 selector for x := range selectors { if *output.LicenseConfigurations[i].Name == selectors[x].Name { licenses = append(licenses, *output.LicenseConfigurations[i].LicenseConfigurationArn) } } } + p.cache.SetDefault(fmt.Sprint(hash), licenses) if p.cm.HasChanged(fmt.Sprintf("license/%t/%s", nodeClass.IsNodeTemplate, nodeClass.Name), licenses) { logging.FromContext(ctx). - With("licenseProvider", lo.Map(licenses, func(s string, _ int) string { return s })). + With("licenseProvider", licenses). Debugf("discovered license configuration") } return licenses, nil diff --git a/pkg/providers/placementgroup/placementgroup.go b/pkg/providers/placementgroup/placementgroup.go index 5f2cbbebc868..f02652dc56d7 100644 --- a/pkg/providers/placementgroup/placementgroup.go +++ b/pkg/providers/placementgroup/placementgroup.go @@ -52,7 +52,7 @@ func (p *Provider) Get(ctx context.Context, nodeClass *v1beta1.EC2NodeClass) (*e defer p.Unlock() // Get selectors from the nodeClass, exit if no selectors defined - selectors := nodeClass.Spec.LicenseSelectorTerms + selectors := nodeClass.Spec.PlacementGroupSelectorTerms if selectors == nil { return nil, nil } @@ -66,30 +66,28 @@ func (p *Provider) Get(ctx context.Context, nodeClass *v1beta1.EC2NodeClass) (*e return cached.(*ec2.PlacementGroup), nil } - var group *ec2.PlacementGroup - // Look up all License Configurations + var match *ec2.PlacementGroup + // Look up all ec2 placement groups output, err := p.ec2api.DescribePlacementGroupsWithContext(ctx, &ec2.DescribePlacementGroupsInput{}) if err != nil { - logging.FromContext(ctx). - With("aws error", err). - Debugf("Error from ec2:describeplacementgroups") + logging.FromContext(ctx).Errorf("discovering placement groups, %w", err) return nil, err } for i := range output.PlacementGroups { // filter results to only include those that match at least 1 selector for x := range selectors { if *output.PlacementGroups[i].GroupName == selectors[x].Name { - group = output.PlacementGroups[i] + match = output.PlacementGroups[i] + p.cache.SetDefault(fmt.Sprint(hash), match) break } } } - - if p.cm.HasChanged(fmt.Sprintf("placementGroups/%t/%s", nodeClass.IsNodeTemplate, nodeClass.Name), group) { + if p.cm.HasChanged(fmt.Sprintf("placementgroup/%t/%s", nodeClass.IsNodeTemplate, nodeClass.Name), match) { logging.FromContext(ctx). - With("placementGroup", group). - Debugf("discovered placement groups") + With("placement group", match). + Debugf("discovered placement group") } - return group, nil + return match, nil } From 7246d1621a37abad883ee405a0bf7a41fd32554b Mon Sep 17 00:00:00 2001 From: Sebastian Cole Date: Tue, 19 Sep 2023 17:09:50 +1000 Subject: [PATCH 17/33] Use nodeClass.Status to populate launch template --- pkg/providers/amifamily/resolver.go | 29 ++++++++++------------------- 1 file changed, 10 insertions(+), 19 deletions(-) diff --git a/pkg/providers/amifamily/resolver.go b/pkg/providers/amifamily/resolver.go index f1d4712db5c0..3974665acf4d 100644 --- a/pkg/providers/amifamily/resolver.go +++ b/pkg/providers/amifamily/resolver.go @@ -145,10 +145,6 @@ func (r Resolver) Resolve(ctx context.Context, nodeClass *v1beta1.EC2NodeClass, if len(mappedAMIs) == 0 { return nil, fmt.Errorf("no instance types satisfy requirements of amis %v", amis) } - licenses, err := r.licenseProvider.Get(ctx, nodeClass) - if err != nil { - return nil, err - } placement, err := r.resolvePlacement(ctx, nodeClass) if err != nil { return nil, err @@ -186,7 +182,7 @@ func (r Resolver) Resolve(ctx context.Context, nodeClass *v1beta1.EC2NodeClass, DetailedMonitoring: aws.BoolValue(nodeClass.Spec.DetailedMonitoring), AMIID: amiID, InstanceTypes: instanceTypes, - Licenses: licenses, + Licenses: nodeClass.Status.Licenses, Placement: placement, } if len(resolved.BlockDeviceMappings) == 0 { @@ -202,22 +198,17 @@ func (r Resolver) Resolve(ctx context.Context, nodeClass *v1beta1.EC2NodeClass, } func (r Resolver) resolvePlacement(ctx context.Context, nodeClass *v1beta1.EC2NodeClass) (*Placement, error) { - placement := &Placement{} - hrg, err := r.hostResourceGroupProvider.Get(ctx, nodeClass) - if err != nil { - return nil, err - } - pg, err := r.placementGroupProvider.Get(ctx, nodeClass) - if err != nil { - return nil, err - } - if pg == nil && hrg == nil { - return nil, nil - } else { - placement.HostResourceGroup = hrg.Name - placement.PlacementGroup = *pg.GroupName + var placement *Placement + hrg := nodeClass.Status.HostResourceGroup + pg := nodeClass.Status.PlacementGroups + if pg != nil { + placement.PlacementGroup = pg.GroupName + } + if hrg != nil { + placement.HostResourceGroup = hrg.Name } + return placement, nil } From d514af5c64fe829ece5d6221abc7b6d0025338db Mon Sep 17 00:00:00 2001 From: Sebastian Cole Date: Tue, 19 Sep 2023 17:18:02 +1000 Subject: [PATCH 18/33] fixes --- pkg/providers/amifamily/resolver.go | 22 ++++++++++++---------- pkg/providers/license/license.go | 1 - 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/pkg/providers/amifamily/resolver.go b/pkg/providers/amifamily/resolver.go index 3974665acf4d..e120e8bfad88 100644 --- a/pkg/providers/amifamily/resolver.go +++ b/pkg/providers/amifamily/resolver.go @@ -145,10 +145,10 @@ func (r Resolver) Resolve(ctx context.Context, nodeClass *v1beta1.EC2NodeClass, if len(mappedAMIs) == 0 { return nil, fmt.Errorf("no instance types satisfy requirements of amis %v", amis) } - placement, err := r.resolvePlacement(ctx, nodeClass) - if err != nil { - return nil, err - } + placement, err := r.resolvePlacement(ctx, nodeClass) + if err != nil { + return nil, err + } var resolvedTemplates []*LaunchTemplate for amiID, instanceTypes := range mappedAMIs { maxPodsToInstanceTypes := lo.GroupBy(instanceTypes, func(instanceType *cloudprovider.InstanceType) int { @@ -182,7 +182,7 @@ func (r Resolver) Resolve(ctx context.Context, nodeClass *v1beta1.EC2NodeClass, DetailedMonitoring: aws.BoolValue(nodeClass.Spec.DetailedMonitoring), AMIID: amiID, InstanceTypes: instanceTypes, - Licenses: nodeClass.Status.Licenses, + Licenses: nodeClass.Status.Licenses, Placement: placement, } if len(resolved.BlockDeviceMappings) == 0 { @@ -199,13 +199,15 @@ func (r Resolver) Resolve(ctx context.Context, nodeClass *v1beta1.EC2NodeClass, func (r Resolver) resolvePlacement(ctx context.Context, nodeClass *v1beta1.EC2NodeClass) (*Placement, error) { var placement *Placement - hrg := nodeClass.Status.HostResourceGroup - pg := nodeClass.Status.PlacementGroups + hrg := nodeClass.Status.HostResourceGroup + pg := nodeClass.Status.PlacementGroups if pg != nil { - placement.PlacementGroup = pg.GroupName - } - if hrg != nil { + if len(pg) > 0 { + placement.PlacementGroup = pg[0] + } + } + if hrg != nil { placement.HostResourceGroup = hrg.Name } diff --git a/pkg/providers/license/license.go b/pkg/providers/license/license.go index c4bec0c6b966..c9343c7a1e0b 100644 --- a/pkg/providers/license/license.go +++ b/pkg/providers/license/license.go @@ -27,7 +27,6 @@ import ( "github.com/aws/karpenter/pkg/apis/v1beta1" "github.com/aws/karpenter-core/pkg/utils/pretty" - "github.com/samber/lo" "knative.dev/pkg/logging" ) From 08f41a1e2d5f5918a5bdd1517002785d577ebf79 Mon Sep 17 00:00:00 2001 From: Sebastian Cole Date: Wed, 20 Sep 2023 11:20:18 +1000 Subject: [PATCH 19/33] Fix fake/rgapi methods --- pkg/controllers/nodeclass/suite_test.go | 4 +-- pkg/fake/rgapi.go | 34 +++++++++++++++---------- pkg/operator/operator.go | 1 - 3 files changed, 23 insertions(+), 16 deletions(-) diff --git a/pkg/controllers/nodeclass/suite_test.go b/pkg/controllers/nodeclass/suite_test.go index f4aaeb6b6d79..db1ec00c6c01 100644 --- a/pkg/controllers/nodeclass/suite_test.go +++ b/pkg/controllers/nodeclass/suite_test.go @@ -55,8 +55,8 @@ var _ = BeforeSuite(func() { ctx = settings.ToContext(ctx, test.Settings()) awsEnv = test.NewEnvironment(ctx, env) - nodeTemplateController = nodeclass.NewNodeTemplateController(env.Client, awsEnv.SubnetProvider, awsEnv.SecurityGroupProvider, awsEnv.AMIProvider) - nodeClassController = nodeclass.NewNodeClassController(env.Client, awsEnv.SubnetProvider, awsEnv.SecurityGroupProvider, awsEnv.AMIProvider) + nodeTemplateController = nodeclass.NewNodeTemplateController(env.Client, awsEnv.SubnetProvider, awsEnv.SecurityGroupProvider, awsEnv.AMIProvider, awsEnv.LicenseProvider, awsEnv.HostResourceGroupProvider, awsEnv.PlacementGroupProvider) + nodeClassController = nodeclass.NewNodeClassController(env.Client, awsEnv.SubnetProvider, awsEnv.SecurityGroupProvider, awsEnv.AMIProvider, awsEnv.LicenseProvider, awsEnv.HostResourceGroupProvider, awsEnv.PlacementGroupProvider) }) var _ = AfterSuite(func() { diff --git a/pkg/fake/rgapi.go b/pkg/fake/rgapi.go index 97bd811c2d62..5b31d5f9504c 100644 --- a/pkg/fake/rgapi.go +++ b/pkg/fake/rgapi.go @@ -15,7 +15,7 @@ limitations under the License. package fake import ( - "errors" + "context" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/aws/request" @@ -28,23 +28,31 @@ type ResourceGroupsAPI struct { ResourceGroupsBehaviour } type ResourceGroupsBehaviour struct { - NextError AtomicError - ListGroupsOutput AtomicPtr[resourcegroups.ListGroupsOutput] + NextError AtomicError + ListGroupsOutput AtomicPtr[resourcegroups.ListGroupsOutput] + ListGroupsBehavior MockedFunction[resourcegroups.ListGroupsInput, resourcegroups.ListGroupsOutput] } func (r *ResourceGroupsAPI) Reset() { r.NextError.Reset() - r.ListGroupsOutput.Reset() + r.ListGroupsOutput.Reset() } -func (r *ResourceGroupsAPI) ListGroups(_ aws.Context, _ *resourcegroups.ListGroupsInput, fn func(*resourcegroups.ListGroupsOutput, bool) bool, _ ...request.Option) error { - if !r.NextError.IsNil() { - return r.NextError.Get() - } - if !r.ListGroupsOutput.IsNil() { - fn(r.ListGroupsOutput.Clone(), false) - return nil +func (r *ResourceGroupsAPI) ListGroupsWithContext(_ aws.Context, input *resourcegroups.ListGroupsInput, opts ...request.Option) (*resourcegroups.ListGroupsOutput, error) { + 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")}, + }, + }, nil + }) +} + +func (r *ResourceGroupsAPI) ListGroupsPagesWithContext(ctx context.Context, input *resourcegroups.ListGroupsInput, fn func(*resourcegroups.ListGroupsOutput, bool) bool, opts ...request.Option) error { + output, err := r.ListGroupsWithContext(ctx, input, opts...) + if err != nil { + return err } - // fail if the test doesn't provide specific data which causes our pricing provider to use its static price list - return errors.New("no resource groups provided") + fn(output, false) + return nil } diff --git a/pkg/operator/operator.go b/pkg/operator/operator.go index 856aee1bb659..f61a95f32744 100644 --- a/pkg/operator/operator.go +++ b/pkg/operator/operator.go @@ -150,7 +150,6 @@ func NewOperator(ctx context.Context, operator *operator.Operator) (context.Cont amiResolver, securityGroupProvider, subnetProvider, - licenseProvider, lo.Must(getCABundle(ctx, operator.GetConfig())), operator.Elected(), kubeDNSIP, From cbf61a70c15384a88e37ba8cbf12b55956c741ee Mon Sep 17 00:00:00 2001 From: Sebastian Cole Date: Wed, 20 Sep 2023 11:43:32 +1000 Subject: [PATCH 20/33] Refactor utils conversions for Selector terms --- pkg/utils/nodeclass/nodeclass.go | 36 ++++++++++++++++---------------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/pkg/utils/nodeclass/nodeclass.go b/pkg/utils/nodeclass/nodeclass.go index 30113ed6ce29..7afac2df0bc9 100644 --- a/pkg/utils/nodeclass/nodeclass.go +++ b/pkg/utils/nodeclass/nodeclass.go @@ -71,28 +71,28 @@ func New(nodeTemplate *v1alpha1.AWSNodeTemplate) *v1beta1.EC2NodeClass { } func NewPlacementGroupSelectorTerms(placementGroupSelector map[string]string) (terms []v1beta1.PlacementGroupSelectorTerm) { - if len(placementGroupSelector) == 0 { - return nil - } - return []v1beta1.PlacementGroupSelectorTerm{ - {Name: placementGroupSelector["name"]}, + if name, ok := placementGroupSelector["name"]; ok { + return []v1beta1.PlacementGroupSelectorTerm{ + {Name: name}, + } } + return nil } func NewHostResourceSelectorTerms(hrgSelector map[string]string) (terms []v1beta1.HostResourceGroupSelectorTerm) { - if len(hrgSelector) == 0 { - return nil - } - return []v1beta1.HostResourceGroupSelectorTerm{ - {Name: hrgSelector["name"]}, + if name, ok := hrgSelector["name"]; ok { + return []v1beta1.HostResourceGroupSelectorTerm{ + {Name: name}, + } } + return nil } func NewHostResourceGroup(hrgs []string) *v1beta1.HostResourceGroup { - if len(hrgs) == 0 { - return nil + for i := range hrgs { + return &v1beta1.HostResourceGroup{ARN: hrgs[i]} } - return &v1beta1.HostResourceGroup{ARN: hrgs[0]} + return nil } func NewSubnetSelectorTerms(subnetSelector map[string]string) (terms []v1beta1.SubnetSelectorTerm) { @@ -182,13 +182,13 @@ func NewAMISelectorTerms(amiSelector map[string]string) (terms []v1beta1.AMISele return terms } func NewLicenseSelectorTerms(selectors map[string]string) (terms []v1beta1.LicenseSelectorTerm) { - if len(selectors) == 0 { - return nil + if name, ok := selectors["name"]; ok { + return []v1beta1.LicenseSelectorTerm{ + {Name: name}, + } } - return []v1beta1.LicenseSelectorTerm{ - {Name: selectors["name"]}, - } + return nil } func NewBlockDeviceMappings(bdms []*v1alpha1.BlockDeviceMapping) []*v1beta1.BlockDeviceMapping { From c5bc5ee8dc918e1e7c535b1b9ec6ca3422f61242 Mon Sep 17 00:00:00 2001 From: Sebastian Cole Date: Wed, 20 Sep 2023 11:47:01 +1000 Subject: [PATCH 21/33] cleanup utils conversion functions --- pkg/utils/nodeclass/nodeclass.go | 1 + pkg/utils/nodetemplate/nodetemplate.go | 13 +++++-------- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/pkg/utils/nodeclass/nodeclass.go b/pkg/utils/nodeclass/nodeclass.go index 7afac2df0bc9..18b2df808216 100644 --- a/pkg/utils/nodeclass/nodeclass.go +++ b/pkg/utils/nodeclass/nodeclass.go @@ -65,6 +65,7 @@ func New(nodeTemplate *v1alpha1.AWSNodeTemplate) *v1beta1.EC2NodeClass { AMIs: NewAMIs(nodeTemplate.Status.AMIs), Licenses: nodeTemplate.Status.Licenses, HostResourceGroup: NewHostResourceGroup(nodeTemplate.Status.HostResourceGroups), + PlacementGroups: nodeTemplate.Status.PlacementGroups, }, IsNodeTemplate: true, } diff --git a/pkg/utils/nodetemplate/nodetemplate.go b/pkg/utils/nodetemplate/nodetemplate.go index 7fc38850e530..00ce02c8b963 100644 --- a/pkg/utils/nodetemplate/nodetemplate.go +++ b/pkg/utils/nodetemplate/nodetemplate.go @@ -49,16 +49,17 @@ func New(nodeClass *v1beta1.EC2NodeClass) *v1alpha1.AWSNodeTemplate { Subnets: NewSubnets(nodeClass.Status.Subnets), SecurityGroups: NewSecurityGroups(nodeClass.Status.SecurityGroups), AMIs: NewAMIs(nodeClass.Status.AMIs), - Licenses: NewLicenses(nodeClass.Status.Licenses), + Licenses: nodeClass.Status.Licenses, HostResourceGroups: NewHostResourceGroups(nodeClass.Status.HostResourceGroup), + PlacementGroups: nodeClass.Status.PlacementGroups, }, } } func NewHostResourceGroups(hrg *v1beta1.HostResourceGroup) []string { - if hrg == nil { - return nil - } + if hrg == nil { + return nil + } return []string{hrg.ARN} } @@ -145,7 +146,3 @@ func NewAMIs(amis []v1beta1.AMI) []v1alpha1.AMI { } }) } - -func NewLicenses(licenses []string) []string { - return licenses -} From b2e8fa8d5443382c247460e07172ef629343cbbc Mon Sep 17 00:00:00 2001 From: Sebastian Cole Date: Wed, 20 Sep 2023 16:20:08 +1000 Subject: [PATCH 22/33] 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 799446b8966c..44cfc7f8918b 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) From a3a7f5d8dd44e7a13b7adf3be95aaea995905e31 Mon Sep 17 00:00:00 2001 From: Sebastian Cole Date: Thu, 21 Sep 2023 11:13:30 +1000 Subject: [PATCH 23/33] Address linting errors --- pkg/apis/settings/zz_generated.deepcopy.go | 1 - pkg/apis/v1alpha1/zz_generated.deepcopy.go | 1 - pkg/apis/v1alpha5/zz_generated.deepcopy.go | 1 - pkg/apis/v1beta1/zz_generated.deepcopy.go | 1 - pkg/controllers/nodeclass/controller.go | 9 +++------ pkg/fake/lmapi.go | 3 +-- pkg/fake/rgapi.go | 8 ++++---- pkg/providers/amifamily/resolver.go | 20 +++++++++----------- pkg/test/environment.go | 6 +++--- pkg/utils/nodeclass/suite_test.go | 12 ++++++------ 10 files changed, 26 insertions(+), 36 deletions(-) diff --git a/pkg/apis/settings/zz_generated.deepcopy.go b/pkg/apis/settings/zz_generated.deepcopy.go index 43eaa173bfe6..51a3bd930dd9 100644 --- a/pkg/apis/settings/zz_generated.deepcopy.go +++ b/pkg/apis/settings/zz_generated.deepcopy.go @@ -1,5 +1,4 @@ //go:build !ignore_autogenerated -// +build !ignore_autogenerated /* Licensed under the Apache License, Version 2.0 (the "License"); diff --git a/pkg/apis/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/v1alpha1/zz_generated.deepcopy.go index 15b0b9d21ac8..55f62e53a512 100644 --- a/pkg/apis/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/v1alpha1/zz_generated.deepcopy.go @@ -1,5 +1,4 @@ //go:build !ignore_autogenerated -// +build !ignore_autogenerated /* Licensed under the Apache License, Version 2.0 (the "License"); diff --git a/pkg/apis/v1alpha5/zz_generated.deepcopy.go b/pkg/apis/v1alpha5/zz_generated.deepcopy.go index e55c4cb4b353..722b9872175f 100644 --- a/pkg/apis/v1alpha5/zz_generated.deepcopy.go +++ b/pkg/apis/v1alpha5/zz_generated.deepcopy.go @@ -1,5 +1,4 @@ //go:build !ignore_autogenerated -// +build !ignore_autogenerated /* Licensed under the Apache License, Version 2.0 (the "License"); diff --git a/pkg/apis/v1beta1/zz_generated.deepcopy.go b/pkg/apis/v1beta1/zz_generated.deepcopy.go index 34ebcc10daf0..66b99865f9a0 100644 --- a/pkg/apis/v1beta1/zz_generated.deepcopy.go +++ b/pkg/apis/v1beta1/zz_generated.deepcopy.go @@ -1,5 +1,4 @@ //go:build !ignore_autogenerated -// +build !ignore_autogenerated /* Licensed under the Apache License, Version 2.0 (the "License"); diff --git a/pkg/controllers/nodeclass/controller.go b/pkg/controllers/nodeclass/controller.go index c5be9a030912..61dcb5909378 100644 --- a/pkg/controllers/nodeclass/controller.go +++ b/pkg/controllers/nodeclass/controller.go @@ -169,8 +169,7 @@ func (c *Controller) resolveAMIs(ctx context.Context, nodeClass *v1beta1.EC2Node func (c *Controller) resolveLicenses(ctx context.Context, nodeClass *v1beta1.EC2NodeClass) error { licenses, err := c.licenseProvider.Get(ctx, nodeClass) if err != nil { - // Errors from license provider should not interrupt the process - return nil + return err } nodeClass.Status.Licenses = licenses @@ -181,8 +180,7 @@ func (c *Controller) resolveLicenses(ctx context.Context, nodeClass *v1beta1.EC2 func (c *Controller) resolveHostResourceGroups(ctx context.Context, nodeClass *v1beta1.EC2NodeClass) error { result, err := c.hostResourceGroupProvider.Get(ctx, nodeClass) if err != nil { - // Errors from host resource group provider should not interrupt the process - return nil + return err } nodeClass.Status.HostResourceGroup = result @@ -193,8 +191,7 @@ func (c *Controller) resolveHostResourceGroups(ctx context.Context, nodeClass *v func (c *Controller) resolvePlacementGroups(ctx context.Context, nodeClass *v1beta1.EC2NodeClass) error { result, err := c.placementGroupProvider.Get(ctx, nodeClass) if err != nil { - // Errors from placement group provider should not interrupt the process - return nil + return err } if result != nil { nodeClass.Status.PlacementGroups = append(nodeClass.Status.PlacementGroups, *result.GroupArn) diff --git a/pkg/fake/lmapi.go b/pkg/fake/lmapi.go index 96277bc1662b..88f0695859fd 100644 --- a/pkg/fake/lmapi.go +++ b/pkg/fake/lmapi.go @@ -28,7 +28,7 @@ type LicenseManagerAPI struct { LicenseManagerBehaviour } type LicenseManagerBehaviour struct { - NextError AtomicError + NextError AtomicError ListLicenseConfigurationsOutput AtomicPtr[licensemanager.ListLicenseConfigurationsOutput] } @@ -48,4 +48,3 @@ func (l *LicenseManagerAPI) ListLicenseConfigurations(_ aws.Context, _ *licensem // fail if the test doesn't provide specific data which causes our pricing provider to use its static price list return errors.New("no license data provided") } - diff --git a/pkg/fake/rgapi.go b/pkg/fake/rgapi.go index f0b4141352f9..082d2df8f06e 100644 --- a/pkg/fake/rgapi.go +++ b/pkg/fake/rgapi.go @@ -24,8 +24,8 @@ import ( ) var ( - TEST_LICENSE_ARN = "arn:aws:license-manager:us-west-2:11111111111:license-configuration:lic-94ba36399bd98eaad808b0ffb1d1604b" - TEST_LICENSE_NAME = "test-license" + TestLicenseArn = "arn:aws:license-manager:us-west-2:11111111111:license-configuration:lic-94ba36399bd98eaad808b0ffb1d1604b" + TestLicenseName = "test-license" ) type ResourceGroupsAPI struct { @@ -43,11 +43,11 @@ func (r *ResourceGroupsAPI) Reset() { r.ListGroupsOutput.Reset() } -func (r *ResourceGroupsAPI) ListGroupsWithContext(_ aws.Context, input *resourcegroups.ListGroupsInput, opts ...request.Option) (*resourcegroups.ListGroupsOutput, error) { +func (r *ResourceGroupsAPI) ListGroupsWithContext(_ aws.Context, input *resourcegroups.ListGroupsInput, _ ...request.Option) (*resourcegroups.ListGroupsOutput, error) { return r.ListGroupsBehavior.Invoke(input, func(input *resourcegroups.ListGroupsInput) (*resourcegroups.ListGroupsOutput, error) { return &resourcegroups.ListGroupsOutput{ GroupIdentifiers: []*resourcegroups.GroupIdentifier{ - {GroupArn: aws.String(TEST_LICENSE_ARN), GroupName: aws.String(TEST_LICENSE_NAME)}, + {GroupArn: aws.String(TestLicenseArn), GroupName: aws.String(TestLicenseName)}, }, }, nil }) diff --git a/pkg/providers/amifamily/resolver.go b/pkg/providers/amifamily/resolver.go index e120e8bfad88..8d004efa78bd 100644 --- a/pkg/providers/amifamily/resolver.go +++ b/pkg/providers/amifamily/resolver.go @@ -145,10 +145,6 @@ func (r Resolver) Resolve(ctx context.Context, nodeClass *v1beta1.EC2NodeClass, if len(mappedAMIs) == 0 { return nil, fmt.Errorf("no instance types satisfy requirements of amis %v", amis) } - placement, err := r.resolvePlacement(ctx, nodeClass) - if err != nil { - return nil, err - } var resolvedTemplates []*LaunchTemplate for amiID, instanceTypes := range mappedAMIs { maxPodsToInstanceTypes := lo.GroupBy(instanceTypes, func(instanceType *cloudprovider.InstanceType) int { @@ -183,7 +179,7 @@ func (r Resolver) Resolve(ctx context.Context, nodeClass *v1beta1.EC2NodeClass, AMIID: amiID, InstanceTypes: instanceTypes, Licenses: nodeClass.Status.Licenses, - Placement: placement, + Placement: r.resolvePlacement(nodeClass), } if len(resolved.BlockDeviceMappings) == 0 { resolved.BlockDeviceMappings = amiFamily.DefaultBlockDeviceMappings() @@ -197,21 +193,23 @@ func (r Resolver) Resolve(ctx context.Context, nodeClass *v1beta1.EC2NodeClass, return resolvedTemplates, nil } -func (r Resolver) resolvePlacement(ctx context.Context, nodeClass *v1beta1.EC2NodeClass) (*Placement, error) { +func (r Resolver) resolvePlacement(nodeClass *v1beta1.EC2NodeClass) *Placement { var placement *Placement hrg := nodeClass.Status.HostResourceGroup pg := nodeClass.Status.PlacementGroups - if pg != nil { + if pg != nil || hrg != nil { + placement = &Placement{} if len(pg) > 0 { placement.PlacementGroup = pg[0] } - } - if hrg != nil { - placement.HostResourceGroup = hrg.Name + + if hrg != nil { + placement.HostResourceGroup = hrg.Name + } } - return placement, nil + return placement } func GetAMIFamily(amiFamily *string, options *Options) AMIFamily { diff --git a/pkg/test/environment.go b/pkg/test/environment.go index 64d43d0ed32a..d14bacde7467 100644 --- a/pkg/test/environment.go +++ b/pkg/test/environment.go @@ -81,7 +81,7 @@ func NewEnvironment(ctx context.Context, env *coretest.Environment) *Environment ec2api := &fake.EC2API{} ssmapi := &fake.SSMAPI{} lmapi := &fake.LicenseManagerAPI{} - rgapi := &fake.ResourceGroupsAPI{} + rgapi := &fake.ResourceGroupsAPI{} // cache ec2Cache := cache.New(awscache.DefaultTTL, awscache.DefaultCleanupInterval) @@ -103,8 +103,8 @@ func NewEnvironment(ctx context.Context, env *coretest.Environment) *Environment versionProvider := version.NewProvider(env.KubernetesInterface, kubernetesVersionCache) amiProvider := amifamily.NewProvider(versionProvider, ssmapi, ec2api, ec2Cache) licenseProvider := license.NewProvider(lmapi.LicenseManagerAPI, licenseCache) - hostResourceGroupProvider := hostresourcegroup.NewProvider(rgapi, hostResourceGroupsCache) - placementGroupProvider := placementgroup.NewProvider(ec2api, placementGroupCache) + hostResourceGroupProvider := hostresourcegroup.NewProvider(rgapi, hostResourceGroupsCache) + placementGroupProvider := placementgroup.NewProvider(ec2api, placementGroupCache) amiResolver := amifamily.New(amiProvider, licenseProvider, hostResourceGroupProvider, placementGroupProvider) instanceTypesProvider := instancetype.NewProvider("", instanceTypeCache, ec2api, subnetProvider, unavailableOfferingsCache, pricingProvider) diff --git a/pkg/utils/nodeclass/suite_test.go b/pkg/utils/nodeclass/suite_test.go index 77be7a488cf6..8277c31769d7 100644 --- a/pkg/utils/nodeclass/suite_test.go +++ b/pkg/utils/nodeclass/suite_test.go @@ -434,13 +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"} + 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)) - }) + 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) From 5b31e3877ef00ecd9b1a73bd8a17d9eceb0287e7 Mon Sep 17 00:00:00 2001 From: Sebastian Cole Date: Thu, 21 Sep 2023 11:49:34 +1000 Subject: [PATCH 24/33] Fix nodeclass suite_test --- pkg/controllers/nodeclass/suite_test.go | 699 ------------------------ pkg/providers/license/suite_test.go | 116 ++++ 2 files changed, 116 insertions(+), 699 deletions(-) create mode 100644 pkg/providers/license/suite_test.go diff --git a/pkg/controllers/nodeclass/suite_test.go b/pkg/controllers/nodeclass/suite_test.go index db1ec00c6c01..18a8247095d5 100644 --- a/pkg/controllers/nodeclass/suite_test.go +++ b/pkg/controllers/nodeclass/suite_test.go @@ -71,702 +71,3 @@ var _ = BeforeEach(func() { var _ = AfterEach(func() { ExpectCleanedUp(ctx, env.Client) }) - -var _ = Describe("AWSNodeTemplateController", func() { - Context("Subnet Status", func() { - It("Should update AWSNodeTemplate status for Subnets", func() { - ExpectApplied(ctx, env.Client, nodeTemplate) - ExpectReconcileSucceeded(ctx, controller, client.ObjectKeyFromObject(nodeTemplate)) - nodeTemplate = ExpectExists(ctx, env.Client, nodeTemplate) - Expect(nodeTemplate.Status.Subnets).To(ConsistOf( - v1alpha1.Subnet{ - ID: "subnet-test1", - Zone: "test-zone-1a", - }, - v1alpha1.Subnet{ - ID: "subnet-test2", - Zone: "test-zone-1b", - }, - v1alpha1.Subnet{ - ID: "subnet-test3", - Zone: "test-zone-1c", - }, - )) - }) - It("Should have the correct ordering for the Subnets", func() { - awsEnv.EC2API.DescribeSubnetsOutput.Set(&ec2.DescribeSubnetsOutput{Subnets: []*ec2.Subnet{ - {SubnetId: aws.String("subnet-test1"), AvailabilityZone: aws.String("test-zone-1a"), AvailableIpAddressCount: aws.Int64(20)}, - {SubnetId: aws.String("subnet-test2"), AvailabilityZone: aws.String("test-zone-1b"), AvailableIpAddressCount: aws.Int64(100)}, - {SubnetId: aws.String("subnet-test3"), AvailabilityZone: aws.String("test-zone-1c"), AvailableIpAddressCount: aws.Int64(50)}, - }}) - ExpectApplied(ctx, env.Client, nodeTemplate) - ExpectReconcileSucceeded(ctx, controller, client.ObjectKeyFromObject(nodeTemplate)) - nodeTemplate = ExpectExists(ctx, env.Client, nodeTemplate) - Expect(nodeTemplate.Status.Subnets).To(ConsistOf( - v1alpha1.Subnet{ - ID: "subnet-test2", - Zone: "test-zone-1b", - }, - v1alpha1.Subnet{ - ID: "subnet-test3", - Zone: "test-zone-1c", - }, - v1alpha1.Subnet{ - ID: "subnet-test1", - Zone: "test-zone-1a", - }, - )) - }) - It("Should resolve a valid selectors for Subnet by tags", func() { - nodeTemplate.Spec.SubnetSelector = map[string]string{`Name`: `test-subnet-1,test-subnet-2`} - ExpectApplied(ctx, env.Client, nodeTemplate) - ExpectReconcileSucceeded(ctx, controller, client.ObjectKeyFromObject(nodeTemplate)) - nodeTemplate = ExpectExists(ctx, env.Client, nodeTemplate) - Expect(nodeTemplate.Status.Subnets).To(ConsistOf( - v1alpha1.Subnet{ - ID: "subnet-test1", - Zone: "test-zone-1a", - }, - v1alpha1.Subnet{ - ID: "subnet-test2", - Zone: "test-zone-1b", - }, - )) - }) - It("Should resolve a valid selectors for Subnet by ids", func() { - nodeTemplate.Spec.SubnetSelector = map[string]string{`aws-ids`: `subnet-test1`} - ExpectApplied(ctx, env.Client, nodeTemplate) - ExpectReconcileSucceeded(ctx, controller, client.ObjectKeyFromObject(nodeTemplate)) - nodeTemplate = ExpectExists(ctx, env.Client, nodeTemplate) - Expect(nodeTemplate.Status.Subnets).To(Equal([]v1alpha1.Subnet{ - { - ID: "subnet-test1", - Zone: "test-zone-1a", - }, - })) - }) - It("Should update Subnet status when the Subnet selector gets updated by tags", func() { - ExpectApplied(ctx, env.Client, nodeTemplate) - ExpectReconcileSucceeded(ctx, controller, client.ObjectKeyFromObject(nodeTemplate)) - nodeTemplate = ExpectExists(ctx, env.Client, nodeTemplate) - Expect(nodeTemplate.Status.Subnets).To(ConsistOf( - v1alpha1.Subnet{ - ID: "subnet-test1", - Zone: "test-zone-1a", - }, - v1alpha1.Subnet{ - ID: "subnet-test2", - Zone: "test-zone-1b", - }, - v1alpha1.Subnet{ - ID: "subnet-test3", - Zone: "test-zone-1c", - }, - )) - - nodeTemplate.Spec.SubnetSelector = map[string]string{`Name`: `test-subnet-1,test-subnet-2`} - ExpectApplied(ctx, env.Client, nodeTemplate) - ExpectReconcileSucceeded(ctx, controller, client.ObjectKeyFromObject(nodeTemplate)) - nodeTemplate = ExpectExists(ctx, env.Client, nodeTemplate) - Expect(nodeTemplate.Status.Subnets).To(ConsistOf( - v1alpha1.Subnet{ - ID: "subnet-test1", - Zone: "test-zone-1a", - }, - v1alpha1.Subnet{ - ID: "subnet-test2", - Zone: "test-zone-1b", - }, - )) - }) - It("Should update Subnet status when the Subnet selector gets updated by ids", func() { - ExpectApplied(ctx, env.Client, nodeTemplate) - ExpectReconcileSucceeded(ctx, controller, client.ObjectKeyFromObject(nodeTemplate)) - nodeTemplate = ExpectExists(ctx, env.Client, nodeTemplate) - Expect(nodeTemplate.Status.Subnets).To(ConsistOf( - v1alpha1.Subnet{ - ID: "subnet-test1", - Zone: "test-zone-1a", - }, - v1alpha1.Subnet{ - ID: "subnet-test2", - Zone: "test-zone-1b", - }, - v1alpha1.Subnet{ - ID: "subnet-test3", - Zone: "test-zone-1c", - }, - )) - - nodeTemplate.Spec.SubnetSelector = map[string]string{`aws-ids`: `subnet-test1`} - ExpectApplied(ctx, env.Client, nodeTemplate) - ExpectReconcileSucceeded(ctx, controller, client.ObjectKeyFromObject(nodeTemplate)) - nodeTemplate = ExpectExists(ctx, env.Client, nodeTemplate) - Expect(nodeTemplate.Status.Subnets).To(ConsistOf( - v1alpha1.Subnet{ - ID: "subnet-test1", - Zone: "test-zone-1a", - }, - )) - }) - It("Should not resolve a invalid selectors for Subnet", func() { - nodeTemplate.Spec.SubnetSelector = map[string]string{`foo`: `invalid`} - ExpectApplied(ctx, env.Client, nodeTemplate) - ExpectReconcileFailed(ctx, controller, client.ObjectKeyFromObject(nodeTemplate)) - nodeTemplate = ExpectExists(ctx, env.Client, nodeTemplate) - Expect(nodeTemplate.Status.Subnets).To(BeNil()) - }) - It("Should not resolve a invalid selectors for an updated Subnet selectors", func() { - ExpectApplied(ctx, env.Client, nodeTemplate) - ExpectReconcileSucceeded(ctx, controller, client.ObjectKeyFromObject(nodeTemplate)) - nodeTemplate = ExpectExists(ctx, env.Client, nodeTemplate) - Expect(nodeTemplate.Status.Subnets).To(ConsistOf( - v1alpha1.Subnet{ - ID: "subnet-test1", - Zone: "test-zone-1a", - }, - v1alpha1.Subnet{ - ID: "subnet-test2", - Zone: "test-zone-1b", - }, - v1alpha1.Subnet{ - ID: "subnet-test3", - Zone: "test-zone-1c", - }, - )) - - nodeTemplate.Spec.SubnetSelector = map[string]string{`foo`: `invalid`} - ExpectApplied(ctx, env.Client, nodeTemplate) - ExpectReconcileFailed(ctx, controller, client.ObjectKeyFromObject(nodeTemplate)) - nodeTemplate = ExpectExists(ctx, env.Client, nodeTemplate) - Expect(nodeTemplate.Status.Subnets).To(BeNil()) - }) - }) - Context("Security Groups Status", func() { - It("Should expect no errors when security groups are not in the AWSNodeTemplate", func() { - // TODO: Remove test for v1beta1, as security groups will be required - nodeTemplate.Spec.SecurityGroupSelector = nil - ExpectApplied(ctx, env.Client, nodeTemplate) - ExpectReconcileSucceeded(ctx, controller, client.ObjectKeyFromObject(nodeTemplate)) - Expect(nodeTemplate.Status.SecurityGroups).To(BeNil()) - }) - It("Should update AWSNodeTemplate status for Security Groups", func() { - ExpectApplied(ctx, env.Client, nodeTemplate) - ExpectReconcileSucceeded(ctx, controller, client.ObjectKeyFromObject(nodeTemplate)) - nodeTemplate = ExpectExists(ctx, env.Client, nodeTemplate) - Expect(nodeTemplate.Status.SecurityGroups).To(ConsistOf( - v1alpha1.SecurityGroup{ - ID: "sg-test1", - Name: "securityGroup-test1", - }, - v1alpha1.SecurityGroup{ - ID: "sg-test2", - Name: "securityGroup-test2", - }, - v1alpha1.SecurityGroup{ - ID: "sg-test3", - Name: "securityGroup-test3", - }, - )) - }) - It("Should resolve a valid selectors for Security Groups by tags", func() { - nodeTemplate.Spec.SecurityGroupSelector = map[string]string{`Name`: `test-security-group-1,test-security-group-2`} - ExpectApplied(ctx, env.Client, nodeTemplate) - ExpectReconcileSucceeded(ctx, controller, client.ObjectKeyFromObject(nodeTemplate)) - nodeTemplate = ExpectExists(ctx, env.Client, nodeTemplate) - Expect(nodeTemplate.Status.SecurityGroups).To(ConsistOf( - v1alpha1.SecurityGroup{ - ID: "sg-test1", - Name: "securityGroup-test1", - }, - v1alpha1.SecurityGroup{ - ID: "sg-test2", - Name: "securityGroup-test2", - }, - )) - }) - It("Should resolve a valid selectors for Security Groups by ids", func() { - nodeTemplate.Spec.SecurityGroupSelector = map[string]string{`aws-ids`: `sg-test1`} - ExpectApplied(ctx, env.Client, nodeTemplate) - ExpectReconcileSucceeded(ctx, controller, client.ObjectKeyFromObject(nodeTemplate)) - nodeTemplate = ExpectExists(ctx, env.Client, nodeTemplate) - Expect(nodeTemplate.Status.SecurityGroups).To(ConsistOf( - v1alpha1.SecurityGroup{ - ID: "sg-test1", - Name: "securityGroup-test1", - }, - )) - }) - It("Should update Security Groups status when the Security Groups selector gets updated by tags", func() { - ExpectApplied(ctx, env.Client, nodeTemplate) - ExpectReconcileSucceeded(ctx, controller, client.ObjectKeyFromObject(nodeTemplate)) - nodeTemplate = ExpectExists(ctx, env.Client, nodeTemplate) - Expect(nodeTemplate.Status.SecurityGroups).To(ConsistOf( - v1alpha1.SecurityGroup{ - ID: "sg-test1", - Name: "securityGroup-test1", - }, - v1alpha1.SecurityGroup{ - ID: "sg-test2", - Name: "securityGroup-test2", - }, - v1alpha1.SecurityGroup{ - ID: "sg-test3", - Name: "securityGroup-test3", - }, - )) - - nodeTemplate.Spec.SecurityGroupSelector = map[string]string{`Name`: `test-security-group-1,test-security-group-2`} - ExpectApplied(ctx, env.Client, nodeTemplate) - ExpectReconcileSucceeded(ctx, controller, client.ObjectKeyFromObject(nodeTemplate)) - nodeTemplate = ExpectExists(ctx, env.Client, nodeTemplate) - Expect(nodeTemplate.Status.SecurityGroups).To(ConsistOf( - v1alpha1.SecurityGroup{ - ID: "sg-test1", - Name: "securityGroup-test1", - }, - v1alpha1.SecurityGroup{ - ID: "sg-test2", - Name: "securityGroup-test2", - }, - )) - }) - It("Should update Security Groups status when the Security Groups selector gets updated by ids", func() { - ExpectApplied(ctx, env.Client, nodeTemplate) - ExpectReconcileSucceeded(ctx, controller, client.ObjectKeyFromObject(nodeTemplate)) - nodeTemplate = ExpectExists(ctx, env.Client, nodeTemplate) - Expect(nodeTemplate.Status.SecurityGroups).To(ConsistOf( - v1alpha1.SecurityGroup{ - ID: "sg-test1", - Name: "securityGroup-test1", - }, - v1alpha1.SecurityGroup{ - ID: "sg-test2", - Name: "securityGroup-test2", - }, - v1alpha1.SecurityGroup{ - ID: "sg-test3", - Name: "securityGroup-test3", - }, - )) - - nodeTemplate.Spec.SecurityGroupSelector = map[string]string{`aws-ids`: `sg-test1`} - ExpectApplied(ctx, env.Client, nodeTemplate) - ExpectReconcileSucceeded(ctx, controller, client.ObjectKeyFromObject(nodeTemplate)) - nodeTemplate = ExpectExists(ctx, env.Client, nodeTemplate) - Expect(nodeTemplate.Status.SecurityGroups).To(ConsistOf( - v1alpha1.SecurityGroup{ - ID: "sg-test1", - Name: "securityGroup-test1", - }, - )) - }) - It("Should not resolve a invalid selectors for Security Groups", func() { - nodeTemplate.Spec.SecurityGroupSelector = map[string]string{`foo`: `invalid`} - ExpectApplied(ctx, env.Client, nodeTemplate) - ExpectReconcileFailed(ctx, controller, client.ObjectKeyFromObject(nodeTemplate)) - nodeTemplate = ExpectExists(ctx, env.Client, nodeTemplate) - Expect(nodeTemplate.Status.SecurityGroups).To(BeNil()) - }) - It("Should not resolve a invalid selectors for an updated Security Groups selector", func() { - ExpectApplied(ctx, env.Client, nodeTemplate) - ExpectReconcileSucceeded(ctx, controller, client.ObjectKeyFromObject(nodeTemplate)) - nodeTemplate = ExpectExists(ctx, env.Client, nodeTemplate) - Expect(nodeTemplate.Status.SecurityGroups).To(ConsistOf( - v1alpha1.SecurityGroup{ - ID: "sg-test1", - Name: "securityGroup-test1", - }, - v1alpha1.SecurityGroup{ - ID: "sg-test2", - Name: "securityGroup-test2", - }, - v1alpha1.SecurityGroup{ - ID: "sg-test3", - Name: "securityGroup-test3", - }, - )) - - nodeTemplate.Spec.SecurityGroupSelector = map[string]string{`foo`: `invalid`} - ExpectApplied(ctx, env.Client, nodeTemplate) - ExpectReconcileFailed(ctx, controller, client.ObjectKeyFromObject(nodeTemplate)) - nodeTemplate = ExpectExists(ctx, env.Client, nodeTemplate) - Expect(nodeTemplate.Status.SecurityGroups).To(BeNil()) - }) - }) - Context("AMI Status", func() { - BeforeEach(func() { - awsEnv.EC2API.DescribeImagesOutput.Set(&ec2.DescribeImagesOutput{ - Images: []*ec2.Image{ - { - Name: aws.String("test-ami-1"), - ImageId: aws.String("ami-test1"), - CreationDate: aws.String(time.Now().Format(time.RFC3339)), - Architecture: aws.String("x86_64"), - Tags: []*ec2.Tag{ - {Key: aws.String("Name"), Value: aws.String("test-ami-1")}, - {Key: aws.String("foo"), Value: aws.String("bar")}, - }, - }, - { - Name: aws.String("test-ami-2"), - ImageId: aws.String("ami-test2"), - CreationDate: aws.String(time.Now().Add(time.Minute).Format(time.RFC3339)), - Architecture: aws.String("x86_64"), - Tags: []*ec2.Tag{ - {Key: aws.String("Name"), Value: aws.String("test-ami-2")}, - {Key: aws.String("foo"), Value: aws.String("bar")}, - }, - }, - { - Name: aws.String("test-ami-3"), - ImageId: aws.String("ami-test3"), - CreationDate: aws.String(time.Now().Add(2 * time.Minute).Format(time.RFC3339)), - Architecture: aws.String("x86_64"), - Tags: []*ec2.Tag{ - {Key: aws.String("Name"), Value: aws.String("test-ami-3")}, - {Key: aws.String("foo"), Value: aws.String("bar")}, - }, - }, - }, - }) - }) - It("should resolve amiSelector AMIs and requirements into status", func() { - version := lo.Must(awsEnv.VersionProvider.Get(ctx)) - - awsEnv.SSMAPI.Parameters = map[string]string{ - fmt.Sprintf("/aws/service/eks/optimized-ami/%s/amazon-linux-2/recommended/image_id", version): "ami-id-123", - fmt.Sprintf("/aws/service/eks/optimized-ami/%s/amazon-linux-2-gpu/recommended/image_id", version): "ami-id-456", - fmt.Sprintf("/aws/service/eks/optimized-ami/%s/amazon-linux-2%s/recommended/image_id", version, fmt.Sprintf("-%s", v1alpha5.ArchitectureArm64)): "ami-id-789", - } - - awsEnv.EC2API.DescribeImagesOutput.Set(&ec2.DescribeImagesOutput{ - Images: []*ec2.Image{ - { - Name: aws.String("test-ami-1"), - ImageId: aws.String("ami-id-123"), - CreationDate: aws.String(time.Now().Format(time.RFC3339)), - Architecture: aws.String("x86_64"), - Tags: []*ec2.Tag{ - {Key: aws.String("Name"), Value: aws.String("test-ami-1")}, - {Key: aws.String("foo"), Value: aws.String("bar")}, - }, - }, - { - Name: aws.String("test-ami-2"), - ImageId: aws.String("ami-id-456"), - CreationDate: aws.String(time.Now().Add(time.Minute).Format(time.RFC3339)), - Architecture: aws.String("x86_64"), - Tags: []*ec2.Tag{ - {Key: aws.String("Name"), Value: aws.String("test-ami-2")}, - {Key: aws.String("foo"), Value: aws.String("bar")}, - }, - }, - { - Name: aws.String("test-ami-3"), - ImageId: aws.String("ami-id-789"), - CreationDate: aws.String(time.Now().Add(2 * time.Minute).Format(time.RFC3339)), - Architecture: aws.String("x86_64"), - Tags: []*ec2.Tag{ - {Key: aws.String("Name"), Value: aws.String("test-ami-3")}, - {Key: aws.String("foo"), Value: aws.String("bar")}, - }, - }, - }, - }) - nodeTemplate.Spec.AMISelector = nil - ExpectApplied(ctx, env.Client, nodeTemplate) - ExpectReconcileSucceeded(ctx, controller, client.ObjectKeyFromObject(nodeTemplate)) - nodeTemplate = ExpectExists(ctx, env.Client, nodeTemplate) - ExpectConsistOfAMIs([]v1alpha1.AMI{ - { - Name: "test-ami-1", - ID: "ami-id-123", - Requirements: []v1.NodeSelectorRequirement{ - { - Key: v1.LabelArchStable, - Operator: v1.NodeSelectorOpIn, - Values: []string{v1alpha5.ArchitectureAmd64}, - }, - { - Key: v1alpha1.LabelInstanceGPUCount, - Operator: v1.NodeSelectorOpDoesNotExist, - }, - { - Key: v1alpha1.LabelInstanceAcceleratorCount, - Operator: v1.NodeSelectorOpDoesNotExist, - }, - }, - }, - { - Name: "test-ami-3", - ID: "ami-id-789", - Requirements: []v1.NodeSelectorRequirement{ - { - Key: v1.LabelArchStable, - Operator: v1.NodeSelectorOpIn, - Values: []string{v1alpha5.ArchitectureArm64}, - }, - { - Key: v1alpha1.LabelInstanceGPUCount, - Operator: v1.NodeSelectorOpDoesNotExist, - }, - { - Key: v1alpha1.LabelInstanceAcceleratorCount, - Operator: v1.NodeSelectorOpDoesNotExist, - }, - }, - }, - { - Name: "test-ami-2", - ID: "ami-id-456", - Requirements: []v1.NodeSelectorRequirement{ - { - Key: v1.LabelArchStable, - Operator: v1.NodeSelectorOpIn, - Values: []string{v1alpha5.ArchitectureAmd64}, - }, - { - Key: v1alpha1.LabelInstanceGPUCount, - Operator: v1.NodeSelectorOpExists, - }, - }, - }, - { - Name: "test-ami-2", - ID: "ami-id-456", - Requirements: []v1.NodeSelectorRequirement{ - { - Key: v1.LabelArchStable, - Operator: v1.NodeSelectorOpIn, - Values: []string{v1alpha5.ArchitectureAmd64}, - }, - { - Key: v1alpha1.LabelInstanceAcceleratorCount, - Operator: v1.NodeSelectorOpExists, - }, - }, - }, - }, nodeTemplate.Status.AMIs) - }) - It("should resolve amiSelector AMis and requirements into status when all SSM aliases don't resolve", func() { - version := lo.Must(awsEnv.VersionProvider.Get(ctx)) - // This parameter set doesn't include any of the Nvidia AMIs - awsEnv.SSMAPI.Parameters = map[string]string{ - fmt.Sprintf("/aws/service/bottlerocket/aws-k8s-%s/x86_64/latest/image_id", version): "ami-id-123", - fmt.Sprintf("/aws/service/bottlerocket/aws-k8s-%s/arm64/latest/image_id", version): "ami-id-456", - } - nodeTemplate.Spec.AMIFamily = &v1alpha1.AMIFamilyBottlerocket - nodeTemplate.Spec.AMISelector = nil - awsEnv.EC2API.DescribeImagesOutput.Set(&ec2.DescribeImagesOutput{ - Images: []*ec2.Image{ - { - Name: aws.String("test-ami-1"), - ImageId: aws.String("ami-id-123"), - CreationDate: aws.String(time.Now().Format(time.RFC3339)), - Architecture: aws.String("x86_64"), - Tags: []*ec2.Tag{ - {Key: aws.String("Name"), Value: aws.String("test-ami-1")}, - {Key: aws.String("foo"), Value: aws.String("bar")}, - }, - }, - { - Name: aws.String("test-ami-2"), - ImageId: aws.String("ami-id-456"), - CreationDate: aws.String(time.Now().Add(time.Minute).Format(time.RFC3339)), - Architecture: aws.String("arm64"), - Tags: []*ec2.Tag{ - {Key: aws.String("Name"), Value: aws.String("test-ami-2")}, - {Key: aws.String("foo"), Value: aws.String("bar")}, - }, - }, - }, - }) - ExpectApplied(ctx, env.Client, nodeTemplate) - ExpectReconcileSucceeded(ctx, controller, client.ObjectKeyFromObject(nodeTemplate)) - nodeTemplate = ExpectExists(ctx, env.Client, nodeTemplate) - - ExpectConsistOfAMIs([]v1alpha1.AMI{ - { - Name: "test-ami-1", - ID: "ami-id-123", - Requirements: []v1.NodeSelectorRequirement{ - { - Key: v1.LabelArchStable, - Operator: v1.NodeSelectorOpIn, - Values: []string{v1alpha5.ArchitectureAmd64}, - }, - { - Key: v1alpha1.LabelInstanceGPUCount, - Operator: v1.NodeSelectorOpDoesNotExist, - }, - { - Key: v1alpha1.LabelInstanceAcceleratorCount, - Operator: v1.NodeSelectorOpDoesNotExist, - }, - }, - }, - { - Name: "test-ami-2", - ID: "ami-id-456", - Requirements: []v1.NodeSelectorRequirement{ - { - Key: v1.LabelArchStable, - Operator: v1.NodeSelectorOpIn, - Values: []string{v1alpha5.ArchitectureArm64}, - }, - { - Key: v1alpha1.LabelInstanceGPUCount, - Operator: v1.NodeSelectorOpDoesNotExist, - }, - { - Key: v1alpha1.LabelInstanceAcceleratorCount, - Operator: v1.NodeSelectorOpDoesNotExist, - }, - }, - }, - }, nodeTemplate.Status.AMIs) - }) - It("Should resolve a valid AMI selector", func() { - ExpectApplied(ctx, env.Client, nodeTemplate) - ExpectReconcileSucceeded(ctx, controller, client.ObjectKeyFromObject(nodeTemplate)) - nodeTemplate = ExpectExists(ctx, env.Client, nodeTemplate) - Expect(nodeTemplate.Status.AMIs).To(ContainElements( - []v1alpha1.AMI{ - { - Name: "test-ami-3", - ID: "ami-test3", - Requirements: []v1.NodeSelectorRequirement{ - { - Key: "kubernetes.io/arch", - Operator: "In", - Values: []string{ - "amd64", - }, - }, - }, - }, - }, - )) - }) - It("should resolve amiSelector AMIs that have well-known tags as AMI requirements into status", func() { - awsEnv.EC2API.DescribeImagesOutput.Set(&ec2.DescribeImagesOutput{ - Images: []*ec2.Image{ - { - Name: aws.String("test-ami-4"), - ImageId: aws.String("ami-test4"), - CreationDate: aws.String(time.Now().Add(2 * time.Minute).Format(time.RFC3339)), - Architecture: aws.String("x86_64"), - Tags: []*ec2.Tag{ - {Key: aws.String("Name"), Value: aws.String("test-ami-3")}, - {Key: aws.String("foo"), Value: aws.String("bar")}, - {Key: aws.String("kubernetes.io/os"), Value: aws.String("test-requirement-1")}, - }, - }, - }, - }) - ExpectApplied(ctx, env.Client, nodeTemplate) - ExpectReconcileSucceeded(ctx, controller, client.ObjectKeyFromObject(nodeTemplate)) - nodeTemplate = ExpectExists(ctx, env.Client, nodeTemplate) - - ExpectConsistOfAMIs([]v1alpha1.AMI{ - { - Name: "test-ami-4", - ID: "ami-test4", - Requirements: []v1.NodeSelectorRequirement{ - { - Key: "kubernetes.io/os", - Operator: "In", - Values: []string{ - "test-requirement-1", - }, - }, - { - Key: "kubernetes.io/arch", - Operator: "In", - Values: []string{ - "amd64", - }, - }, - }, - }, - }, nodeTemplate.Status.AMIs) - }) - }) - Context("License Status", func() { - It("Should resolve a valid License selector", func() { - ExpectApplied(ctx, env.Client, nodeTemplate) - ExpectReconcileSucceeded(ctx, controller, client.ObjectKeyFromObject(nodeTemplate)) - nodeTemplate = ExpectExists(ctx, env.Client, nodeTemplate) - }) - }) - Context("AWSNodeTemplate Static Drift Hash", func() { - DescribeTable("should update the static drift hash when nodeTemplate static field is updated", func(awsnodetemplatespec v1alpha1.AWSNodeTemplateSpec) { - updatedAWSNodeTemplate := test.AWSNodeTemplate(*nodeTemplate.Spec.DeepCopy(), awsnodetemplatespec) - updatedAWSNodeTemplate.ObjectMeta = nodeTemplate.ObjectMeta - updatedAWSNodeTemplate.Annotations = map[string]string{v1alpha1.AnnotationNodeTemplateHash: updatedAWSNodeTemplate.Hash()} - - ExpectApplied(ctx, env.Client, nodeTemplate) - ExpectReconcileSucceeded(ctx, controller, client.ObjectKeyFromObject(nodeTemplate)) - nodeTemplate = ExpectExists(ctx, env.Client, nodeTemplate) - - expectedHash := nodeTemplate.Hash() - Expect(nodeTemplate.ObjectMeta.Annotations[v1alpha1.AnnotationNodeTemplateHash]).To(Equal(expectedHash)) - - ExpectApplied(ctx, env.Client, updatedAWSNodeTemplate) - ExpectReconcileSucceeded(ctx, controller, client.ObjectKeyFromObject(nodeTemplate)) - nodeTemplate = ExpectExists(ctx, env.Client, nodeTemplate) - - expectedHashTwo := nodeTemplate.Hash() - Expect(expectedHash).ToNot(Equal(expectedHashTwo)) - Expect(nodeTemplate.ObjectMeta.Annotations[v1alpha1.AnnotationNodeTemplateHash]).To(Equal(expectedHashTwo)) - }, - 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)}), - Entry("AMIFamily Drift", v1alpha1.AWSNodeTemplateSpec{AWS: v1alpha1.AWS{AMIFamily: aws.String(v1alpha1.AMIFamilyBottlerocket)}}), - ) - It("should not update the static drift hash when nodeTemplate dynamic field is updated", func() { - ExpectApplied(ctx, env.Client, nodeTemplate) - ExpectReconcileSucceeded(ctx, controller, client.ObjectKeyFromObject(nodeTemplate)) - nodeTemplate = ExpectExists(ctx, env.Client, nodeTemplate) - - expectedHash := nodeTemplate.Hash() - Expect(nodeTemplate.ObjectMeta.Annotations[v1alpha1.AnnotationNodeTemplateHash]).To(Equal(expectedHash)) - - nodeTemplate.Spec.SubnetSelector = map[string]string{"aws-ids": "subnet-test1"} - nodeTemplate.Spec.SecurityGroupSelector = map[string]string{"aws-ids": "sg-test1"} - nodeTemplate.Spec.AMISelector = map[string]string{"ami-test-key": "ami-test-value"} - - ExpectApplied(ctx, env.Client, nodeTemplate) - ExpectReconcileSucceeded(ctx, controller, client.ObjectKeyFromObject(nodeTemplate)) - nodeTemplate = ExpectExists(ctx, env.Client, nodeTemplate) - - Expect(nodeTemplate.ObjectMeta.Annotations[v1alpha1.AnnotationNodeTemplateHash]).To(Equal(expectedHash)) - }) - It("should maintain the same hash, before and after the NodeClass conversion", func() { - hash := nodeTemplate.Hash() - nodeClass := nodeclassutil.New(nodeTemplate) - convertedHash := nodeclassutil.HashAnnotation(nodeClass) - Expect(convertedHash).To(HaveKeyWithValue(v1alpha1.AnnotationNodeTemplateHash, hash)) - }) - }) -}) - -func ExpectConsistOfAMIs(expected, actual []v1alpha1.AMI) { - GinkgoHelper() - Expect(actual).To(HaveLen(len(expected))) - - for _, list := range [][]v1alpha1.AMI{expected, actual} { - for _, elem := range list { - sort.Slice(elem.Requirements, func(i, j int) bool { - return elem.Requirements[i].Key < elem.Requirements[j].Key - }) - } - } - Expect(actual).To(ConsistOf(lo.Map(expected, func(a v1alpha1.AMI, _ int) interface{} { return a })...)) -} diff --git a/pkg/providers/license/suite_test.go b/pkg/providers/license/suite_test.go new file mode 100644 index 000000000000..4854aa102e53 --- /dev/null +++ b/pkg/providers/license/suite_test.go @@ -0,0 +1,116 @@ +/* +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package license_test + +import ( + "context" + "testing" + + "github.com/aws/aws-sdk-go/aws" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + . "knative.dev/pkg/logging/testing" + + "github.com/aws/karpenter/pkg/apis" + "github.com/aws/karpenter/pkg/apis/settings" + "github.com/aws/karpenter/pkg/apis/v1beta1" + "github.com/aws/karpenter/pkg/fake" + "github.com/aws/karpenter/pkg/test" + + coresettings "github.com/aws/karpenter-core/pkg/apis/settings" + "github.com/aws/karpenter-core/pkg/operator/injection" + "github.com/aws/karpenter-core/pkg/operator/options" + "github.com/aws/karpenter-core/pkg/operator/scheme" + coretest "github.com/aws/karpenter-core/pkg/test" + . "github.com/aws/karpenter-core/pkg/test/expectations" +) + +var ctx context.Context +var stop context.CancelFunc +var opts options.Options +var env *coretest.Environment +var awsEnv *test.Environment +var nodeClass *v1beta1.EC2NodeClass + +func TestAWS(t *testing.T) { + ctx = TestContextWithLogger(t) + RegisterFailHandler(Fail) + RunSpecs(t, "Provider/AWS") +} + +var _ = BeforeSuite(func() { + env = coretest.NewEnvironment(scheme.Scheme, coretest.WithCRDs(apis.CRDs...)) + ctx = coresettings.ToContext(ctx, coretest.Settings()) + ctx = settings.ToContext(ctx, test.Settings()) + ctx, stop = context.WithCancel(ctx) + awsEnv = test.NewEnvironment(ctx, env) +}) + +var _ = AfterSuite(func() { + stop() + Expect(env.Stop()).To(Succeed(), "Failed to stop environment") +}) + +var _ = BeforeEach(func() { + ctx = injection.WithOptions(ctx, opts) + ctx = coresettings.ToContext(ctx, coretest.Settings()) + ctx = settings.ToContext(ctx, test.Settings()) + nodeClass = test.EC2NodeClass(v1beta1.EC2NodeClass{ + Spec: v1beta1.EC2NodeClassSpec{ + AMIFamily: aws.String(v1beta1.AMIFamilyAL2), + SubnetSelectorTerms: []v1beta1.SubnetSelectorTerm{ + { + Tags: map[string]string{ + "*": "*", + }, + }, + }, + SecurityGroupSelectorTerms: []v1beta1.SecurityGroupSelectorTerm{ + { + Tags: map[string]string{ + "*": "*", + }, + }, + }, + }, + }) + awsEnv.Reset() +}) + +var _ = AfterEach(func() { + ExpectCleanedUp(ctx, env.Client) +}) + +var _ = Describe("SubnetProvider", func() { + Context("List", func() { + It("should discover license by name", func() { + nodeClass.Spec.LicenseSelectorTerms = []v1beta1.LicenseSelectorTerm{ + { + Name: "test-license", + }, + } + licenses, err := awsEnv.LicenseProvider.Get(ctx, nodeClass) + Expect(err).To(BeNil()) + ExpectConsistsOfLicenses([]string{ + fake.TestLicenseArn, + }, licenses) + }) + }) +}) + +func ExpectConsistsOfLicenses(expected, actual []string) { + GinkgoHelper() + Expect(actual).To(Equal(expected)) +} From 04b081ab677d40f6f904cadc223af812188d51e9 Mon Sep 17 00:00:00 2001 From: Sebastian Cole Date: Thu, 21 Sep 2023 13:32:04 +1000 Subject: [PATCH 25/33] Add license provider tests --- pkg/fake/lmapi.go | 9 ++++---- pkg/fake/rgapi.go | 6 ++++-- pkg/providers/license/suite_test.go | 23 +++++++++++++++++--- pkg/test/environment.go | 33 +++++++++++++++++------------ 4 files changed, 47 insertions(+), 24 deletions(-) diff --git a/pkg/fake/lmapi.go b/pkg/fake/lmapi.go index 88f0695859fd..d36f48b68341 100644 --- a/pkg/fake/lmapi.go +++ b/pkg/fake/lmapi.go @@ -37,14 +37,13 @@ func (l *LicenseManagerAPI) Reset() { l.ListLicenseConfigurationsOutput.Reset() } -func (l *LicenseManagerAPI) ListLicenseConfigurations(_ aws.Context, _ *licensemanager.ListLicenseConfigurationsInput, fn func(*licensemanager.ListLicenseConfigurationsOutput, bool) bool, _ ...request.Option) error { +func (l *LicenseManagerAPI) ListLicenseConfigurationsWithContext(_ aws.Context, _ *licensemanager.ListLicenseConfigurationsInput, _ ...request.Option) (*licensemanager.ListLicenseConfigurationsOutput, error) { if !l.NextError.IsNil() { - return l.NextError.Get() + return nil, l.NextError.Get() } if !l.ListLicenseConfigurationsOutput.IsNil() { - fn(l.ListLicenseConfigurationsOutput.Clone(), false) - return nil + return l.ListLicenseConfigurationsOutput.Clone(), nil } // fail if the test doesn't provide specific data which causes our pricing provider to use its static price list - return errors.New("no license data provided") + return nil, errors.New("no license data provided") } diff --git a/pkg/fake/rgapi.go b/pkg/fake/rgapi.go index 082d2df8f06e..bacf40e106c0 100644 --- a/pkg/fake/rgapi.go +++ b/pkg/fake/rgapi.go @@ -34,16 +34,18 @@ type ResourceGroupsAPI struct { } type ResourceGroupsBehaviour struct { NextError AtomicError - ListGroupsOutput AtomicPtr[resourcegroups.ListGroupsOutput] ListGroupsBehavior MockedFunction[resourcegroups.ListGroupsInput, resourcegroups.ListGroupsOutput] } func (r *ResourceGroupsAPI) Reset() { r.NextError.Reset() - r.ListGroupsOutput.Reset() + r.ListGroupsBehavior.Reset() } func (r *ResourceGroupsAPI) ListGroupsWithContext(_ aws.Context, input *resourcegroups.ListGroupsInput, _ ...request.Option) (*resourcegroups.ListGroupsOutput, error) { + if !r.NextError.IsNil() { + return nil, r.NextError.Get() + } return r.ListGroupsBehavior.Invoke(input, func(input *resourcegroups.ListGroupsInput) (*resourcegroups.ListGroupsOutput, error) { return &resourcegroups.ListGroupsOutput{ GroupIdentifiers: []*resourcegroups.GroupIdentifier{ diff --git a/pkg/providers/license/suite_test.go b/pkg/providers/license/suite_test.go index 4854aa102e53..0dde5e9037b1 100644 --- a/pkg/providers/license/suite_test.go +++ b/pkg/providers/license/suite_test.go @@ -19,6 +19,7 @@ import ( "testing" "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/licensemanager" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" . "knative.dev/pkg/logging/testing" @@ -93,12 +94,15 @@ var _ = AfterEach(func() { ExpectCleanedUp(ctx, env.Client) }) -var _ = Describe("SubnetProvider", func() { - Context("List", func() { +var _ = Describe("LicenseProvider", func() { + Context("Get", func() { It("should discover license by name", func() { + awsEnv.LicenseManagerAPI.ListLicenseConfigurationsOutput.Set(&licensemanager.ListLicenseConfigurationsOutput{ + LicenseConfigurations: []*licensemanager.LicenseConfiguration{{LicenseConfigurationArn: aws.String(fake.TestLicenseArn), Name: aws.String(fake.TestLicenseName)}}, + }) nodeClass.Spec.LicenseSelectorTerms = []v1beta1.LicenseSelectorTerm{ { - Name: "test-license", + Name: fake.TestLicenseName, }, } licenses, err := awsEnv.LicenseProvider.Get(ctx, nodeClass) @@ -107,6 +111,19 @@ var _ = Describe("SubnetProvider", func() { fake.TestLicenseArn, }, licenses) }) + It("should only return matches", func() { + awsEnv.LicenseManagerAPI.ListLicenseConfigurationsOutput.Set(&licensemanager.ListLicenseConfigurationsOutput{ + LicenseConfigurations: []*licensemanager.LicenseConfiguration{{LicenseConfigurationArn: aws.String(fake.TestLicenseArn), Name: aws.String(fake.TestLicenseName)}}, + }) + nodeClass.Spec.LicenseSelectorTerms = []v1beta1.LicenseSelectorTerm{ + { + Name: "does-not-match", + }, + } + licenses, err := awsEnv.LicenseProvider.Get(ctx, nodeClass) + Expect(err).To(BeNil()) + Expect(licenses).To(BeNil()) + }) }) }) diff --git a/pkg/test/environment.go b/pkg/test/environment.go index d14bacde7467..f340d7760a66 100644 --- a/pkg/test/environment.go +++ b/pkg/test/environment.go @@ -102,7 +102,7 @@ func NewEnvironment(ctx context.Context, env *coretest.Environment) *Environment securityGroupProvider := securitygroup.NewProvider(ec2api, securityGroupCache) versionProvider := version.NewProvider(env.KubernetesInterface, kubernetesVersionCache) amiProvider := amifamily.NewProvider(versionProvider, ssmapi, ec2api, ec2Cache) - licenseProvider := license.NewProvider(lmapi.LicenseManagerAPI, licenseCache) + licenseProvider := license.NewProvider(lmapi, licenseCache) hostResourceGroupProvider := hostresourcegroup.NewProvider(rgapi, hostResourceGroupsCache) placementGroupProvider := placementgroup.NewProvider(ec2api, placementGroupCache) @@ -132,9 +132,11 @@ func NewEnvironment(ctx context.Context, env *coretest.Environment) *Environment ) return &Environment{ - EC2API: ec2api, - SSMAPI: ssmapi, - PricingAPI: fakePricingAPI, + EC2API: ec2api, + SSMAPI: ssmapi, + LicenseManagerAPI: lmapi, + PricingAPI: fakePricingAPI, + ResourceGroupsAPI: rgapi, EC2Cache: ec2Cache, KubernetesVersionCache: kubernetesVersionCache, @@ -145,16 +147,18 @@ func NewEnvironment(ctx context.Context, env *coretest.Environment) *Environment UnavailableOfferingsCache: unavailableOfferingsCache, LicenseCache: licenseCache, - InstanceTypesProvider: instanceTypesProvider, - InstanceProvider: instanceProvider, - SubnetProvider: subnetProvider, - SecurityGroupProvider: securityGroupProvider, - PricingProvider: pricingProvider, - AMIProvider: amiProvider, - AMIResolver: amiResolver, - VersionProvider: versionProvider, - LaunchTemplateProvider: launchTemplateProvider, - LicenseProvider: licenseProvider, + InstanceTypesProvider: instanceTypesProvider, + InstanceProvider: instanceProvider, + SubnetProvider: subnetProvider, + SecurityGroupProvider: securityGroupProvider, + PricingProvider: pricingProvider, + AMIProvider: amiProvider, + AMIResolver: amiResolver, + VersionProvider: versionProvider, + LaunchTemplateProvider: launchTemplateProvider, + LicenseProvider: licenseProvider, + HostResourceGroupProvider: hostResourceGroupProvider, + PlacementGroupProvider: placementGroupProvider, } } @@ -162,6 +166,7 @@ func (env *Environment) Reset() { env.EC2API.Reset() env.SSMAPI.Reset() env.LicenseManagerAPI.Reset() + env.ResourceGroupsAPI.Reset() env.PricingAPI.Reset() env.PricingProvider.Reset() From 7416e23eb07a260fe51bcebf62509bda0d6ecbc8 Mon Sep 17 00:00:00 2001 From: Sebastian Cole Date: Thu, 21 Sep 2023 13:50:33 +1000 Subject: [PATCH 26/33] Adding test suites for host resource groups provider --- pkg/providers/hostresourcegroup/suite_test.go | 131 ++++++++++++++++++ 1 file changed, 131 insertions(+) create mode 100644 pkg/providers/hostresourcegroup/suite_test.go diff --git a/pkg/providers/hostresourcegroup/suite_test.go b/pkg/providers/hostresourcegroup/suite_test.go new file mode 100644 index 000000000000..785b9ca060da --- /dev/null +++ b/pkg/providers/hostresourcegroup/suite_test.go @@ -0,0 +1,131 @@ +/* +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package hostresourcegroup_test + +import ( + "context" + "testing" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/resourcegroups" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + . "knative.dev/pkg/logging/testing" + + "github.com/aws/karpenter/pkg/apis" + "github.com/aws/karpenter/pkg/apis/settings" + "github.com/aws/karpenter/pkg/apis/v1beta1" + "github.com/aws/karpenter/pkg/test" + + coresettings "github.com/aws/karpenter-core/pkg/apis/settings" + "github.com/aws/karpenter-core/pkg/operator/injection" + "github.com/aws/karpenter-core/pkg/operator/options" + "github.com/aws/karpenter-core/pkg/operator/scheme" + coretest "github.com/aws/karpenter-core/pkg/test" + . "github.com/aws/karpenter-core/pkg/test/expectations" +) + +var ctx context.Context +var stop context.CancelFunc +var opts options.Options +var env *coretest.Environment +var awsEnv *test.Environment +var nodeClass *v1beta1.EC2NodeClass + +func TestAWS(t *testing.T) { + ctx = TestContextWithLogger(t) + RegisterFailHandler(Fail) + RunSpecs(t, "Provider/AWS") +} + +var _ = BeforeSuite(func() { + env = coretest.NewEnvironment(scheme.Scheme, coretest.WithCRDs(apis.CRDs...)) + ctx = coresettings.ToContext(ctx, coretest.Settings()) + ctx = settings.ToContext(ctx, test.Settings()) + ctx, stop = context.WithCancel(ctx) + awsEnv = test.NewEnvironment(ctx, env) +}) + +var _ = AfterSuite(func() { + stop() + Expect(env.Stop()).To(Succeed(), "Failed to stop environment") +}) + +var _ = BeforeEach(func() { + ctx = injection.WithOptions(ctx, opts) + ctx = coresettings.ToContext(ctx, coretest.Settings()) + ctx = settings.ToContext(ctx, test.Settings()) + nodeClass = test.EC2NodeClass(v1beta1.EC2NodeClass{ + Spec: v1beta1.EC2NodeClassSpec{ + AMIFamily: aws.String(v1beta1.AMIFamilyAL2), + SubnetSelectorTerms: []v1beta1.SubnetSelectorTerm{ + { + Tags: map[string]string{ + "*": "*", + }, + }, + }, + SecurityGroupSelectorTerms: []v1beta1.SecurityGroupSelectorTerm{ + { + Tags: map[string]string{ + "*": "*", + }, + }, + }, + }, + }) + awsEnv.Reset() +}) + +var _ = AfterEach(func() { + ExpectCleanedUp(ctx, env.Client) +}) + +var _ = Describe("HostResourceGroupProvider", func() { + Context("Get", func() { + It("should discover host resource groups by name", func() { + expected := &v1beta1.HostResourceGroup{ARN: "expected-group-arn", Name: "expected-group-name"} + awsEnv.ResourceGroupsAPI.ListGroupsBehavior.Output.Set(&resourcegroups.ListGroupsOutput{ + GroupIdentifiers: []*resourcegroups.GroupIdentifier{ + {GroupArn: aws.String(expected.ARN), GroupName: aws.String(expected.Name)}, + }, + }) + nodeClass.Spec.HostResourceGroupSelectorTerms = []v1beta1.HostResourceGroupSelectorTerm{ + { + Name: expected.Name, + }, + } + hrgs, err := awsEnv.HostResourceGroupProvider.Get(ctx, nodeClass) + Expect(err).To(BeNil()) + Expect(hrgs).To(Equal(expected)) + }) + It("should filter only matching groups", func() { + expected := &v1beta1.HostResourceGroup{ARN: "expected-group-arn", Name: "expected-group-name"} + awsEnv.ResourceGroupsAPI.ListGroupsBehavior.Output.Set(&resourcegroups.ListGroupsOutput{ + GroupIdentifiers: []*resourcegroups.GroupIdentifier{ + {GroupArn: aws.String(expected.ARN), GroupName: aws.String(expected.Name)}, + }, + }) + nodeClass.Spec.HostResourceGroupSelectorTerms = []v1beta1.HostResourceGroupSelectorTerm{ + { + Name: "does-not-match", + }, + } + hrgs, err := awsEnv.HostResourceGroupProvider.Get(ctx, nodeClass) + Expect(err).To(BeNil()) + Expect(hrgs).To(BeNil()) + }) + }) +}) From 1e4ed56712213855bbb486f555ae68982771e742 Mon Sep 17 00:00:00 2001 From: Sebastian Cole Date: Thu, 21 Sep 2023 14:24:56 +1000 Subject: [PATCH 27/33] Adding suite_tests for placement group provider --- pkg/fake/ec2api.go | 14 +++ pkg/providers/placementgroup/suite_test.go | 123 +++++++++++++++++++++ 2 files changed, 137 insertions(+) create mode 100644 pkg/providers/placementgroup/suite_test.go diff --git a/pkg/fake/ec2api.go b/pkg/fake/ec2api.go index 092c5b88f5eb..b1a39dd30bc0 100644 --- a/pkg/fake/ec2api.go +++ b/pkg/fake/ec2api.go @@ -55,6 +55,7 @@ type EC2Behavior struct { DescribeAvailabilityZonesOutput AtomicPtr[ec2.DescribeAvailabilityZonesOutput] DescribeSpotPriceHistoryInput AtomicPtr[ec2.DescribeSpotPriceHistoryInput] DescribeSpotPriceHistoryOutput AtomicPtr[ec2.DescribeSpotPriceHistoryOutput] + DescribePlacementGroupsOutput AtomicPtr[ec2.DescribePlacementGroupsOutput] CreateFleetBehavior MockedFunction[ec2.CreateFleetInput, ec2.CreateFleetOutput] TerminateInstancesBehavior MockedFunction[ec2.TerminateInstancesInput, ec2.TerminateInstancesOutput] DescribeInstancesBehavior MockedFunction[ec2.DescribeInstancesInput, ec2.DescribeInstancesOutput] @@ -85,6 +86,7 @@ func (e *EC2API) Reset() { e.DescribeInstanceTypesOutput.Reset() e.DescribeInstanceTypeOfferingsOutput.Reset() e.DescribeAvailabilityZonesOutput.Reset() + e.DescribePlacementGroupsOutput.Reset() e.CreateFleetBehavior.Reset() e.TerminateInstancesBehavior.Reset() e.DescribeInstancesBehavior.Reset() @@ -640,3 +642,15 @@ func (e *EC2API) DescribeSpotPriceHistoryPagesWithContext(ctx aws.Context, input fn(out, false) return nil } + +func (e *EC2API) DescribePlacementGroupsWithContext(ctx aws.Context, input *ec2.DescribePlacementGroupsInput, _ ...request.Option) (*ec2.DescribePlacementGroupsOutput, error) { + if !e.NextError.IsNil() { + defer e.NextError.Reset() + return nil, e.NextError.Get() + } + if !e.DescribePlacementGroupsOutput.IsNil() { + return e.DescribePlacementGroupsOutput.Clone(), nil + } + return nil, errors.New("no placement groups data provided") + +} diff --git a/pkg/providers/placementgroup/suite_test.go b/pkg/providers/placementgroup/suite_test.go new file mode 100644 index 000000000000..3b7f2492d6a0 --- /dev/null +++ b/pkg/providers/placementgroup/suite_test.go @@ -0,0 +1,123 @@ +/* +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package placementgroup_test + +import ( + "context" + "testing" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/ec2" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + . "knative.dev/pkg/logging/testing" + + "github.com/aws/karpenter/pkg/apis" + "github.com/aws/karpenter/pkg/apis/settings" + "github.com/aws/karpenter/pkg/apis/v1beta1" + "github.com/aws/karpenter/pkg/test" + + coresettings "github.com/aws/karpenter-core/pkg/apis/settings" + "github.com/aws/karpenter-core/pkg/operator/injection" + "github.com/aws/karpenter-core/pkg/operator/options" + "github.com/aws/karpenter-core/pkg/operator/scheme" + coretest "github.com/aws/karpenter-core/pkg/test" + . "github.com/aws/karpenter-core/pkg/test/expectations" +) + +var ctx context.Context +var stop context.CancelFunc +var opts options.Options +var env *coretest.Environment +var awsEnv *test.Environment +var nodeClass *v1beta1.EC2NodeClass + +func TestAWS(t *testing.T) { + ctx = TestContextWithLogger(t) + RegisterFailHandler(Fail) + RunSpecs(t, "Provider/AWS") +} + +var _ = BeforeSuite(func() { + env = coretest.NewEnvironment(scheme.Scheme, coretest.WithCRDs(apis.CRDs...)) + ctx = coresettings.ToContext(ctx, coretest.Settings()) + ctx = settings.ToContext(ctx, test.Settings()) + ctx, stop = context.WithCancel(ctx) + awsEnv = test.NewEnvironment(ctx, env) +}) + +var _ = AfterSuite(func() { + stop() + Expect(env.Stop()).To(Succeed(), "Failed to stop environment") +}) + +var _ = BeforeEach(func() { + ctx = injection.WithOptions(ctx, opts) + ctx = coresettings.ToContext(ctx, coretest.Settings()) + ctx = settings.ToContext(ctx, test.Settings()) + nodeClass = test.EC2NodeClass(v1beta1.EC2NodeClass{ + Spec: v1beta1.EC2NodeClassSpec{ + AMIFamily: aws.String(v1beta1.AMIFamilyAL2), + SubnetSelectorTerms: []v1beta1.SubnetSelectorTerm{ + { + Tags: map[string]string{ + "*": "*", + }, + }, + }, + SecurityGroupSelectorTerms: []v1beta1.SecurityGroupSelectorTerm{ + { + Tags: map[string]string{ + "*": "*", + }, + }, + }, + }, + }) + awsEnv.Reset() +}) + +var _ = AfterEach(func() { + ExpectCleanedUp(ctx, env.Client) +}) + +var _ = Describe("PlacementGroupProvider", func() { + Context("Get", func() { + It("should discover placement groups by name", func() { + expected := &ec2.PlacementGroup{GroupArn: aws.String("test-group-arn"), GroupName: aws.String("test-group-name")} + awsEnv.EC2API.DescribePlacementGroupsOutput.Set(&ec2.DescribePlacementGroupsOutput{PlacementGroups: []*ec2.PlacementGroup{expected}}) + nodeClass.Spec.PlacementGroupSelectorTerms = []v1beta1.PlacementGroupSelectorTerm{ + { + Name: *expected.GroupName, + }, + } + pg, err := awsEnv.PlacementGroupProvider.Get(ctx, nodeClass) + Expect(err).To(BeNil()) + Expect(pg).To(Equal(expected)) + }) + It("should filter only matches", func() { + expected := &ec2.PlacementGroup{GroupArn: aws.String("test-group-arn"), GroupName: aws.String("test-group-name")} + awsEnv.EC2API.DescribePlacementGroupsOutput.Set(&ec2.DescribePlacementGroupsOutput{PlacementGroups: []*ec2.PlacementGroup{expected}}) + nodeClass.Spec.PlacementGroupSelectorTerms = []v1beta1.PlacementGroupSelectorTerm{ + { + Name: "does-not-match", + }, + } + pg, err := awsEnv.PlacementGroupProvider.Get(ctx, nodeClass) + Expect(err).To(BeNil()) + Expect(pg).To(BeNil()) + }) + }) +}) From 16aa9c67453aa91c19b385571bb146f3fb6867b4 Mon Sep 17 00:00:00 2001 From: Sebastian Cole Date: Thu, 21 Sep 2023 14:30:15 +1000 Subject: [PATCH 28/33] Fix fake api signatures --- pkg/fake/ec2api.go | 2 +- pkg/fake/lmapi.go | 2 +- pkg/fake/rgapi.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/fake/ec2api.go b/pkg/fake/ec2api.go index b1a39dd30bc0..30124374fc9c 100644 --- a/pkg/fake/ec2api.go +++ b/pkg/fake/ec2api.go @@ -643,7 +643,7 @@ func (e *EC2API) DescribeSpotPriceHistoryPagesWithContext(ctx aws.Context, input return nil } -func (e *EC2API) DescribePlacementGroupsWithContext(ctx aws.Context, input *ec2.DescribePlacementGroupsInput, _ ...request.Option) (*ec2.DescribePlacementGroupsOutput, error) { +func (e *EC2API) DescribePlacementGroupsWithContext(_ aws.Context, _ *ec2.DescribePlacementGroupsInput, _ ...request.Option) (*ec2.DescribePlacementGroupsOutput, error) { if !e.NextError.IsNil() { defer e.NextError.Reset() return nil, e.NextError.Get() diff --git a/pkg/fake/lmapi.go b/pkg/fake/lmapi.go index d36f48b68341..fca0ab52f416 100644 --- a/pkg/fake/lmapi.go +++ b/pkg/fake/lmapi.go @@ -42,7 +42,7 @@ func (l *LicenseManagerAPI) ListLicenseConfigurationsWithContext(_ aws.Context, return nil, l.NextError.Get() } if !l.ListLicenseConfigurationsOutput.IsNil() { - return l.ListLicenseConfigurationsOutput.Clone(), nil + return l.ListLicenseConfigurationsOutput.Clone(), nil } // fail if the test doesn't provide specific data which causes our pricing provider to use its static price list return nil, errors.New("no license data provided") diff --git a/pkg/fake/rgapi.go b/pkg/fake/rgapi.go index bacf40e106c0..f8b8e94739f6 100644 --- a/pkg/fake/rgapi.go +++ b/pkg/fake/rgapi.go @@ -39,7 +39,7 @@ type ResourceGroupsBehaviour struct { func (r *ResourceGroupsAPI) Reset() { r.NextError.Reset() - r.ListGroupsBehavior.Reset() + r.ListGroupsBehavior.Reset() } func (r *ResourceGroupsAPI) ListGroupsWithContext(_ aws.Context, input *resourcegroups.ListGroupsInput, _ ...request.Option) (*resourcegroups.ListGroupsOutput, error) { From 16ff1cff6eeb6770739e68e470824fdb92eb9f87 Mon Sep 17 00:00:00 2001 From: Sebastian Cole Date: Thu, 21 Sep 2023 15:01:26 +1000 Subject: [PATCH 29/33] Fix pagination logic for host resource group provider --- pkg/providers/hostresourcegroup/hostresourcegroup.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/providers/hostresourcegroup/hostresourcegroup.go b/pkg/providers/hostresourcegroup/hostresourcegroup.go index 1e6c0a580fd6..041a9ca8f60c 100644 --- a/pkg/providers/hostresourcegroup/hostresourcegroup.go +++ b/pkg/providers/hostresourcegroup/hostresourcegroup.go @@ -76,7 +76,7 @@ func (p *Provider) Get(ctx context.Context, nodeClass *v1beta1.EC2NodeClass) (*v } } } - return lastPage + return !lastPage }) if err != nil { From 42cbd6b515357268dc5a078113bad99d76408034 Mon Sep 17 00:00:00 2001 From: Sebastian Cole Date: Mon, 9 Oct 2023 09:51:17 +1100 Subject: [PATCH 30/33] Revert changes, remove support in v1alpha1/awsnodetemplate --- pkg/apis/v1alpha1/awsnodetemplate.go | 12 ---- pkg/apis/v1alpha1/provider.go | 9 --- pkg/utils/nodeclass/nodeclass.go | 79 ++++++-------------------- pkg/utils/nodeclass/suite_test.go | 7 --- pkg/utils/nodetemplate/nodetemplate.go | 30 +++------- 5 files changed, 27 insertions(+), 110 deletions(-) diff --git a/pkg/apis/v1alpha1/awsnodetemplate.go b/pkg/apis/v1alpha1/awsnodetemplate.go index 744cbb6182ff..7722bdf8443b 100644 --- a/pkg/apis/v1alpha1/awsnodetemplate.go +++ b/pkg/apis/v1alpha1/awsnodetemplate.go @@ -69,18 +69,6 @@ type AWSNodeTemplateStatus struct { // cluster under the AMI selectors. // +optional AMIs []AMI `json:"amis,omitempty"` - // Licenses contains the current License Configuration Specifications - // that currently apply given the LicenseConfiguration selectors. - // +optional - Licenses []string `json:"licenses,omitempty"` - // HostResourceGroups contains the current Host Resource Groups - // that currently apply given the HostResourceGroup selectors. - // +optional - HostResourceGroups []string `json:"hostResourceGroups,omitempty"` - // PlacementGroups contains the current placement groups - // that currently apply given the PlacementGroup selectors. - // +optional - PlacementGroups []string `json:"placementGroups,omitempty"` } // AWSNodeTemplateSpec is the top level specification for the AWS Karpenter Provider. diff --git a/pkg/apis/v1alpha1/provider.go b/pkg/apis/v1alpha1/provider.go index c72f0a9c88d8..0fe2f8071b06 100644 --- a/pkg/apis/v1alpha1/provider.go +++ b/pkg/apis/v1alpha1/provider.go @@ -41,15 +41,6 @@ type AWS struct { // SecurityGroups specify the names of the security groups. // +optional SecurityGroupSelector map[string]string `json:"securityGroupSelector,omitempty" hash:"ignore"` - // LicenseSelector specifies the license configuration specifications to launch instances with - // +optional - LicenseSelector map[string]string `json:"licenseSelector,omitempty" hash:"ignore"` - // HostResourceGroups specifies the hostResourceGroup names to use for ec2 placement - // +optional - HostResourceGroupSelector map[string]string `json:"hostResourceGroupSelector,omitempty" hash:"ignore"` - // PlacementGroup specifies the placement group names to use for ec2 placement - // +optional - PlacementGroupSelector map[string]string `json:"placementGroupSelector,omitempty" hash:"ignore"` // Tags to be applied on ec2 resources like instances and launch templates. // +optional Tags map[string]string `json:"tags,omitempty"` diff --git a/pkg/utils/nodeclass/nodeclass.go b/pkg/utils/nodeclass/nodeclass.go index 18b2df808216..1f818689beab 100644 --- a/pkg/utils/nodeclass/nodeclass.go +++ b/pkg/utils/nodeclass/nodeclass.go @@ -37,65 +37,31 @@ func New(nodeTemplate *v1alpha1.AWSNodeTemplate) *v1beta1.EC2NodeClass { TypeMeta: nodeTemplate.TypeMeta, ObjectMeta: nodeTemplate.ObjectMeta, Spec: v1beta1.EC2NodeClassSpec{ - SubnetSelectorTerms: NewSubnetSelectorTerms(nodeTemplate.Spec.SubnetSelector), - OriginalSubnetSelector: nodeTemplate.Spec.SubnetSelector, - SecurityGroupSelectorTerms: NewSecurityGroupSelectorTerms(nodeTemplate.Spec.SecurityGroupSelector), - OriginalSecurityGroupSelector: nodeTemplate.Spec.SecurityGroupSelector, - AMISelectorTerms: NewAMISelectorTerms(nodeTemplate.Spec.AMISelector), - OriginalAMISelector: nodeTemplate.Spec.AMISelector, - AMIFamily: nodeTemplate.Spec.AMIFamily, - LicenseSelectorTerms: NewLicenseSelectorTerms(nodeTemplate.Spec.LicenseSelector), - OriginalLicenseSelector: nodeTemplate.Spec.LicenseSelector, - OriginalPlacementGroupSelector: nodeTemplate.Spec.PlacementGroupSelector, - PlacementGroupSelectorTerms: NewPlacementGroupSelectorTerms(nodeTemplate.Spec.PlacementGroupSelector), - UserData: nodeTemplate.Spec.UserData, - Tags: nodeTemplate.Spec.Tags, - BlockDeviceMappings: NewBlockDeviceMappings(nodeTemplate.Spec.BlockDeviceMappings), - DetailedMonitoring: nodeTemplate.Spec.DetailedMonitoring, - MetadataOptions: NewMetadataOptions(nodeTemplate.Spec.MetadataOptions), - Context: nodeTemplate.Spec.Context, - LaunchTemplateName: nodeTemplate.Spec.LaunchTemplateName, - InstanceProfile: nodeTemplate.Spec.InstanceProfile, - OriginalHostResourceGroupSelector: nodeTemplate.Spec.HostResourceGroupSelector, - HostResourceGroupSelectorTerms: NewHostResourceSelectorTerms(nodeTemplate.Spec.HostResourceGroupSelector), + SubnetSelectorTerms: NewSubnetSelectorTerms(nodeTemplate.Spec.SubnetSelector), + OriginalSubnetSelector: nodeTemplate.Spec.SubnetSelector, + SecurityGroupSelectorTerms: NewSecurityGroupSelectorTerms(nodeTemplate.Spec.SecurityGroupSelector), + OriginalSecurityGroupSelector: nodeTemplate.Spec.SecurityGroupSelector, + AMISelectorTerms: NewAMISelectorTerms(nodeTemplate.Spec.AMISelector), + OriginalAMISelector: nodeTemplate.Spec.AMISelector, + AMIFamily: nodeTemplate.Spec.AMIFamily, + UserData: nodeTemplate.Spec.UserData, + Tags: nodeTemplate.Spec.Tags, + BlockDeviceMappings: NewBlockDeviceMappings(nodeTemplate.Spec.BlockDeviceMappings), + DetailedMonitoring: nodeTemplate.Spec.DetailedMonitoring, + MetadataOptions: NewMetadataOptions(nodeTemplate.Spec.MetadataOptions), + Context: nodeTemplate.Spec.Context, + LaunchTemplateName: nodeTemplate.Spec.LaunchTemplateName, + InstanceProfile: nodeTemplate.Spec.InstanceProfile, }, Status: v1beta1.EC2NodeClassStatus{ - Subnets: NewSubnets(nodeTemplate.Status.Subnets), - SecurityGroups: NewSecurityGroups(nodeTemplate.Status.SecurityGroups), - AMIs: NewAMIs(nodeTemplate.Status.AMIs), - Licenses: nodeTemplate.Status.Licenses, - HostResourceGroup: NewHostResourceGroup(nodeTemplate.Status.HostResourceGroups), - PlacementGroups: nodeTemplate.Status.PlacementGroups, + Subnets: NewSubnets(nodeTemplate.Status.Subnets), + SecurityGroups: NewSecurityGroups(nodeTemplate.Status.SecurityGroups), + AMIs: NewAMIs(nodeTemplate.Status.AMIs), }, IsNodeTemplate: true, } } -func NewPlacementGroupSelectorTerms(placementGroupSelector map[string]string) (terms []v1beta1.PlacementGroupSelectorTerm) { - if name, ok := placementGroupSelector["name"]; ok { - return []v1beta1.PlacementGroupSelectorTerm{ - {Name: name}, - } - } - return nil -} - -func NewHostResourceSelectorTerms(hrgSelector map[string]string) (terms []v1beta1.HostResourceGroupSelectorTerm) { - if name, ok := hrgSelector["name"]; ok { - return []v1beta1.HostResourceGroupSelectorTerm{ - {Name: name}, - } - } - return nil -} - -func NewHostResourceGroup(hrgs []string) *v1beta1.HostResourceGroup { - for i := range hrgs { - return &v1beta1.HostResourceGroup{ARN: hrgs[i]} - } - return nil -} - func NewSubnetSelectorTerms(subnetSelector map[string]string) (terms []v1beta1.SubnetSelectorTerm) { if len(subnetSelector) == 0 { return nil @@ -182,15 +148,6 @@ func NewAMISelectorTerms(amiSelector map[string]string) (terms []v1beta1.AMISele } return terms } -func NewLicenseSelectorTerms(selectors map[string]string) (terms []v1beta1.LicenseSelectorTerm) { - if name, ok := selectors["name"]; ok { - return []v1beta1.LicenseSelectorTerm{ - {Name: name}, - } - } - - return nil -} func NewBlockDeviceMappings(bdms []*v1alpha1.BlockDeviceMapping) []*v1beta1.BlockDeviceMapping { if bdms == nil { diff --git a/pkg/utils/nodeclass/suite_test.go b/pkg/utils/nodeclass/suite_test.go index 8277c31769d7..7b996b3a0659 100644 --- a/pkg/utils/nodeclass/suite_test.go +++ b/pkg/utils/nodeclass/suite_test.go @@ -434,13 +434,6 @@ 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) diff --git a/pkg/utils/nodetemplate/nodetemplate.go b/pkg/utils/nodetemplate/nodetemplate.go index 00ce02c8b963..3112a04b51e9 100644 --- a/pkg/utils/nodetemplate/nodetemplate.go +++ b/pkg/utils/nodetemplate/nodetemplate.go @@ -28,14 +28,12 @@ func New(nodeClass *v1beta1.EC2NodeClass) *v1alpha1.AWSNodeTemplate { Spec: v1alpha1.AWSNodeTemplateSpec{ UserData: nodeClass.Spec.UserData, AWS: v1alpha1.AWS{ - AMIFamily: nodeClass.Spec.AMIFamily, - Context: nodeClass.Spec.Context, - InstanceProfile: nodeClass.Spec.InstanceProfile, - SubnetSelector: nodeClass.Spec.OriginalSubnetSelector, - SecurityGroupSelector: nodeClass.Spec.OriginalSecurityGroupSelector, - LicenseSelector: nodeClass.Spec.OriginalLicenseSelector, - HostResourceGroupSelector: nodeClass.Spec.OriginalHostResourceGroupSelector, - Tags: nodeClass.Spec.Tags, + AMIFamily: nodeClass.Spec.AMIFamily, + Context: nodeClass.Spec.Context, + InstanceProfile: nodeClass.Spec.InstanceProfile, + SubnetSelector: nodeClass.Spec.OriginalSubnetSelector, + SecurityGroupSelector: nodeClass.Spec.OriginalSecurityGroupSelector, + Tags: nodeClass.Spec.Tags, LaunchTemplate: v1alpha1.LaunchTemplate{ LaunchTemplateName: nodeClass.Spec.LaunchTemplateName, MetadataOptions: NewMetadataOptions(nodeClass.Spec.MetadataOptions), @@ -46,23 +44,13 @@ func New(nodeClass *v1beta1.EC2NodeClass) *v1alpha1.AWSNodeTemplate { DetailedMonitoring: nodeClass.Spec.DetailedMonitoring, }, Status: v1alpha1.AWSNodeTemplateStatus{ - Subnets: NewSubnets(nodeClass.Status.Subnets), - SecurityGroups: NewSecurityGroups(nodeClass.Status.SecurityGroups), - AMIs: NewAMIs(nodeClass.Status.AMIs), - Licenses: nodeClass.Status.Licenses, - HostResourceGroups: NewHostResourceGroups(nodeClass.Status.HostResourceGroup), - PlacementGroups: nodeClass.Status.PlacementGroups, + Subnets: NewSubnets(nodeClass.Status.Subnets), + SecurityGroups: NewSecurityGroups(nodeClass.Status.SecurityGroups), + AMIs: NewAMIs(nodeClass.Status.AMIs), }, } } -func NewHostResourceGroups(hrg *v1beta1.HostResourceGroup) []string { - if hrg == nil { - return nil - } - return []string{hrg.ARN} -} - func NewBlockDeviceMappings(bdms []*v1beta1.BlockDeviceMapping) []*v1alpha1.BlockDeviceMapping { if bdms == nil { return nil From 914e187d1eee034dff9a946d0830c395d55a2018 Mon Sep 17 00:00:00 2001 From: Sebastian Cole Date: Mon, 9 Oct 2023 11:48:31 +1100 Subject: [PATCH 31/33] Updating documentation --- .../en/preview/concepts/nodeclasses.md | 89 ++++++++++++++++++- .../en/v0.31/concepts/node-templates.md | 65 +++----------- 2 files changed, 99 insertions(+), 55 deletions(-) diff --git a/website/content/en/preview/concepts/nodeclasses.md b/website/content/en/preview/concepts/nodeclasses.md index ca668aef11f0..20b608bc89bd 100644 --- a/website/content/en/preview/concepts/nodeclasses.md +++ b/website/content/en/preview/concepts/nodeclasses.md @@ -80,6 +80,14 @@ spec: # optional, configures detailed monitoring for the instance detailedMonitoring: true + + # optional, configures launch template settings related to host placement + hostResourceGroupSelectorTerms: + - name: my-hrg + licenseSelectorTerms: + - name: my-license + placementGroupSelectorTerms: + - name: my-placement-group status: # resolved subnets subnets: @@ -122,6 +130,15 @@ status: # generated instance profile name instanceProfile: "${CLUSTER_NAME}-0123456778901234567789" + + # resolved placement information + licenses: + - arn:aws:license-manager:us-west-2:111122223333:license/lic-123456789 + hostResourceGroup: + name: my-hrg + arn:aws:resource-groups:us-west-2:111122223333:group/my-hrg + placementGroup: + - arn:aws:ec2:us-west-2:111122223333:group/my-placement-group ``` Refer to the [NodePool docs]({{}}) for settings applicable to all providers. To explore various `EC2NodeClass` configurations, refer to the examples provided [in the Karpenter Github repository](https://github.com/aws/karpenter/blob/main/examples/nodepool/). @@ -745,6 +762,37 @@ spec: detailedMonitoring: true ``` + +## spec.hostResourceGroupsSelectorTerms + +Enables use of Host Resource Groups, primarily for using dedicated hosts. Groups are matched on results from the [AWS ResourceGroups API](https://docs.aws.amazon.com/ARG/latest/APIReference/API_ListGroups.html) +```yaml +spec: + hostResourceGroupSelectorTerms: + - name: my-hrg +``` + + +## spec.licenseSelectorTerms +Enables use of License Configuration Specifications, primarily for using dedicated hosts. Groups are matched on results from the [AWS LicenseManager API](https://docs.aws.amazon.com/license-manager/latest/APIReference/API_ListLicenses.html) + +```yaml +spec: + licenseSelectorTerms: + - name: my-license +``` + +## spec.placementGroupSelectorTerms +Enables use of EC2 placement groups. Groups are matched on results from the [AWS EC2 API](https://docs.aws.amazon.com/AWSEC2/latest/APIReference/API_DescribePlacementGroups.html) + +```yaml +spec: + placementGroupSelectorTerms: + - name: my-placement-group +``` + + + ## status.subnets [`status.subnets`]({{< ref "#statussubnets" >}}) contains the resolved `id` and `zone` of the subnets that were selected by the [`spec.subnetSelectorTerms`]({{< ref "#specsubnetselectorterms" >}}) for the node class. The subnets will be sorted by the available IP address count in decreasing order. @@ -880,4 +928,43 @@ spec: role: "KarpenterNodeRole-${CLUSTER_NAME}" status: instanceProfile: "${CLUSTER_NAME}-0123456778901234567789" -``` \ No newline at end of file +``` + +## status.hostResourceGroup + +[`status.hostResourceGroup`]({{< ref "#statushostResourceGroup" >}}) containts the resolved host resource groups specified in the [`spec.hostResourceGroupSelectorTerms`]({{< ref "#spechostResourceGroupSelectorTerms" >}}) + +```yaml +spec: + hostResourceGroupSelectorTerms: + - name: my-hrg +status: + hostResourceGroup: + name: my-hrg + arn:aws:resource-groups:us-west-2:111122223333:group/my-hrg +``` + + +## status.licenses +[`status.licenses`]({{< ref "#statuslicenses" >}}) containts the resolved license configuration specifications as specified in the [`spec.licenseSelectorTerms`]({{< ref "#speclicenseSelectorTerms" >}}) + +```yaml +spec: + licenseSelectorTerms: + - name: my-license +status: + licenses: + - arn:aws:license-manager:us-west-2:111122223333:license/lic-12345689 +``` + +## status.placementGroups +[`status.placementGroups`]({{< ref "#statusplacementGroups" >}}) containts the resolved EC2 placement groups as specified in the [`spec.placementGroupSelectorTerms`]({{< ref "#specplacementGroupSelectorTerms" >}}) + +```yaml +spec: + placementGroupSelectorTerms: + - name: my-placement-group +status: + placementGroups: + - arn:aws:ec2:us-west-2:111122223333:group/my-placement-group +``` diff --git a/website/content/en/v0.31/concepts/node-templates.md b/website/content/en/v0.31/concepts/node-templates.md index 159ad995c76a..ecab793e0984 100644 --- a/website/content/en/v0.31/concepts/node-templates.md +++ b/website/content/en/v0.31/concepts/node-templates.md @@ -24,27 +24,20 @@ kind: AWSNodeTemplate metadata: name: default spec: - subnetSelector: { ... } # required, discovers tagged subnets to attach to instances - securityGroupSelector: { ... } # required, discovers tagged security groups to attach to instances - instanceProfile: "..." # optional, overrides the node's identity from global settings - amiFamily: "..." # optional, resolves a default ami and userdata - amiSelector: { ... } # optional, discovers tagged amis to override the amiFamily's default - userData: "..." # optional, overrides autogenerated userdata with a merge semantic - tags: { ... } # optional, propagates tags to underlying EC2 resources - metadataOptions: { ... } # optional, configures IMDS for the instance - blockDeviceMappings: [ ... ] # optional, configures storage devices for the instance - detailedMonitoring: "..." # optional, configures detailed monitoring for the instance - licenseSelector: { ... } #optional, discovers resource group license configurations - hostResourceGroupSelector: { ... } # optional, discovers resource groups to launch instances into - placementGroupSelector: { ... } # optional, discovers resource groups to launch instances into - + subnetSelector: { ... } # required, discovers tagged subnets to attach to instances + securityGroupSelector: { ... } # required, discovers tagged security groups to attach to instances + instanceProfile: "..." # optional, overrides the node's identity from global settings + amiFamily: "..." # optional, resolves a default ami and userdata + amiSelector: { ... } # optional, discovers tagged amis to override the amiFamily's default + userData: "..." # optional, overrides autogenerated userdata with a merge semantic + tags: { ... } # optional, propagates tags to underlying EC2 resources + metadataOptions: { ... } # optional, configures IMDS for the instance + blockDeviceMappings: [ ... ] # optional, configures storage devices for the instance + detailedMonitoring: "..." # optional, configures detailed monitoring for the instance status: subnets: { ... } # resolved subnets securityGroups: { ... } # resolved security groups amis: { ... } # resolved AMIs - licenseConfigrations: { ... } # resolved licenseConfigurationSpecifications - hostResourceGroups: { ... } # resolved hostResourceGroups - placementGroups: { ... } # resolved ec2 placementGroups ``` Refer to the [Provisioner docs]({{}}) for settings applicable to all providers. To explore various `AWSNodeTemplate` configurations, refer to the examples provided [in the Karpenter Github repository](https://github.com/aws/karpenter/blob/main/examples/provisioner/). @@ -614,30 +607,6 @@ spec: detailedMonitoring: true ``` -## spec.licenseSelector - licenseSelector: { ... } #optional, discovers resource group license configurations - Example: - ``` - licenseSelector: - name: myLicense - ``` - -## spec.hostResourceGroupSelector -``` - hostResourceGroupSelector: - name: "r5-1" -``` - -## spec.placementGroupSelector - placementGroupSelector: { ... } # optional, discovers resource groups to launch instances into - ``` - - placementGroupSelector: - name: placementGroup - tags: - key: value - ``` - ## status.subnets `status.subnets` contains the `id` and `zone` of the subnets utilized during node launch. The subnets are sorted by the available IP address count in decreasing order. @@ -717,16 +686,4 @@ status: values: - aws - nvidia -``` - -## status.licenseConfigurations - -contains the arn's of the licenses used to launch instances - -## status.hostResourceGroups - -contains the arns of the host resource groups - -## status.placementGroups - -contains the arns of the ec2 placement groups +``` \ No newline at end of file From 25d1e913c8155dab5a52f03df5bf1bbddd3be345 Mon Sep 17 00:00:00 2001 From: Sebastian Cole Date: Wed, 11 Oct 2023 14:16:32 +1100 Subject: [PATCH 32/33] Commit missing generated functions --- pkg/apis/v1beta1/zz_generated.deepcopy.go | 21 --------------------- 1 file changed, 21 deletions(-) diff --git a/pkg/apis/v1beta1/zz_generated.deepcopy.go b/pkg/apis/v1beta1/zz_generated.deepcopy.go index 66b99865f9a0..630855a09637 100644 --- a/pkg/apis/v1beta1/zz_generated.deepcopy.go +++ b/pkg/apis/v1beta1/zz_generated.deepcopy.go @@ -319,27 +319,6 @@ func (in *EC2NodeClassSpec) DeepCopyInto(out *EC2NodeClassSpec) { (*out)[key] = val } } - if in.OriginalLicenseSelector != nil { - in, out := &in.OriginalLicenseSelector, &out.OriginalLicenseSelector - *out = make(map[string]string, len(*in)) - for key, val := range *in { - (*out)[key] = val - } - } - if in.OriginalHostResourceGroupSelector != nil { - in, out := &in.OriginalHostResourceGroupSelector, &out.OriginalHostResourceGroupSelector - *out = make(map[string]string, len(*in)) - for key, val := range *in { - (*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. From 85f0860a93276187361c6da83168b56d58a4c6c2 Mon Sep 17 00:00:00 2001 From: Sebastian Cole Date: Fri, 3 Nov 2023 11:45:00 +1100 Subject: [PATCH 33/33] feedback: Remove HRG Name, use ARN only --- pkg/apis/crds/karpenter.k8s.aws_ec2nodeclasses.yaml | 3 --- pkg/apis/v1beta1/ec2nodeclass_status.go | 3 --- pkg/providers/amifamily/resolver.go | 2 +- .../hostresourcegroup/hostresourcegroup.go | 2 +- pkg/providers/hostresourcegroup/suite_test.go | 13 +++++++------ pkg/providers/launchtemplate/launchtemplate.go | 1 + website/content/en/preview/concepts/nodeclasses.md | 6 ++---- 7 files changed, 12 insertions(+), 18 deletions(-) diff --git a/pkg/apis/crds/karpenter.k8s.aws_ec2nodeclasses.yaml b/pkg/apis/crds/karpenter.k8s.aws_ec2nodeclasses.yaml index 28ba4297040e..da14b316b0f2 100644 --- a/pkg/apis/crds/karpenter.k8s.aws_ec2nodeclasses.yaml +++ b/pkg/apis/crds/karpenter.k8s.aws_ec2nodeclasses.yaml @@ -483,9 +483,6 @@ spec: arn: description: Arn of the HRG type: string - name: - description: Name of the HRG - type: string type: object instanceProfile: description: InstanceProfile contains the resolved instance profile diff --git a/pkg/apis/v1beta1/ec2nodeclass_status.go b/pkg/apis/v1beta1/ec2nodeclass_status.go index b44a9a7cbb60..5110dd7acc9c 100644 --- a/pkg/apis/v1beta1/ec2nodeclass_status.go +++ b/pkg/apis/v1beta1/ec2nodeclass_status.go @@ -51,9 +51,6 @@ 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"` diff --git a/pkg/providers/amifamily/resolver.go b/pkg/providers/amifamily/resolver.go index 8d004efa78bd..d14067ec9ec1 100644 --- a/pkg/providers/amifamily/resolver.go +++ b/pkg/providers/amifamily/resolver.go @@ -205,7 +205,7 @@ func (r Resolver) resolvePlacement(nodeClass *v1beta1.EC2NodeClass) *Placement { } if hrg != nil { - placement.HostResourceGroup = hrg.Name + placement.HostResourceGroup = hrg.ARN } } diff --git a/pkg/providers/hostresourcegroup/hostresourcegroup.go b/pkg/providers/hostresourcegroup/hostresourcegroup.go index 041a9ca8f60c..c70aa4183c30 100644 --- a/pkg/providers/hostresourcegroup/hostresourcegroup.go +++ b/pkg/providers/hostresourcegroup/hostresourcegroup.go @@ -70,7 +70,7 @@ func (p *Provider) Get(ctx context.Context, nodeClass *v1beta1.EC2NodeClass) (*v 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} + match = &v1beta1.HostResourceGroup{ARN: *page.GroupIdentifiers[i].GroupArn} p.cache.SetDefault(fmt.Sprint(hash), match) return false } diff --git a/pkg/providers/hostresourcegroup/suite_test.go b/pkg/providers/hostresourcegroup/suite_test.go index 8d14eba806ba..23c6178383b3 100644 --- a/pkg/providers/hostresourcegroup/suite_test.go +++ b/pkg/providers/hostresourcegroup/suite_test.go @@ -92,26 +92,27 @@ var _ = AfterEach(func() { var _ = Describe("HostResourceGroupProvider", func() { Context("Get", func() { It("should discover host resource groups by name", func() { - expected := &v1beta1.HostResourceGroup{ARN: "expected-group-arn", Name: "expected-group-name"} + expectedArn := "arn:aws:resource-groups:us-west-2:111122223333:group/my-hrg" + expectedName := "my-hrg" awsEnv.ResourceGroupsAPI.ListGroupsBehavior.Output.Set(&resourcegroups.ListGroupsOutput{ GroupIdentifiers: []*resourcegroups.GroupIdentifier{ - {GroupArn: aws.String(expected.ARN), GroupName: aws.String(expected.Name)}, + {GroupArn: aws.String(expectedArn), GroupName: aws.String(expectedName)}, }, }) nodeClass.Spec.HostResourceGroupSelectorTerms = []v1beta1.HostResourceGroupSelectorTerm{ { - Name: expected.Name, + Name: expectedName, }, } hrgs, err := awsEnv.HostResourceGroupProvider.Get(ctx, nodeClass) Expect(err).To(BeNil()) - Expect(hrgs).To(Equal(expected)) + Expect(hrgs.ARN).To(Equal(expectedArn)) }) It("should filter only matching groups", func() { - expected := &v1beta1.HostResourceGroup{ARN: "expected-group-arn", Name: "expected-group-name"} + expected := &v1beta1.HostResourceGroup{ARN: "expected-group-arn"} awsEnv.ResourceGroupsAPI.ListGroupsBehavior.Output.Set(&resourcegroups.ListGroupsOutput{ GroupIdentifiers: []*resourcegroups.GroupIdentifier{ - {GroupArn: aws.String(expected.ARN), GroupName: aws.String(expected.Name)}, + {GroupArn: aws.String(expected.ARN), GroupName: aws.String("")}, }, }) nodeClass.Spec.HostResourceGroupSelectorTerms = []v1beta1.HostResourceGroupSelectorTerm{ diff --git a/pkg/providers/launchtemplate/launchtemplate.go b/pkg/providers/launchtemplate/launchtemplate.go index 09a1f65f38a6..06d3c8a29447 100644 --- a/pkg/providers/launchtemplate/launchtemplate.go +++ b/pkg/providers/launchtemplate/launchtemplate.go @@ -282,6 +282,7 @@ func generatePlacement(placement *amifamily.Placement) *ec2.LaunchTemplatePlacem return nil } return &ec2.LaunchTemplatePlacementRequest{ + GroupName: aws.String(placement.PlacementGroup), HostResourceGroupArn: aws.String(placement.HostResourceGroup), } diff --git a/website/content/en/preview/concepts/nodeclasses.md b/website/content/en/preview/concepts/nodeclasses.md index 480ecfac38b6..5aadc938c9e9 100644 --- a/website/content/en/preview/concepts/nodeclasses.md +++ b/website/content/en/preview/concepts/nodeclasses.md @@ -135,8 +135,7 @@ status: licenses: - arn:aws:license-manager:us-west-2:111122223333:license/lic-123456789 hostResourceGroup: - name: my-hrg - arn:aws:resource-groups:us-west-2:111122223333:group/my-hrg + arn: arn:aws:resource-groups:us-west-2:111122223333:group/my-hrg placementGroup: - arn:aws:ec2:us-west-2:111122223333:group/my-placement-group ``` @@ -940,8 +939,7 @@ spec: - name: my-hrg status: hostResourceGroup: - name: my-hrg - arn:aws:resource-groups:us-west-2:111122223333:group/my-hrg + arn: arn:aws:resource-groups:us-west-2:111122223333:group/my-hrg ```