From 0b41c5bad17767fc06164e537ce239a264f66044 Mon Sep 17 00:00:00 2001 From: Amanuel Engeda Date: Sun, 31 Mar 2024 16:31:48 -0700 Subject: [PATCH] refactor nodeclass controller --- pkg/controllers/controllers.go | 8 +- pkg/controllers/nodeclass/controller.go | 316 ---- pkg/controllers/nodeclass/hash/controller.go | 117 ++ pkg/controllers/nodeclass/hash/suite_test.go | 279 ++++ pkg/controllers/nodeclass/status/ami.go | 58 + pkg/controllers/nodeclass/status/ami_test.go | 346 +++++ .../nodeclass/status/controller.go | 113 ++ .../nodeclass/status/instanceprofile.go | 44 + .../nodeclass/status/instanceprofile_test.go | 134 ++ .../nodeclass/status/launchtemplate.go | 42 + .../nodeclass/status/launchtemplate_test.go | 90 ++ .../nodeclass/status/securitygroup.go | 54 + .../nodeclass/status/securitygroup_test.go | 224 +++ pkg/controllers/nodeclass/status/subnet.go | 58 + .../nodeclass/status/subnet_test.go | 271 ++++ .../nodeclass/status/suite_test.go | 80 + pkg/controllers/nodeclass/suite_test.go | 1291 ----------------- .../nodeclass/termination/controller.go | 142 ++ .../nodeclass/{ => termination}/events.go | 2 +- .../nodeclass/termination/suite_test.go | 285 ++++ .../en/preview/upgrading/upgrade-guide.md | 4 + 21 files changed, 2348 insertions(+), 1610 deletions(-) delete mode 100644 pkg/controllers/nodeclass/controller.go create mode 100644 pkg/controllers/nodeclass/hash/controller.go create mode 100644 pkg/controllers/nodeclass/hash/suite_test.go create mode 100644 pkg/controllers/nodeclass/status/ami.go create mode 100644 pkg/controllers/nodeclass/status/ami_test.go create mode 100644 pkg/controllers/nodeclass/status/controller.go create mode 100644 pkg/controllers/nodeclass/status/instanceprofile.go create mode 100644 pkg/controllers/nodeclass/status/instanceprofile_test.go create mode 100644 pkg/controllers/nodeclass/status/launchtemplate.go create mode 100644 pkg/controllers/nodeclass/status/launchtemplate_test.go create mode 100644 pkg/controllers/nodeclass/status/securitygroup.go create mode 100644 pkg/controllers/nodeclass/status/securitygroup_test.go create mode 100644 pkg/controllers/nodeclass/status/subnet.go create mode 100644 pkg/controllers/nodeclass/status/subnet_test.go create mode 100644 pkg/controllers/nodeclass/status/suite_test.go delete mode 100644 pkg/controllers/nodeclass/suite_test.go create mode 100644 pkg/controllers/nodeclass/termination/controller.go rename pkg/controllers/nodeclass/{ => termination}/events.go (98%) create mode 100644 pkg/controllers/nodeclass/termination/suite_test.go diff --git a/pkg/controllers/controllers.go b/pkg/controllers/controllers.go index e428418123f1..c4c80843a9a5 100644 --- a/pkg/controllers/controllers.go +++ b/pkg/controllers/controllers.go @@ -19,6 +19,9 @@ import ( "sigs.k8s.io/karpenter/pkg/cloudprovider" + nodeclasshash "github.com/aws/karpenter-provider-aws/pkg/controllers/nodeclass/hash" + nodeclassstatus "github.com/aws/karpenter-provider-aws/pkg/controllers/nodeclass/status" + nodeclasstermination "github.com/aws/karpenter-provider-aws/pkg/controllers/nodeclass/termination" controllerspricing "github.com/aws/karpenter-provider-aws/pkg/controllers/pricing" "github.com/aws/karpenter-provider-aws/pkg/providers/launchtemplate" @@ -35,7 +38,6 @@ import ( "github.com/aws/karpenter-provider-aws/pkg/controllers/interruption" nodeclaimgarbagecollection "github.com/aws/karpenter-provider-aws/pkg/controllers/nodeclaim/garbagecollection" nodeclaimtagging "github.com/aws/karpenter-provider-aws/pkg/controllers/nodeclaim/tagging" - "github.com/aws/karpenter-provider-aws/pkg/controllers/nodeclass" "github.com/aws/karpenter-provider-aws/pkg/operator/options" "github.com/aws/karpenter-provider-aws/pkg/providers/amifamily" "github.com/aws/karpenter-provider-aws/pkg/providers/instance" @@ -52,7 +54,9 @@ func NewControllers(ctx context.Context, sess *session.Session, clk clock.Clock, pricingProvider pricing.Provider, amiProvider amifamily.Provider, launchTemplateProvider launchtemplate.Provider) []controller.Controller { controllers := []controller.Controller{ - nodeclass.NewController(kubeClient, recorder, subnetProvider, securityGroupProvider, amiProvider, instanceProfileProvider, launchTemplateProvider), + nodeclasshash.NewController(kubeClient), + nodeclassstatus.NewController(kubeClient, subnetProvider, securityGroupProvider, amiProvider, instanceProfileProvider, launchTemplateProvider), + nodeclasstermination.NewController(kubeClient, recorder, instanceProfileProvider, launchTemplateProvider), nodeclaimgarbagecollection.NewController(kubeClient, cloudProvider), nodeclaimtagging.NewController(kubeClient, instanceProvider), controllerspricing.NewController(pricingProvider), diff --git a/pkg/controllers/nodeclass/controller.go b/pkg/controllers/nodeclass/controller.go deleted file mode 100644 index d901db019582..000000000000 --- a/pkg/controllers/nodeclass/controller.go +++ /dev/null @@ -1,316 +0,0 @@ -/* -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 nodeclass - -import ( - "context" - "fmt" - "sort" - "time" - - "k8s.io/apimachinery/pkg/api/errors" - - "github.com/aws/karpenter-provider-aws/pkg/providers/launchtemplate" - - "github.com/aws/aws-sdk-go/service/ec2" - "github.com/samber/lo" - "go.uber.org/multierr" - "golang.org/x/time/rate" - "k8s.io/apimachinery/pkg/api/equality" - "k8s.io/apimachinery/pkg/types" - "k8s.io/client-go/util/workqueue" - controllerruntime "sigs.k8s.io/controller-runtime" - "sigs.k8s.io/controller-runtime/pkg/builder" - "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/controller" - "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" - "sigs.k8s.io/controller-runtime/pkg/event" - "sigs.k8s.io/controller-runtime/pkg/handler" - "sigs.k8s.io/controller-runtime/pkg/manager" - "sigs.k8s.io/controller-runtime/pkg/predicate" - "sigs.k8s.io/controller-runtime/pkg/reconcile" - - corev1beta1 "sigs.k8s.io/karpenter/pkg/apis/v1beta1" - "sigs.k8s.io/karpenter/pkg/events" - corecontroller "sigs.k8s.io/karpenter/pkg/operator/controller" - - "github.com/aws/karpenter-provider-aws/pkg/apis/v1beta1" - "github.com/aws/karpenter-provider-aws/pkg/providers/amifamily" - "github.com/aws/karpenter-provider-aws/pkg/providers/instanceprofile" - "github.com/aws/karpenter-provider-aws/pkg/providers/securitygroup" - "github.com/aws/karpenter-provider-aws/pkg/providers/subnet" -) - -var _ corecontroller.FinalizingTypedController[*v1beta1.EC2NodeClass] = (*Controller)(nil) - -type Controller struct { - kubeClient client.Client - recorder events.Recorder - - subnetProvider subnet.Provider - securityGroupProvider securitygroup.Provider - amiProvider amifamily.Provider - instanceProfileProvider instanceprofile.Provider - launchTemplateProvider launchtemplate.Provider -} - -func NewController(kubeClient client.Client, recorder events.Recorder, subnetProvider subnet.Provider, securityGroupProvider securitygroup.Provider, - amiProvider amifamily.Provider, instanceProfileProvider instanceprofile.Provider, launchTemplateProvider launchtemplate.Provider) corecontroller.Controller { - - return corecontroller.Typed[*v1beta1.EC2NodeClass](kubeClient, &Controller{ - kubeClient: kubeClient, - recorder: recorder, - - subnetProvider: subnetProvider, - securityGroupProvider: securityGroupProvider, - amiProvider: amiProvider, - instanceProfileProvider: instanceProfileProvider, - launchTemplateProvider: launchTemplateProvider, - }) -} - -func (c *Controller) Reconcile(ctx context.Context, nodeClass *v1beta1.EC2NodeClass) (reconcile.Result, error) { - stored := nodeClass.DeepCopy() - controllerutil.AddFinalizer(nodeClass, v1beta1.TerminationFinalizer) - - if nodeClass.Annotations[v1beta1.AnnotationEC2NodeClassHashVersion] != v1beta1.EC2NodeClassHashVersion { - if err := c.updateNodeClaimHash(ctx, nodeClass); err != nil { - return reconcile.Result{}, err - } - } - nodeClass.Annotations = lo.Assign(nodeClass.Annotations, map[string]string{ - v1beta1.AnnotationEC2NodeClassHash: nodeClass.Hash(), - v1beta1.AnnotationEC2NodeClassHashVersion: v1beta1.EC2NodeClassHashVersion, - }) - - errs := multierr.Combine( - c.resolveSubnets(ctx, nodeClass), - c.resolveSecurityGroups(ctx, nodeClass), - c.resolveAMIs(ctx, nodeClass), - c.resolveInstanceProfile(ctx, nodeClass), - ) - if lo.FromPtr(nodeClass.Spec.AMIFamily) == v1beta1.AMIFamilyAL2023 { - if err := c.launchTemplateProvider.ResolveClusterCIDR(ctx); err != nil { - errs = multierr.Append(errs, fmt.Errorf("resolving cluster CIDR, %w", err)) - } - } - if !equality.Semantic.DeepEqual(stored, nodeClass) { - statusCopy := nodeClass.DeepCopy() - if err := c.kubeClient.Patch(ctx, nodeClass, client.MergeFrom(stored)); err != nil { - errs = multierr.Append(errs, client.IgnoreNotFound(err)) - } - if err := c.kubeClient.Status().Patch(ctx, statusCopy, client.MergeFrom(stored)); err != nil { - errs = multierr.Append(errs, client.IgnoreNotFound(err)) - } - } - if errs != nil { - return reconcile.Result{}, errs - } - return reconcile.Result{RequeueAfter: 5 * time.Minute}, nil -} - -func (c *Controller) Finalize(ctx context.Context, nodeClass *v1beta1.EC2NodeClass) (reconcile.Result, error) { - stored := nodeClass.DeepCopy() - if !controllerutil.ContainsFinalizer(nodeClass, v1beta1.TerminationFinalizer) { - return reconcile.Result{}, nil - } - nodeClaimList := &corev1beta1.NodeClaimList{} - if err := c.kubeClient.List(ctx, nodeClaimList, client.MatchingFields{"spec.nodeClassRef.name": nodeClass.Name}); err != nil { - return reconcile.Result{}, fmt.Errorf("listing nodeclaims that are using nodeclass, %w", err) - } - if len(nodeClaimList.Items) > 0 { - c.recorder.Publish(WaitingOnNodeClaimTerminationEvent(nodeClass, lo.Map(nodeClaimList.Items, func(nc corev1beta1.NodeClaim, _ int) string { return nc.Name }))) - return reconcile.Result{RequeueAfter: time.Minute * 10}, nil // periodically fire the event - } - if nodeClass.Spec.Role != "" { - if err := c.instanceProfileProvider.Delete(ctx, nodeClass); err != nil { - return reconcile.Result{}, fmt.Errorf("deleting instance profile, %w", err) - } - } - if err := c.launchTemplateProvider.DeleteAll(ctx, nodeClass); err != nil { - return reconcile.Result{}, fmt.Errorf("deleting launch templates, %w", err) - } - controllerutil.RemoveFinalizer(nodeClass, v1beta1.TerminationFinalizer) - if !equality.Semantic.DeepEqual(stored, nodeClass) { - // We call Update() here rather than Patch() because patching a list with a JSON merge patch - // can cause races due to the fact that it fully replaces the list on a change - // https://github.com/kubernetes/kubernetes/issues/111643#issuecomment-2016489732 - if err := c.kubeClient.Update(ctx, nodeClass); err != nil { - if errors.IsConflict(err) { - return reconcile.Result{Requeue: true}, nil - } - return reconcile.Result{}, client.IgnoreNotFound(fmt.Errorf("removing termination finalizer, %w", err)) - } - } - return reconcile.Result{}, nil -} - -func (c *Controller) resolveSubnets(ctx context.Context, nodeClass *v1beta1.EC2NodeClass) error { - subnets, err := c.subnetProvider.List(ctx, nodeClass) - if err != nil { - return err - } - if len(subnets) == 0 { - nodeClass.Status.Subnets = nil - return fmt.Errorf("no subnets exist given constraints %v", nodeClass.Spec.SubnetSelectorTerms) - } - sort.Slice(subnets, func(i, j int) bool { - if int(*subnets[i].AvailableIpAddressCount) != int(*subnets[j].AvailableIpAddressCount) { - return int(*subnets[i].AvailableIpAddressCount) > int(*subnets[j].AvailableIpAddressCount) - } - return *subnets[i].SubnetId < *subnets[j].SubnetId - }) - nodeClass.Status.Subnets = lo.Map(subnets, func(ec2subnet *ec2.Subnet, _ int) v1beta1.Subnet { - return v1beta1.Subnet{ - ID: *ec2subnet.SubnetId, - Zone: *ec2subnet.AvailabilityZone, - } - }) - return nil -} - -func (c *Controller) resolveSecurityGroups(ctx context.Context, nodeClass *v1beta1.EC2NodeClass) error { - securityGroups, err := c.securityGroupProvider.List(ctx, nodeClass) - if err != nil { - return err - } - if len(securityGroups) == 0 && len(nodeClass.Spec.SecurityGroupSelectorTerms) > 0 { - nodeClass.Status.SecurityGroups = nil - return fmt.Errorf("no security groups exist given constraints") - } - sort.Slice(securityGroups, func(i, j int) bool { - return *securityGroups[i].GroupId < *securityGroups[j].GroupId - }) - nodeClass.Status.SecurityGroups = lo.Map(securityGroups, func(securityGroup *ec2.SecurityGroup, _ int) v1beta1.SecurityGroup { - return v1beta1.SecurityGroup{ - ID: *securityGroup.GroupId, - Name: *securityGroup.GroupName, - } - }) - return nil -} - -func (c *Controller) resolveAMIs(ctx context.Context, nodeClass *v1beta1.EC2NodeClass) error { - amis, err := c.amiProvider.Get(ctx, nodeClass, &amifamily.Options{}) - if err != nil { - return err - } - if len(amis) == 0 { - nodeClass.Status.AMIs = nil - return fmt.Errorf("no amis exist given constraints") - } - nodeClass.Status.AMIs = lo.Map(amis, func(ami amifamily.AMI, _ int) v1beta1.AMI { - reqs := ami.Requirements.NodeSelectorRequirements() - sort.Slice(reqs, func(i, j int) bool { - if len(reqs[i].Key) != len(reqs[j].Key) { - return len(reqs[i].Key) < len(reqs[j].Key) - } - return reqs[i].Key < reqs[j].Key - }) - return v1beta1.AMI{ - Name: ami.Name, - ID: ami.AmiID, - Requirements: reqs, - } - }) - - return nil -} - -func (c *Controller) resolveInstanceProfile(ctx context.Context, nodeClass *v1beta1.EC2NodeClass) error { - if nodeClass.Spec.Role != "" { - name, err := c.instanceProfileProvider.Create(ctx, nodeClass) - if err != nil { - return fmt.Errorf("creating instance profile, %w", err) - } - nodeClass.Status.InstanceProfile = name - } else { - nodeClass.Status.InstanceProfile = lo.FromPtr(nodeClass.Spec.InstanceProfile) - } - return nil -} - -// Updating `ec2nodeclass-hash-version` annotation inside the karpenter controller means a breaking change has been made to the hash calculation. -// `ec2nodeclass-hash` annotation on the EC2NodeClass will be updated, due to the breaking change, making the `ec2nodeclass-hash` on the NodeClaim different from -// EC2NodeClass. Since, we cannot rely on the `ec2nodeclass-hash` on the NodeClaims, due to the breaking change, we will need to re-calculate the hash and update the annotation. -// For more information on the Drift Hash Versioning: https://github.com/kubernetes-sigs/karpenter/blob/main/designs/drift-hash-versioning.md -func (c *Controller) updateNodeClaimHash(ctx context.Context, nodeClass *v1beta1.EC2NodeClass) error { - ncList := &corev1beta1.NodeClaimList{} - if err := c.kubeClient.List(ctx, ncList, client.MatchingFields{"spec.nodeClassRef.name": nodeClass.Name}); err != nil { - return err - } - - errs := make([]error, len(ncList.Items)) - for i := range ncList.Items { - nc := ncList.Items[i] - stored := nc.DeepCopy() - - if nc.Annotations[v1beta1.AnnotationEC2NodeClassHashVersion] != v1beta1.EC2NodeClassHashVersion { - nc.Annotations = lo.Assign(nc.Annotations, map[string]string{ - v1beta1.AnnotationEC2NodeClassHashVersion: v1beta1.EC2NodeClassHashVersion, - }) - - // Any NodeClaim that is already drifted will remain drifted if the karpenter.k8s.aws/nodepool-hash-version doesn't match - // Since the hashing mechanism has changed we will not be able to determine if the drifted status of the NodeClaim has changed - if nc.StatusConditions().GetCondition(corev1beta1.Drifted) == nil { - nc.Annotations = lo.Assign(nc.Annotations, map[string]string{ - v1beta1.AnnotationEC2NodeClassHash: nodeClass.Hash(), - }) - } - - if !equality.Semantic.DeepEqual(stored, nc) { - if err := c.kubeClient.Patch(ctx, &nc, client.MergeFrom(stored)); err != nil { - errs[i] = client.IgnoreNotFound(err) - } - } - } - } - - return multierr.Combine(errs...) -} - -func (c *Controller) Name() string { - return "nodeclass" -} - -func (c *Controller) Builder(_ context.Context, m manager.Manager) corecontroller.Builder { - return corecontroller.Adapt(controllerruntime. - NewControllerManagedBy(m). - For(&v1beta1.EC2NodeClass{}). - Watches( - &corev1beta1.NodeClaim{}, - handler.EnqueueRequestsFromMapFunc(func(_ context.Context, o client.Object) []reconcile.Request { - nc := o.(*corev1beta1.NodeClaim) - if nc.Spec.NodeClassRef == nil { - return nil - } - return []reconcile.Request{{NamespacedName: types.NamespacedName{Name: nc.Spec.NodeClassRef.Name}}} - }), - // Watch for NodeClaim deletion events - builder.WithPredicates(predicate.Funcs{ - CreateFunc: func(e event.CreateEvent) bool { return false }, - UpdateFunc: func(e event.UpdateEvent) bool { return false }, - DeleteFunc: func(e event.DeleteEvent) bool { return true }, - }), - ). - WithOptions(controller.Options{ - RateLimiter: workqueue.NewMaxOfRateLimiter( - workqueue.NewItemExponentialFailureRateLimiter(100*time.Millisecond, 1*time.Minute), - // 10 qps, 100 bucket size - &workqueue.BucketRateLimiter{Limiter: rate.NewLimiter(rate.Limit(10), 100)}, - ), - MaxConcurrentReconciles: 10, - })) -} diff --git a/pkg/controllers/nodeclass/hash/controller.go b/pkg/controllers/nodeclass/hash/controller.go new file mode 100644 index 000000000000..8cc70c28c7bb --- /dev/null +++ b/pkg/controllers/nodeclass/hash/controller.go @@ -0,0 +1,117 @@ +/* +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 hash + +import ( + "context" + + "github.com/samber/lo" + "go.uber.org/multierr" + "k8s.io/apimachinery/pkg/api/equality" + controllerruntime "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller" + "sigs.k8s.io/controller-runtime/pkg/manager" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + corev1beta1 "sigs.k8s.io/karpenter/pkg/apis/v1beta1" + corecontroller "sigs.k8s.io/karpenter/pkg/operator/controller" + + "github.com/aws/karpenter-provider-aws/pkg/apis/v1beta1" +) + +var _ corecontroller.TypedController[*v1beta1.EC2NodeClass] = (*Controller)(nil) + +type Controller struct { + kubeClient client.Client +} + +func NewController(kubeClient client.Client) corecontroller.Controller { + return corecontroller.Typed[*v1beta1.EC2NodeClass](kubeClient, &Controller{ + kubeClient: kubeClient, + }) +} + +func (c *Controller) Reconcile(ctx context.Context, nodeClass *v1beta1.EC2NodeClass) (reconcile.Result, error) { + stored := nodeClass.DeepCopy() + + if nodeClass.Annotations[v1beta1.AnnotationEC2NodeClassHashVersion] != v1beta1.EC2NodeClassHashVersion { + if err := c.updateNodeClaimHash(ctx, nodeClass); err != nil { + return reconcile.Result{}, err + } + } + nodeClass.Annotations = lo.Assign(nodeClass.Annotations, map[string]string{ + v1beta1.AnnotationEC2NodeClassHash: nodeClass.Hash(), + v1beta1.AnnotationEC2NodeClassHashVersion: v1beta1.EC2NodeClassHashVersion, + }) + + if !equality.Semantic.DeepEqual(stored, nodeClass) { + if err := c.kubeClient.Patch(ctx, nodeClass, client.MergeFrom(stored)); err != nil { + return reconcile.Result{}, err + } + } + + return reconcile.Result{}, nil +} + +func (c *Controller) Name() string { + return "nodeclass.hash" +} + +func (c *Controller) Builder(_ context.Context, m manager.Manager) corecontroller.Builder { + return corecontroller.Adapt(controllerruntime. + NewControllerManagedBy(m). + For(&v1beta1.EC2NodeClass{}). + WithOptions(controller.Options{MaxConcurrentReconciles: 10})) +} + +// Updating `ec2nodeclass-hash-version` annotation inside the karpenter controller means a breaking change has been made to the hash calculation. +// `ec2nodeclass-hash` annotation on the EC2NodeClass will be updated, due to the breaking change, making the `ec2nodeclass-hash` on the NodeClaim different from +// EC2NodeClass. Since, we cannot rely on the `ec2nodeclass-hash` on the NodeClaims, due to the breaking change, we will need to re-calculate the hash and update the annotation. +// For more information on the Drift Hash Versioning: https://github.com/kubernetes-sigs/karpenter/blob/main/designs/drift-hash-versioning.md +func (c *Controller) updateNodeClaimHash(ctx context.Context, nodeClass *v1beta1.EC2NodeClass) error { + ncList := &corev1beta1.NodeClaimList{} + if err := c.kubeClient.List(ctx, ncList, client.MatchingFields{"spec.nodeClassRef.name": nodeClass.Name}); err != nil { + return err + } + + errs := make([]error, len(ncList.Items)) + for i := range ncList.Items { + nc := ncList.Items[i] + stored := nc.DeepCopy() + + if nc.Annotations[v1beta1.AnnotationEC2NodeClassHashVersion] != v1beta1.EC2NodeClassHashVersion { + nc.Annotations = lo.Assign(nc.Annotations, map[string]string{ + v1beta1.AnnotationEC2NodeClassHashVersion: v1beta1.EC2NodeClassHashVersion, + }) + + // Any NodeClaim that is already drifted will remain drifted if the karpenter.k8s.aws/nodepool-hash-version doesn't match + // Since the hashing mechanism has changed we will not be able to determine if the drifted status of the NodeClaim has changed + if nc.StatusConditions().GetCondition(corev1beta1.Drifted) == nil { + nc.Annotations = lo.Assign(nc.Annotations, map[string]string{ + v1beta1.AnnotationEC2NodeClassHash: nodeClass.Hash(), + }) + } + + if !equality.Semantic.DeepEqual(stored, nc) { + if err := c.kubeClient.Patch(ctx, &nc, client.MergeFrom(stored)); err != nil { + errs[i] = client.IgnoreNotFound(err) + } + } + } + } + + return multierr.Combine(errs...) +} diff --git a/pkg/controllers/nodeclass/hash/suite_test.go b/pkg/controllers/nodeclass/hash/suite_test.go new file mode 100644 index 000000000000..a1e6bf19ff7f --- /dev/null +++ b/pkg/controllers/nodeclass/hash/suite_test.go @@ -0,0 +1,279 @@ +/* +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 hash_test + +import ( + "context" + "testing" + + "github.com/imdario/mergo" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + _ "knative.dev/pkg/system/testing" + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/aws/aws-sdk-go/aws" + corev1beta1 "sigs.k8s.io/karpenter/pkg/apis/v1beta1" + corecontroller "sigs.k8s.io/karpenter/pkg/operator/controller" + coreoptions "sigs.k8s.io/karpenter/pkg/operator/options" + "sigs.k8s.io/karpenter/pkg/operator/scheme" + coretest "sigs.k8s.io/karpenter/pkg/test" + + "github.com/aws/karpenter-provider-aws/pkg/apis" + "github.com/aws/karpenter-provider-aws/pkg/apis/v1beta1" + "github.com/aws/karpenter-provider-aws/pkg/controllers/nodeclass/hash" + "github.com/aws/karpenter-provider-aws/pkg/operator/options" + "github.com/aws/karpenter-provider-aws/pkg/test" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + . "knative.dev/pkg/logging/testing" + . "sigs.k8s.io/karpenter/pkg/test/expectations" +) + +var ctx context.Context +var env *coretest.Environment +var awsEnv *test.Environment +var hashController corecontroller.Controller + +func TestAPIs(t *testing.T) { + ctx = TestContextWithLogger(t) + RegisterFailHandler(Fail) + RunSpecs(t, "EC2NodeClass") +} + +var _ = BeforeSuite(func() { + env = coretest.NewEnvironment(scheme.Scheme, coretest.WithCRDs(apis.CRDs...), coretest.WithFieldIndexers(test.EC2NodeClassFieldIndexer(ctx))) + ctx = coreoptions.ToContext(ctx, coretest.Options()) + ctx = options.ToContext(ctx, test.Options()) + awsEnv = test.NewEnvironment(ctx, env) + + hashController = hash.NewController(env.Client) +}) + +var _ = AfterSuite(func() { + Expect(env.Stop()).To(Succeed(), "Failed to stop environment") +}) + +var _ = BeforeEach(func() { + ctx = coreoptions.ToContext(ctx, coretest.Options()) + awsEnv.Reset() +}) + +var _ = AfterEach(func() { + ExpectCleanedUp(ctx, env.Client) +}) + +var _ = Describe("NodeClass Hash Controller", func() { + var nodeClass *v1beta1.EC2NodeClass + BeforeEach(func() { + nodeClass = test.EC2NodeClass(v1beta1.EC2NodeClass{ + Spec: v1beta1.EC2NodeClassSpec{ + SubnetSelectorTerms: []v1beta1.SubnetSelectorTerm{ + { + Tags: map[string]string{"*": "*"}, + }, + }, + SecurityGroupSelectorTerms: []v1beta1.SecurityGroupSelectorTerm{ + { + Tags: map[string]string{"*": "*"}, + }, + }, + AMISelectorTerms: []v1beta1.AMISelectorTerm{ + { + Tags: map[string]string{"*": "*"}, + }, + }, + }, + }) + }) + DescribeTable("should update the drift hash when static field is updated", func(changes *v1beta1.EC2NodeClass) { + ExpectApplied(ctx, env.Client, nodeClass) + ExpectReconcileSucceeded(ctx, hashController, client.ObjectKeyFromObject(nodeClass)) + nodeClass = ExpectExists(ctx, env.Client, nodeClass) + + expectedHash := nodeClass.Hash() + Expect(nodeClass.ObjectMeta.Annotations[v1beta1.AnnotationEC2NodeClassHash]).To(Equal(expectedHash)) + + Expect(mergo.Merge(nodeClass, changes, mergo.WithOverride)).To(Succeed()) + + ExpectApplied(ctx, env.Client, nodeClass) + ExpectReconcileSucceeded(ctx, hashController, client.ObjectKeyFromObject(nodeClass)) + nodeClass = ExpectExists(ctx, env.Client, nodeClass) + + expectedHashTwo := nodeClass.Hash() + Expect(nodeClass.Annotations[v1beta1.AnnotationEC2NodeClassHash]).To(Equal(expectedHashTwo)) + Expect(expectedHash).ToNot(Equal(expectedHashTwo)) + + }, + Entry("AMIFamily Drift", &v1beta1.EC2NodeClass{Spec: v1beta1.EC2NodeClassSpec{AMIFamily: aws.String(v1beta1.AMIFamilyBottlerocket)}}), + Entry("UserData Drift", &v1beta1.EC2NodeClass{Spec: v1beta1.EC2NodeClassSpec{UserData: aws.String("userdata-test-2")}}), + Entry("Tags Drift", &v1beta1.EC2NodeClass{Spec: v1beta1.EC2NodeClassSpec{Tags: map[string]string{"keyTag-test-3": "valueTag-test-3"}}}), + Entry("BlockDeviceMappings Drift", &v1beta1.EC2NodeClass{Spec: v1beta1.EC2NodeClassSpec{BlockDeviceMappings: []*v1beta1.BlockDeviceMapping{{DeviceName: aws.String("map-device-test-3")}}}}), + Entry("DetailedMonitoring Drift", &v1beta1.EC2NodeClass{Spec: v1beta1.EC2NodeClassSpec{DetailedMonitoring: aws.Bool(true)}}), + Entry("MetadataOptions Drift", &v1beta1.EC2NodeClass{Spec: v1beta1.EC2NodeClassSpec{MetadataOptions: &v1beta1.MetadataOptions{HTTPEndpoint: aws.String("disabled")}}}), + Entry("Context Drift", &v1beta1.EC2NodeClass{Spec: v1beta1.EC2NodeClassSpec{Context: aws.String("context-2")}}), + ) + It("should not update the drift hash when dynamic field is updated", func() { + ExpectApplied(ctx, env.Client, nodeClass) + ExpectReconcileSucceeded(ctx, hashController, client.ObjectKeyFromObject(nodeClass)) + nodeClass = ExpectExists(ctx, env.Client, nodeClass) + + expectedHash := nodeClass.Hash() + Expect(nodeClass.Annotations[v1beta1.AnnotationEC2NodeClassHash]).To(Equal(expectedHash)) + + nodeClass.Spec.SubnetSelectorTerms = []v1beta1.SubnetSelectorTerm{ + { + ID: "subnet-test1", + }, + } + nodeClass.Spec.SecurityGroupSelectorTerms = []v1beta1.SecurityGroupSelectorTerm{ + { + ID: "sg-test1", + }, + } + nodeClass.Spec.AMISelectorTerms = []v1beta1.AMISelectorTerm{ + { + Tags: map[string]string{"ami-test-key": "ami-test-value"}, + }, + } + + ExpectApplied(ctx, env.Client, nodeClass) + ExpectReconcileSucceeded(ctx, hashController, client.ObjectKeyFromObject(nodeClass)) + nodeClass = ExpectExists(ctx, env.Client, nodeClass) + Expect(nodeClass.Annotations[v1beta1.AnnotationEC2NodeClassHash]).To(Equal(expectedHash)) + }) + It("should update ec2nodeclass-hash-version annotation when the ec2nodeclass-hash-version on the NodeClass does not match with the controller hash version", func() { + nodeClass.Annotations = map[string]string{ + v1beta1.AnnotationEC2NodeClassHash: "abceduefed", + v1beta1.AnnotationEC2NodeClassHashVersion: "test", + } + ExpectApplied(ctx, env.Client, nodeClass) + + ExpectReconcileSucceeded(ctx, hashController, client.ObjectKeyFromObject(nodeClass)) + nodeClass = ExpectExists(ctx, env.Client, nodeClass) + + expectedHash := nodeClass.Hash() + // Expect ec2nodeclass-hash on the NodeClass to be updated + Expect(nodeClass.Annotations).To(HaveKeyWithValue(v1beta1.AnnotationEC2NodeClassHash, expectedHash)) + Expect(nodeClass.Annotations).To(HaveKeyWithValue(v1beta1.AnnotationEC2NodeClassHashVersion, v1beta1.EC2NodeClassHashVersion)) + }) + It("should update ec2nodeclass-hash-versions on all NodeClaims when the ec2nodeclass-hash-version does not match with the controller hash version", func() { + nodeClass.Annotations = map[string]string{ + v1beta1.AnnotationEC2NodeClassHash: "abceduefed", + v1beta1.AnnotationEC2NodeClassHashVersion: "test", + } + nodeClaimOne := coretest.NodeClaim(corev1beta1.NodeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + v1beta1.AnnotationEC2NodeClassHash: "123456", + v1beta1.AnnotationEC2NodeClassHashVersion: "test", + }, + }, + Spec: corev1beta1.NodeClaimSpec{ + NodeClassRef: &corev1beta1.NodeClassReference{ + Name: nodeClass.Name, + }, + }, + }) + nodeClaimTwo := coretest.NodeClaim(corev1beta1.NodeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + v1beta1.AnnotationEC2NodeClassHash: "123456", + v1beta1.AnnotationEC2NodeClassHashVersion: "test", + }, + }, + Spec: corev1beta1.NodeClaimSpec{ + NodeClassRef: &corev1beta1.NodeClassReference{ + Name: nodeClass.Name, + }, + }, + }) + + ExpectApplied(ctx, env.Client, nodeClass, nodeClaimOne, nodeClaimTwo) + + ExpectReconcileSucceeded(ctx, hashController, client.ObjectKeyFromObject(nodeClass)) + nodeClass = ExpectExists(ctx, env.Client, nodeClass) + nodeClaimOne = ExpectExists(ctx, env.Client, nodeClaimOne) + nodeClaimTwo = ExpectExists(ctx, env.Client, nodeClaimTwo) + + expectedHash := nodeClass.Hash() + // Expect ec2nodeclass-hash on the NodeClaims to be updated + Expect(nodeClaimOne.Annotations).To(HaveKeyWithValue(v1beta1.AnnotationEC2NodeClassHash, expectedHash)) + Expect(nodeClaimOne.Annotations).To(HaveKeyWithValue(v1beta1.AnnotationEC2NodeClassHashVersion, v1beta1.EC2NodeClassHashVersion)) + Expect(nodeClaimTwo.Annotations).To(HaveKeyWithValue(v1beta1.AnnotationEC2NodeClassHash, expectedHash)) + Expect(nodeClaimTwo.Annotations).To(HaveKeyWithValue(v1beta1.AnnotationEC2NodeClassHashVersion, v1beta1.EC2NodeClassHashVersion)) + }) + It("should not update ec2nodeclass-hash on all NodeClaims when the ec2nodeclass-hash-version matches the controller hash version", func() { + nodeClass.Annotations = map[string]string{ + v1beta1.AnnotationEC2NodeClassHash: "abceduefed", + v1beta1.AnnotationEC2NodeClassHashVersion: "test-version", + } + nodeClaim := coretest.NodeClaim(corev1beta1.NodeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + v1beta1.AnnotationEC2NodeClassHash: "1234564654", + v1beta1.AnnotationEC2NodeClassHashVersion: v1beta1.EC2NodeClassHashVersion, + }, + }, + Spec: corev1beta1.NodeClaimSpec{ + NodeClassRef: &corev1beta1.NodeClassReference{ + Name: nodeClass.Name, + }, + }, + }) + ExpectApplied(ctx, env.Client, nodeClass, nodeClaim) + + ExpectReconcileSucceeded(ctx, hashController, client.ObjectKeyFromObject(nodeClass)) + nodeClass = ExpectExists(ctx, env.Client, nodeClass) + nodeClaim = ExpectExists(ctx, env.Client, nodeClaim) + + expectedHash := nodeClass.Hash() + + // Expect ec2nodeclass-hash on the NodeClass to be updated + Expect(nodeClass.Annotations).To(HaveKeyWithValue(v1beta1.AnnotationEC2NodeClassHash, expectedHash)) + Expect(nodeClass.Annotations).To(HaveKeyWithValue(v1beta1.AnnotationEC2NodeClassHashVersion, v1beta1.EC2NodeClassHashVersion)) + // Expect ec2nodeclass-hash on the NodeClaims to stay the same + Expect(nodeClaim.Annotations).To(HaveKeyWithValue(v1beta1.AnnotationEC2NodeClassHash, "1234564654")) + Expect(nodeClaim.Annotations).To(HaveKeyWithValue(v1beta1.AnnotationEC2NodeClassHashVersion, v1beta1.EC2NodeClassHashVersion)) + }) + It("should not update ec2nodeclass-hash on the NodeClaim if it's drifted and the ec2nodeclass-hash-version does not match the controller hash version", func() { + nodeClass.Annotations = map[string]string{ + v1beta1.AnnotationEC2NodeClassHash: "abceduefed", + v1beta1.AnnotationEC2NodeClassHashVersion: "test", + } + nodeClaim := coretest.NodeClaim(corev1beta1.NodeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + v1beta1.AnnotationEC2NodeClassHash: "123456", + v1beta1.AnnotationEC2NodeClassHashVersion: "test", + }, + }, + Spec: corev1beta1.NodeClaimSpec{ + NodeClassRef: &corev1beta1.NodeClassReference{ + Name: nodeClass.Name, + }, + }, + }) + nodeClaim.StatusConditions().MarkTrue(corev1beta1.Drifted) + ExpectApplied(ctx, env.Client, nodeClass, nodeClaim) + + ExpectReconcileSucceeded(ctx, hashController, client.ObjectKeyFromObject(nodeClass)) + nodeClaim = ExpectExists(ctx, env.Client, nodeClaim) + + // Expect ec2nodeclass-hash on the NodeClaims to stay the same + Expect(nodeClaim.Annotations).To(HaveKeyWithValue(v1beta1.AnnotationEC2NodeClassHash, "123456")) + Expect(nodeClaim.Annotations).To(HaveKeyWithValue(v1beta1.AnnotationEC2NodeClassHashVersion, v1beta1.EC2NodeClassHashVersion)) + }) +}) diff --git a/pkg/controllers/nodeclass/status/ami.go b/pkg/controllers/nodeclass/status/ami.go new file mode 100644 index 000000000000..71a80f51f130 --- /dev/null +++ b/pkg/controllers/nodeclass/status/ami.go @@ -0,0 +1,58 @@ +/* +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 status + +import ( + "context" + "fmt" + "sort" + "time" + + "github.com/samber/lo" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + "github.com/aws/karpenter-provider-aws/pkg/apis/v1beta1" + "github.com/aws/karpenter-provider-aws/pkg/providers/amifamily" +) + +type AMI struct { + amiProvider amifamily.Provider +} + +func (a *AMI) Reconcile(ctx context.Context, nodeClass *v1beta1.EC2NodeClass) (reconcile.Result, error) { + amis, err := a.amiProvider.Get(ctx, nodeClass, &amifamily.Options{}) + if err != nil { + return reconcile.Result{}, err + } + if len(amis) == 0 { + nodeClass.Status.AMIs = nil + return reconcile.Result{}, fmt.Errorf("no amis exist given constraints") + } + nodeClass.Status.AMIs = lo.Map(amis, func(ami amifamily.AMI, _ int) v1beta1.AMI { + reqs := ami.Requirements.NodeSelectorRequirements() + sort.Slice(reqs, func(i, j int) bool { + if len(reqs[i].Key) != len(reqs[j].Key) { + return len(reqs[i].Key) < len(reqs[j].Key) + } + return reqs[i].Key < reqs[j].Key + }) + return v1beta1.AMI{ + Name: ami.Name, + ID: ami.AmiID, + Requirements: reqs, + } + }) + return reconcile.Result{RequeueAfter: 5 * time.Minute}, nil +} diff --git a/pkg/controllers/nodeclass/status/ami_test.go b/pkg/controllers/nodeclass/status/ami_test.go new file mode 100644 index 000000000000..0e442cbf33a1 --- /dev/null +++ b/pkg/controllers/nodeclass/status/ami_test.go @@ -0,0 +1,346 @@ +/* +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 status_test + +import ( + "fmt" + "time" + + "github.com/samber/lo" + v1 "k8s.io/api/core/v1" + _ "knative.dev/pkg/system/testing" + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/ec2" + corev1beta1 "sigs.k8s.io/karpenter/pkg/apis/v1beta1" + + "github.com/aws/karpenter-provider-aws/pkg/apis/v1beta1" + "github.com/aws/karpenter-provider-aws/pkg/test" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + . "sigs.k8s.io/karpenter/pkg/test/expectations" +) + +var _ = Describe("NodeClass AMI Status Controller", func() { + BeforeEach(func() { + nodeClass = test.EC2NodeClass(v1beta1.EC2NodeClass{ + Spec: v1beta1.EC2NodeClassSpec{ + SubnetSelectorTerms: []v1beta1.SubnetSelectorTerm{ + { + Tags: map[string]string{"*": "*"}, + }, + }, + SecurityGroupSelectorTerms: []v1beta1.SecurityGroupSelectorTerm{ + { + Tags: map[string]string{"*": "*"}, + }, + }, + AMISelectorTerms: []v1beta1.AMISelectorTerm{ + { + Tags: map[string]string{"*": "*"}, + }, + }, + }, + }) + 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", corev1beta1.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")}, + }, + }, + }, + }) + nodeClass.Spec.AMISelectorTerms = nil + ExpectApplied(ctx, env.Client, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) + nodeClass = ExpectExists(ctx, env.Client, nodeClass) + Expect(nodeClass.Status.AMIs).To(Equal([]v1beta1.AMI{ + { + Name: "test-ami-3", + ID: "ami-id-789", + Requirements: []corev1beta1.NodeSelectorRequirementWithMinValues{ + { + NodeSelectorRequirement: v1.NodeSelectorRequirement{ + Key: v1.LabelArchStable, + Operator: v1.NodeSelectorOpIn, + Values: []string{corev1beta1.ArchitectureArm64}, + }, + }, + { + NodeSelectorRequirement: v1.NodeSelectorRequirement{ + Key: v1beta1.LabelInstanceGPUCount, + Operator: v1.NodeSelectorOpDoesNotExist, + }, + }, + { + NodeSelectorRequirement: v1.NodeSelectorRequirement{ + Key: v1beta1.LabelInstanceAcceleratorCount, + Operator: v1.NodeSelectorOpDoesNotExist, + }, + }, + }, + }, + { + Name: "test-ami-2", + ID: "ami-id-456", + Requirements: []corev1beta1.NodeSelectorRequirementWithMinValues{ + { + NodeSelectorRequirement: v1.NodeSelectorRequirement{ + Key: v1.LabelArchStable, + Operator: v1.NodeSelectorOpIn, + Values: []string{corev1beta1.ArchitectureAmd64}, + }, + }, + { + NodeSelectorRequirement: v1.NodeSelectorRequirement{ + Key: v1beta1.LabelInstanceGPUCount, + Operator: v1.NodeSelectorOpExists, + }, + }, + }, + }, + { + Name: "test-ami-2", + ID: "ami-id-456", + Requirements: []corev1beta1.NodeSelectorRequirementWithMinValues{ + { + NodeSelectorRequirement: v1.NodeSelectorRequirement{ + Key: v1.LabelArchStable, + Operator: v1.NodeSelectorOpIn, + Values: []string{corev1beta1.ArchitectureAmd64}, + }, + }, + { + NodeSelectorRequirement: v1.NodeSelectorRequirement{ + Key: v1beta1.LabelInstanceAcceleratorCount, + Operator: v1.NodeSelectorOpExists, + }, + }, + }, + }, + { + Name: "test-ami-1", + ID: "ami-id-123", + Requirements: []corev1beta1.NodeSelectorRequirementWithMinValues{ + { + NodeSelectorRequirement: v1.NodeSelectorRequirement{ + Key: v1.LabelArchStable, + Operator: v1.NodeSelectorOpIn, + Values: []string{corev1beta1.ArchitectureAmd64}, + }, + }, + { + NodeSelectorRequirement: v1.NodeSelectorRequirement{ + Key: v1beta1.LabelInstanceGPUCount, + Operator: v1.NodeSelectorOpDoesNotExist, + }, + }, + { + NodeSelectorRequirement: v1.NodeSelectorRequirement{ + Key: v1beta1.LabelInstanceAcceleratorCount, + Operator: v1.NodeSelectorOpDoesNotExist, + }, + }, + }, + }, + })) + }) + 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", + } + nodeClass.Spec.AMIFamily = &v1beta1.AMIFamilyBottlerocket + nodeClass.Spec.AMISelectorTerms = 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, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) + nodeClass = ExpectExists(ctx, env.Client, nodeClass) + + Expect(nodeClass.Status.AMIs).To(Equal([]v1beta1.AMI{ + { + Name: "test-ami-2", + ID: "ami-id-456", + Requirements: []corev1beta1.NodeSelectorRequirementWithMinValues{ + { + NodeSelectorRequirement: v1.NodeSelectorRequirement{ + Key: v1.LabelArchStable, + Operator: v1.NodeSelectorOpIn, + Values: []string{corev1beta1.ArchitectureArm64}, + }, + }, + { + NodeSelectorRequirement: v1.NodeSelectorRequirement{ + Key: v1beta1.LabelInstanceGPUCount, + Operator: v1.NodeSelectorOpDoesNotExist, + }, + }, + { + NodeSelectorRequirement: v1.NodeSelectorRequirement{ + Key: v1beta1.LabelInstanceAcceleratorCount, + Operator: v1.NodeSelectorOpDoesNotExist, + }, + }, + }, + }, + { + Name: "test-ami-1", + ID: "ami-id-123", + Requirements: []corev1beta1.NodeSelectorRequirementWithMinValues{ + { + NodeSelectorRequirement: v1.NodeSelectorRequirement{ + Key: v1.LabelArchStable, + Operator: v1.NodeSelectorOpIn, + Values: []string{corev1beta1.ArchitectureAmd64}, + }, + }, + { + NodeSelectorRequirement: v1.NodeSelectorRequirement{ + Key: v1beta1.LabelInstanceGPUCount, + Operator: v1.NodeSelectorOpDoesNotExist, + }, + }, + { + NodeSelectorRequirement: v1.NodeSelectorRequirement{ + Key: v1beta1.LabelInstanceAcceleratorCount, + Operator: v1.NodeSelectorOpDoesNotExist, + }, + }, + }, + }, + })) + }) + It("Should resolve a valid AMI selector", func() { + ExpectApplied(ctx, env.Client, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) + nodeClass = ExpectExists(ctx, env.Client, nodeClass) + Expect(nodeClass.Status.AMIs).To(Equal( + []v1beta1.AMI{ + { + Name: "test-ami-3", + ID: "ami-test3", + Requirements: []corev1beta1.NodeSelectorRequirementWithMinValues{ + { + NodeSelectorRequirement: v1.NodeSelectorRequirement{ + Key: "kubernetes.io/arch", + Operator: "In", + Values: []string{ + "amd64", + }, + }, + }, + }, + }, + }, + )) + }) +}) diff --git a/pkg/controllers/nodeclass/status/controller.go b/pkg/controllers/nodeclass/status/controller.go new file mode 100644 index 000000000000..e39981d9f510 --- /dev/null +++ b/pkg/controllers/nodeclass/status/controller.go @@ -0,0 +1,113 @@ +/* +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 status + +import ( + "context" + + "go.uber.org/multierr" + "k8s.io/apimachinery/pkg/api/equality" + controllerruntime "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + "sigs.k8s.io/controller-runtime/pkg/manager" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + corecontroller "sigs.k8s.io/karpenter/pkg/operator/controller" + "sigs.k8s.io/karpenter/pkg/utils/result" + + "github.com/aws/karpenter-provider-aws/pkg/apis/v1beta1" + "github.com/aws/karpenter-provider-aws/pkg/providers/amifamily" + "github.com/aws/karpenter-provider-aws/pkg/providers/instanceprofile" + "github.com/aws/karpenter-provider-aws/pkg/providers/launchtemplate" + "github.com/aws/karpenter-provider-aws/pkg/providers/securitygroup" + "github.com/aws/karpenter-provider-aws/pkg/providers/subnet" +) + +var _ corecontroller.TypedController[*v1beta1.EC2NodeClass] = (*Controller)(nil) + +type nodeClassStatusReconciler interface { + Reconcile(context.Context, *v1beta1.EC2NodeClass) (reconcile.Result, error) +} + +type Controller struct { + kubeClient client.Client + + ami *AMI + instanceprofile *InstanceProfile + subnet *Subnet + securitygroup *SecurityGroup + launchtemplate *LaunchTemplate +} + +func NewController(kubeClient client.Client, subnetProvider subnet.Provider, securityGroupProvider securitygroup.Provider, + amiProvider amifamily.Provider, instanceProfileProvider instanceprofile.Provider, launchTemplateProvider launchtemplate.Provider) corecontroller.Controller { + return corecontroller.Typed[*v1beta1.EC2NodeClass](kubeClient, &Controller{ + kubeClient: kubeClient, + + ami: &AMI{amiProvider: amiProvider}, + subnet: &Subnet{subnetProvider: subnetProvider}, + securitygroup: &SecurityGroup{securityGroupProvider: securityGroupProvider}, + instanceprofile: &InstanceProfile{instanceProfileProvider: instanceProfileProvider}, + launchtemplate: &LaunchTemplate{launchTemplateProvider: launchTemplateProvider}, + }) +} + +func (c *Controller) Reconcile(ctx context.Context, nodeClass *v1beta1.EC2NodeClass) (reconcile.Result, error) { + if !controllerutil.ContainsFinalizer(nodeClass, v1beta1.TerminationFinalizer) { + stored := nodeClass.DeepCopy() + controllerutil.AddFinalizer(nodeClass, v1beta1.TerminationFinalizer) + if err := c.kubeClient.Patch(ctx, nodeClass, client.MergeFrom(stored)); err != nil { + return reconcile.Result{}, err + } + } + stored := nodeClass.DeepCopy() + + var results []reconcile.Result + var errs error + for _, reconciler := range []nodeClassStatusReconciler{ + c.ami, + c.subnet, + c.securitygroup, + c.instanceprofile, + c.launchtemplate, + } { + res, err := reconciler.Reconcile(ctx, nodeClass) + errs = multierr.Append(errs, err) + results = append(results, res) + } + + if !equality.Semantic.DeepEqual(stored, nodeClass) { + if err := c.kubeClient.Status().Patch(ctx, nodeClass, client.MergeFrom(stored)); err != nil { + errs = multierr.Append(errs, client.IgnoreNotFound(err)) + } + } + if errs != nil { + return reconcile.Result{}, errs + } + return result.Min(results...), nil +} + +func (c *Controller) Name() string { + return "nodeclass.status" +} + +func (c *Controller) Builder(_ context.Context, m manager.Manager) corecontroller.Builder { + return corecontroller.Adapt(controllerruntime. + NewControllerManagedBy(m). + For(&v1beta1.EC2NodeClass{}). + WithOptions(controller.Options{MaxConcurrentReconciles: 10})) +} diff --git a/pkg/controllers/nodeclass/status/instanceprofile.go b/pkg/controllers/nodeclass/status/instanceprofile.go new file mode 100644 index 000000000000..10988803db5c --- /dev/null +++ b/pkg/controllers/nodeclass/status/instanceprofile.go @@ -0,0 +1,44 @@ +/* +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 status + +import ( + "context" + "fmt" + + "github.com/samber/lo" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + "github.com/aws/karpenter-provider-aws/pkg/apis/v1beta1" + "github.com/aws/karpenter-provider-aws/pkg/providers/instanceprofile" +) + +type InstanceProfile struct { + instanceProfileProvider instanceprofile.Provider +} + +func (ip *InstanceProfile) Reconcile(ctx context.Context, nodeClass *v1beta1.EC2NodeClass) (reconcile.Result, error) { + if nodeClass.Spec.Role != "" { + name, err := ip.instanceProfileProvider.Create(ctx, nodeClass) + if err != nil { + return reconcile.Result{}, fmt.Errorf("creating instance profile, %w", err) + } + nodeClass.Status.InstanceProfile = name + } else { + nodeClass.Status.InstanceProfile = lo.FromPtr(nodeClass.Spec.InstanceProfile) + } + + return reconcile.Result{}, nil +} diff --git a/pkg/controllers/nodeclass/status/instanceprofile_test.go b/pkg/controllers/nodeclass/status/instanceprofile_test.go new file mode 100644 index 000000000000..3b51968bc9df --- /dev/null +++ b/pkg/controllers/nodeclass/status/instanceprofile_test.go @@ -0,0 +1,134 @@ +/* +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 status_test + +import ( + "github.com/samber/lo" + _ "knative.dev/pkg/system/testing" + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/iam" + + "github.com/aws/karpenter-provider-aws/pkg/fake" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + . "sigs.k8s.io/karpenter/pkg/test/expectations" +) + +var _ = Describe("NodeClass InstanceProfile Status Controller", func() { + var profileName string + BeforeEach(func() { + profileName = awsEnv.InstanceProfileProvider.GetProfileName(ctx, fake.DefaultRegion, nodeClass.Name) + }) + It("should create the instance profile when it doesn't exist", func() { + nodeClass.Spec.Role = "test-role" + ExpectApplied(ctx, env.Client, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) + + Expect(awsEnv.IAMAPI.InstanceProfiles).To(HaveLen(1)) + Expect(awsEnv.IAMAPI.InstanceProfiles[profileName].Roles).To(HaveLen(1)) + Expect(*awsEnv.IAMAPI.InstanceProfiles[profileName].Roles[0].RoleName).To(Equal("test-role")) + + nodeClass = ExpectExists(ctx, env.Client, nodeClass) + Expect(nodeClass.Status.InstanceProfile).To(Equal(profileName)) + }) + It("should add the role to the instance profile when it exists without a role", func() { + awsEnv.IAMAPI.InstanceProfiles = map[string]*iam.InstanceProfile{ + profileName: { + InstanceProfileId: aws.String(fake.InstanceProfileID()), + InstanceProfileName: aws.String(profileName), + }, + } + + nodeClass.Spec.Role = "test-role" + ExpectApplied(ctx, env.Client, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) + + Expect(awsEnv.IAMAPI.InstanceProfiles).To(HaveLen(1)) + Expect(awsEnv.IAMAPI.InstanceProfiles[profileName].Roles).To(HaveLen(1)) + Expect(*awsEnv.IAMAPI.InstanceProfiles[profileName].Roles[0].RoleName).To(Equal("test-role")) + + nodeClass = ExpectExists(ctx, env.Client, nodeClass) + Expect(nodeClass.Status.InstanceProfile).To(Equal(profileName)) + }) + It("should update the role for the instance profile when the wrong role exists", func() { + awsEnv.IAMAPI.InstanceProfiles = map[string]*iam.InstanceProfile{ + profileName: { + InstanceProfileId: aws.String(fake.InstanceProfileID()), + InstanceProfileName: aws.String(profileName), + Roles: []*iam.Role{ + { + RoleName: aws.String("other-role"), + }, + }, + }, + } + + nodeClass.Spec.Role = "test-role" + ExpectApplied(ctx, env.Client, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) + + Expect(awsEnv.IAMAPI.InstanceProfiles).To(HaveLen(1)) + Expect(awsEnv.IAMAPI.InstanceProfiles[profileName].Roles).To(HaveLen(1)) + Expect(*awsEnv.IAMAPI.InstanceProfiles[profileName].Roles[0].RoleName).To(Equal("test-role")) + + nodeClass = ExpectExists(ctx, env.Client, nodeClass) + Expect(nodeClass.Status.InstanceProfile).To(Equal(profileName)) + }) + It("should not call CreateInstanceProfile or AddRoleToInstanceProfile when instance profile exists with correct role", func() { + awsEnv.IAMAPI.InstanceProfiles = map[string]*iam.InstanceProfile{ + profileName: { + InstanceProfileId: aws.String(fake.InstanceProfileID()), + InstanceProfileName: aws.String(profileName), + Roles: []*iam.Role{ + { + RoleName: aws.String("test-role"), + }, + }, + }, + } + + nodeClass.Spec.Role = "test-role" + ExpectApplied(ctx, env.Client, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) + + Expect(awsEnv.IAMAPI.InstanceProfiles).To(HaveLen(1)) + Expect(awsEnv.IAMAPI.InstanceProfiles[profileName].Roles).To(HaveLen(1)) + Expect(*awsEnv.IAMAPI.InstanceProfiles[profileName].Roles[0].RoleName).To(Equal("test-role")) + + Expect(awsEnv.IAMAPI.CreateInstanceProfileBehavior.Calls()).To(BeZero()) + Expect(awsEnv.IAMAPI.AddRoleToInstanceProfileBehavior.Calls()).To(BeZero()) + }) + It("should resolve the specified instance profile into the status when using instanceProfile field", func() { + nodeClass.Spec.Role = "" + nodeClass.Spec.InstanceProfile = lo.ToPtr("test-instance-profile") + ExpectApplied(ctx, env.Client, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) + + nodeClass = ExpectExists(ctx, env.Client, nodeClass) + Expect(nodeClass.Status.InstanceProfile).To(Equal(lo.FromPtr(nodeClass.Spec.InstanceProfile))) + }) + It("should not call the the IAM API when specifying an instance profile", func() { + nodeClass.Spec.Role = "" + nodeClass.Spec.InstanceProfile = lo.ToPtr("test-instance-profile") + ExpectApplied(ctx, env.Client, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) + + Expect(awsEnv.IAMAPI.CreateInstanceProfileBehavior.Calls()).To(BeZero()) + Expect(awsEnv.IAMAPI.AddRoleToInstanceProfileBehavior.Calls()).To(BeZero()) + }) +}) diff --git a/pkg/controllers/nodeclass/status/launchtemplate.go b/pkg/controllers/nodeclass/status/launchtemplate.go new file mode 100644 index 000000000000..7f8477b099fe --- /dev/null +++ b/pkg/controllers/nodeclass/status/launchtemplate.go @@ -0,0 +1,42 @@ +/* +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 status + +import ( + "context" + "fmt" + + "github.com/samber/lo" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + "github.com/aws/karpenter-provider-aws/pkg/apis/v1beta1" + "github.com/aws/karpenter-provider-aws/pkg/providers/launchtemplate" +) + +type LaunchTemplate struct { + launchTemplateProvider launchtemplate.Provider +} + +func (lt *LaunchTemplate) Reconcile(ctx context.Context, nodeClass *v1beta1.EC2NodeClass) (reconcile.Result, error) { + // A NodeClass that use AL2023 requires the cluster CIDR for launching nodes. + // To allow Karpenter to be used for Non-EKS clusters, resolving the Cluster CIDR + // will not be done at startup but instead in a reconcile loop. + if lo.FromPtr(nodeClass.Spec.AMIFamily) == v1beta1.AMIFamilyAL2023 { + if err := lt.launchTemplateProvider.ResolveClusterCIDR(ctx); err != nil { + return reconcile.Result{}, fmt.Errorf("unable to detect the cluster CIDR, %w", err) + } + } + return reconcile.Result{}, nil +} diff --git a/pkg/controllers/nodeclass/status/launchtemplate_test.go b/pkg/controllers/nodeclass/status/launchtemplate_test.go new file mode 100644 index 000000000000..8d5bbb8457ea --- /dev/null +++ b/pkg/controllers/nodeclass/status/launchtemplate_test.go @@ -0,0 +1,90 @@ +/* +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 status_test + +import ( + _ "knative.dev/pkg/system/testing" + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/aws/aws-sdk-go/service/eks" + "github.com/samber/lo" + + "github.com/aws/karpenter-provider-aws/pkg/apis/v1beta1" + "github.com/aws/karpenter-provider-aws/pkg/test" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + . "sigs.k8s.io/karpenter/pkg/test/expectations" +) + +var _ = Describe("NodeClass Launch Template CIDR Resolution Controller", func() { + BeforeEach(func() { + nodeClass = test.EC2NodeClass(v1beta1.EC2NodeClass{ + Spec: v1beta1.EC2NodeClassSpec{ + SubnetSelectorTerms: []v1beta1.SubnetSelectorTerm{ + { + Tags: map[string]string{"*": "*"}, + }, + }, + SecurityGroupSelectorTerms: []v1beta1.SecurityGroupSelectorTerm{ + { + Tags: map[string]string{"*": "*"}, + }, + }, + AMISelectorTerms: []v1beta1.AMISelectorTerm{ + { + Tags: map[string]string{"*": "*"}, + }, + }, + }, + }) + // Cluster CIDR will only be resolved once per lifetime of the launch template provider, reset to nil between tests + awsEnv.LaunchTemplateProvider.ClusterCIDR.Store(nil) + }) + It("shouldn't resolve cluster CIDR for non-AL2023 NodeClasses", func() { + for _, family := range []string{ + v1beta1.AMIFamilyAL2, + v1beta1.AMIFamilyBottlerocket, + v1beta1.AMIFamilyUbuntu, + v1beta1.AMIFamilyWindows2019, + v1beta1.AMIFamilyWindows2022, + v1beta1.AMIFamilyCustom, + } { + nodeClass.Spec.AMIFamily = lo.ToPtr(family) + ExpectApplied(ctx, env.Client, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) + Expect(awsEnv.LaunchTemplateProvider.ClusterCIDR.Load()).To(BeNil()) + } + }) + It("should resolve cluster CIDR for IPv4 clusters", func() { + nodeClass.Spec.AMIFamily = lo.ToPtr(v1beta1.AMIFamilyAL2023) + ExpectApplied(ctx, env.Client, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) + Expect(lo.FromPtr(awsEnv.LaunchTemplateProvider.ClusterCIDR.Load())).To(Equal("10.100.0.0/16")) + }) + It("should resolve cluster CIDR for IPv6 clusters", func() { + awsEnv.EKSAPI.DescribeClusterBehavior.Output.Set(&eks.DescribeClusterOutput{ + Cluster: &eks.Cluster{ + KubernetesNetworkConfig: &eks.KubernetesNetworkConfigResponse{ + ServiceIpv6Cidr: lo.ToPtr("2001:db8::/64"), + }, + }, + }) + nodeClass.Spec.AMIFamily = lo.ToPtr(v1beta1.AMIFamilyAL2023) + ExpectApplied(ctx, env.Client, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) + Expect(lo.FromPtr(awsEnv.LaunchTemplateProvider.ClusterCIDR.Load())).To(Equal("2001:db8::/64")) + }) +}) diff --git a/pkg/controllers/nodeclass/status/securitygroup.go b/pkg/controllers/nodeclass/status/securitygroup.go new file mode 100644 index 000000000000..378398ef04c0 --- /dev/null +++ b/pkg/controllers/nodeclass/status/securitygroup.go @@ -0,0 +1,54 @@ +/* +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 status + +import ( + "context" + "fmt" + "sort" + "time" + + "github.com/aws/aws-sdk-go/service/ec2" + "github.com/samber/lo" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + "github.com/aws/karpenter-provider-aws/pkg/apis/v1beta1" + "github.com/aws/karpenter-provider-aws/pkg/providers/securitygroup" +) + +type SecurityGroup struct { + securityGroupProvider securitygroup.Provider +} + +func (sg *SecurityGroup) Reconcile(ctx context.Context, nodeClass *v1beta1.EC2NodeClass) (reconcile.Result, error) { + securityGroups, err := sg.securityGroupProvider.List(ctx, nodeClass) + if err != nil { + return reconcile.Result{}, err + } + if len(securityGroups) == 0 && len(nodeClass.Spec.SecurityGroupSelectorTerms) > 0 { + nodeClass.Status.SecurityGroups = nil + return reconcile.Result{}, fmt.Errorf("no security groups exist given constraints") + } + sort.Slice(securityGroups, func(i, j int) bool { + return *securityGroups[i].GroupId < *securityGroups[j].GroupId + }) + nodeClass.Status.SecurityGroups = lo.Map(securityGroups, func(securityGroup *ec2.SecurityGroup, _ int) v1beta1.SecurityGroup { + return v1beta1.SecurityGroup{ + ID: *securityGroup.GroupId, + Name: *securityGroup.GroupName, + } + }) + return reconcile.Result{RequeueAfter: 5 * time.Minute}, nil +} diff --git a/pkg/controllers/nodeclass/status/securitygroup_test.go b/pkg/controllers/nodeclass/status/securitygroup_test.go new file mode 100644 index 000000000000..ad0589122921 --- /dev/null +++ b/pkg/controllers/nodeclass/status/securitygroup_test.go @@ -0,0 +1,224 @@ +/* +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 status_test + +import ( + _ "knative.dev/pkg/system/testing" + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/aws/karpenter-provider-aws/pkg/apis/v1beta1" + "github.com/aws/karpenter-provider-aws/pkg/test" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + . "sigs.k8s.io/karpenter/pkg/test/expectations" +) + +var _ = Describe("NodeClass Security Group Status Controller", func() { + BeforeEach(func() { + nodeClass = test.EC2NodeClass(v1beta1.EC2NodeClass{ + Spec: v1beta1.EC2NodeClassSpec{ + SubnetSelectorTerms: []v1beta1.SubnetSelectorTerm{ + { + Tags: map[string]string{"*": "*"}, + }, + }, + SecurityGroupSelectorTerms: []v1beta1.SecurityGroupSelectorTerm{ + { + Tags: map[string]string{"*": "*"}, + }, + }, + AMISelectorTerms: []v1beta1.AMISelectorTerm{ + { + Tags: map[string]string{"*": "*"}, + }, + }, + }, + }) + }) + It("Should update EC2NodeClass status for Security Groups", func() { + ExpectApplied(ctx, env.Client, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) + nodeClass = ExpectExists(ctx, env.Client, nodeClass) + Expect(nodeClass.Status.SecurityGroups).To(Equal([]v1beta1.SecurityGroup{ + { + ID: "sg-test1", + Name: "securityGroup-test1", + }, + { + ID: "sg-test2", + Name: "securityGroup-test2", + }, + { + ID: "sg-test3", + Name: "securityGroup-test3", + }, + })) + }) + It("Should resolve a valid selectors for Security Groups by tags", func() { + nodeClass.Spec.SecurityGroupSelectorTerms = []v1beta1.SecurityGroupSelectorTerm{ + { + Tags: map[string]string{"Name": "test-security-group-1"}, + }, + { + Tags: map[string]string{"Name": "test-security-group-2"}, + }, + } + ExpectApplied(ctx, env.Client, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) + nodeClass = ExpectExists(ctx, env.Client, nodeClass) + Expect(nodeClass.Status.SecurityGroups).To(Equal([]v1beta1.SecurityGroup{ + { + ID: "sg-test1", + Name: "securityGroup-test1", + }, + { + ID: "sg-test2", + Name: "securityGroup-test2", + }, + })) + }) + It("Should resolve a valid selectors for Security Groups by ids", func() { + nodeClass.Spec.SecurityGroupSelectorTerms = []v1beta1.SecurityGroupSelectorTerm{ + { + ID: "sg-test1", + }, + } + ExpectApplied(ctx, env.Client, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) + nodeClass = ExpectExists(ctx, env.Client, nodeClass) + Expect(nodeClass.Status.SecurityGroups).To(Equal([]v1beta1.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, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) + nodeClass = ExpectExists(ctx, env.Client, nodeClass) + Expect(nodeClass.Status.SecurityGroups).To(Equal([]v1beta1.SecurityGroup{ + { + ID: "sg-test1", + Name: "securityGroup-test1", + }, + { + ID: "sg-test2", + Name: "securityGroup-test2", + }, + { + ID: "sg-test3", + Name: "securityGroup-test3", + }, + })) + + nodeClass.Spec.SecurityGroupSelectorTerms = []v1beta1.SecurityGroupSelectorTerm{ + { + Tags: map[string]string{"Name": "test-security-group-1"}, + }, + { + Tags: map[string]string{"Name": "test-security-group-2"}, + }, + } + ExpectApplied(ctx, env.Client, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) + nodeClass = ExpectExists(ctx, env.Client, nodeClass) + Expect(nodeClass.Status.SecurityGroups).To(Equal([]v1beta1.SecurityGroup{ + { + ID: "sg-test1", + Name: "securityGroup-test1", + }, + { + 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, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) + nodeClass = ExpectExists(ctx, env.Client, nodeClass) + Expect(nodeClass.Status.SecurityGroups).To(Equal([]v1beta1.SecurityGroup{ + { + ID: "sg-test1", + Name: "securityGroup-test1", + }, + { + ID: "sg-test2", + Name: "securityGroup-test2", + }, + { + ID: "sg-test3", + Name: "securityGroup-test3", + }, + })) + + nodeClass.Spec.SecurityGroupSelectorTerms = []v1beta1.SecurityGroupSelectorTerm{ + { + ID: "sg-test1", + }, + } + ExpectApplied(ctx, env.Client, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) + nodeClass = ExpectExists(ctx, env.Client, nodeClass) + Expect(nodeClass.Status.SecurityGroups).To(Equal([]v1beta1.SecurityGroup{ + { + ID: "sg-test1", + Name: "securityGroup-test1", + }, + })) + }) + It("Should not resolve a invalid selectors for Security Groups", func() { + nodeClass.Spec.SecurityGroupSelectorTerms = []v1beta1.SecurityGroupSelectorTerm{ + { + Tags: map[string]string{`foo`: `invalid`}, + }, + } + ExpectApplied(ctx, env.Client, nodeClass) + ExpectReconcileFailed(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) + nodeClass = ExpectExists(ctx, env.Client, nodeClass) + Expect(nodeClass.Status.SecurityGroups).To(BeNil()) + }) + It("Should not resolve a invalid selectors for an updated Security Groups selector", func() { + ExpectApplied(ctx, env.Client, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) + nodeClass = ExpectExists(ctx, env.Client, nodeClass) + Expect(nodeClass.Status.SecurityGroups).To(Equal([]v1beta1.SecurityGroup{ + { + ID: "sg-test1", + Name: "securityGroup-test1", + }, + { + ID: "sg-test2", + Name: "securityGroup-test2", + }, + { + ID: "sg-test3", + Name: "securityGroup-test3", + }, + })) + + nodeClass.Spec.SecurityGroupSelectorTerms = []v1beta1.SecurityGroupSelectorTerm{ + { + Tags: map[string]string{`foo`: `invalid`}, + }, + } + ExpectApplied(ctx, env.Client, nodeClass) + ExpectReconcileFailed(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) + nodeClass = ExpectExists(ctx, env.Client, nodeClass) + Expect(nodeClass.Status.SecurityGroups).To(BeNil()) + }) +}) diff --git a/pkg/controllers/nodeclass/status/subnet.go b/pkg/controllers/nodeclass/status/subnet.go new file mode 100644 index 000000000000..861131fa8cbc --- /dev/null +++ b/pkg/controllers/nodeclass/status/subnet.go @@ -0,0 +1,58 @@ +/* +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 status + +import ( + "context" + "fmt" + "sort" + "time" + + "github.com/aws/aws-sdk-go/service/ec2" + "github.com/samber/lo" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + "github.com/aws/karpenter-provider-aws/pkg/apis/v1beta1" + "github.com/aws/karpenter-provider-aws/pkg/providers/subnet" +) + +type Subnet struct { + subnetProvider subnet.Provider +} + +func (s *Subnet) Reconcile(ctx context.Context, nodeClass *v1beta1.EC2NodeClass) (reconcile.Result, error) { + subnets, err := s.subnetProvider.List(ctx, nodeClass) + if err != nil { + return reconcile.Result{}, err + } + if len(subnets) == 0 { + nodeClass.Status.Subnets = nil + return reconcile.Result{}, fmt.Errorf("no subnets exist given constraints %v", nodeClass.Spec.SubnetSelectorTerms) + } + sort.Slice(subnets, func(i, j int) bool { + if int(*subnets[i].AvailableIpAddressCount) != int(*subnets[j].AvailableIpAddressCount) { + return int(*subnets[i].AvailableIpAddressCount) > int(*subnets[j].AvailableIpAddressCount) + } + return *subnets[i].SubnetId < *subnets[j].SubnetId + }) + nodeClass.Status.Subnets = lo.Map(subnets, func(ec2subnet *ec2.Subnet, _ int) v1beta1.Subnet { + return v1beta1.Subnet{ + ID: *ec2subnet.SubnetId, + Zone: *ec2subnet.AvailabilityZone, + } + }) + + return reconcile.Result{RequeueAfter: 5 * time.Minute}, nil +} diff --git a/pkg/controllers/nodeclass/status/subnet_test.go b/pkg/controllers/nodeclass/status/subnet_test.go new file mode 100644 index 000000000000..ba37f1d3b11a --- /dev/null +++ b/pkg/controllers/nodeclass/status/subnet_test.go @@ -0,0 +1,271 @@ +/* +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 status_test + +import ( + _ "knative.dev/pkg/system/testing" + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/ec2" + + "github.com/aws/karpenter-provider-aws/pkg/apis/v1beta1" + "github.com/aws/karpenter-provider-aws/pkg/test" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + . "sigs.k8s.io/karpenter/pkg/test/expectations" +) + +var _ = Describe("NodeClass Subnet Status Controller", func() { + BeforeEach(func() { + nodeClass = test.EC2NodeClass(v1beta1.EC2NodeClass{ + Spec: v1beta1.EC2NodeClassSpec{ + SubnetSelectorTerms: []v1beta1.SubnetSelectorTerm{ + { + Tags: map[string]string{"*": "*"}, + }, + }, + SecurityGroupSelectorTerms: []v1beta1.SecurityGroupSelectorTerm{ + { + Tags: map[string]string{"*": "*"}, + }, + }, + AMISelectorTerms: []v1beta1.AMISelectorTerm{ + { + Tags: map[string]string{"*": "*"}, + }, + }, + }, + }) + }) + It("Should update EC2NodeClass status for Subnets", func() { + ExpectApplied(ctx, env.Client, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) + nodeClass = ExpectExists(ctx, env.Client, nodeClass) + Expect(nodeClass.Status.Subnets).To(Equal([]v1beta1.Subnet{ + { + ID: "subnet-test1", + Zone: "test-zone-1a", + }, + { + ID: "subnet-test2", + Zone: "test-zone-1b", + }, + { + ID: "subnet-test3", + Zone: "test-zone-1c", + }, + { + ID: "subnet-test4", + Zone: "test-zone-1a-local", + }, + })) + }) + 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, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) + nodeClass = ExpectExists(ctx, env.Client, nodeClass) + Expect(nodeClass.Status.Subnets).To(Equal([]v1beta1.Subnet{ + { + ID: "subnet-test2", + Zone: "test-zone-1b", + }, + { + ID: "subnet-test3", + Zone: "test-zone-1c", + }, + { + ID: "subnet-test1", + Zone: "test-zone-1a", + }, + })) + }) + It("Should resolve a valid selectors for Subnet by tags", func() { + nodeClass.Spec.SubnetSelectorTerms = []v1beta1.SubnetSelectorTerm{ + { + Tags: map[string]string{`Name`: `test-subnet-1`}, + }, + { + Tags: map[string]string{`Name`: `test-subnet-2`}, + }, + } + ExpectApplied(ctx, env.Client, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) + nodeClass = ExpectExists(ctx, env.Client, nodeClass) + Expect(nodeClass.Status.Subnets).To(Equal([]v1beta1.Subnet{ + { + ID: "subnet-test1", + Zone: "test-zone-1a", + }, + { + ID: "subnet-test2", + Zone: "test-zone-1b", + }, + })) + }) + It("Should resolve a valid selectors for Subnet by ids", func() { + nodeClass.Spec.SubnetSelectorTerms = []v1beta1.SubnetSelectorTerm{ + { + ID: "subnet-test1", + }, + } + ExpectApplied(ctx, env.Client, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) + nodeClass = ExpectExists(ctx, env.Client, nodeClass) + Expect(nodeClass.Status.Subnets).To(Equal([]v1beta1.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, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) + nodeClass = ExpectExists(ctx, env.Client, nodeClass) + Expect(nodeClass.Status.Subnets).To(Equal([]v1beta1.Subnet{ + { + ID: "subnet-test1", + Zone: "test-zone-1a", + }, + { + ID: "subnet-test2", + Zone: "test-zone-1b", + }, + { + ID: "subnet-test3", + Zone: "test-zone-1c", + }, + { + ID: "subnet-test4", + Zone: "test-zone-1a-local", + }, + })) + + nodeClass.Spec.SubnetSelectorTerms = []v1beta1.SubnetSelectorTerm{ + { + Tags: map[string]string{ + "Name": "test-subnet-1", + }, + }, + { + Tags: map[string]string{ + "Name": "test-subnet-2", + }, + }, + } + ExpectApplied(ctx, env.Client, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) + nodeClass = ExpectExists(ctx, env.Client, nodeClass) + Expect(nodeClass.Status.Subnets).To(Equal([]v1beta1.Subnet{ + { + ID: "subnet-test1", + Zone: "test-zone-1a", + }, + { + 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, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) + nodeClass = ExpectExists(ctx, env.Client, nodeClass) + Expect(nodeClass.Status.Subnets).To(Equal([]v1beta1.Subnet{ + { + ID: "subnet-test1", + Zone: "test-zone-1a", + }, + { + ID: "subnet-test2", + Zone: "test-zone-1b", + }, + { + ID: "subnet-test3", + Zone: "test-zone-1c", + }, + { + ID: "subnet-test4", + Zone: "test-zone-1a-local", + }, + })) + + nodeClass.Spec.SubnetSelectorTerms = []v1beta1.SubnetSelectorTerm{ + { + ID: "subnet-test1", + }, + } + ExpectApplied(ctx, env.Client, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) + nodeClass = ExpectExists(ctx, env.Client, nodeClass) + Expect(nodeClass.Status.Subnets).To(Equal([]v1beta1.Subnet{ + { + ID: "subnet-test1", + Zone: "test-zone-1a", + }, + })) + }) + It("Should not resolve a invalid selectors for Subnet", func() { + nodeClass.Spec.SubnetSelectorTerms = []v1beta1.SubnetSelectorTerm{ + { + Tags: map[string]string{`foo`: `invalid`}, + }, + } + ExpectApplied(ctx, env.Client, nodeClass) + ExpectReconcileFailed(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) + nodeClass = ExpectExists(ctx, env.Client, nodeClass) + Expect(nodeClass.Status.Subnets).To(BeNil()) + }) + It("Should not resolve a invalid selectors for an updated subnet selector", func() { + ExpectApplied(ctx, env.Client, nodeClass) + ExpectReconcileSucceeded(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) + nodeClass = ExpectExists(ctx, env.Client, nodeClass) + Expect(nodeClass.Status.Subnets).To(Equal([]v1beta1.Subnet{ + { + ID: "subnet-test1", + Zone: "test-zone-1a", + }, + { + ID: "subnet-test2", + Zone: "test-zone-1b", + }, + { + ID: "subnet-test3", + Zone: "test-zone-1c", + }, + { + ID: "subnet-test4", + Zone: "test-zone-1a-local", + }, + })) + + nodeClass.Spec.SubnetSelectorTerms = []v1beta1.SubnetSelectorTerm{ + { + Tags: map[string]string{`foo`: `invalid`}, + }, + } + ExpectApplied(ctx, env.Client, nodeClass) + ExpectReconcileFailed(ctx, statusController, client.ObjectKeyFromObject(nodeClass)) + nodeClass = ExpectExists(ctx, env.Client, nodeClass) + Expect(nodeClass.Status.Subnets).To(BeNil()) + }) +}) diff --git a/pkg/controllers/nodeclass/status/suite_test.go b/pkg/controllers/nodeclass/status/suite_test.go new file mode 100644 index 000000000000..545c9ba6cfd5 --- /dev/null +++ b/pkg/controllers/nodeclass/status/suite_test.go @@ -0,0 +1,80 @@ +/* +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 status_test + +import ( + "context" + "testing" + + _ "knative.dev/pkg/system/testing" + + corecontroller "sigs.k8s.io/karpenter/pkg/operator/controller" + coreoptions "sigs.k8s.io/karpenter/pkg/operator/options" + "sigs.k8s.io/karpenter/pkg/operator/scheme" + coretest "sigs.k8s.io/karpenter/pkg/test" + + "github.com/aws/karpenter-provider-aws/pkg/apis" + "github.com/aws/karpenter-provider-aws/pkg/apis/v1beta1" + "github.com/aws/karpenter-provider-aws/pkg/controllers/nodeclass/status" + "github.com/aws/karpenter-provider-aws/pkg/operator/options" + "github.com/aws/karpenter-provider-aws/pkg/test" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + . "knative.dev/pkg/logging/testing" + . "sigs.k8s.io/karpenter/pkg/test/expectations" +) + +var ctx context.Context +var env *coretest.Environment +var awsEnv *test.Environment +var nodeClass *v1beta1.EC2NodeClass +var statusController corecontroller.Controller + +func TestAPIs(t *testing.T) { + ctx = TestContextWithLogger(t) + RegisterFailHandler(Fail) + RunSpecs(t, "EC2NodeClass") +} + +var _ = BeforeSuite(func() { + env = coretest.NewEnvironment(scheme.Scheme, coretest.WithCRDs(apis.CRDs...), coretest.WithFieldIndexers(test.EC2NodeClassFieldIndexer(ctx))) + ctx = coreoptions.ToContext(ctx, coretest.Options()) + ctx = options.ToContext(ctx, test.Options()) + awsEnv = test.NewEnvironment(ctx, env) + + statusController = status.NewController( + env.Client, + awsEnv.SubnetProvider, + awsEnv.SecurityGroupProvider, + awsEnv.AMIProvider, + awsEnv.InstanceProfileProvider, + awsEnv.LaunchTemplateProvider, + ) +}) + +var _ = AfterSuite(func() { + Expect(env.Stop()).To(Succeed(), "Failed to stop environment") +}) + +var _ = BeforeEach(func() { + ctx = coreoptions.ToContext(ctx, coretest.Options()) + nodeClass = test.EC2NodeClass() + awsEnv.Reset() +}) + +var _ = AfterEach(func() { + ExpectCleanedUp(ctx, env.Client) +}) diff --git a/pkg/controllers/nodeclass/suite_test.go b/pkg/controllers/nodeclass/suite_test.go deleted file mode 100644 index 222fa5b07092..000000000000 --- a/pkg/controllers/nodeclass/suite_test.go +++ /dev/null @@ -1,1291 +0,0 @@ -/* -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 nodeclass_test - -import ( - "context" - "fmt" - "testing" - "time" - - "github.com/imdario/mergo" - "github.com/samber/lo" - v1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/client-go/tools/record" - _ "knative.dev/pkg/system/testing" - "sigs.k8s.io/controller-runtime/pkg/client" - - "github.com/aws/aws-sdk-go/aws" - "github.com/aws/aws-sdk-go/service/ec2" - "github.com/aws/aws-sdk-go/service/eks" - "github.com/aws/aws-sdk-go/service/iam" - corev1beta1 "sigs.k8s.io/karpenter/pkg/apis/v1beta1" - "sigs.k8s.io/karpenter/pkg/events" - corecontroller "sigs.k8s.io/karpenter/pkg/operator/controller" - coreoptions "sigs.k8s.io/karpenter/pkg/operator/options" - "sigs.k8s.io/karpenter/pkg/operator/scheme" - coretest "sigs.k8s.io/karpenter/pkg/test" - - "github.com/aws/karpenter-provider-aws/pkg/apis" - "github.com/aws/karpenter-provider-aws/pkg/apis/v1beta1" - "github.com/aws/karpenter-provider-aws/pkg/controllers/nodeclass" - "github.com/aws/karpenter-provider-aws/pkg/fake" - "github.com/aws/karpenter-provider-aws/pkg/operator/options" - "github.com/aws/karpenter-provider-aws/pkg/test" - - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" - . "knative.dev/pkg/logging/testing" - . "sigs.k8s.io/karpenter/pkg/test/expectations" -) - -var ctx context.Context -var env *coretest.Environment -var awsEnv *test.Environment -var nodeClassController corecontroller.Controller - -func TestAPIs(t *testing.T) { - ctx = TestContextWithLogger(t) - RegisterFailHandler(Fail) - RunSpecs(t, "EC2NodeClass") -} - -var _ = BeforeSuite(func() { - env = coretest.NewEnvironment(scheme.Scheme, coretest.WithCRDs(apis.CRDs...), coretest.WithFieldIndexers(test.EC2NodeClassFieldIndexer(ctx))) - ctx = coreoptions.ToContext(ctx, coretest.Options()) - ctx = options.ToContext(ctx, test.Options()) - awsEnv = test.NewEnvironment(ctx, env) - - nodeClassController = nodeclass.NewController(env.Client, events.NewRecorder(&record.FakeRecorder{}), awsEnv.SubnetProvider, awsEnv.SecurityGroupProvider, awsEnv.AMIProvider, awsEnv.InstanceProfileProvider, awsEnv.LaunchTemplateProvider) -}) - -var _ = AfterSuite(func() { - Expect(env.Stop()).To(Succeed(), "Failed to stop environment") -}) - -var _ = BeforeEach(func() { - ctx = coreoptions.ToContext(ctx, coretest.Options()) - awsEnv.Reset() -}) - -var _ = AfterEach(func() { - ExpectCleanedUp(ctx, env.Client) -}) - -var _ = Describe("NodeClassController", func() { - var nodeClass *v1beta1.EC2NodeClass - BeforeEach(func() { - nodeClass = test.EC2NodeClass(v1beta1.EC2NodeClass{ - Spec: v1beta1.EC2NodeClassSpec{ - SubnetSelectorTerms: []v1beta1.SubnetSelectorTerm{ - { - Tags: map[string]string{"*": "*"}, - }, - }, - SecurityGroupSelectorTerms: []v1beta1.SecurityGroupSelectorTerm{ - { - Tags: map[string]string{"*": "*"}, - }, - }, - AMISelectorTerms: []v1beta1.AMISelectorTerm{ - { - Tags: map[string]string{"*": "*"}, - }, - }, - }, - }) - }) - Context("Cluster CIDR Resolution", func() { - BeforeEach(func() { - // Cluster CIDR will only be resolved once per lifetime of the launch template provider, reset to nil between tests - awsEnv.LaunchTemplateProvider.ClusterCIDR.Store(nil) - }) - It("shouldn't resolve cluster CIDR for non-AL2023 NodeClasses", func() { - for _, family := range []string{ - v1beta1.AMIFamilyAL2, - v1beta1.AMIFamilyBottlerocket, - v1beta1.AMIFamilyUbuntu, - v1beta1.AMIFamilyWindows2019, - v1beta1.AMIFamilyWindows2022, - v1beta1.AMIFamilyCustom, - } { - nodeClass.Spec.AMIFamily = lo.ToPtr(family) - ExpectApplied(ctx, env.Client, nodeClass) - ExpectReconcileSucceeded(ctx, nodeClassController, client.ObjectKeyFromObject(nodeClass)) - Expect(awsEnv.LaunchTemplateProvider.ClusterCIDR.Load()).To(BeNil()) - } - }) - It("should resolve cluster CIDR for IPv4 clusters", func() { - nodeClass.Spec.AMIFamily = lo.ToPtr(v1beta1.AMIFamilyAL2023) - ExpectApplied(ctx, env.Client, nodeClass) - ExpectReconcileSucceeded(ctx, nodeClassController, client.ObjectKeyFromObject(nodeClass)) - Expect(lo.FromPtr(awsEnv.LaunchTemplateProvider.ClusterCIDR.Load())).To(Equal("10.100.0.0/16")) - }) - It("should resolve cluster CIDR for IPv6 clusters", func() { - awsEnv.EKSAPI.DescribeClusterBehavior.Output.Set(&eks.DescribeClusterOutput{ - Cluster: &eks.Cluster{ - KubernetesNetworkConfig: &eks.KubernetesNetworkConfigResponse{ - ServiceIpv6Cidr: lo.ToPtr("2001:db8::/64"), - }, - }, - }) - nodeClass.Spec.AMIFamily = lo.ToPtr(v1beta1.AMIFamilyAL2023) - ExpectApplied(ctx, env.Client, nodeClass) - ExpectReconcileSucceeded(ctx, nodeClassController, client.ObjectKeyFromObject(nodeClass)) - Expect(lo.FromPtr(awsEnv.LaunchTemplateProvider.ClusterCIDR.Load())).To(Equal("2001:db8::/64")) - }) - }) - Context("Subnet Status", func() { - It("Should update EC2NodeClass status for Subnets", func() { - ExpectApplied(ctx, env.Client, nodeClass) - ExpectReconcileSucceeded(ctx, nodeClassController, client.ObjectKeyFromObject(nodeClass)) - nodeClass = ExpectExists(ctx, env.Client, nodeClass) - Expect(nodeClass.Status.Subnets).To(Equal([]v1beta1.Subnet{ - { - ID: "subnet-test1", - Zone: "test-zone-1a", - }, - { - ID: "subnet-test2", - Zone: "test-zone-1b", - }, - { - ID: "subnet-test3", - Zone: "test-zone-1c", - }, - { - ID: "subnet-test4", - Zone: "test-zone-1a-local", - }, - })) - }) - 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, nodeClass) - ExpectReconcileSucceeded(ctx, nodeClassController, client.ObjectKeyFromObject(nodeClass)) - nodeClass = ExpectExists(ctx, env.Client, nodeClass) - Expect(nodeClass.Status.Subnets).To(Equal([]v1beta1.Subnet{ - { - ID: "subnet-test2", - Zone: "test-zone-1b", - }, - { - ID: "subnet-test3", - Zone: "test-zone-1c", - }, - { - ID: "subnet-test1", - Zone: "test-zone-1a", - }, - })) - }) - It("Should resolve a valid selectors for Subnet by tags", func() { - nodeClass.Spec.SubnetSelectorTerms = []v1beta1.SubnetSelectorTerm{ - { - Tags: map[string]string{`Name`: `test-subnet-1`}, - }, - { - Tags: map[string]string{`Name`: `test-subnet-2`}, - }, - } - ExpectApplied(ctx, env.Client, nodeClass) - ExpectReconcileSucceeded(ctx, nodeClassController, client.ObjectKeyFromObject(nodeClass)) - nodeClass = ExpectExists(ctx, env.Client, nodeClass) - Expect(nodeClass.Status.Subnets).To(Equal([]v1beta1.Subnet{ - { - ID: "subnet-test1", - Zone: "test-zone-1a", - }, - { - ID: "subnet-test2", - Zone: "test-zone-1b", - }, - })) - }) - It("Should resolve a valid selectors for Subnet by ids", func() { - nodeClass.Spec.SubnetSelectorTerms = []v1beta1.SubnetSelectorTerm{ - { - ID: "subnet-test1", - }, - } - ExpectApplied(ctx, env.Client, nodeClass) - ExpectReconcileSucceeded(ctx, nodeClassController, client.ObjectKeyFromObject(nodeClass)) - nodeClass = ExpectExists(ctx, env.Client, nodeClass) - Expect(nodeClass.Status.Subnets).To(Equal([]v1beta1.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, nodeClass) - ExpectReconcileSucceeded(ctx, nodeClassController, client.ObjectKeyFromObject(nodeClass)) - nodeClass = ExpectExists(ctx, env.Client, nodeClass) - Expect(nodeClass.Status.Subnets).To(Equal([]v1beta1.Subnet{ - { - ID: "subnet-test1", - Zone: "test-zone-1a", - }, - { - ID: "subnet-test2", - Zone: "test-zone-1b", - }, - { - ID: "subnet-test3", - Zone: "test-zone-1c", - }, - { - ID: "subnet-test4", - Zone: "test-zone-1a-local", - }, - })) - - nodeClass.Spec.SubnetSelectorTerms = []v1beta1.SubnetSelectorTerm{ - { - Tags: map[string]string{ - "Name": "test-subnet-1", - }, - }, - { - Tags: map[string]string{ - "Name": "test-subnet-2", - }, - }, - } - ExpectApplied(ctx, env.Client, nodeClass) - ExpectReconcileSucceeded(ctx, nodeClassController, client.ObjectKeyFromObject(nodeClass)) - nodeClass = ExpectExists(ctx, env.Client, nodeClass) - Expect(nodeClass.Status.Subnets).To(Equal([]v1beta1.Subnet{ - { - ID: "subnet-test1", - Zone: "test-zone-1a", - }, - { - 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, nodeClass) - ExpectReconcileSucceeded(ctx, nodeClassController, client.ObjectKeyFromObject(nodeClass)) - nodeClass = ExpectExists(ctx, env.Client, nodeClass) - Expect(nodeClass.Status.Subnets).To(Equal([]v1beta1.Subnet{ - { - ID: "subnet-test1", - Zone: "test-zone-1a", - }, - { - ID: "subnet-test2", - Zone: "test-zone-1b", - }, - { - ID: "subnet-test3", - Zone: "test-zone-1c", - }, - { - ID: "subnet-test4", - Zone: "test-zone-1a-local", - }, - })) - - nodeClass.Spec.SubnetSelectorTerms = []v1beta1.SubnetSelectorTerm{ - { - ID: "subnet-test1", - }, - } - ExpectApplied(ctx, env.Client, nodeClass) - ExpectReconcileSucceeded(ctx, nodeClassController, client.ObjectKeyFromObject(nodeClass)) - nodeClass = ExpectExists(ctx, env.Client, nodeClass) - Expect(nodeClass.Status.Subnets).To(Equal([]v1beta1.Subnet{ - { - ID: "subnet-test1", - Zone: "test-zone-1a", - }, - })) - }) - It("Should not resolve a invalid selectors for Subnet", func() { - nodeClass.Spec.SubnetSelectorTerms = []v1beta1.SubnetSelectorTerm{ - { - Tags: map[string]string{`foo`: `invalid`}, - }, - } - ExpectApplied(ctx, env.Client, nodeClass) - ExpectReconcileFailed(ctx, nodeClassController, client.ObjectKeyFromObject(nodeClass)) - nodeClass = ExpectExists(ctx, env.Client, nodeClass) - Expect(nodeClass.Status.Subnets).To(BeNil()) - }) - It("Should not resolve a invalid selectors for an updated subnet selector", func() { - ExpectApplied(ctx, env.Client, nodeClass) - ExpectReconcileSucceeded(ctx, nodeClassController, client.ObjectKeyFromObject(nodeClass)) - nodeClass = ExpectExists(ctx, env.Client, nodeClass) - Expect(nodeClass.Status.Subnets).To(Equal([]v1beta1.Subnet{ - { - ID: "subnet-test1", - Zone: "test-zone-1a", - }, - { - ID: "subnet-test2", - Zone: "test-zone-1b", - }, - { - ID: "subnet-test3", - Zone: "test-zone-1c", - }, - { - ID: "subnet-test4", - Zone: "test-zone-1a-local", - }, - })) - - nodeClass.Spec.SubnetSelectorTerms = []v1beta1.SubnetSelectorTerm{ - { - Tags: map[string]string{`foo`: `invalid`}, - }, - } - ExpectApplied(ctx, env.Client, nodeClass) - ExpectReconcileFailed(ctx, nodeClassController, client.ObjectKeyFromObject(nodeClass)) - nodeClass = ExpectExists(ctx, env.Client, nodeClass) - Expect(nodeClass.Status.Subnets).To(BeNil()) - }) - }) - Context("Security Groups Status", func() { - It("Should update EC2NodeClass status for Security Groups", func() { - ExpectApplied(ctx, env.Client, nodeClass) - ExpectReconcileSucceeded(ctx, nodeClassController, client.ObjectKeyFromObject(nodeClass)) - nodeClass = ExpectExists(ctx, env.Client, nodeClass) - Expect(nodeClass.Status.SecurityGroups).To(Equal([]v1beta1.SecurityGroup{ - { - ID: "sg-test1", - Name: "securityGroup-test1", - }, - { - ID: "sg-test2", - Name: "securityGroup-test2", - }, - { - ID: "sg-test3", - Name: "securityGroup-test3", - }, - })) - }) - It("Should resolve a valid selectors for Security Groups by tags", func() { - nodeClass.Spec.SecurityGroupSelectorTerms = []v1beta1.SecurityGroupSelectorTerm{ - { - Tags: map[string]string{"Name": "test-security-group-1"}, - }, - { - Tags: map[string]string{"Name": "test-security-group-2"}, - }, - } - ExpectApplied(ctx, env.Client, nodeClass) - ExpectReconcileSucceeded(ctx, nodeClassController, client.ObjectKeyFromObject(nodeClass)) - nodeClass = ExpectExists(ctx, env.Client, nodeClass) - Expect(nodeClass.Status.SecurityGroups).To(Equal([]v1beta1.SecurityGroup{ - { - ID: "sg-test1", - Name: "securityGroup-test1", - }, - { - ID: "sg-test2", - Name: "securityGroup-test2", - }, - })) - }) - It("Should resolve a valid selectors for Security Groups by ids", func() { - nodeClass.Spec.SecurityGroupSelectorTerms = []v1beta1.SecurityGroupSelectorTerm{ - { - ID: "sg-test1", - }, - } - ExpectApplied(ctx, env.Client, nodeClass) - ExpectReconcileSucceeded(ctx, nodeClassController, client.ObjectKeyFromObject(nodeClass)) - nodeClass = ExpectExists(ctx, env.Client, nodeClass) - Expect(nodeClass.Status.SecurityGroups).To(Equal([]v1beta1.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, nodeClass) - ExpectReconcileSucceeded(ctx, nodeClassController, client.ObjectKeyFromObject(nodeClass)) - nodeClass = ExpectExists(ctx, env.Client, nodeClass) - Expect(nodeClass.Status.SecurityGroups).To(Equal([]v1beta1.SecurityGroup{ - { - ID: "sg-test1", - Name: "securityGroup-test1", - }, - { - ID: "sg-test2", - Name: "securityGroup-test2", - }, - { - ID: "sg-test3", - Name: "securityGroup-test3", - }, - })) - - nodeClass.Spec.SecurityGroupSelectorTerms = []v1beta1.SecurityGroupSelectorTerm{ - { - Tags: map[string]string{"Name": "test-security-group-1"}, - }, - { - Tags: map[string]string{"Name": "test-security-group-2"}, - }, - } - ExpectApplied(ctx, env.Client, nodeClass) - ExpectReconcileSucceeded(ctx, nodeClassController, client.ObjectKeyFromObject(nodeClass)) - nodeClass = ExpectExists(ctx, env.Client, nodeClass) - Expect(nodeClass.Status.SecurityGroups).To(Equal([]v1beta1.SecurityGroup{ - { - ID: "sg-test1", - Name: "securityGroup-test1", - }, - { - 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, nodeClass) - ExpectReconcileSucceeded(ctx, nodeClassController, client.ObjectKeyFromObject(nodeClass)) - nodeClass = ExpectExists(ctx, env.Client, nodeClass) - Expect(nodeClass.Status.SecurityGroups).To(Equal([]v1beta1.SecurityGroup{ - { - ID: "sg-test1", - Name: "securityGroup-test1", - }, - { - ID: "sg-test2", - Name: "securityGroup-test2", - }, - { - ID: "sg-test3", - Name: "securityGroup-test3", - }, - })) - - nodeClass.Spec.SecurityGroupSelectorTerms = []v1beta1.SecurityGroupSelectorTerm{ - { - ID: "sg-test1", - }, - } - ExpectApplied(ctx, env.Client, nodeClass) - ExpectReconcileSucceeded(ctx, nodeClassController, client.ObjectKeyFromObject(nodeClass)) - nodeClass = ExpectExists(ctx, env.Client, nodeClass) - Expect(nodeClass.Status.SecurityGroups).To(Equal([]v1beta1.SecurityGroup{ - { - ID: "sg-test1", - Name: "securityGroup-test1", - }, - })) - }) - It("Should not resolve a invalid selectors for Security Groups", func() { - nodeClass.Spec.SecurityGroupSelectorTerms = []v1beta1.SecurityGroupSelectorTerm{ - { - Tags: map[string]string{`foo`: `invalid`}, - }, - } - ExpectApplied(ctx, env.Client, nodeClass) - ExpectReconcileFailed(ctx, nodeClassController, client.ObjectKeyFromObject(nodeClass)) - nodeClass = ExpectExists(ctx, env.Client, nodeClass) - Expect(nodeClass.Status.SecurityGroups).To(BeNil()) - }) - It("Should not resolve a invalid selectors for an updated Security Groups selector", func() { - ExpectApplied(ctx, env.Client, nodeClass) - ExpectReconcileSucceeded(ctx, nodeClassController, client.ObjectKeyFromObject(nodeClass)) - nodeClass = ExpectExists(ctx, env.Client, nodeClass) - Expect(nodeClass.Status.SecurityGroups).To(Equal([]v1beta1.SecurityGroup{ - { - ID: "sg-test1", - Name: "securityGroup-test1", - }, - { - ID: "sg-test2", - Name: "securityGroup-test2", - }, - { - ID: "sg-test3", - Name: "securityGroup-test3", - }, - })) - - nodeClass.Spec.SecurityGroupSelectorTerms = []v1beta1.SecurityGroupSelectorTerm{ - { - Tags: map[string]string{`foo`: `invalid`}, - }, - } - ExpectApplied(ctx, env.Client, nodeClass) - ExpectReconcileFailed(ctx, nodeClassController, client.ObjectKeyFromObject(nodeClass)) - nodeClass = ExpectExists(ctx, env.Client, nodeClass) - Expect(nodeClass.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", corev1beta1.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")}, - }, - }, - }, - }) - nodeClass.Spec.AMISelectorTerms = nil - ExpectApplied(ctx, env.Client, nodeClass) - ExpectReconcileSucceeded(ctx, nodeClassController, client.ObjectKeyFromObject(nodeClass)) - nodeClass = ExpectExists(ctx, env.Client, nodeClass) - Expect(nodeClass.Status.AMIs).To(Equal([]v1beta1.AMI{ - { - Name: "test-ami-3", - ID: "ami-id-789", - Requirements: []corev1beta1.NodeSelectorRequirementWithMinValues{ - { - NodeSelectorRequirement: v1.NodeSelectorRequirement{ - Key: v1.LabelArchStable, - Operator: v1.NodeSelectorOpIn, - Values: []string{corev1beta1.ArchitectureArm64}, - }, - }, - { - NodeSelectorRequirement: v1.NodeSelectorRequirement{ - Key: v1beta1.LabelInstanceGPUCount, - Operator: v1.NodeSelectorOpDoesNotExist, - }, - }, - { - NodeSelectorRequirement: v1.NodeSelectorRequirement{ - Key: v1beta1.LabelInstanceAcceleratorCount, - Operator: v1.NodeSelectorOpDoesNotExist, - }, - }, - }, - }, - { - Name: "test-ami-2", - ID: "ami-id-456", - Requirements: []corev1beta1.NodeSelectorRequirementWithMinValues{ - { - NodeSelectorRequirement: v1.NodeSelectorRequirement{ - Key: v1.LabelArchStable, - Operator: v1.NodeSelectorOpIn, - Values: []string{corev1beta1.ArchitectureAmd64}, - }, - }, - { - NodeSelectorRequirement: v1.NodeSelectorRequirement{ - Key: v1beta1.LabelInstanceGPUCount, - Operator: v1.NodeSelectorOpExists, - }, - }, - }, - }, - { - Name: "test-ami-2", - ID: "ami-id-456", - Requirements: []corev1beta1.NodeSelectorRequirementWithMinValues{ - { - NodeSelectorRequirement: v1.NodeSelectorRequirement{ - Key: v1.LabelArchStable, - Operator: v1.NodeSelectorOpIn, - Values: []string{corev1beta1.ArchitectureAmd64}, - }, - }, - { - NodeSelectorRequirement: v1.NodeSelectorRequirement{ - Key: v1beta1.LabelInstanceAcceleratorCount, - Operator: v1.NodeSelectorOpExists, - }, - }, - }, - }, - { - Name: "test-ami-1", - ID: "ami-id-123", - Requirements: []corev1beta1.NodeSelectorRequirementWithMinValues{ - { - NodeSelectorRequirement: v1.NodeSelectorRequirement{ - Key: v1.LabelArchStable, - Operator: v1.NodeSelectorOpIn, - Values: []string{corev1beta1.ArchitectureAmd64}, - }, - }, - { - NodeSelectorRequirement: v1.NodeSelectorRequirement{ - Key: v1beta1.LabelInstanceGPUCount, - Operator: v1.NodeSelectorOpDoesNotExist, - }, - }, - { - NodeSelectorRequirement: v1.NodeSelectorRequirement{ - Key: v1beta1.LabelInstanceAcceleratorCount, - Operator: v1.NodeSelectorOpDoesNotExist, - }, - }, - }, - }, - })) - }) - 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", - } - nodeClass.Spec.AMIFamily = &v1beta1.AMIFamilyBottlerocket - nodeClass.Spec.AMISelectorTerms = 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, nodeClass) - ExpectReconcileSucceeded(ctx, nodeClassController, client.ObjectKeyFromObject(nodeClass)) - nodeClass = ExpectExists(ctx, env.Client, nodeClass) - - Expect(nodeClass.Status.AMIs).To(Equal([]v1beta1.AMI{ - { - Name: "test-ami-2", - ID: "ami-id-456", - Requirements: []corev1beta1.NodeSelectorRequirementWithMinValues{ - { - NodeSelectorRequirement: v1.NodeSelectorRequirement{ - Key: v1.LabelArchStable, - Operator: v1.NodeSelectorOpIn, - Values: []string{corev1beta1.ArchitectureArm64}, - }, - }, - { - NodeSelectorRequirement: v1.NodeSelectorRequirement{ - Key: v1beta1.LabelInstanceGPUCount, - Operator: v1.NodeSelectorOpDoesNotExist, - }, - }, - { - NodeSelectorRequirement: v1.NodeSelectorRequirement{ - Key: v1beta1.LabelInstanceAcceleratorCount, - Operator: v1.NodeSelectorOpDoesNotExist, - }, - }, - }, - }, - { - Name: "test-ami-1", - ID: "ami-id-123", - Requirements: []corev1beta1.NodeSelectorRequirementWithMinValues{ - { - NodeSelectorRequirement: v1.NodeSelectorRequirement{ - Key: v1.LabelArchStable, - Operator: v1.NodeSelectorOpIn, - Values: []string{corev1beta1.ArchitectureAmd64}, - }, - }, - { - NodeSelectorRequirement: v1.NodeSelectorRequirement{ - Key: v1beta1.LabelInstanceGPUCount, - Operator: v1.NodeSelectorOpDoesNotExist, - }, - }, - { - NodeSelectorRequirement: v1.NodeSelectorRequirement{ - Key: v1beta1.LabelInstanceAcceleratorCount, - Operator: v1.NodeSelectorOpDoesNotExist, - }, - }, - }, - }, - })) - }) - It("Should resolve a valid AMI selector", func() { - ExpectApplied(ctx, env.Client, nodeClass) - ExpectReconcileSucceeded(ctx, nodeClassController, client.ObjectKeyFromObject(nodeClass)) - nodeClass = ExpectExists(ctx, env.Client, nodeClass) - Expect(nodeClass.Status.AMIs).To(Equal( - []v1beta1.AMI{ - { - Name: "test-ami-3", - ID: "ami-test3", - Requirements: []corev1beta1.NodeSelectorRequirementWithMinValues{ - { - NodeSelectorRequirement: v1.NodeSelectorRequirement{ - Key: "kubernetes.io/arch", - Operator: "In", - Values: []string{ - "amd64", - }, - }, - }, - }, - }, - }, - )) - }) - }) - Context("Static Drift Hash", func() { - DescribeTable("should update the drift hash when static field is updated", func(changes *v1beta1.EC2NodeClass) { - ExpectApplied(ctx, env.Client, nodeClass) - ExpectReconcileSucceeded(ctx, nodeClassController, client.ObjectKeyFromObject(nodeClass)) - nodeClass = ExpectExists(ctx, env.Client, nodeClass) - - expectedHash := nodeClass.Hash() - Expect(nodeClass.ObjectMeta.Annotations[v1beta1.AnnotationEC2NodeClassHash]).To(Equal(expectedHash)) - - Expect(mergo.Merge(nodeClass, changes, mergo.WithOverride)).To(Succeed()) - - ExpectApplied(ctx, env.Client, nodeClass) - ExpectReconcileSucceeded(ctx, nodeClassController, client.ObjectKeyFromObject(nodeClass)) - nodeClass = ExpectExists(ctx, env.Client, nodeClass) - - expectedHashTwo := nodeClass.Hash() - Expect(nodeClass.Annotations[v1beta1.AnnotationEC2NodeClassHash]).To(Equal(expectedHashTwo)) - Expect(expectedHash).ToNot(Equal(expectedHashTwo)) - - }, - Entry("AMIFamily Drift", &v1beta1.EC2NodeClass{Spec: v1beta1.EC2NodeClassSpec{AMIFamily: aws.String(v1beta1.AMIFamilyBottlerocket)}}), - Entry("UserData Drift", &v1beta1.EC2NodeClass{Spec: v1beta1.EC2NodeClassSpec{UserData: aws.String("userdata-test-2")}}), - Entry("Tags Drift", &v1beta1.EC2NodeClass{Spec: v1beta1.EC2NodeClassSpec{Tags: map[string]string{"keyTag-test-3": "valueTag-test-3"}}}), - Entry("BlockDeviceMappings Drift", &v1beta1.EC2NodeClass{Spec: v1beta1.EC2NodeClassSpec{BlockDeviceMappings: []*v1beta1.BlockDeviceMapping{{DeviceName: aws.String("map-device-test-3")}}}}), - Entry("DetailedMonitoring Drift", &v1beta1.EC2NodeClass{Spec: v1beta1.EC2NodeClassSpec{DetailedMonitoring: aws.Bool(true)}}), - Entry("MetadataOptions Drift", &v1beta1.EC2NodeClass{Spec: v1beta1.EC2NodeClassSpec{MetadataOptions: &v1beta1.MetadataOptions{HTTPEndpoint: aws.String("disabled")}}}), - Entry("Context Drift", &v1beta1.EC2NodeClass{Spec: v1beta1.EC2NodeClassSpec{Context: aws.String("context-2")}}), - ) - It("should not update the drift hash when dynamic field is updated", func() { - ExpectApplied(ctx, env.Client, nodeClass) - ExpectReconcileSucceeded(ctx, nodeClassController, client.ObjectKeyFromObject(nodeClass)) - nodeClass = ExpectExists(ctx, env.Client, nodeClass) - - expectedHash := nodeClass.Hash() - Expect(nodeClass.Annotations[v1beta1.AnnotationEC2NodeClassHash]).To(Equal(expectedHash)) - - nodeClass.Spec.SubnetSelectorTerms = []v1beta1.SubnetSelectorTerm{ - { - ID: "subnet-test1", - }, - } - nodeClass.Spec.SecurityGroupSelectorTerms = []v1beta1.SecurityGroupSelectorTerm{ - { - ID: "sg-test1", - }, - } - nodeClass.Spec.AMISelectorTerms = []v1beta1.AMISelectorTerm{ - { - Tags: map[string]string{"ami-test-key": "ami-test-value"}, - }, - } - - ExpectApplied(ctx, env.Client, nodeClass) - ExpectReconcileSucceeded(ctx, nodeClassController, client.ObjectKeyFromObject(nodeClass)) - nodeClass = ExpectExists(ctx, env.Client, nodeClass) - Expect(nodeClass.Annotations[v1beta1.AnnotationEC2NodeClassHash]).To(Equal(expectedHash)) - }) - It("should update ec2nodeclass-hash-version annotation when the ec2nodeclass-hash-version on the NodeClass does not match with the controller hash version", func() { - nodeClass.Annotations = map[string]string{ - v1beta1.AnnotationEC2NodeClassHash: "abceduefed", - v1beta1.AnnotationEC2NodeClassHashVersion: "test", - } - ExpectApplied(ctx, env.Client, nodeClass) - - ExpectReconcileSucceeded(ctx, nodeClassController, client.ObjectKeyFromObject(nodeClass)) - nodeClass = ExpectExists(ctx, env.Client, nodeClass) - - expectedHash := nodeClass.Hash() - // Expect ec2nodeclass-hash on the NodeClass to be updated - Expect(nodeClass.Annotations).To(HaveKeyWithValue(v1beta1.AnnotationEC2NodeClassHash, expectedHash)) - Expect(nodeClass.Annotations).To(HaveKeyWithValue(v1beta1.AnnotationEC2NodeClassHashVersion, v1beta1.EC2NodeClassHashVersion)) - }) - It("should update ec2nodeclass-hash-versions on all NodeClaims when the ec2nodeclass-hash-version does not match with the controller hash version", func() { - nodeClass.Annotations = map[string]string{ - v1beta1.AnnotationEC2NodeClassHash: "abceduefed", - v1beta1.AnnotationEC2NodeClassHashVersion: "test", - } - nodeClaimOne := coretest.NodeClaim(corev1beta1.NodeClaim{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - v1beta1.AnnotationEC2NodeClassHash: "123456", - v1beta1.AnnotationEC2NodeClassHashVersion: "test", - }, - }, - Spec: corev1beta1.NodeClaimSpec{ - NodeClassRef: &corev1beta1.NodeClassReference{ - Name: nodeClass.Name, - }, - }, - }) - nodeClaimTwo := coretest.NodeClaim(corev1beta1.NodeClaim{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - v1beta1.AnnotationEC2NodeClassHash: "123456", - v1beta1.AnnotationEC2NodeClassHashVersion: "test", - }, - }, - Spec: corev1beta1.NodeClaimSpec{ - NodeClassRef: &corev1beta1.NodeClassReference{ - Name: nodeClass.Name, - }, - }, - }) - - ExpectApplied(ctx, env.Client, nodeClass, nodeClaimOne, nodeClaimTwo) - - ExpectReconcileSucceeded(ctx, nodeClassController, client.ObjectKeyFromObject(nodeClass)) - nodeClass = ExpectExists(ctx, env.Client, nodeClass) - nodeClaimOne = ExpectExists(ctx, env.Client, nodeClaimOne) - nodeClaimTwo = ExpectExists(ctx, env.Client, nodeClaimTwo) - - expectedHash := nodeClass.Hash() - // Expect ec2nodeclass-hash on the NodeClaims to be updated - Expect(nodeClaimOne.Annotations).To(HaveKeyWithValue(v1beta1.AnnotationEC2NodeClassHash, expectedHash)) - Expect(nodeClaimOne.Annotations).To(HaveKeyWithValue(v1beta1.AnnotationEC2NodeClassHashVersion, v1beta1.EC2NodeClassHashVersion)) - Expect(nodeClaimTwo.Annotations).To(HaveKeyWithValue(v1beta1.AnnotationEC2NodeClassHash, expectedHash)) - Expect(nodeClaimTwo.Annotations).To(HaveKeyWithValue(v1beta1.AnnotationEC2NodeClassHashVersion, v1beta1.EC2NodeClassHashVersion)) - }) - It("should not update ec2nodeclass-hash on all NodeClaims when the ec2nodeclass-hash-version matches the controller hash version", func() { - nodeClass.Annotations = map[string]string{ - v1beta1.AnnotationEC2NodeClassHash: "abceduefed", - v1beta1.AnnotationEC2NodeClassHashVersion: "test-version", - } - nodeClaim := coretest.NodeClaim(corev1beta1.NodeClaim{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - v1beta1.AnnotationEC2NodeClassHash: "1234564654", - v1beta1.AnnotationEC2NodeClassHashVersion: v1beta1.EC2NodeClassHashVersion, - }, - }, - Spec: corev1beta1.NodeClaimSpec{ - NodeClassRef: &corev1beta1.NodeClassReference{ - Name: nodeClass.Name, - }, - }, - }) - ExpectApplied(ctx, env.Client, nodeClass, nodeClaim) - - ExpectReconcileSucceeded(ctx, nodeClassController, client.ObjectKeyFromObject(nodeClass)) - nodeClass = ExpectExists(ctx, env.Client, nodeClass) - nodeClaim = ExpectExists(ctx, env.Client, nodeClaim) - - expectedHash := nodeClass.Hash() - - // Expect ec2nodeclass-hash on the NodeClass to be updated - Expect(nodeClass.Annotations).To(HaveKeyWithValue(v1beta1.AnnotationEC2NodeClassHash, expectedHash)) - Expect(nodeClass.Annotations).To(HaveKeyWithValue(v1beta1.AnnotationEC2NodeClassHashVersion, v1beta1.EC2NodeClassHashVersion)) - // Expect ec2nodeclass-hash on the NodeClaims to stay the same - Expect(nodeClaim.Annotations).To(HaveKeyWithValue(v1beta1.AnnotationEC2NodeClassHash, "1234564654")) - Expect(nodeClaim.Annotations).To(HaveKeyWithValue(v1beta1.AnnotationEC2NodeClassHashVersion, v1beta1.EC2NodeClassHashVersion)) - }) - It("should not update ec2nodeclass-hash on the NodeClaim if it's drifted and the ec2nodeclass-hash-version does not match the controller hash version", func() { - nodeClass.Annotations = map[string]string{ - v1beta1.AnnotationEC2NodeClassHash: "abceduefed", - v1beta1.AnnotationEC2NodeClassHashVersion: "test", - } - nodeClaim := coretest.NodeClaim(corev1beta1.NodeClaim{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - v1beta1.AnnotationEC2NodeClassHash: "123456", - v1beta1.AnnotationEC2NodeClassHashVersion: "test", - }, - }, - Spec: corev1beta1.NodeClaimSpec{ - NodeClassRef: &corev1beta1.NodeClassReference{ - Name: nodeClass.Name, - }, - }, - }) - nodeClaim.StatusConditions().MarkTrue(corev1beta1.Drifted) - ExpectApplied(ctx, env.Client, nodeClass, nodeClaim) - - ExpectReconcileSucceeded(ctx, nodeClassController, client.ObjectKeyFromObject(nodeClass)) - nodeClaim = ExpectExists(ctx, env.Client, nodeClaim) - - // Expect ec2nodeclass-hash on the NodeClaims to stay the same - Expect(nodeClaim.Annotations).To(HaveKeyWithValue(v1beta1.AnnotationEC2NodeClassHash, "123456")) - Expect(nodeClaim.Annotations).To(HaveKeyWithValue(v1beta1.AnnotationEC2NodeClassHashVersion, v1beta1.EC2NodeClassHashVersion)) - }) - }) - Context("NodeClass Termination", func() { - var profileName string - BeforeEach(func() { - profileName = awsEnv.InstanceProfileProvider.GetProfileName(ctx, fake.DefaultRegion, nodeClass.Name) - }) - It("should not delete the NodeClass if launch template deletion fails", func() { - launchTemplateName := aws.String(fake.LaunchTemplateName()) - awsEnv.EC2API.LaunchTemplates.Store(launchTemplateName, &ec2.LaunchTemplate{LaunchTemplateName: launchTemplateName, LaunchTemplateId: aws.String(fake.LaunchTemplateID()), Tags: []*ec2.Tag{&ec2.Tag{Key: aws.String("karpenter.k8s.aws/cluster"), Value: aws.String("test-cluster")}}}) - _, ok := awsEnv.EC2API.LaunchTemplates.Load(launchTemplateName) - Expect(ok).To(BeTrue()) - ExpectApplied(ctx, env.Client, nodeClass) - ExpectReconcileSucceeded(ctx, nodeClassController, client.ObjectKeyFromObject(nodeClass)) - - Expect(env.Client.Delete(ctx, nodeClass)).To(Succeed()) - awsEnv.EC2API.NextError.Set(fmt.Errorf("delete Launch Template Error")) - ExpectReconcileFailed(ctx, nodeClassController, client.ObjectKeyFromObject(nodeClass)) - ExpectExists(ctx, env.Client, nodeClass) - }) - It("should not delete the launch template not associated with the nodeClass", func() { - launchTemplateName := aws.String(fake.LaunchTemplateName()) - awsEnv.EC2API.LaunchTemplates.Store(launchTemplateName, &ec2.LaunchTemplate{LaunchTemplateName: launchTemplateName, LaunchTemplateId: aws.String(fake.LaunchTemplateID()), Tags: []*ec2.Tag{&ec2.Tag{Key: aws.String("karpenter.k8s.aws/cluster"), Value: aws.String("test-cluster")}}}) - _, ok := awsEnv.EC2API.LaunchTemplates.Load(launchTemplateName) - Expect(ok).To(BeTrue()) - ExpectApplied(ctx, env.Client, nodeClass) - ExpectReconcileSucceeded(ctx, nodeClassController, client.ObjectKeyFromObject(nodeClass)) - - Expect(env.Client.Delete(ctx, nodeClass)).To(Succeed()) - ExpectReconcileSucceeded(ctx, nodeClassController, client.ObjectKeyFromObject(nodeClass)) - _, ok = awsEnv.EC2API.LaunchTemplates.Load(launchTemplateName) - Expect(ok).To(BeTrue()) - ExpectNotFound(ctx, env.Client, nodeClass) - }) - It("should succeed to delete the launch template", func() { - ltName1 := aws.String(fake.LaunchTemplateName()) - awsEnv.EC2API.LaunchTemplates.Store(ltName1, &ec2.LaunchTemplate{LaunchTemplateName: ltName1, LaunchTemplateId: aws.String(fake.LaunchTemplateID()), Tags: []*ec2.Tag{&ec2.Tag{Key: aws.String("karpenter.k8s.aws/cluster"), Value: aws.String("test-cluster")}, {Key: aws.String("karpenter.k8s.aws/ec2nodeclass"), Value: aws.String(nodeClass.Name)}}}) - ltName2 := aws.String(fake.LaunchTemplateName()) - awsEnv.EC2API.LaunchTemplates.Store(ltName2, &ec2.LaunchTemplate{LaunchTemplateName: ltName2, LaunchTemplateId: aws.String(fake.LaunchTemplateID()), Tags: []*ec2.Tag{&ec2.Tag{Key: aws.String("karpenter.k8s.aws/cluster"), Value: aws.String("test-cluster")}, {Key: aws.String("karpenter.k8s.aws/ec2nodeclass"), Value: aws.String(nodeClass.Name)}}}) - _, ok := awsEnv.EC2API.LaunchTemplates.Load(ltName1) - Expect(ok).To(BeTrue()) - _, ok = awsEnv.EC2API.LaunchTemplates.Load(ltName2) - Expect(ok).To(BeTrue()) - ExpectApplied(ctx, env.Client, nodeClass) - ExpectReconcileSucceeded(ctx, nodeClassController, client.ObjectKeyFromObject(nodeClass)) - - Expect(env.Client.Delete(ctx, nodeClass)).To(Succeed()) - ExpectReconcileSucceeded(ctx, nodeClassController, client.ObjectKeyFromObject(nodeClass)) - _, ok = awsEnv.EC2API.LaunchTemplates.Load(ltName1) - Expect(ok).To(BeFalse()) - _, ok = awsEnv.EC2API.LaunchTemplates.Load(ltName2) - Expect(ok).To(BeFalse()) - ExpectNotFound(ctx, env.Client, nodeClass) - }) - It("should succeed to delete the instance profile with no NodeClaims", func() { - awsEnv.IAMAPI.InstanceProfiles = map[string]*iam.InstanceProfile{ - profileName: { - InstanceProfileName: aws.String(profileName), - Roles: []*iam.Role{ - { - RoleId: aws.String(fake.RoleID()), - RoleName: aws.String(nodeClass.Spec.Role), - }, - }, - }, - } - ExpectApplied(ctx, env.Client, nodeClass) - ExpectReconcileSucceeded(ctx, nodeClassController, client.ObjectKeyFromObject(nodeClass)) - Expect(awsEnv.IAMAPI.InstanceProfiles).To(HaveLen(1)) - - Expect(env.Client.Delete(ctx, nodeClass)).To(Succeed()) - ExpectReconcileSucceeded(ctx, nodeClassController, client.ObjectKeyFromObject(nodeClass)) - Expect(awsEnv.IAMAPI.InstanceProfiles).To(HaveLen(0)) - ExpectNotFound(ctx, env.Client, nodeClass) - }) - It("should succeed to delete the instance profile when no roles exist with no NodeClaims", func() { - awsEnv.IAMAPI.InstanceProfiles = map[string]*iam.InstanceProfile{ - profileName: { - InstanceProfileName: aws.String(profileName), - }, - } - ExpectApplied(ctx, env.Client, nodeClass) - ExpectReconcileSucceeded(ctx, nodeClassController, client.ObjectKeyFromObject(nodeClass)) - Expect(awsEnv.IAMAPI.InstanceProfiles).To(HaveLen(1)) - - Expect(env.Client.Delete(ctx, nodeClass)).To(Succeed()) - ExpectReconcileSucceeded(ctx, nodeClassController, client.ObjectKeyFromObject(nodeClass)) - Expect(awsEnv.IAMAPI.InstanceProfiles).To(HaveLen(0)) - ExpectNotFound(ctx, env.Client, nodeClass) - }) - It("should succeed to delete the NodeClass when the instance profile doesn't exist", func() { - Expect(awsEnv.IAMAPI.InstanceProfiles).To(HaveLen(0)) - ExpectApplied(ctx, env.Client, nodeClass) - - Expect(env.Client.Delete(ctx, nodeClass)).To(Succeed()) - ExpectReconcileSucceeded(ctx, nodeClassController, client.ObjectKeyFromObject(nodeClass)) - Expect(awsEnv.IAMAPI.InstanceProfiles).To(HaveLen(0)) - ExpectNotFound(ctx, env.Client, nodeClass) - }) - It("should not delete the EC2NodeClass until all associated NodeClaims are terminated", func() { - var nodeClaims []*corev1beta1.NodeClaim - for i := 0; i < 2; i++ { - nc := coretest.NodeClaim(corev1beta1.NodeClaim{ - Spec: corev1beta1.NodeClaimSpec{ - NodeClassRef: &corev1beta1.NodeClassReference{ - Name: nodeClass.Name, - }, - }, - }) - ExpectApplied(ctx, env.Client, nc) - nodeClaims = append(nodeClaims, nc) - } - awsEnv.IAMAPI.InstanceProfiles = map[string]*iam.InstanceProfile{ - profileName: { - InstanceProfileName: aws.String(profileName), - Roles: []*iam.Role{ - { - RoleId: aws.String(fake.RoleID()), - RoleName: aws.String(nodeClass.Spec.Role), - }, - }, - }, - } - ExpectApplied(ctx, env.Client, nodeClass) - ExpectReconcileSucceeded(ctx, nodeClassController, client.ObjectKeyFromObject(nodeClass)) - Expect(awsEnv.IAMAPI.InstanceProfiles).To(HaveLen(1)) - - Expect(env.Client.Delete(ctx, nodeClass)).To(Succeed()) - res := ExpectReconcileSucceeded(ctx, nodeClassController, client.ObjectKeyFromObject(nodeClass)) - Expect(res.RequeueAfter).To(Equal(time.Minute * 10)) - Expect(awsEnv.IAMAPI.InstanceProfiles).To(HaveLen(1)) - ExpectExists(ctx, env.Client, nodeClass) - - // Delete one of the NodeClaims - // The NodeClass should still not delete - ExpectDeleted(ctx, env.Client, nodeClaims[0]) - res = ExpectReconcileSucceeded(ctx, nodeClassController, client.ObjectKeyFromObject(nodeClass)) - Expect(res.RequeueAfter).To(Equal(time.Minute * 10)) - Expect(awsEnv.IAMAPI.InstanceProfiles).To(HaveLen(1)) - ExpectExists(ctx, env.Client, nodeClass) - - // Delete the last NodeClaim - // The NodeClass should now delete - ExpectDeleted(ctx, env.Client, nodeClaims[1]) - ExpectReconcileSucceeded(ctx, nodeClassController, client.ObjectKeyFromObject(nodeClass)) - Expect(awsEnv.IAMAPI.InstanceProfiles).To(HaveLen(0)) - ExpectNotFound(ctx, env.Client, nodeClass) - }) - It("should not call the IAM API when deleting a NodeClass with an instanceProfile specified", func() { - awsEnv.IAMAPI.InstanceProfiles = map[string]*iam.InstanceProfile{ - profileName: { - InstanceProfileName: aws.String("test-instance-profile"), - Roles: []*iam.Role{ - { - RoleId: aws.String(fake.RoleID()), - RoleName: aws.String("fake-role"), - }, - }, - }, - } - nodeClass.Spec.Role = "" - nodeClass.Spec.InstanceProfile = lo.ToPtr("test-instance-profile") - ExpectApplied(ctx, env.Client, nodeClass) - ExpectReconcileSucceeded(ctx, nodeClassController, client.ObjectKeyFromObject(nodeClass)) - Expect(awsEnv.IAMAPI.InstanceProfiles).To(HaveLen(1)) - - Expect(env.Client.Delete(ctx, nodeClass)).To(Succeed()) - ExpectReconcileSucceeded(ctx, nodeClassController, client.ObjectKeyFromObject(nodeClass)) - Expect(awsEnv.IAMAPI.InstanceProfiles).To(HaveLen(1)) - ExpectNotFound(ctx, env.Client, nodeClass) - - Expect(awsEnv.IAMAPI.DeleteInstanceProfileBehavior.Calls()).To(BeZero()) - Expect(awsEnv.IAMAPI.RemoveRoleFromInstanceProfileBehavior.Calls()).To(BeZero()) - }) - }) - Context("Instance Profile Status", func() { - var profileName string - BeforeEach(func() { - profileName = awsEnv.InstanceProfileProvider.GetProfileName(ctx, fake.DefaultRegion, nodeClass.Name) - }) - It("should create the instance profile when it doesn't exist", func() { - nodeClass.Spec.Role = "test-role" - ExpectApplied(ctx, env.Client, nodeClass) - ExpectReconcileSucceeded(ctx, nodeClassController, client.ObjectKeyFromObject(nodeClass)) - - Expect(awsEnv.IAMAPI.InstanceProfiles).To(HaveLen(1)) - Expect(awsEnv.IAMAPI.InstanceProfiles[profileName].Roles).To(HaveLen(1)) - Expect(*awsEnv.IAMAPI.InstanceProfiles[profileName].Roles[0].RoleName).To(Equal("test-role")) - - nodeClass = ExpectExists(ctx, env.Client, nodeClass) - Expect(nodeClass.Status.InstanceProfile).To(Equal(profileName)) - }) - It("should add the role to the instance profile when it exists without a role", func() { - awsEnv.IAMAPI.InstanceProfiles = map[string]*iam.InstanceProfile{ - profileName: { - InstanceProfileId: aws.String(fake.InstanceProfileID()), - InstanceProfileName: aws.String(profileName), - }, - } - - nodeClass.Spec.Role = "test-role" - ExpectApplied(ctx, env.Client, nodeClass) - ExpectReconcileSucceeded(ctx, nodeClassController, client.ObjectKeyFromObject(nodeClass)) - - Expect(awsEnv.IAMAPI.InstanceProfiles).To(HaveLen(1)) - Expect(awsEnv.IAMAPI.InstanceProfiles[profileName].Roles).To(HaveLen(1)) - Expect(*awsEnv.IAMAPI.InstanceProfiles[profileName].Roles[0].RoleName).To(Equal("test-role")) - - nodeClass = ExpectExists(ctx, env.Client, nodeClass) - Expect(nodeClass.Status.InstanceProfile).To(Equal(profileName)) - }) - It("should update the role for the instance profile when the wrong role exists", func() { - awsEnv.IAMAPI.InstanceProfiles = map[string]*iam.InstanceProfile{ - profileName: { - InstanceProfileId: aws.String(fake.InstanceProfileID()), - InstanceProfileName: aws.String(profileName), - Roles: []*iam.Role{ - { - RoleName: aws.String("other-role"), - }, - }, - }, - } - - nodeClass.Spec.Role = "test-role" - ExpectApplied(ctx, env.Client, nodeClass) - ExpectReconcileSucceeded(ctx, nodeClassController, client.ObjectKeyFromObject(nodeClass)) - - Expect(awsEnv.IAMAPI.InstanceProfiles).To(HaveLen(1)) - Expect(awsEnv.IAMAPI.InstanceProfiles[profileName].Roles).To(HaveLen(1)) - Expect(*awsEnv.IAMAPI.InstanceProfiles[profileName].Roles[0].RoleName).To(Equal("test-role")) - - nodeClass = ExpectExists(ctx, env.Client, nodeClass) - Expect(nodeClass.Status.InstanceProfile).To(Equal(profileName)) - }) - It("should not call CreateInstanceProfile or AddRoleToInstanceProfile when instance profile exists with correct role", func() { - awsEnv.IAMAPI.InstanceProfiles = map[string]*iam.InstanceProfile{ - profileName: { - InstanceProfileId: aws.String(fake.InstanceProfileID()), - InstanceProfileName: aws.String(profileName), - Roles: []*iam.Role{ - { - RoleName: aws.String("test-role"), - }, - }, - }, - } - - nodeClass.Spec.Role = "test-role" - ExpectApplied(ctx, env.Client, nodeClass) - ExpectReconcileSucceeded(ctx, nodeClassController, client.ObjectKeyFromObject(nodeClass)) - - Expect(awsEnv.IAMAPI.InstanceProfiles).To(HaveLen(1)) - Expect(awsEnv.IAMAPI.InstanceProfiles[profileName].Roles).To(HaveLen(1)) - Expect(*awsEnv.IAMAPI.InstanceProfiles[profileName].Roles[0].RoleName).To(Equal("test-role")) - - Expect(awsEnv.IAMAPI.CreateInstanceProfileBehavior.Calls()).To(BeZero()) - Expect(awsEnv.IAMAPI.AddRoleToInstanceProfileBehavior.Calls()).To(BeZero()) - }) - It("should resolve the specified instance profile into the status when using instanceProfile field", func() { - nodeClass.Spec.Role = "" - nodeClass.Spec.InstanceProfile = lo.ToPtr("test-instance-profile") - ExpectApplied(ctx, env.Client, nodeClass) - ExpectReconcileSucceeded(ctx, nodeClassController, client.ObjectKeyFromObject(nodeClass)) - - nodeClass = ExpectExists(ctx, env.Client, nodeClass) - Expect(nodeClass.Status.InstanceProfile).To(Equal(lo.FromPtr(nodeClass.Spec.InstanceProfile))) - }) - It("should not call the the IAM API when specifying an instance profile", func() { - nodeClass.Spec.Role = "" - nodeClass.Spec.InstanceProfile = lo.ToPtr("test-instance-profile") - ExpectApplied(ctx, env.Client, nodeClass) - ExpectReconcileSucceeded(ctx, nodeClassController, client.ObjectKeyFromObject(nodeClass)) - - Expect(awsEnv.IAMAPI.CreateInstanceProfileBehavior.Calls()).To(BeZero()) - Expect(awsEnv.IAMAPI.AddRoleToInstanceProfileBehavior.Calls()).To(BeZero()) - }) - }) -}) diff --git a/pkg/controllers/nodeclass/termination/controller.go b/pkg/controllers/nodeclass/termination/controller.go new file mode 100644 index 000000000000..39a740357700 --- /dev/null +++ b/pkg/controllers/nodeclass/termination/controller.go @@ -0,0 +1,142 @@ +/* +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 termination + +import ( + "context" + "fmt" + "time" + + "k8s.io/apimachinery/pkg/api/errors" + + "github.com/aws/karpenter-provider-aws/pkg/providers/launchtemplate" + + "github.com/samber/lo" + "golang.org/x/time/rate" + "k8s.io/apimachinery/pkg/api/equality" + "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/util/workqueue" + controllerruntime "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/builder" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + "sigs.k8s.io/controller-runtime/pkg/event" + "sigs.k8s.io/controller-runtime/pkg/handler" + "sigs.k8s.io/controller-runtime/pkg/manager" + "sigs.k8s.io/controller-runtime/pkg/predicate" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + corev1beta1 "sigs.k8s.io/karpenter/pkg/apis/v1beta1" + "sigs.k8s.io/karpenter/pkg/events" + corecontroller "sigs.k8s.io/karpenter/pkg/operator/controller" + + "github.com/aws/karpenter-provider-aws/pkg/apis/v1beta1" + "github.com/aws/karpenter-provider-aws/pkg/providers/instanceprofile" +) + +var _ corecontroller.FinalizingTypedController[*v1beta1.EC2NodeClass] = (*Controller)(nil) + +type Controller struct { + kubeClient client.Client + recorder events.Recorder + instanceProfileProvider instanceprofile.Provider + launchTemplateProvider launchtemplate.Provider +} + +func NewController(kubeClient client.Client, recorder events.Recorder, + instanceProfileProvider instanceprofile.Provider, launchTemplateProvider launchtemplate.Provider) corecontroller.Controller { + + return corecontroller.Typed[*v1beta1.EC2NodeClass](kubeClient, &Controller{ + kubeClient: kubeClient, + recorder: recorder, + instanceProfileProvider: instanceProfileProvider, + launchTemplateProvider: launchTemplateProvider, + }) +} + +func (c *Controller) Reconcile(ctx context.Context, nodeClass *v1beta1.EC2NodeClass) (reconcile.Result, error) { + return reconcile.Result{}, nil +} + +func (c *Controller) Finalize(ctx context.Context, nodeClass *v1beta1.EC2NodeClass) (reconcile.Result, error) { + stored := nodeClass.DeepCopy() + if !controllerutil.ContainsFinalizer(nodeClass, v1beta1.TerminationFinalizer) { + return reconcile.Result{}, nil + } + nodeClaimList := &corev1beta1.NodeClaimList{} + if err := c.kubeClient.List(ctx, nodeClaimList, client.MatchingFields{"spec.nodeClassRef.name": nodeClass.Name}); err != nil { + return reconcile.Result{}, fmt.Errorf("listing nodeclaims that are using nodeclass, %w", err) + } + if len(nodeClaimList.Items) > 0 { + c.recorder.Publish(WaitingOnNodeClaimTerminationEvent(nodeClass, lo.Map(nodeClaimList.Items, func(nc corev1beta1.NodeClaim, _ int) string { return nc.Name }))) + return reconcile.Result{RequeueAfter: time.Minute * 10}, nil // periodically fire the event + } + if nodeClass.Spec.Role != "" { + if err := c.instanceProfileProvider.Delete(ctx, nodeClass); err != nil { + return reconcile.Result{}, fmt.Errorf("deleting instance profile, %w", err) + } + } + if err := c.launchTemplateProvider.DeleteAll(ctx, nodeClass); err != nil { + return reconcile.Result{}, fmt.Errorf("deleting launch templates, %w", err) + } + controllerutil.RemoveFinalizer(nodeClass, v1beta1.TerminationFinalizer) + if !equality.Semantic.DeepEqual(stored, nodeClass) { + // We call Update() here rather than Patch() because patching a list with a JSON merge patch + // can cause races due to the fact that it fully replaces the list on a change + // https://github.com/kubernetes/kubernetes/issues/111643#issuecomment-2016489732 + if err := c.kubeClient.Update(ctx, nodeClass); err != nil { + if errors.IsConflict(err) { + return reconcile.Result{Requeue: true}, nil + } + return reconcile.Result{}, client.IgnoreNotFound(fmt.Errorf("removing termination finalizer, %w", err)) + } + } + return reconcile.Result{}, nil +} + +func (c *Controller) Name() string { + return "nodeclass.termination" +} + +func (c *Controller) Builder(_ context.Context, m manager.Manager) corecontroller.Builder { + return corecontroller.Adapt(controllerruntime. + NewControllerManagedBy(m). + For(&v1beta1.EC2NodeClass{}). + Watches( + &corev1beta1.NodeClaim{}, + handler.EnqueueRequestsFromMapFunc(func(_ context.Context, o client.Object) []reconcile.Request { + nc := o.(*corev1beta1.NodeClaim) + if nc.Spec.NodeClassRef == nil { + return nil + } + return []reconcile.Request{{NamespacedName: types.NamespacedName{Name: nc.Spec.NodeClassRef.Name}}} + }), + // Watch for NodeClaim deletion events + builder.WithPredicates(predicate.Funcs{ + CreateFunc: func(e event.CreateEvent) bool { return false }, + UpdateFunc: func(e event.UpdateEvent) bool { return false }, + DeleteFunc: func(e event.DeleteEvent) bool { return true }, + }), + ). + WithOptions(controller.Options{ + RateLimiter: workqueue.NewMaxOfRateLimiter( + workqueue.NewItemExponentialFailureRateLimiter(100*time.Millisecond, 1*time.Minute), + // 10 qps, 100 bucket size + &workqueue.BucketRateLimiter{Limiter: rate.NewLimiter(rate.Limit(10), 100)}, + ), + MaxConcurrentReconciles: 10, + })) +} diff --git a/pkg/controllers/nodeclass/events.go b/pkg/controllers/nodeclass/termination/events.go similarity index 98% rename from pkg/controllers/nodeclass/events.go rename to pkg/controllers/nodeclass/termination/events.go index e2076f574ba7..8da543ed2bd8 100644 --- a/pkg/controllers/nodeclass/events.go +++ b/pkg/controllers/nodeclass/termination/events.go @@ -12,7 +12,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package nodeclass +package termination import ( "fmt" diff --git a/pkg/controllers/nodeclass/termination/suite_test.go b/pkg/controllers/nodeclass/termination/suite_test.go new file mode 100644 index 000000000000..236490b99e5d --- /dev/null +++ b/pkg/controllers/nodeclass/termination/suite_test.go @@ -0,0 +1,285 @@ +/* +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 termination_test + +import ( + "context" + "fmt" + "testing" + "time" + + "github.com/samber/lo" + "k8s.io/client-go/tools/record" + _ "knative.dev/pkg/system/testing" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/ec2" + "github.com/aws/aws-sdk-go/service/iam" + corev1beta1 "sigs.k8s.io/karpenter/pkg/apis/v1beta1" + "sigs.k8s.io/karpenter/pkg/events" + corecontroller "sigs.k8s.io/karpenter/pkg/operator/controller" + coreoptions "sigs.k8s.io/karpenter/pkg/operator/options" + "sigs.k8s.io/karpenter/pkg/operator/scheme" + coretest "sigs.k8s.io/karpenter/pkg/test" + + "github.com/aws/karpenter-provider-aws/pkg/apis" + "github.com/aws/karpenter-provider-aws/pkg/apis/v1beta1" + "github.com/aws/karpenter-provider-aws/pkg/controllers/nodeclass/termination" + "github.com/aws/karpenter-provider-aws/pkg/fake" + "github.com/aws/karpenter-provider-aws/pkg/operator/options" + "github.com/aws/karpenter-provider-aws/pkg/test" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + . "knative.dev/pkg/logging/testing" + . "sigs.k8s.io/karpenter/pkg/test/expectations" +) + +var ctx context.Context +var env *coretest.Environment +var awsEnv *test.Environment +var terminationController corecontroller.Controller + +func TestAPIs(t *testing.T) { + ctx = TestContextWithLogger(t) + RegisterFailHandler(Fail) + RunSpecs(t, "EC2NodeClass") +} + +var _ = BeforeSuite(func() { + env = coretest.NewEnvironment(scheme.Scheme, coretest.WithCRDs(apis.CRDs...), coretest.WithFieldIndexers(test.EC2NodeClassFieldIndexer(ctx))) + ctx = coreoptions.ToContext(ctx, coretest.Options()) + ctx = options.ToContext(ctx, test.Options()) + awsEnv = test.NewEnvironment(ctx, env) + + terminationController = termination.NewController(env.Client, events.NewRecorder(&record.FakeRecorder{}), awsEnv.InstanceProfileProvider, awsEnv.LaunchTemplateProvider) +}) + +var _ = AfterSuite(func() { + Expect(env.Stop()).To(Succeed(), "Failed to stop environment") +}) + +var _ = BeforeEach(func() { + ctx = coreoptions.ToContext(ctx, coretest.Options()) + awsEnv.Reset() +}) + +var _ = AfterEach(func() { + ExpectCleanedUp(ctx, env.Client) +}) + +var _ = Describe("NodeClass Termination", func() { + var nodeClass *v1beta1.EC2NodeClass + var profileName string + BeforeEach(func() { + nodeClass = test.EC2NodeClass(v1beta1.EC2NodeClass{ + Spec: v1beta1.EC2NodeClassSpec{ + SubnetSelectorTerms: []v1beta1.SubnetSelectorTerm{ + { + Tags: map[string]string{"*": "*"}, + }, + }, + SecurityGroupSelectorTerms: []v1beta1.SecurityGroupSelectorTerm{ + { + Tags: map[string]string{"*": "*"}, + }, + }, + AMISelectorTerms: []v1beta1.AMISelectorTerm{ + { + Tags: map[string]string{"*": "*"}, + }, + }, + }, + }) + profileName = awsEnv.InstanceProfileProvider.GetProfileName(ctx, fake.DefaultRegion, nodeClass.Name) + }) + It("should not delete the NodeClass if launch template deletion fails", func() { + launchTemplateName := aws.String(fake.LaunchTemplateName()) + awsEnv.EC2API.LaunchTemplates.Store(launchTemplateName, &ec2.LaunchTemplate{LaunchTemplateName: launchTemplateName, LaunchTemplateId: aws.String(fake.LaunchTemplateID()), Tags: []*ec2.Tag{&ec2.Tag{Key: aws.String("karpenter.k8s.aws/cluster"), Value: aws.String("test-cluster")}}}) + _, ok := awsEnv.EC2API.LaunchTemplates.Load(launchTemplateName) + Expect(ok).To(BeTrue()) + controllerutil.AddFinalizer(nodeClass, v1beta1.TerminationFinalizer) + ExpectApplied(ctx, env.Client, nodeClass) + ExpectReconcileSucceeded(ctx, terminationController, client.ObjectKeyFromObject(nodeClass)) + + Expect(env.Client.Delete(ctx, nodeClass)).To(Succeed()) + awsEnv.EC2API.NextError.Set(fmt.Errorf("delete Launch Template Error")) + ExpectReconcileFailed(ctx, terminationController, client.ObjectKeyFromObject(nodeClass)) + ExpectExists(ctx, env.Client, nodeClass) + }) + It("should not delete the launch template not associated with the nodeClass", func() { + launchTemplateName := aws.String(fake.LaunchTemplateName()) + awsEnv.EC2API.LaunchTemplates.Store(launchTemplateName, &ec2.LaunchTemplate{LaunchTemplateName: launchTemplateName, LaunchTemplateId: aws.String(fake.LaunchTemplateID()), Tags: []*ec2.Tag{&ec2.Tag{Key: aws.String("karpenter.k8s.aws/cluster"), Value: aws.String("test-cluster")}}}) + _, ok := awsEnv.EC2API.LaunchTemplates.Load(launchTemplateName) + Expect(ok).To(BeTrue()) + controllerutil.AddFinalizer(nodeClass, v1beta1.TerminationFinalizer) + ExpectApplied(ctx, env.Client, nodeClass) + ExpectReconcileSucceeded(ctx, terminationController, client.ObjectKeyFromObject(nodeClass)) + + Expect(env.Client.Delete(ctx, nodeClass)).To(Succeed()) + ExpectReconcileSucceeded(ctx, terminationController, client.ObjectKeyFromObject(nodeClass)) + _, ok = awsEnv.EC2API.LaunchTemplates.Load(launchTemplateName) + Expect(ok).To(BeTrue()) + ExpectNotFound(ctx, env.Client, nodeClass) + }) + It("should succeed to delete the launch template", func() { + ltName1 := aws.String(fake.LaunchTemplateName()) + awsEnv.EC2API.LaunchTemplates.Store(ltName1, &ec2.LaunchTemplate{LaunchTemplateName: ltName1, LaunchTemplateId: aws.String(fake.LaunchTemplateID()), Tags: []*ec2.Tag{&ec2.Tag{Key: aws.String("karpenter.k8s.aws/cluster"), Value: aws.String("test-cluster")}, {Key: aws.String("karpenter.k8s.aws/ec2nodeclass"), Value: aws.String(nodeClass.Name)}}}) + ltName2 := aws.String(fake.LaunchTemplateName()) + awsEnv.EC2API.LaunchTemplates.Store(ltName2, &ec2.LaunchTemplate{LaunchTemplateName: ltName2, LaunchTemplateId: aws.String(fake.LaunchTemplateID()), Tags: []*ec2.Tag{&ec2.Tag{Key: aws.String("karpenter.k8s.aws/cluster"), Value: aws.String("test-cluster")}, {Key: aws.String("karpenter.k8s.aws/ec2nodeclass"), Value: aws.String(nodeClass.Name)}}}) + _, ok := awsEnv.EC2API.LaunchTemplates.Load(ltName1) + Expect(ok).To(BeTrue()) + _, ok = awsEnv.EC2API.LaunchTemplates.Load(ltName2) + Expect(ok).To(BeTrue()) + controllerutil.AddFinalizer(nodeClass, v1beta1.TerminationFinalizer) + ExpectApplied(ctx, env.Client, nodeClass) + ExpectReconcileSucceeded(ctx, terminationController, client.ObjectKeyFromObject(nodeClass)) + + Expect(env.Client.Delete(ctx, nodeClass)).To(Succeed()) + ExpectReconcileSucceeded(ctx, terminationController, client.ObjectKeyFromObject(nodeClass)) + _, ok = awsEnv.EC2API.LaunchTemplates.Load(ltName1) + Expect(ok).To(BeFalse()) + _, ok = awsEnv.EC2API.LaunchTemplates.Load(ltName2) + Expect(ok).To(BeFalse()) + ExpectNotFound(ctx, env.Client, nodeClass) + }) + It("should succeed to delete the instance profile with no NodeClaims", func() { + awsEnv.IAMAPI.InstanceProfiles = map[string]*iam.InstanceProfile{ + profileName: { + InstanceProfileName: aws.String(profileName), + Roles: []*iam.Role{ + { + RoleId: aws.String(fake.RoleID()), + RoleName: aws.String(nodeClass.Spec.Role), + }, + }, + }, + } + controllerutil.AddFinalizer(nodeClass, v1beta1.TerminationFinalizer) + ExpectApplied(ctx, env.Client, nodeClass) + ExpectReconcileSucceeded(ctx, terminationController, client.ObjectKeyFromObject(nodeClass)) + Expect(awsEnv.IAMAPI.InstanceProfiles).To(HaveLen(1)) + + Expect(env.Client.Delete(ctx, nodeClass)).To(Succeed()) + ExpectReconcileSucceeded(ctx, terminationController, client.ObjectKeyFromObject(nodeClass)) + Expect(awsEnv.IAMAPI.InstanceProfiles).To(HaveLen(0)) + ExpectNotFound(ctx, env.Client, nodeClass) + }) + It("should succeed to delete the instance profile when no roles exist with no NodeClaims", func() { + awsEnv.IAMAPI.InstanceProfiles = map[string]*iam.InstanceProfile{ + profileName: { + InstanceProfileName: aws.String(profileName), + }, + } + controllerutil.AddFinalizer(nodeClass, v1beta1.TerminationFinalizer) + ExpectApplied(ctx, env.Client, nodeClass) + ExpectReconcileSucceeded(ctx, terminationController, client.ObjectKeyFromObject(nodeClass)) + Expect(awsEnv.IAMAPI.InstanceProfiles).To(HaveLen(1)) + + Expect(env.Client.Delete(ctx, nodeClass)).To(Succeed()) + ExpectReconcileSucceeded(ctx, terminationController, client.ObjectKeyFromObject(nodeClass)) + Expect(awsEnv.IAMAPI.InstanceProfiles).To(HaveLen(0)) + ExpectNotFound(ctx, env.Client, nodeClass) + }) + It("should succeed to delete the NodeClass when the instance profile doesn't exist", func() { + Expect(awsEnv.IAMAPI.InstanceProfiles).To(HaveLen(0)) + controllerutil.AddFinalizer(nodeClass, v1beta1.TerminationFinalizer) + ExpectApplied(ctx, env.Client, nodeClass) + + Expect(env.Client.Delete(ctx, nodeClass)).To(Succeed()) + ExpectReconcileSucceeded(ctx, terminationController, client.ObjectKeyFromObject(nodeClass)) + Expect(awsEnv.IAMAPI.InstanceProfiles).To(HaveLen(0)) + ExpectNotFound(ctx, env.Client, nodeClass) + }) + It("should not delete the EC2NodeClass until all associated NodeClaims are terminated", func() { + var nodeClaims []*corev1beta1.NodeClaim + for i := 0; i < 2; i++ { + nc := coretest.NodeClaim(corev1beta1.NodeClaim{ + Spec: corev1beta1.NodeClaimSpec{ + NodeClassRef: &corev1beta1.NodeClassReference{ + Name: nodeClass.Name, + }, + }, + }) + ExpectApplied(ctx, env.Client, nc) + nodeClaims = append(nodeClaims, nc) + } + awsEnv.IAMAPI.InstanceProfiles = map[string]*iam.InstanceProfile{ + profileName: { + InstanceProfileName: aws.String(profileName), + Roles: []*iam.Role{ + { + RoleId: aws.String(fake.RoleID()), + RoleName: aws.String(nodeClass.Spec.Role), + }, + }, + }, + } + controllerutil.AddFinalizer(nodeClass, v1beta1.TerminationFinalizer) + ExpectApplied(ctx, env.Client, nodeClass) + ExpectReconcileSucceeded(ctx, terminationController, client.ObjectKeyFromObject(nodeClass)) + Expect(awsEnv.IAMAPI.InstanceProfiles).To(HaveLen(1)) + + Expect(env.Client.Delete(ctx, nodeClass)).To(Succeed()) + res := ExpectReconcileSucceeded(ctx, terminationController, client.ObjectKeyFromObject(nodeClass)) + Expect(res.RequeueAfter).To(Equal(time.Minute * 10)) + Expect(awsEnv.IAMAPI.InstanceProfiles).To(HaveLen(1)) + ExpectExists(ctx, env.Client, nodeClass) + + // Delete one of the NodeClaims + // The NodeClass should still not delete + ExpectDeleted(ctx, env.Client, nodeClaims[0]) + res = ExpectReconcileSucceeded(ctx, terminationController, client.ObjectKeyFromObject(nodeClass)) + Expect(res.RequeueAfter).To(Equal(time.Minute * 10)) + Expect(awsEnv.IAMAPI.InstanceProfiles).To(HaveLen(1)) + ExpectExists(ctx, env.Client, nodeClass) + + // Delete the last NodeClaim + // The NodeClass should now delete + ExpectDeleted(ctx, env.Client, nodeClaims[1]) + ExpectReconcileSucceeded(ctx, terminationController, client.ObjectKeyFromObject(nodeClass)) + Expect(awsEnv.IAMAPI.InstanceProfiles).To(HaveLen(0)) + ExpectNotFound(ctx, env.Client, nodeClass) + }) + It("should not call the IAM API when deleting a NodeClass with an instanceProfile specified", func() { + awsEnv.IAMAPI.InstanceProfiles = map[string]*iam.InstanceProfile{ + profileName: { + InstanceProfileName: aws.String("test-instance-profile"), + Roles: []*iam.Role{ + { + RoleId: aws.String(fake.RoleID()), + RoleName: aws.String("fake-role"), + }, + }, + }, + } + nodeClass.Spec.Role = "" + nodeClass.Spec.InstanceProfile = lo.ToPtr("test-instance-profile") + controllerutil.AddFinalizer(nodeClass, v1beta1.TerminationFinalizer) + ExpectApplied(ctx, env.Client, nodeClass) + ExpectReconcileSucceeded(ctx, terminationController, client.ObjectKeyFromObject(nodeClass)) + Expect(awsEnv.IAMAPI.InstanceProfiles).To(HaveLen(1)) + + Expect(env.Client.Delete(ctx, nodeClass)).To(Succeed()) + ExpectReconcileSucceeded(ctx, terminationController, client.ObjectKeyFromObject(nodeClass)) + Expect(awsEnv.IAMAPI.InstanceProfiles).To(HaveLen(1)) + ExpectNotFound(ctx, env.Client, nodeClass) + + Expect(awsEnv.IAMAPI.DeleteInstanceProfileBehavior.Calls()).To(BeZero()) + Expect(awsEnv.IAMAPI.RemoveRoleFromInstanceProfileBehavior.Calls()).To(BeZero()) + }) +}) diff --git a/website/content/en/preview/upgrading/upgrade-guide.md b/website/content/en/preview/upgrading/upgrade-guide.md index 1dfdfb3c8e7a..b991ad2dd856 100644 --- a/website/content/en/preview/upgrading/upgrade-guide.md +++ b/website/content/en/preview/upgrading/upgrade-guide.md @@ -37,6 +37,10 @@ kubectl apply -f https://raw.githubusercontent.com/aws/karpenter{{< githubRelRef WHEN CREATING A NEW SECTION OF THE UPGRADE GUIDANCE FOR NEWER VERSIONS, ENSURE THAT YOU COPY THE BETA API ALERT SECTION FROM THE LAST RELEASE TO PROPERLY WARN USERS OF THE RISK OF UPGRADING WITHOUT GOING TO 0.32.x FIRST --> +### Upgrading to `0.37.0`+ + +* Karpenter updated the NodeClass controller naming in the following way: `nodeclass` -> `nodeclass.status`, `nodeclass.hash`, `nodeclass.termination` + ### Upgrading to `0.36.0`+ {{% alert title="Warning" color="warning" %}}