From c3941b2ba5e83762be6d028a4ca0330251e70bd3 Mon Sep 17 00:00:00 2001 From: Nick Tran <10810510+njtran@users.noreply.github.com> Date: Tue, 31 Oct 2023 17:34:31 -0700 Subject: [PATCH] chore: remove v1alpha5 hash and disruption controllers (#645) --- pkg/controllers/controllers.go | 2 - .../nodeclaim/disruption/controller.go | 64 --- ...{nodeclaim_drift_test.go => drift_test.go} | 2 +- ...im_emptiness_test.go => emptiness_test.go} | 2 +- ..._expiration_test.go => expiration_test.go} | 2 +- .../disruption/machine_drift_test.go | 437 ------------------ .../disruption/machine_emptiness_test.go | 141 ------ .../disruption/machine_expiration_test.go | 110 ----- .../nodeclaim/disruption/machine_test.go | 84 ---- .../nodeclaim/disruption/nodeclaim_test.go | 84 ---- .../nodeclaim/disruption/suite_test.go | 56 ++- pkg/controllers/nodepool/hash/controller.go | 29 -- .../nodepool/hash/nodepool_test.go | 108 ----- .../nodepool/hash/provisioner_test.go | 107 ----- pkg/controllers/nodepool/hash/suite_test.go | 90 +++- 15 files changed, 144 insertions(+), 1174 deletions(-) rename pkg/controllers/nodeclaim/disruption/{nodeclaim_drift_test.go => drift_test.go} (99%) rename pkg/controllers/nodeclaim/disruption/{nodeclaim_emptiness_test.go => emptiness_test.go} (99%) rename pkg/controllers/nodeclaim/disruption/{nodeclaim_expiration_test.go => expiration_test.go} (98%) delete mode 100644 pkg/controllers/nodeclaim/disruption/machine_drift_test.go delete mode 100644 pkg/controllers/nodeclaim/disruption/machine_emptiness_test.go delete mode 100644 pkg/controllers/nodeclaim/disruption/machine_expiration_test.go delete mode 100644 pkg/controllers/nodeclaim/disruption/machine_test.go delete mode 100644 pkg/controllers/nodeclaim/disruption/nodeclaim_test.go delete mode 100644 pkg/controllers/nodepool/hash/nodepool_test.go delete mode 100644 pkg/controllers/nodepool/hash/provisioner_test.go diff --git a/pkg/controllers/controllers.go b/pkg/controllers/controllers.go index 8410f81877..96aff9c9ae 100644 --- a/pkg/controllers/controllers.go +++ b/pkg/controllers/controllers.go @@ -57,7 +57,6 @@ func NewControllers( p, evictionQueue, disruption.NewController(clock, kubeClient, p, cloudProvider, recorder, cluster), provisioning.NewController(kubeClient, p, recorder), - nodepoolhash.NewProvisionerController(kubeClient), nodepoolhash.NewNodePoolController(kubeClient), informer.NewDaemonSetController(kubeClient, cluster), informer.NewNodeController(kubeClient, cluster), @@ -78,7 +77,6 @@ func NewControllers( nodeclaimgarbagecollection.NewController(clock, kubeClient, cloudProvider), nodeclaimtermination.NewMachineController(kubeClient, cloudProvider), nodeclaimtermination.NewNodeClaimController(kubeClient, cloudProvider), - nodeclaimdisruption.NewMachineController(clock, kubeClient, cluster, cloudProvider), nodeclaimdisruption.NewNodeClaimController(clock, kubeClient, cluster, cloudProvider), leasegarbagecollection.NewController(kubeClient), } diff --git a/pkg/controllers/nodeclaim/disruption/controller.go b/pkg/controllers/nodeclaim/disruption/controller.go index 325d72adb1..3cf873793d 100644 --- a/pkg/controllers/nodeclaim/disruption/controller.go +++ b/pkg/controllers/nodeclaim/disruption/controller.go @@ -31,12 +31,10 @@ import ( "sigs.k8s.io/controller-runtime/pkg/predicate" "sigs.k8s.io/controller-runtime/pkg/reconcile" - "github.com/aws/karpenter-core/pkg/apis/v1alpha5" "github.com/aws/karpenter-core/pkg/apis/v1beta1" "github.com/aws/karpenter-core/pkg/cloudprovider" "github.com/aws/karpenter-core/pkg/controllers/state" corecontroller "github.com/aws/karpenter-core/pkg/operator/controller" - machineutil "github.com/aws/karpenter-core/pkg/utils/machine" nodeclaimutil "github.com/aws/karpenter-core/pkg/utils/nodeclaim" "github.com/aws/karpenter-core/pkg/utils/result" ) @@ -163,65 +161,3 @@ func (c *NodeClaimController) Builder(_ context.Context, m manager.Manager) core ), ) } - -var _ corecontroller.TypedController[*v1alpha5.Machine] = (*MachineController)(nil) - -type MachineController struct { - *Controller -} - -func NewMachineController(clk clock.Clock, kubeClient client.Client, cluster *state.Cluster, cloudProvider cloudprovider.CloudProvider) corecontroller.Controller { - return corecontroller.Typed[*v1alpha5.Machine](kubeClient, &MachineController{ - Controller: NewController(clk, kubeClient, cluster, cloudProvider), - }) -} - -func (c *MachineController) Name() string { - return "machine.disruption" -} - -func (c *MachineController) Reconcile(ctx context.Context, machine *v1alpha5.Machine) (reconcile.Result, error) { - return c.Controller.Reconcile(ctx, nodeclaimutil.New(machine)) -} - -func (c *Controller) Builder(_ context.Context, m manager.Manager) corecontroller.Builder { - return corecontroller.Adapt(controllerruntime. - NewControllerManagedBy(m). - For(&v1alpha5.Machine{}, builder.WithPredicates( - predicate.Or( - predicate.GenerationChangedPredicate{}, - predicate.Funcs{ - UpdateFunc: func(e event.UpdateEvent) bool { - oldMachine := e.ObjectOld.(*v1alpha5.Machine) - newMachine := e.ObjectNew.(*v1alpha5.Machine) - - // One of the status conditions that affects disruption has changed - // which means that we should re-consider this for disruption - for _, cond := range v1alpha5.LivingConditions { - if !equality.Semantic.DeepEqual( - oldMachine.StatusConditions().GetCondition(cond), - newMachine.StatusConditions().GetCondition(cond), - ) { - return true - } - } - return false - }, - }, - ), - )). - WithOptions(controller.Options{MaxConcurrentReconciles: 10}). - Watches( - &v1alpha5.Provisioner{}, - machineutil.ProvisionerEventHandler(c.kubeClient), - ). - Watches( - &v1.Node{}, - machineutil.NodeEventHandler(c.kubeClient), - ). - Watches( - &v1.Pod{}, - machineutil.PodEventHandler(c.kubeClient), - ), - ) -} diff --git a/pkg/controllers/nodeclaim/disruption/nodeclaim_drift_test.go b/pkg/controllers/nodeclaim/disruption/drift_test.go similarity index 99% rename from pkg/controllers/nodeclaim/disruption/nodeclaim_drift_test.go rename to pkg/controllers/nodeclaim/disruption/drift_test.go index dea839bbf5..0eb3af6062 100644 --- a/pkg/controllers/nodeclaim/disruption/nodeclaim_drift_test.go +++ b/pkg/controllers/nodeclaim/disruption/drift_test.go @@ -36,7 +36,7 @@ import ( . "github.com/onsi/gomega" ) -var _ = Describe("NodeClaim/Drift", func() { +var _ = Describe("Drift", func() { var nodePool *v1beta1.NodePool var nodeClaim *v1beta1.NodeClaim var node *v1.Node diff --git a/pkg/controllers/nodeclaim/disruption/nodeclaim_emptiness_test.go b/pkg/controllers/nodeclaim/disruption/emptiness_test.go similarity index 99% rename from pkg/controllers/nodeclaim/disruption/nodeclaim_emptiness_test.go rename to pkg/controllers/nodeclaim/disruption/emptiness_test.go index 3f07209bb5..f0b6e13f44 100644 --- a/pkg/controllers/nodeclaim/disruption/nodeclaim_emptiness_test.go +++ b/pkg/controllers/nodeclaim/disruption/emptiness_test.go @@ -32,7 +32,7 @@ import ( . "github.com/aws/karpenter-core/pkg/test/expectations" ) -var _ = Describe("NodeClaim/Emptiness", func() { +var _ = Describe("Emptiness", func() { var nodePool *v1beta1.NodePool var nodeClaim *v1beta1.NodeClaim var node *v1.Node diff --git a/pkg/controllers/nodeclaim/disruption/nodeclaim_expiration_test.go b/pkg/controllers/nodeclaim/disruption/expiration_test.go similarity index 98% rename from pkg/controllers/nodeclaim/disruption/nodeclaim_expiration_test.go rename to pkg/controllers/nodeclaim/disruption/expiration_test.go index ca034fc022..0c97e13f4c 100644 --- a/pkg/controllers/nodeclaim/disruption/nodeclaim_expiration_test.go +++ b/pkg/controllers/nodeclaim/disruption/expiration_test.go @@ -31,7 +31,7 @@ import ( . "github.com/aws/karpenter-core/pkg/test/expectations" ) -var _ = Describe("NodeClaim/Expiration", func() { +var _ = Describe("Expiration", func() { var nodePool *v1beta1.NodePool var nodeClaim *v1beta1.NodeClaim var node *v1.Node diff --git a/pkg/controllers/nodeclaim/disruption/machine_drift_test.go b/pkg/controllers/nodeclaim/disruption/machine_drift_test.go deleted file mode 100644 index 0738433a75..0000000000 --- a/pkg/controllers/nodeclaim/disruption/machine_drift_test.go +++ /dev/null @@ -1,437 +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 disruption_test - -import ( - "github.com/samber/lo" - v1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "knative.dev/pkg/apis" - "knative.dev/pkg/ptr" - - "github.com/aws/karpenter-core/pkg/controllers/nodeclaim/disruption" - controllerprov "github.com/aws/karpenter-core/pkg/controllers/nodepool/hash" - "github.com/aws/karpenter-core/pkg/operator/controller" - "github.com/aws/karpenter-core/pkg/operator/options" - . "github.com/aws/karpenter-core/pkg/test/expectations" - - "sigs.k8s.io/controller-runtime/pkg/client" - - "github.com/aws/karpenter-core/pkg/apis/v1alpha5" - "github.com/aws/karpenter-core/pkg/test" - - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" -) - -var _ = Describe("Machine/Drift", func() { - var provisioner *v1alpha5.Provisioner - var machine *v1alpha5.Machine - var node *v1.Node - BeforeEach(func() { - provisioner = test.Provisioner() - machine, node = test.MachineAndNode(v1alpha5.Machine{ - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{ - v1alpha5.ProvisionerNameLabelKey: provisioner.Name, - v1.LabelInstanceTypeStable: test.RandomName(), - }, - Annotations: map[string]string{ - v1alpha5.ProvisionerHashAnnotationKey: provisioner.Hash(), - }, - }, - }) - // Machines are required to be launched before they can be evaluated for drift - machine.StatusConditions().MarkTrue(v1alpha5.MachineLaunched) - }) - It("should detect drift", func() { - cp.Drifted = "drifted" - ExpectApplied(ctx, env.Client, provisioner, machine) - ExpectReconcileSucceeded(ctx, machineDisruptionController, client.ObjectKeyFromObject(machine)) - - machine = ExpectExists(ctx, env.Client, machine) - Expect(machine.StatusConditions().GetCondition(v1alpha5.MachineDrifted).IsTrue()).To(BeTrue()) - }) - It("should detect static drift before cloud provider drift", func() { - cp.Drifted = "drifted" - provisioner.Annotations = lo.Assign(provisioner.Annotations, map[string]string{ - v1alpha5.ProvisionerHashAnnotationKey: "123456789", - }) - ExpectApplied(ctx, env.Client, provisioner, machine) - ExpectReconcileSucceeded(ctx, machineDisruptionController, client.ObjectKeyFromObject(machine)) - - machine = ExpectExists(ctx, env.Client, machine) - Expect(machine.StatusConditions().GetCondition(v1alpha5.MachineDrifted).IsTrue()).To(BeTrue()) - Expect(machine.StatusConditions().GetCondition(v1alpha5.MachineDrifted).Reason).To(Equal(string(disruption.ProvisionerDrifted))) - }) - It("should detect node requirement drift before cloud provider drift", func() { - cp.Drifted = "drifted" - provisioner.Spec.Requirements = []v1.NodeSelectorRequirement{ - v1.NodeSelectorRequirement{ - Key: v1.LabelInstanceTypeStable, - Operator: v1.NodeSelectorOpDoesNotExist, - }, - } - ExpectApplied(ctx, env.Client, provisioner, machine) - ExpectReconcileSucceeded(ctx, machineDisruptionController, client.ObjectKeyFromObject(machine)) - - machine = ExpectExists(ctx, env.Client, machine) - Expect(machine.StatusConditions().GetCondition(v1alpha5.MachineDrifted).IsTrue()).To(BeTrue()) - Expect(machine.StatusConditions().GetCondition(v1alpha5.MachineDrifted).Reason).To(Equal(string(disruption.RequirementsDrifted))) - }) - It("should not detect drift if the feature flag is disabled", func() { - cp.Drifted = "drifted" - ctx = options.ToContext(ctx, test.Options(test.OptionsFields{FeatureGates: test.FeatureGates{Drift: lo.ToPtr(false)}})) - ExpectApplied(ctx, env.Client, provisioner, machine) - ExpectReconcileSucceeded(ctx, machineDisruptionController, client.ObjectKeyFromObject(machine)) - - machine = ExpectExists(ctx, env.Client, machine) - Expect(machine.StatusConditions().GetCondition(v1alpha5.MachineDrifted)).To(BeNil()) - }) - It("should remove the status condition from the machine if the feature flag is disabled", func() { - cp.Drifted = "drifted" - ctx = options.ToContext(ctx, test.Options(test.OptionsFields{FeatureGates: test.FeatureGates{Drift: lo.ToPtr(false)}})) - machine.StatusConditions().MarkTrue(v1alpha5.MachineDrifted) - ExpectApplied(ctx, env.Client, provisioner, machine) - - ExpectReconcileSucceeded(ctx, machineDisruptionController, client.ObjectKeyFromObject(machine)) - - machine = ExpectExists(ctx, env.Client, machine) - Expect(machine.StatusConditions().GetCondition(v1alpha5.MachineDrifted)).To(BeNil()) - }) - It("should remove the status condition from the machine when the machine launch condition is false", func() { - cp.Drifted = "drifted" - machine.StatusConditions().MarkTrue(v1alpha5.MachineDrifted) - ExpectApplied(ctx, env.Client, provisioner, machine, node) - machine.StatusConditions().MarkFalse(v1alpha5.MachineLaunched, "", "") - ExpectApplied(ctx, env.Client, machine) - - ExpectReconcileSucceeded(ctx, machineDisruptionController, client.ObjectKeyFromObject(machine)) - - machine = ExpectExists(ctx, env.Client, machine) - Expect(machine.StatusConditions().GetCondition(v1alpha5.MachineDrifted)).To(BeNil()) - }) - It("should remove the status condition from the machine when the machine launch condition doesn't exist", func() { - cp.Drifted = "drifted" - machine.StatusConditions().MarkTrue(v1alpha5.MachineDrifted) - ExpectApplied(ctx, env.Client, provisioner, machine, node) - machine.Status.Conditions = lo.Reject(machine.Status.Conditions, func(s apis.Condition, _ int) bool { - return s.Type == v1alpha5.MachineLaunched - }) - ExpectApplied(ctx, env.Client, machine) - - ExpectReconcileSucceeded(ctx, machineDisruptionController, client.ObjectKeyFromObject(machine)) - - machine = ExpectExists(ctx, env.Client, machine) - Expect(machine.StatusConditions().GetCondition(v1alpha5.MachineDrifted)).To(BeNil()) - }) - It("should not detect drift if the provisioner does not exist", func() { - cp.Drifted = "drifted" - ExpectApplied(ctx, env.Client, machine) - ExpectReconcileSucceeded(ctx, machineDisruptionController, client.ObjectKeyFromObject(machine)) - - machine = ExpectExists(ctx, env.Client, machine) - Expect(machine.StatusConditions().GetCondition(v1alpha5.MachineDrifted)).To(BeNil()) - }) - It("should remove the status condition from the machine if the machine is no longer drifted", func() { - cp.Drifted = "" - machine.StatusConditions().MarkTrue(v1alpha5.MachineDrifted) - ExpectApplied(ctx, env.Client, provisioner, machine) - - ExpectReconcileSucceeded(ctx, machineDisruptionController, client.ObjectKeyFromObject(machine)) - - machine = ExpectExists(ctx, env.Client, machine) - Expect(machine.StatusConditions().GetCondition(v1alpha5.MachineDrifted)).To(BeNil()) - }) - Context("NodeRequirement Drift", func() { - DescribeTable("", - func(oldProvisionerReq []v1.NodeSelectorRequirement, newProvisionerReq []v1.NodeSelectorRequirement, machineLabels map[string]string, drifted bool) { - cp.Drifted = "" - provisioner.Spec.Requirements = oldProvisionerReq - machine.Labels = lo.Assign(machine.Labels, machineLabels) - - ExpectApplied(ctx, env.Client, provisioner, machine) - ExpectReconcileSucceeded(ctx, machineDisruptionController, client.ObjectKeyFromObject(machine)) - machine = ExpectExists(ctx, env.Client, machine) - Expect(machine.StatusConditions().GetCondition(v1alpha5.MachineDrifted)).To(BeNil()) - - provisioner.Spec.Requirements = newProvisionerReq - ExpectApplied(ctx, env.Client, provisioner) - ExpectReconcileSucceeded(ctx, machineDisruptionController, client.ObjectKeyFromObject(machine)) - machine = ExpectExists(ctx, env.Client, machine) - if drifted { - Expect(machine.StatusConditions().GetCondition(v1alpha5.MachineDrifted).IsTrue()).To(BeTrue()) - } else { - Expect(machine.StatusConditions().GetCondition(v1alpha5.MachineDrifted)).To(BeNil()) - } - }, - Entry( - "should return drifted if the provisioner node requirement is updated", - []v1.NodeSelectorRequirement{ - {Key: v1alpha5.LabelCapacityType, Operator: v1.NodeSelectorOpIn, Values: []string{v1alpha5.CapacityTypeOnDemand}}, - {Key: v1.LabelArchStable, Operator: v1.NodeSelectorOpIn, Values: []string{v1alpha5.ArchitectureAmd64}}, - {Key: v1.LabelOSStable, Operator: v1.NodeSelectorOpIn, Values: []string{string(v1.Linux)}}, - }, - []v1.NodeSelectorRequirement{ - {Key: v1alpha5.LabelCapacityType, Operator: v1.NodeSelectorOpIn, Values: []string{v1alpha5.CapacityTypeSpot}}, - }, - map[string]string{ - v1alpha5.LabelCapacityType: v1alpha5.CapacityTypeOnDemand, - v1.LabelArchStable: v1alpha5.ArchitectureAmd64, - v1.LabelOSStable: string(v1.Linux), - }, - true), - Entry( - "should return drifted if a new node requirement is added", - []v1.NodeSelectorRequirement{ - {Key: v1alpha5.LabelCapacityType, Operator: v1.NodeSelectorOpIn, Values: []string{v1alpha5.CapacityTypeOnDemand}}, - {Key: v1.LabelOSStable, Operator: v1.NodeSelectorOpIn, Values: []string{string(v1.Linux)}}, - }, - []v1.NodeSelectorRequirement{ - {Key: v1alpha5.LabelCapacityType, Operator: v1.NodeSelectorOpIn, Values: []string{v1alpha5.CapacityTypeOnDemand}}, - {Key: v1.LabelOSStable, Operator: v1.NodeSelectorOpIn, Values: []string{string(v1.Linux)}}, - {Key: v1.LabelArchStable, Operator: v1.NodeSelectorOpIn, Values: []string{v1alpha5.ArchitectureAmd64}}, - }, - map[string]string{ - v1alpha5.LabelCapacityType: v1alpha5.CapacityTypeOnDemand, - v1.LabelOSStable: string(v1.Linux), - }, - true, - ), - Entry( - "should return drifted if a node requirement is reduced", - []v1.NodeSelectorRequirement{ - {Key: v1alpha5.LabelCapacityType, Operator: v1.NodeSelectorOpIn, Values: []string{v1alpha5.CapacityTypeOnDemand}}, - {Key: v1.LabelOSStable, Operator: v1.NodeSelectorOpIn, Values: []string{string(v1.Linux), string(v1.Windows)}}, - }, - []v1.NodeSelectorRequirement{ - {Key: v1alpha5.LabelCapacityType, Operator: v1.NodeSelectorOpIn, Values: []string{v1alpha5.CapacityTypeOnDemand}}, - {Key: v1.LabelOSStable, Operator: v1.NodeSelectorOpIn, Values: []string{string(v1.Windows)}}, - }, - map[string]string{ - v1alpha5.LabelCapacityType: v1alpha5.CapacityTypeOnDemand, - v1.LabelOSStable: string(v1.Linux), - }, - true, - ), - Entry( - "should not return drifted if a node requirement is expanded", - []v1.NodeSelectorRequirement{ - {Key: v1alpha5.LabelCapacityType, Operator: v1.NodeSelectorOpIn, Values: []string{v1alpha5.CapacityTypeOnDemand}}, - {Key: v1.LabelOSStable, Operator: v1.NodeSelectorOpIn, Values: []string{string(v1.Linux)}}, - }, - []v1.NodeSelectorRequirement{ - {Key: v1alpha5.LabelCapacityType, Operator: v1.NodeSelectorOpIn, Values: []string{v1alpha5.CapacityTypeOnDemand}}, - {Key: v1.LabelOSStable, Operator: v1.NodeSelectorOpIn, Values: []string{string(v1.Linux), string(v1.Windows)}}, - }, - map[string]string{ - v1alpha5.LabelCapacityType: v1alpha5.CapacityTypeOnDemand, - v1.LabelOSStable: string(v1.Linux), - }, - false, - ), - Entry( - "should not return drifted if a node requirement set to Exists", - []v1.NodeSelectorRequirement{ - {Key: v1alpha5.LabelCapacityType, Operator: v1.NodeSelectorOpIn, Values: []string{v1alpha5.CapacityTypeOnDemand}}, - {Key: v1.LabelOSStable, Operator: v1.NodeSelectorOpIn, Values: []string{string(v1.Linux)}}, - }, - []v1.NodeSelectorRequirement{ - {Key: v1alpha5.LabelCapacityType, Operator: v1.NodeSelectorOpIn, Values: []string{v1alpha5.CapacityTypeOnDemand}}, - {Key: v1.LabelOSStable, Operator: v1.NodeSelectorOpExists, Values: []string{}}, - }, - map[string]string{ - v1alpha5.LabelCapacityType: v1alpha5.CapacityTypeOnDemand, - v1.LabelOSStable: string(v1.Linux), - }, - false, - ), - Entry( - "should return drifted if a node requirement set to DoesNotExists", - []v1.NodeSelectorRequirement{ - {Key: v1alpha5.LabelCapacityType, Operator: v1.NodeSelectorOpIn, Values: []string{v1alpha5.CapacityTypeOnDemand}}, - {Key: v1.LabelOSStable, Operator: v1.NodeSelectorOpIn, Values: []string{string(v1.Linux)}}, - }, - []v1.NodeSelectorRequirement{ - {Key: v1alpha5.LabelCapacityType, Operator: v1.NodeSelectorOpIn, Values: []string{v1alpha5.CapacityTypeOnDemand}}, - {Key: v1.LabelOSStable, Operator: v1.NodeSelectorOpDoesNotExist, Values: []string{}}, - }, - map[string]string{ - v1alpha5.LabelCapacityType: v1alpha5.CapacityTypeOnDemand, - v1.LabelOSStable: string(v1.Linux), - }, - true, - ), - Entry( - "should not return drifted if a machine is grater then node requirement", - []v1.NodeSelectorRequirement{ - {Key: v1alpha5.LabelCapacityType, Operator: v1.NodeSelectorOpIn, Values: []string{v1alpha5.CapacityTypeOnDemand}}, - {Key: v1.LabelInstanceTypeStable, Operator: v1.NodeSelectorOpGt, Values: []string{"2"}}, - }, - []v1.NodeSelectorRequirement{ - {Key: v1alpha5.LabelCapacityType, Operator: v1.NodeSelectorOpIn, Values: []string{v1alpha5.CapacityTypeOnDemand}}, - {Key: v1.LabelInstanceTypeStable, Operator: v1.NodeSelectorOpGt, Values: []string{"10"}}, - }, - map[string]string{ - v1alpha5.LabelCapacityType: v1alpha5.CapacityTypeOnDemand, - v1.LabelInstanceTypeStable: "5", - }, - true, - ), - Entry( - "should not return drifted if a machine is less then node requirement", - []v1.NodeSelectorRequirement{ - {Key: v1alpha5.LabelCapacityType, Operator: v1.NodeSelectorOpIn, Values: []string{v1alpha5.CapacityTypeOnDemand}}, - {Key: v1.LabelInstanceTypeStable, Operator: v1.NodeSelectorOpLt, Values: []string{"5"}}, - }, - []v1.NodeSelectorRequirement{ - {Key: v1alpha5.LabelCapacityType, Operator: v1.NodeSelectorOpIn, Values: []string{v1alpha5.CapacityTypeOnDemand}}, - {Key: v1.LabelInstanceTypeStable, Operator: v1.NodeSelectorOpLt, Values: []string{"1"}}, - }, - map[string]string{ - v1alpha5.LabelCapacityType: v1alpha5.CapacityTypeOnDemand, - v1.LabelInstanceTypeStable: "2", - }, - true, - ), - ) - It("should return drifted only on machines that are drifted from an updated provisioner", func() { - cp.Drifted = "" - provisioner.Spec.Requirements = []v1.NodeSelectorRequirement{ - {Key: v1alpha5.LabelCapacityType, Operator: v1.NodeSelectorOpIn, Values: []string{v1alpha5.CapacityTypeOnDemand}}, - {Key: v1.LabelOSStable, Operator: v1.NodeSelectorOpIn, Values: []string{string(v1.Linux), string(v1.Windows)}}, - } - machine.Labels = lo.Assign(machine.Labels, map[string]string{ - v1alpha5.LabelCapacityType: v1alpha5.CapacityTypeOnDemand, - v1.LabelOSStable: string(v1.Linux), - }) - machineTwo, _ := test.MachineAndNode(v1alpha5.Machine{ - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{ - v1alpha5.ProvisionerNameLabelKey: provisioner.Name, - v1.LabelInstanceTypeStable: test.RandomName(), - v1alpha5.LabelCapacityType: v1alpha5.CapacityTypeOnDemand, - v1.LabelOSStable: string(v1.Windows), - }, - Annotations: map[string]string{ - v1alpha5.ProvisionerHashAnnotationKey: provisioner.Hash(), - }, - }, - Status: v1alpha5.MachineStatus{ - ProviderID: test.RandomProviderID(), - }, - }) - machineTwo.StatusConditions().MarkTrue(v1alpha5.MachineLaunched) - ExpectApplied(ctx, env.Client, provisioner, machine, machineTwo) - - ExpectReconcileSucceeded(ctx, machineDisruptionController, client.ObjectKeyFromObject(machine)) - ExpectReconcileSucceeded(ctx, machineDisruptionController, client.ObjectKeyFromObject(machineTwo)) - machine = ExpectExists(ctx, env.Client, machine) - machineTwo = ExpectExists(ctx, env.Client, machineTwo) - Expect(machine.StatusConditions().GetCondition(v1alpha5.MachineDrifted)).To(BeNil()) - Expect(machineTwo.StatusConditions().GetCondition(v1alpha5.MachineDrifted)).To(BeNil()) - - // Removed Windows OS - provisioner.Spec.Requirements = []v1.NodeSelectorRequirement{ - {Key: v1alpha5.LabelCapacityType, Operator: v1.NodeSelectorOpIn, Values: []string{v1alpha5.CapacityTypeOnDemand}}, - {Key: v1.LabelOSStable, Operator: v1.NodeSelectorOpIn, Values: []string{string(v1.Linux)}}, - } - ExpectApplied(ctx, env.Client, provisioner) - - ExpectReconcileSucceeded(ctx, machineDisruptionController, client.ObjectKeyFromObject(machine)) - machine = ExpectExists(ctx, env.Client, machine) - Expect(machine.StatusConditions().GetCondition(v1alpha5.MachineDrifted)).To(BeNil()) - - ExpectReconcileSucceeded(ctx, machineDisruptionController, client.ObjectKeyFromObject(machineTwo)) - machineTwo = ExpectExists(ctx, env.Client, machineTwo) - Expect(machineTwo.StatusConditions().GetCondition(v1alpha5.MachineDrifted).IsTrue()).To(BeTrue()) - }) - - }) - Context("Provisioner Static Drift", func() { - var testProvisionerOptions test.ProvisionerOptions - var provisionerController controller.Controller - BeforeEach(func() { - cp.Drifted = "" - provisionerController = controllerprov.NewProvisionerController(env.Client) - testProvisionerOptions = test.ProvisionerOptions{ - ObjectMeta: provisioner.ObjectMeta, - Taints: []v1.Taint{ - { - Key: "keyValue1", - Effect: v1.TaintEffectNoExecute, - }, - }, - StartupTaints: []v1.Taint{ - { - Key: "startupKeyValue1", - Effect: v1.TaintEffectNoExecute, - }, - }, - Labels: map[string]string{ - "keyLabel": "valueLabel", - "keyLabel2": "valueLabel2", - }, - Kubelet: &v1alpha5.KubeletConfiguration{ - MaxPods: ptr.Int32(10), - }, - Annotations: map[string]string{ - "keyAnnotation": "valueAnnotation", - "keyAnnotation2": "valueAnnotation2", - }, - } - provisioner = test.Provisioner(testProvisionerOptions) - machine.ObjectMeta.Annotations[v1alpha5.ProvisionerHashAnnotationKey] = provisioner.Hash() - }) - It("should detect drift on changes for all static fields", func() { - ExpectApplied(ctx, env.Client, provisioner, machine) - ExpectReconcileSucceeded(ctx, provisionerController, client.ObjectKeyFromObject(provisioner)) - ExpectReconcileSucceeded(ctx, machineDisruptionController, client.ObjectKeyFromObject(machine)) - machine = ExpectExists(ctx, env.Client, machine) - Expect(machine.StatusConditions().GetCondition(v1alpha5.MachineDrifted)).To(BeNil()) - - // Change one static field for the same provisioner - provisionerFieldToChange := []*v1alpha5.Provisioner{ - test.Provisioner(testProvisionerOptions, test.ProvisionerOptions{ObjectMeta: provisioner.ObjectMeta, Annotations: map[string]string{"keyAnnotationTest": "valueAnnotationTest"}}), - test.Provisioner(testProvisionerOptions, test.ProvisionerOptions{ObjectMeta: provisioner.ObjectMeta, Labels: map[string]string{"keyLabelTest": "valueLabelTest"}}), - test.Provisioner(testProvisionerOptions, test.ProvisionerOptions{ObjectMeta: provisioner.ObjectMeta, Taints: []v1.Taint{{Key: "keytest2Taint", Effect: v1.TaintEffectNoExecute}}}), - test.Provisioner(testProvisionerOptions, test.ProvisionerOptions{ObjectMeta: provisioner.ObjectMeta, StartupTaints: []v1.Taint{{Key: "keytest2StartupTaint", Effect: v1.TaintEffectNoExecute}}}), - test.Provisioner(testProvisionerOptions, test.ProvisionerOptions{ObjectMeta: provisioner.ObjectMeta, Kubelet: &v1alpha5.KubeletConfiguration{MaxPods: ptr.Int32(30)}}), - } - - for _, updatedProvisioner := range provisionerFieldToChange { - ExpectApplied(ctx, env.Client, updatedProvisioner) - ExpectReconcileSucceeded(ctx, provisionerController, client.ObjectKeyFromObject(updatedProvisioner)) - ExpectReconcileSucceeded(ctx, machineDisruptionController, client.ObjectKeyFromObject(machine)) - machine = ExpectExists(ctx, env.Client, machine) - Expect(machine.StatusConditions().GetCondition(v1alpha5.MachineDrifted).IsTrue()).To(BeTrue()) - } - }) - It("should not return drifted if karpenter.sh/provisioner-hash annotation is not present on the provisioner", func() { - provisioner.ObjectMeta.Annotations = map[string]string{} - ExpectApplied(ctx, env.Client, provisioner, machine) - ExpectReconcileSucceeded(ctx, machineDisruptionController, client.ObjectKeyFromObject(machine)) - machine = ExpectExists(ctx, env.Client, machine) - Expect(machine.StatusConditions().GetCondition(v1alpha5.MachineDrifted)).To(BeNil()) - }) - It("should not return drifted if karpenter.sh/provisioner-hash annotation is not present on the machine", func() { - machine.ObjectMeta.Annotations = map[string]string{} - ExpectApplied(ctx, env.Client, provisioner, machine) - ExpectReconcileSucceeded(ctx, machineDisruptionController, client.ObjectKeyFromObject(machine)) - machine = ExpectExists(ctx, env.Client, machine) - Expect(machine.StatusConditions().GetCondition(v1alpha5.MachineDrifted)).To(BeNil()) - }) - }) -}) diff --git a/pkg/controllers/nodeclaim/disruption/machine_emptiness_test.go b/pkg/controllers/nodeclaim/disruption/machine_emptiness_test.go deleted file mode 100644 index d9eb3e636c..0000000000 --- a/pkg/controllers/nodeclaim/disruption/machine_emptiness_test.go +++ /dev/null @@ -1,141 +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 disruption_test - -import ( - "time" - - "github.com/samber/lo" - v1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "knative.dev/pkg/apis" - "knative.dev/pkg/ptr" - "sigs.k8s.io/controller-runtime/pkg/client" - - "github.com/aws/karpenter-core/pkg/apis/v1alpha5" - "github.com/aws/karpenter-core/pkg/test" - - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" - - . "github.com/aws/karpenter-core/pkg/test/expectations" -) - -var _ = Describe("Machine/Emptiness", func() { - var provisioner *v1alpha5.Provisioner - var machine *v1alpha5.Machine - var node *v1.Node - BeforeEach(func() { - provisioner = test.Provisioner(test.ProvisionerOptions{ - TTLSecondsAfterEmpty: ptr.Int64(30), - }) - machine, node = test.MachineAndNode(v1alpha5.Machine{ - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{ - v1alpha5.ProvisionerNameLabelKey: provisioner.Name, - v1.LabelInstanceTypeStable: "default-instance-type", // need the instance type for the cluster state update - }, - }, - }) - }) - - It("should mark machines as empty", func() { - ExpectApplied(ctx, env.Client, provisioner, machine, node) - ExpectMakeMachinesInitialized(ctx, env.Client, machine) - - ExpectReconcileSucceeded(ctx, machineDisruptionController, client.ObjectKeyFromObject(machine)) - - machine = ExpectExists(ctx, env.Client, machine) - Expect(machine.StatusConditions().GetCondition(v1alpha5.MachineEmpty).IsTrue()).To(BeTrue()) - }) - It("should remove the status condition from the machine when emptiness is disabled", func() { - provisioner.Spec.TTLSecondsAfterEmpty = nil - machine.StatusConditions().MarkTrue(v1alpha5.MachineEmpty) - ExpectApplied(ctx, env.Client, provisioner, machine, node) - ExpectMakeMachinesInitialized(ctx, env.Client, machine) - - ExpectReconcileSucceeded(ctx, machineDisruptionController, client.ObjectKeyFromObject(machine)) - - machine = ExpectExists(ctx, env.Client, machine) - Expect(machine.StatusConditions().GetCondition(v1alpha5.MachineEmpty)).To(BeNil()) - }) - It("should remove the status condition from the machine when the machine initialization condition is false", func() { - machine.StatusConditions().MarkTrue(v1alpha5.MachineEmpty) - ExpectApplied(ctx, env.Client, provisioner, machine, node) - ExpectMakeMachinesInitialized(ctx, env.Client, machine) - machine.StatusConditions().MarkFalse(v1alpha5.MachineInitialized, "", "") - ExpectApplied(ctx, env.Client, machine) - - ExpectReconcileSucceeded(ctx, machineDisruptionController, client.ObjectKeyFromObject(machine)) - - machine = ExpectExists(ctx, env.Client, machine) - Expect(machine.StatusConditions().GetCondition(v1alpha5.MachineEmpty)).To(BeNil()) - }) - It("should remove the status condition from the machine when the machine initialization condition doesn't exist", func() { - machine.StatusConditions().MarkTrue(v1alpha5.MachineEmpty) - ExpectApplied(ctx, env.Client, provisioner, machine, node) - ExpectMakeMachinesInitialized(ctx, env.Client, machine) - machine.Status.Conditions = lo.Reject(machine.Status.Conditions, func(s apis.Condition, _ int) bool { - return s.Type == v1alpha5.MachineInitialized - }) - ExpectApplied(ctx, env.Client, machine) - - ExpectReconcileSucceeded(ctx, machineDisruptionController, client.ObjectKeyFromObject(machine)) - - machine = ExpectExists(ctx, env.Client, machine) - Expect(machine.StatusConditions().GetCondition(v1alpha5.MachineEmpty)).To(BeNil()) - }) - It("should remove the status condition from the machine when the node doesn't exist", func() { - provisioner.Spec.TTLSecondsAfterEmpty = ptr.Int64(30) - machine.StatusConditions().MarkTrue(v1alpha5.MachineEmpty) - ExpectApplied(ctx, env.Client, provisioner, machine) - ExpectMakeMachinesInitialized(ctx, env.Client, machine) - - ExpectReconcileSucceeded(ctx, machineDisruptionController, client.ObjectKeyFromObject(machine)) - - machine = ExpectExists(ctx, env.Client, machine) - Expect(machine.StatusConditions().GetCondition(v1alpha5.MachineEmpty)).To(BeNil()) - }) - It("should remove the status condition from non-empty machines", func() { - machine.StatusConditions().MarkTrue(v1alpha5.MachineEmpty) - ExpectApplied(ctx, env.Client, provisioner, machine, node) - ExpectMakeMachinesInitialized(ctx, env.Client, machine) - - ExpectApplied(ctx, env.Client, test.Pod(test.PodOptions{ - NodeName: node.Name, - Conditions: []v1.PodCondition{{Type: v1.PodReady, Status: v1.ConditionTrue}}, - })) - - ExpectReconcileSucceeded(ctx, machineDisruptionController, client.ObjectKeyFromObject(machine)) - - machine = ExpectExists(ctx, env.Client, machine) - Expect(machine.StatusConditions().GetCondition(v1alpha5.MachineEmpty)).To(BeNil()) - }) - It("should remove the status condition when the cluster state node is nominated", func() { - machine.StatusConditions().MarkTrue(v1alpha5.MachineEmpty) - ExpectApplied(ctx, env.Client, provisioner, machine, node) - ExpectMakeMachinesInitialized(ctx, env.Client, machine) - - // Add the node to the cluster state and nominate it in the internal cluster state - Expect(cluster.UpdateNode(ctx, node)).To(Succeed()) - cluster.NominateNodeForPod(ctx, node.Spec.ProviderID) - - result := ExpectReconcileSucceeded(ctx, machineDisruptionController, client.ObjectKeyFromObject(machine)) - Expect(result.RequeueAfter).To(Equal(time.Second * 30)) - - machine = ExpectExists(ctx, env.Client, machine) - Expect(machine.StatusConditions().GetCondition(v1alpha5.MachineEmpty)).To(BeNil()) - }) -}) diff --git a/pkg/controllers/nodeclaim/disruption/machine_expiration_test.go b/pkg/controllers/nodeclaim/disruption/machine_expiration_test.go deleted file mode 100644 index 06e1a71857..0000000000 --- a/pkg/controllers/nodeclaim/disruption/machine_expiration_test.go +++ /dev/null @@ -1,110 +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 disruption_test - -import ( - "time" - - v1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "knative.dev/pkg/ptr" - "sigs.k8s.io/controller-runtime/pkg/client" - - "github.com/aws/karpenter-core/pkg/apis/v1alpha5" - "github.com/aws/karpenter-core/pkg/test" - - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" - - . "github.com/aws/karpenter-core/pkg/test/expectations" -) - -var _ = Describe("Machine/Expiration", func() { - var provisioner *v1alpha5.Provisioner - var machine *v1alpha5.Machine - var node *v1.Node - BeforeEach(func() { - provisioner = test.Provisioner() - machine, node = test.MachineAndNode(v1alpha5.Machine{ - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{v1alpha5.ProvisionerNameLabelKey: provisioner.Name}, - }, - }) - }) - - It("should remove the status condition from the machines when expiration is disabled", func() { - machine.StatusConditions().MarkTrue(v1alpha5.MachineExpired) - ExpectApplied(ctx, env.Client, provisioner, machine) - - ExpectReconcileSucceeded(ctx, machineDisruptionController, client.ObjectKeyFromObject(machine)) - - machine = ExpectExists(ctx, env.Client, machine) - Expect(machine.StatusConditions().GetCondition(v1alpha5.MachineExpired)).To(BeNil()) - }) - It("should mark machines as expired", func() { - provisioner.Spec.TTLSecondsUntilExpired = ptr.Int64(30) - ExpectApplied(ctx, env.Client, provisioner, machine) - - // step forward to make the node expired - fakeClock.Step(60 * time.Second) - ExpectReconcileSucceeded(ctx, machineDisruptionController, client.ObjectKeyFromObject(machine)) - - machine = ExpectExists(ctx, env.Client, machine) - Expect(machine.StatusConditions().GetCondition(v1alpha5.MachineExpired).IsTrue()).To(BeTrue()) - }) - It("should remove the status condition from non-expired machines", func() { - provisioner.Spec.TTLSecondsUntilExpired = ptr.Int64(200) - machine.StatusConditions().MarkTrue(v1alpha5.MachineExpired) - ExpectApplied(ctx, env.Client, provisioner, machine) - - ExpectReconcileSucceeded(ctx, machineDisruptionController, client.ObjectKeyFromObject(machine)) - - machine = ExpectExists(ctx, env.Client, machine) - Expect(machine.StatusConditions().GetCondition(v1alpha5.MachineExpired)).To(BeNil()) - }) - It("should mark machines as expired if the node is expired but the machine isn't", func() { - provisioner.Spec.TTLSecondsUntilExpired = ptr.Int64(30) - ExpectApplied(ctx, env.Client, provisioner, node) - - // step forward to make the node expired - fakeClock.Step(60 * time.Second) - ExpectApplied(ctx, env.Client, machine) // machine shouldn't be expired, but node will be - ExpectReconcileSucceeded(ctx, machineDisruptionController, client.ObjectKeyFromObject(machine)) - - machine = ExpectExists(ctx, env.Client, machine) - Expect(machine.StatusConditions().GetCondition(v1alpha5.MachineExpired).IsTrue()).To(BeTrue()) - }) - It("should mark machines as expired if the machine is expired but the node isn't", func() { - provisioner.Spec.TTLSecondsUntilExpired = ptr.Int64(30) - ExpectApplied(ctx, env.Client, provisioner, machine) - - // step forward to make the node expired - fakeClock.Step(60 * time.Second) - ExpectApplied(ctx, env.Client, node) // node shouldn't be expired, but machine will be - ExpectReconcileSucceeded(ctx, machineDisruptionController, client.ObjectKeyFromObject(machine)) - - machine = ExpectExists(ctx, env.Client, machine) - Expect(machine.StatusConditions().GetCondition(v1alpha5.MachineExpired).IsTrue()).To(BeTrue()) - }) - It("should return the requeue interval for the time between now and when the machine expires", func() { - provisioner.Spec.TTLSecondsUntilExpired = ptr.Int64(200) - ExpectApplied(ctx, env.Client, provisioner, machine, node) - - fakeClock.Step(time.Second * 100) - - result := ExpectReconcileSucceeded(ctx, machineDisruptionController, client.ObjectKeyFromObject(machine)) - Expect(result.RequeueAfter).To(BeNumerically("~", time.Second*100, time.Second)) - }) -}) diff --git a/pkg/controllers/nodeclaim/disruption/machine_test.go b/pkg/controllers/nodeclaim/disruption/machine_test.go deleted file mode 100644 index 5b03835bf1..0000000000 --- a/pkg/controllers/nodeclaim/disruption/machine_test.go +++ /dev/null @@ -1,84 +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 disruption_test - -import ( - "time" - - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" - "github.com/samber/lo" - v1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "knative.dev/pkg/ptr" - "sigs.k8s.io/controller-runtime/pkg/client" - - "github.com/aws/karpenter-core/pkg/apis/v1alpha5" - "github.com/aws/karpenter-core/pkg/operator/options" - . "github.com/aws/karpenter-core/pkg/test/expectations" - - "github.com/aws/karpenter-core/pkg/test" -) - -var _ = Describe("Machine/Disruption", func() { - var provisioner *v1alpha5.Provisioner - var machine *v1alpha5.Machine - var node *v1.Node - - BeforeEach(func() { - provisioner = test.Provisioner() - machine, node = test.MachineAndNode(v1alpha5.Machine{ - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{v1alpha5.ProvisionerNameLabelKey: provisioner.Name}, - }, - }) - }) - It("should set multiple disruption conditions simultaneously", func() { - cp.Drifted = "drifted" - provisioner.Spec.TTLSecondsAfterEmpty = ptr.Int64(30) - provisioner.Spec.TTLSecondsUntilExpired = ptr.Int64(30) - node.Annotations = lo.Assign(node.Annotations, map[string]string{ - v1alpha5.EmptinessTimestampAnnotationKey: fakeClock.Now().Format(time.RFC3339), - }) - ExpectApplied(ctx, env.Client, provisioner, machine, node) - ExpectMakeMachinesInitialized(ctx, env.Client, machine) - - // step forward to make the node expired and empty - fakeClock.Step(60 * time.Second) - ExpectReconcileSucceeded(ctx, machineDisruptionController, client.ObjectKeyFromObject(machine)) - - machine = ExpectExists(ctx, env.Client, machine) - Expect(machine.StatusConditions().GetCondition(v1alpha5.MachineDrifted).IsTrue()).To(BeTrue()) - Expect(machine.StatusConditions().GetCondition(v1alpha5.MachineEmpty).IsTrue()).To(BeTrue()) - Expect(machine.StatusConditions().GetCondition(v1alpha5.MachineExpired).IsTrue()).To(BeTrue()) - }) - It("should remove multiple disruption conditions simultaneously", func() { - machine.StatusConditions().MarkTrue(v1alpha5.MachineDrifted) - machine.StatusConditions().MarkTrue(v1alpha5.MachineEmpty) - machine.StatusConditions().MarkTrue(v1alpha5.MachineExpired) - - ExpectApplied(ctx, env.Client, provisioner, machine, node) - ExpectMakeMachinesInitialized(ctx, env.Client, machine) - - // Drift, Expiration, and Emptiness are disabled through configuration - ctx = options.ToContext(ctx, test.Options(test.OptionsFields{FeatureGates: test.FeatureGates{Drift: lo.ToPtr(false)}})) - ExpectReconcileSucceeded(ctx, machineDisruptionController, client.ObjectKeyFromObject(machine)) - - machine = ExpectExists(ctx, env.Client, machine) - Expect(machine.StatusConditions().GetCondition(v1alpha5.MachineDrifted)).To(BeNil()) - Expect(machine.StatusConditions().GetCondition(v1alpha5.MachineEmpty)).To(BeNil()) - Expect(machine.StatusConditions().GetCondition(v1alpha5.MachineExpired)).To(BeNil()) - }) -}) diff --git a/pkg/controllers/nodeclaim/disruption/nodeclaim_test.go b/pkg/controllers/nodeclaim/disruption/nodeclaim_test.go deleted file mode 100644 index c6ce9e5ae2..0000000000 --- a/pkg/controllers/nodeclaim/disruption/nodeclaim_test.go +++ /dev/null @@ -1,84 +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 disruption_test - -import ( - "time" - - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" - "github.com/samber/lo" - v1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "sigs.k8s.io/controller-runtime/pkg/client" - - "github.com/aws/karpenter-core/pkg/apis/v1beta1" - "github.com/aws/karpenter-core/pkg/operator/options" - . "github.com/aws/karpenter-core/pkg/test/expectations" - - "github.com/aws/karpenter-core/pkg/test" -) - -var _ = Describe("NodeClaim/Disruption", func() { - var nodePool *v1beta1.NodePool - var nodeClaim *v1beta1.NodeClaim - var node *v1.Node - - BeforeEach(func() { - nodePool = test.NodePool() - nodeClaim, node = test.NodeClaimAndNode(v1beta1.NodeClaim{ - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{v1beta1.NodePoolLabelKey: nodePool.Name}, - }, - }) - }) - It("should set multiple disruption conditions simultaneously", func() { - cp.Drifted = "drifted" - nodePool.Spec.Disruption.ConsolidationPolicy = v1beta1.ConsolidationPolicyWhenEmpty - nodePool.Spec.Disruption.ConsolidateAfter = &v1beta1.NillableDuration{Duration: lo.ToPtr(time.Second * 30)} - nodePool.Spec.Disruption.ExpireAfter.Duration = lo.ToPtr(time.Second * 30) - ExpectApplied(ctx, env.Client, nodePool, nodeClaim, node) - ExpectMakeNodeClaimsInitialized(ctx, env.Client, nodeClaim) - - // step forward to make the node expired and empty - fakeClock.Step(60 * time.Second) - ExpectReconcileSucceeded(ctx, nodeClaimDisruptionController, client.ObjectKeyFromObject(nodeClaim)) - - nodeClaim = ExpectExists(ctx, env.Client, nodeClaim) - Expect(nodeClaim.StatusConditions().GetCondition(v1beta1.Drifted).IsTrue()).To(BeTrue()) - Expect(nodeClaim.StatusConditions().GetCondition(v1beta1.Empty).IsTrue()).To(BeTrue()) - Expect(nodeClaim.StatusConditions().GetCondition(v1beta1.Expired).IsTrue()).To(BeTrue()) - }) - It("should remove multiple disruption conditions simultaneously", func() { - nodePool.Spec.Disruption.ExpireAfter.Duration = nil - nodePool.Spec.Disruption.ConsolidateAfter = &v1beta1.NillableDuration{Duration: nil} - - nodeClaim.StatusConditions().MarkTrue(v1beta1.Drifted) - nodeClaim.StatusConditions().MarkTrue(v1beta1.Empty) - nodeClaim.StatusConditions().MarkTrue(v1beta1.Expired) - - ExpectApplied(ctx, env.Client, nodePool, nodeClaim, node) - ExpectMakeNodeClaimsInitialized(ctx, env.Client, nodeClaim) - - // Drift, Expiration, and Emptiness are disabled through configuration - ctx = options.ToContext(ctx, test.Options(test.OptionsFields{FeatureGates: test.FeatureGates{Drift: lo.ToPtr(false)}})) - ExpectReconcileSucceeded(ctx, nodeClaimDisruptionController, client.ObjectKeyFromObject(nodeClaim)) - - nodeClaim = ExpectExists(ctx, env.Client, nodeClaim) - Expect(nodeClaim.StatusConditions().GetCondition(v1beta1.Drifted)).To(BeNil()) - Expect(nodeClaim.StatusConditions().GetCondition(v1beta1.Empty)).To(BeNil()) - Expect(nodeClaim.StatusConditions().GetCondition(v1beta1.Expired)).To(BeNil()) - }) -}) diff --git a/pkg/controllers/nodeclaim/disruption/suite_test.go b/pkg/controllers/nodeclaim/disruption/suite_test.go index 62aab8b51e..f7963f2c2d 100644 --- a/pkg/controllers/nodeclaim/disruption/suite_test.go +++ b/pkg/controllers/nodeclaim/disruption/suite_test.go @@ -23,12 +23,14 @@ import ( . "github.com/onsi/gomega" "github.com/samber/lo" v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" clock "k8s.io/utils/clock/testing" . "knative.dev/pkg/logging/testing" "sigs.k8s.io/controller-runtime/pkg/cache" "sigs.k8s.io/controller-runtime/pkg/client" "github.com/aws/karpenter-core/pkg/apis" + "github.com/aws/karpenter-core/pkg/apis/v1beta1" "github.com/aws/karpenter-core/pkg/cloudprovider/fake" nodeclaimdisruption "github.com/aws/karpenter-core/pkg/controllers/nodeclaim/disruption" "github.com/aws/karpenter-core/pkg/controllers/state" @@ -41,7 +43,6 @@ import ( ) var ctx context.Context -var machineDisruptionController controller.Controller var nodeClaimDisruptionController controller.Controller var env *test.Environment var fakeClock *clock.FakeClock @@ -64,7 +65,6 @@ var _ = BeforeSuite(func() { ctx = options.ToContext(ctx, test.Options()) cp = fake.NewCloudProvider() cluster = state.NewCluster(fakeClock, env.Client, cp) - machineDisruptionController = nodeclaimdisruption.NewMachineController(fakeClock, env.Client, cluster, cp) nodeClaimDisruptionController = nodeclaimdisruption.NewNodeClaimController(fakeClock, env.Client, cluster, cp) }) @@ -82,3 +82,55 @@ var _ = AfterEach(func() { cluster.Reset() ExpectCleanedUp(ctx, env.Client) }) + +var _ = Describe("Disruption", func() { + var nodePool *v1beta1.NodePool + var nodeClaim *v1beta1.NodeClaim + var node *v1.Node + + BeforeEach(func() { + nodePool = test.NodePool() + nodeClaim, node = test.NodeClaimAndNode(v1beta1.NodeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{v1beta1.NodePoolLabelKey: nodePool.Name}, + }, + }) + }) + It("should set multiple disruption conditions simultaneously", func() { + cp.Drifted = "drifted" + nodePool.Spec.Disruption.ConsolidationPolicy = v1beta1.ConsolidationPolicyWhenEmpty + nodePool.Spec.Disruption.ConsolidateAfter = &v1beta1.NillableDuration{Duration: lo.ToPtr(time.Second * 30)} + nodePool.Spec.Disruption.ExpireAfter.Duration = lo.ToPtr(time.Second * 30) + ExpectApplied(ctx, env.Client, nodePool, nodeClaim, node) + ExpectMakeNodeClaimsInitialized(ctx, env.Client, nodeClaim) + + // step forward to make the node expired and empty + fakeClock.Step(60 * time.Second) + ExpectReconcileSucceeded(ctx, nodeClaimDisruptionController, client.ObjectKeyFromObject(nodeClaim)) + + nodeClaim = ExpectExists(ctx, env.Client, nodeClaim) + Expect(nodeClaim.StatusConditions().GetCondition(v1beta1.Drifted).IsTrue()).To(BeTrue()) + Expect(nodeClaim.StatusConditions().GetCondition(v1beta1.Empty).IsTrue()).To(BeTrue()) + Expect(nodeClaim.StatusConditions().GetCondition(v1beta1.Expired).IsTrue()).To(BeTrue()) + }) + It("should remove multiple disruption conditions simultaneously", func() { + nodePool.Spec.Disruption.ExpireAfter.Duration = nil + nodePool.Spec.Disruption.ConsolidateAfter = &v1beta1.NillableDuration{Duration: nil} + + nodeClaim.StatusConditions().MarkTrue(v1beta1.Drifted) + nodeClaim.StatusConditions().MarkTrue(v1beta1.Empty) + nodeClaim.StatusConditions().MarkTrue(v1beta1.Expired) + + ExpectApplied(ctx, env.Client, nodePool, nodeClaim, node) + ExpectMakeNodeClaimsInitialized(ctx, env.Client, nodeClaim) + + // Drift, Expiration, and Emptiness are disabled through configuration + ctx = options.ToContext(ctx, test.Options(test.OptionsFields{FeatureGates: test.FeatureGates{Drift: lo.ToPtr(false)}})) + ExpectReconcileSucceeded(ctx, nodeClaimDisruptionController, client.ObjectKeyFromObject(nodeClaim)) + + nodeClaim = ExpectExists(ctx, env.Client, nodeClaim) + Expect(nodeClaim.StatusConditions().GetCondition(v1beta1.Drifted)).To(BeNil()) + Expect(nodeClaim.StatusConditions().GetCondition(v1beta1.Empty)).To(BeNil()) + Expect(nodeClaim.StatusConditions().GetCondition(v1beta1.Expired)).To(BeNil()) + }) +}) diff --git a/pkg/controllers/nodepool/hash/controller.go b/pkg/controllers/nodepool/hash/controller.go index 1b0a92dd9e..5e71f7a880 100644 --- a/pkg/controllers/nodepool/hash/controller.go +++ b/pkg/controllers/nodepool/hash/controller.go @@ -26,7 +26,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/predicate" "sigs.k8s.io/controller-runtime/pkg/reconcile" - "github.com/aws/karpenter-core/pkg/apis/v1alpha5" "github.com/aws/karpenter-core/pkg/apis/v1beta1" corecontroller "github.com/aws/karpenter-core/pkg/operator/controller" nodepoolutil "github.com/aws/karpenter-core/pkg/utils/nodepool" @@ -57,34 +56,6 @@ func (c *Controller) Reconcile(ctx context.Context, np *v1beta1.NodePool) (recon return reconcile.Result{}, nil } -//nolint:revive -type ProvisionerController struct { - *Controller -} - -func NewProvisionerController(kubeClient client.Client) corecontroller.Controller { - return corecontroller.Typed[*v1alpha5.Provisioner](kubeClient, &ProvisionerController{ - Controller: NewController(kubeClient), - }) -} - -func (c *ProvisionerController) Reconcile(ctx context.Context, p *v1alpha5.Provisioner) (reconcile.Result, error) { - return c.Controller.Reconcile(ctx, nodepoolutil.New(p)) -} - -func (c *ProvisionerController) Name() string { - return "provisioner.hash" -} - -func (c *ProvisionerController) Builder(_ context.Context, m manager.Manager) corecontroller.Builder { - return corecontroller.Adapt(controllerruntime. - NewControllerManagedBy(m). - WithEventFilter(predicate.GenerationChangedPredicate{}). - For(&v1alpha5.Provisioner{}). - WithOptions(controller.Options{MaxConcurrentReconciles: 10}), - ) -} - type NodePoolController struct { *Controller } diff --git a/pkg/controllers/nodepool/hash/nodepool_test.go b/pkg/controllers/nodepool/hash/nodepool_test.go deleted file mode 100644 index 6c72a87be3..0000000000 --- a/pkg/controllers/nodepool/hash/nodepool_test.go +++ /dev/null @@ -1,108 +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 hash_test - -import ( - "time" - - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" - "github.com/samber/lo" - v1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/resource" - "knative.dev/pkg/ptr" - "sigs.k8s.io/controller-runtime/pkg/client" - - "github.com/aws/karpenter-core/pkg/apis/v1beta1" - "github.com/aws/karpenter-core/pkg/test" - . "github.com/aws/karpenter-core/pkg/test/expectations" -) - -var _ = Describe("NodePool Static Drift Hash", func() { - var nodePool *v1beta1.NodePool - BeforeEach(func() { - nodePool = test.NodePool(v1beta1.NodePool{ - Spec: v1beta1.NodePoolSpec{ - Template: v1beta1.NodeClaimTemplate{ - ObjectMeta: v1beta1.ObjectMeta{ - Annotations: map[string]string{ - "keyAnnotation": "valueAnnotation", - "keyAnnotation2": "valueAnnotation2", - }, - Labels: map[string]string{ - "keyLabel": "valueLabel", - }, - }, - Spec: v1beta1.NodeClaimSpec{ - Taints: []v1.Taint{ - { - Key: "key", - Effect: v1.TaintEffectNoExecute, - }, - }, - StartupTaints: []v1.Taint{ - { - Key: "key", - Effect: v1.TaintEffectNoExecute, - }, - }, - Kubelet: &v1beta1.KubeletConfiguration{ - MaxPods: ptr.Int32(10), - }, - }, - }, - }, - }) - }) - It("should update the static drift hash when NodePool static field is updated", func() { - ExpectApplied(ctx, env.Client, nodePool) - ExpectReconcileSucceeded(ctx, nodePoolController, client.ObjectKeyFromObject(nodePool)) - nodePool = ExpectExists(ctx, env.Client, nodePool) - - expectedHash := nodePool.Hash() - Expect(nodePool.Annotations).To(HaveKeyWithValue(v1beta1.NodePoolHashAnnotationKey, expectedHash)) - - nodePool.Spec.Template.Labels = map[string]string{"keyLabeltest": "valueLabeltest"} - nodePool.Spec.Template.Annotations = map[string]string{"keyAnnotation2": "valueAnnotation2", "keyAnnotation": "valueAnnotation"} - ExpectReconcileSucceeded(ctx, nodePoolController, client.ObjectKeyFromObject(nodePool)) - nodePool = ExpectExists(ctx, env.Client, nodePool) - - expectedHashTwo := nodePool.Hash() - Expect(nodePool.Annotations).To(HaveKeyWithValue(v1beta1.NodePoolHashAnnotationKey, expectedHashTwo)) - }) - It("should not update the static drift hash when NodePool behavior field is updated", func() { - ExpectApplied(ctx, env.Client, nodePool) - ExpectReconcileSucceeded(ctx, nodePoolController, client.ObjectKeyFromObject(nodePool)) - nodePool = ExpectExists(ctx, env.Client, nodePool) - - expectedHash := nodePool.Hash() - Expect(nodePool.Annotations).To(HaveKeyWithValue(v1beta1.NodePoolHashAnnotationKey, expectedHash)) - - nodePool.Spec.Limits = v1beta1.Limits(v1.ResourceList{"cpu": resource.MustParse("16")}) - nodePool.Spec.Disruption.ConsolidationPolicy = v1beta1.ConsolidationPolicyWhenEmpty - nodePool.Spec.Disruption.ConsolidateAfter = &v1beta1.NillableDuration{Duration: lo.ToPtr(30 * time.Second)} - nodePool.Spec.Disruption.ExpireAfter.Duration = lo.ToPtr(30 * time.Second) - nodePool.Spec.Template.Spec.Requirements = []v1.NodeSelectorRequirement{ - {Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpIn, Values: []string{"test"}}, - {Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpGt, Values: []string{"1"}}, - {Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpLt, Values: []string{"1"}}, - } - nodePool.Spec.Weight = lo.ToPtr(int32(80)) - ExpectReconcileSucceeded(ctx, nodePoolController, client.ObjectKeyFromObject(nodePool)) - nodePool = ExpectExists(ctx, env.Client, nodePool) - - Expect(nodePool.Annotations).To(HaveKeyWithValue(v1beta1.NodePoolHashAnnotationKey, expectedHash)) - }) -}) diff --git a/pkg/controllers/nodepool/hash/provisioner_test.go b/pkg/controllers/nodepool/hash/provisioner_test.go deleted file mode 100644 index 5ae7523e3a..0000000000 --- a/pkg/controllers/nodepool/hash/provisioner_test.go +++ /dev/null @@ -1,107 +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 hash_test - -import ( - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" - "github.com/samber/lo" - v1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/resource" - "knative.dev/pkg/ptr" - "sigs.k8s.io/controller-runtime/pkg/client" - - "github.com/aws/karpenter-core/pkg/apis/v1alpha5" - "github.com/aws/karpenter-core/pkg/test" - . "github.com/aws/karpenter-core/pkg/test/expectations" - nodepoolutil "github.com/aws/karpenter-core/pkg/utils/nodepool" -) - -var _ = Describe("Provisioner Static Drift Hash", func() { - var provisioner *v1alpha5.Provisioner - BeforeEach(func() { - provisioner = test.Provisioner(test.ProvisionerOptions{ - Taints: []v1.Taint{ - { - Key: "key", - Effect: v1.TaintEffectNoExecute, - }, - }, - StartupTaints: []v1.Taint{ - { - Key: "key", - Effect: v1.TaintEffectNoExecute, - }, - }, - Labels: map[string]string{ - "keyLabel": "valueLabel", - }, - Kubelet: &v1alpha5.KubeletConfiguration{ - MaxPods: ptr.Int32(10), - }, - Annotations: map[string]string{ - "keyAnnotation": "valueAnnotation", - "keyAnnotation2": "valueAnnotation2", - }, - }) - }) - It("should maintain the same hash, before and after the NodePool conversion", func() { - hash := provisioner.Hash() - nodePool := nodepoolutil.New(provisioner) - convertedHash := nodepoolutil.HashAnnotation(nodePool) - Expect(convertedHash).To(HaveKeyWithValue(v1alpha5.ProvisionerHashAnnotationKey, hash)) - }) - It("should update the static drift hash when provisioner static field is updated", func() { - ExpectApplied(ctx, env.Client, provisioner) - ExpectReconcileSucceeded(ctx, provisionerController, client.ObjectKeyFromObject(provisioner)) - provisioner = ExpectExists(ctx, env.Client, provisioner) - - expectedHash := provisioner.Hash() - Expect(provisioner.Annotations).To(HaveKeyWithValue(v1alpha5.ProvisionerHashAnnotationKey, expectedHash)) - - provisioner.Spec.Labels = map[string]string{"keyLabeltest": "valueLabeltest"} - provisioner.Spec.Annotations = map[string]string{"keyAnnotation2": "valueAnnotation2", "keyAnnotation": "valueAnnotation"} - ExpectReconcileSucceeded(ctx, provisionerController, client.ObjectKeyFromObject(provisioner)) - provisioner = ExpectExists(ctx, env.Client, provisioner) - - expectedHashTwo := provisioner.Hash() - Expect(provisioner.Annotations).To(HaveKeyWithValue(v1alpha5.ProvisionerHashAnnotationKey, expectedHashTwo)) - }) - It("should not update the static drift hash when provisioner behavior field is updated", func() { - ExpectApplied(ctx, env.Client, provisioner) - ExpectReconcileSucceeded(ctx, provisionerController, client.ObjectKeyFromObject(provisioner)) - provisioner = ExpectExists(ctx, env.Client, provisioner) - - expectedHash := provisioner.Hash() - Expect(provisioner.Annotations).To(HaveKeyWithValue(v1alpha5.ProvisionerHashAnnotationKey, expectedHash)) - - provisioner.Spec.Limits = &v1alpha5.Limits{Resources: v1.ResourceList{"cpu": resource.MustParse("16")}} - provisioner.Spec.Consolidation = &v1alpha5.Consolidation{Enabled: lo.ToPtr(true)} - provisioner.Spec.TTLSecondsAfterEmpty = lo.ToPtr(int64(30)) - provisioner.Spec.TTLSecondsUntilExpired = lo.ToPtr(int64(50)) - provisioner.Spec.Requirements = []v1.NodeSelectorRequirement{ - {Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpIn, Values: []string{"test"}}, - {Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpGt, Values: []string{"1"}}, - {Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpLt, Values: []string{"1"}}, - } - provisioner.Spec.Provider = &v1alpha5.Provider{} - provisioner.Spec.ProviderRef = &v1alpha5.MachineTemplateRef{Kind: "NodeTemplate", Name: "default"} - provisioner.Spec.Weight = lo.ToPtr(int32(80)) - ExpectReconcileSucceeded(ctx, provisionerController, client.ObjectKeyFromObject(provisioner)) - provisioner = ExpectExists(ctx, env.Client, provisioner) - - Expect(provisioner.Annotations).To(HaveKeyWithValue(v1alpha5.ProvisionerHashAnnotationKey, expectedHash)) - }) -}) diff --git a/pkg/controllers/nodepool/hash/suite_test.go b/pkg/controllers/nodepool/hash/suite_test.go index f6c1352186..f11166f53f 100644 --- a/pkg/controllers/nodepool/hash/suite_test.go +++ b/pkg/controllers/nodepool/hash/suite_test.go @@ -17,19 +17,27 @@ package hash_test import ( "context" "testing" + "time" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + "github.com/samber/lo" + v1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" . "knative.dev/pkg/logging/testing" + "knative.dev/pkg/ptr" + "sigs.k8s.io/controller-runtime/pkg/client" + + . "github.com/aws/karpenter-core/pkg/test/expectations" "github.com/aws/karpenter-core/pkg/apis" + "github.com/aws/karpenter-core/pkg/apis/v1beta1" "github.com/aws/karpenter-core/pkg/controllers/nodepool/hash" "github.com/aws/karpenter-core/pkg/operator/controller" "github.com/aws/karpenter-core/pkg/operator/scheme" "github.com/aws/karpenter-core/pkg/test" ) -var provisionerController controller.Controller var nodePoolController controller.Controller var ctx context.Context var env *test.Environment @@ -37,15 +45,91 @@ var env *test.Environment func TestAPIs(t *testing.T) { ctx = TestContextWithLogger(t) RegisterFailHandler(Fail) - RunSpecs(t, "ProvisionerController") + RunSpecs(t, "Hash") } var _ = BeforeSuite(func() { env = test.NewEnvironment(scheme.Scheme, test.WithCRDs(apis.CRDs...)) - provisionerController = hash.NewProvisionerController(env.Client) nodePoolController = hash.NewNodePoolController(env.Client) }) var _ = AfterSuite(func() { Expect(env.Stop()).To(Succeed(), "Failed to stop environment") }) + +var _ = Describe("Static Drift Hash", func() { + var nodePool *v1beta1.NodePool + BeforeEach(func() { + nodePool = test.NodePool(v1beta1.NodePool{ + Spec: v1beta1.NodePoolSpec{ + Template: v1beta1.NodeClaimTemplate{ + ObjectMeta: v1beta1.ObjectMeta{ + Annotations: map[string]string{ + "keyAnnotation": "valueAnnotation", + "keyAnnotation2": "valueAnnotation2", + }, + Labels: map[string]string{ + "keyLabel": "valueLabel", + }, + }, + Spec: v1beta1.NodeClaimSpec{ + Taints: []v1.Taint{ + { + Key: "key", + Effect: v1.TaintEffectNoExecute, + }, + }, + StartupTaints: []v1.Taint{ + { + Key: "key", + Effect: v1.TaintEffectNoExecute, + }, + }, + Kubelet: &v1beta1.KubeletConfiguration{ + MaxPods: ptr.Int32(10), + }, + }, + }, + }, + }) + }) + It("should update the static drift hash when NodePool static field is updated", func() { + ExpectApplied(ctx, env.Client, nodePool) + ExpectReconcileSucceeded(ctx, nodePoolController, client.ObjectKeyFromObject(nodePool)) + nodePool = ExpectExists(ctx, env.Client, nodePool) + + expectedHash := nodePool.Hash() + Expect(nodePool.Annotations).To(HaveKeyWithValue(v1beta1.NodePoolHashAnnotationKey, expectedHash)) + + nodePool.Spec.Template.Labels = map[string]string{"keyLabeltest": "valueLabeltest"} + nodePool.Spec.Template.Annotations = map[string]string{"keyAnnotation2": "valueAnnotation2", "keyAnnotation": "valueAnnotation"} + ExpectReconcileSucceeded(ctx, nodePoolController, client.ObjectKeyFromObject(nodePool)) + nodePool = ExpectExists(ctx, env.Client, nodePool) + + expectedHashTwo := nodePool.Hash() + Expect(nodePool.Annotations).To(HaveKeyWithValue(v1beta1.NodePoolHashAnnotationKey, expectedHashTwo)) + }) + It("should not update the static drift hash when NodePool behavior field is updated", func() { + ExpectApplied(ctx, env.Client, nodePool) + ExpectReconcileSucceeded(ctx, nodePoolController, client.ObjectKeyFromObject(nodePool)) + nodePool = ExpectExists(ctx, env.Client, nodePool) + + expectedHash := nodePool.Hash() + Expect(nodePool.Annotations).To(HaveKeyWithValue(v1beta1.NodePoolHashAnnotationKey, expectedHash)) + + nodePool.Spec.Limits = v1beta1.Limits(v1.ResourceList{"cpu": resource.MustParse("16")}) + nodePool.Spec.Disruption.ConsolidationPolicy = v1beta1.ConsolidationPolicyWhenEmpty + nodePool.Spec.Disruption.ConsolidateAfter = &v1beta1.NillableDuration{Duration: lo.ToPtr(30 * time.Second)} + nodePool.Spec.Disruption.ExpireAfter.Duration = lo.ToPtr(30 * time.Second) + nodePool.Spec.Template.Spec.Requirements = []v1.NodeSelectorRequirement{ + {Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpIn, Values: []string{"test"}}, + {Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpGt, Values: []string{"1"}}, + {Key: v1.LabelTopologyZone, Operator: v1.NodeSelectorOpLt, Values: []string{"1"}}, + } + nodePool.Spec.Weight = lo.ToPtr(int32(80)) + ExpectReconcileSucceeded(ctx, nodePoolController, client.ObjectKeyFromObject(nodePool)) + nodePool = ExpectExists(ctx, env.Client, nodePool) + + Expect(nodePool.Annotations).To(HaveKeyWithValue(v1beta1.NodePoolHashAnnotationKey, expectedHash)) + }) +})