Don't reassign the provisioner in the launcher

pkg/apis/provisioning/v1alpha5/limits.go

@@ -15,8 +15,6 @@ limitations under the License.
 package v1alpha5
 
 import (
-	"fmt"
-
 	v1 "k8s.io/api/core/v1"
 )
 
@@ -25,14 +23,3 @@ type Limits struct {
 	// Resources contains all the allocatable resources that Karpenter supports for limiting.
 	Resources v1.ResourceList `json:"resources,omitempty"`
 }
-
-func (p *Provisioner) HasExceededResources() error {
-	for resourceName, usage := range p.Status.Resources {
-		if limit, ok := p.Spec.Limits.Resources[resourceName]; ok {
-			if usage.Cmp(limit) >= 0 {
-				return fmt.Errorf("resource limit %s exceeded", resourceName)
-			}
-		}
-	}
-	return nil
-}

pkg/controllers/counter/controller.go

@@ -58,18 +58,15 @@ func (c *Controller) Reconcile(ctx context.Context, req reconcile.Request) (reco
 		return reconcile.Result{}, nil
 	}
 	persisted := provisioner.DeepCopy()
-
 	// Determine resource usage and update provisioner.status.resources
 	resourceCounts, err := c.resourceCountsFor(ctx, provisioner.Name)
 	if err != nil {
 		return reconcile.Result{}, err
 	}
-
 	provisioner.Status.Resources = resourceCounts
 	if err := c.kubeClient.Status().Patch(ctx, provisioner, client.MergeFrom(persisted)); err != nil {
 		return reconcile.Result{}, fmt.Errorf("failed to persist changes to %s, %w", req.NamespacedName, err)
 	}
-
 	return reconcile.Result{}, nil
 }
 
@@ -78,10 +75,8 @@ func (c *Controller) resourceCountsFor(ctx context.Context, provisionerName stri
 	if err := c.kubeClient.List(ctx, &nodes, client.MatchingLabels{v1alpha5.ProvisionerNameLabelKey: provisionerName}); err != nil {
 		return nil, err
 	}
-
 	var cpu = resource.NewScaledQuantity(0, 0)
 	var memory = resource.NewScaledQuantity(0, resource.Giga)
-
 	for _, node := range nodes.Items {
 		cpu.Add(*node.Status.Capacity.Cpu())
 		memory.Add(*node.Status.Capacity.Memory())
@@ -90,7 +85,6 @@ func (c *Controller) resourceCountsFor(ctx context.Context, provisionerName stri
 		v1.ResourceCPU:    *cpu,
 		v1.ResourceMemory: *memory,
 	}, nil
-
 }
 
 // Register the controller to the manager

pkg/controllers/provisioning/controller.go

@@ -98,7 +98,7 @@ func (c *Controller) Apply(ctx context.Context, provisioner *v1alpha5.Provisione
 	provisioner.Spec.Requirements = provisioner.Spec.Requirements.
 		With(requirements(instanceTypes)).
 		With(v1alpha5.LabelRequirements(provisioner.Spec.Labels))
-	if currentProvisioner, ok := c.provisioners.Load(provisioner.Name); ok && c.areEqual(currentProvisioner.(*Provisioner).Spec, provisioner.Spec) {
+	if !c.hasChanged(ctx, provisioner) {
 		// If the provisionerSpecs haven't changed, we don't need to stop and drain the current Provisioner.
 		return nil
 	}
@@ -122,10 +122,21 @@ func (c *Controller) Apply(ctx context.Context, provisioner *v1alpha5.Provisione
 	return nil
 }
 
-func (c *Controller) areEqual(provisionerSpecOld v1alpha5.ProvisionerSpec, provisionerSpecNew v1alpha5.ProvisionerSpec) bool {
-	hashKeyOld, _ := hashstructure.Hash(provisionerSpecOld, hashstructure.FormatV2, &hashstructure.HashOptions{SlicesAsSets: true})
-	hashKeyNew, _ := hashstructure.Hash(provisionerSpecNew, hashstructure.FormatV2, &hashstructure.HashOptions{SlicesAsSets: true})
-	return hashKeyOld == hashKeyNew
+// Returns true if the new candidate provisioner is different than the provisioner in memory.
+func (c *Controller) hasChanged(ctx context.Context, provisionerNew *v1alpha5.Provisioner) bool {
+	oldProvisioner, _ := c.provisioners.Load(provisionerNew.Name)
+	if oldProvisioner == nil {
+		return true
+	}
+	hashKeyOld, err := hashstructure.Hash(oldProvisioner.(*Provisioner).Spec, hashstructure.FormatV2, &hashstructure.HashOptions{SlicesAsSets: true})
+	if err != nil {
+		logging.FromContext(ctx).Fatalf("Unable to hash old provisioner spec: %s", err.Error())
+	}
+	hashKeyNew, err := hashstructure.Hash(provisionerNew.Spec, hashstructure.FormatV2, &hashstructure.HashOptions{SlicesAsSets: true})
+	if err != nil {
+		logging.FromContext(ctx).Fatalf("Unable to hash new provisioner spec: %s", err.Error())
+	}
+	return hashKeyOld != hashKeyNew
 }
 
 // List the active provisioners

pkg/controllers/provisioning/launcher.go

@@ -51,13 +51,9 @@ func (l *Launcher) Launch(ctx context.Context, provisioner *v1alpha5.Provisioner
 
 	// Pack and bind pods
 	errs := make([]error, len(schedules))
-	provisioner, err := l.updateState(ctx, provisioner)
-	if err != nil {
-		return fmt.Errorf("unable to determine status of provisioner")
-	}
 	workqueue.ParallelizeUntil(ctx, len(schedules), len(schedules), func(index int) {
 		for _, packing := range l.Packer.Pack(ctx, schedules[index], instanceTypes) {
-			if err := provisioner.HasExceededResources(); err != nil {
+			if err := l.verifyResourceLimits(ctx, provisioner); err != nil {
 				errs[index] = multierr.Append(errs[index], err)
 				continue
 			}
@@ -122,12 +118,19 @@ func (l *Launcher) bind(ctx context.Context, node *v1.Node, pods []*v1.Pod) (err
 	return nil
 }
 
-func (l *Launcher) updateState(ctx context.Context, provisioner *v1alpha5.Provisioner) (*v1alpha5.Provisioner, error) {
-	provisionerCopy := &v1alpha5.Provisioner{}
-	if err := l.KubeClient.Get(ctx, types.NamespacedName{Name: provisioner.Name, Namespace: provisioner.Namespace}, provisionerCopy); err != nil {
-		return nil, err
+func (l *Launcher) verifyResourceLimits(ctx context.Context, provisioner *v1alpha5.Provisioner) error {
+	provisionerLatest := &v1alpha5.Provisioner{}
+	if err := l.KubeClient.Get(ctx, types.NamespacedName{Name: provisioner.Name, Namespace: provisioner.Namespace}, provisionerLatest); err != nil {
+		return err
 	}
-	return provisionerCopy, nil
+	for resourceName, usage := range provisionerLatest.Status.Resources {
+		if limit, ok := provisionerLatest.Spec.Limits.Resources[resourceName]; ok {
+			if usage.Cmp(limit) >= 0 {
+				return fmt.Errorf("%s resource usage of %v exceeds limit of %v", resourceName, usage.AsDec(), limit.AsDec())
+			}
+		}
+	}
+	return nil
 }
 
 var bindTimeHistogram = prometheus.NewHistogramVec(