chore: cleanup deprovisioning types

pkg/controllers/deprovisioning/consolidation.go

@@ -110,7 +110,7 @@ func (c *consolidation) computeConsolidation(ctx context.Context, candidates ...
 	if err != nil {
 		// if a candidate node is now deleting, just retry
 		if errors.Is(err, errCandidateDeleting) {
-			return Command{action: actionDoNothing}, nil
+			return Command{}, nil
 		}
 		return Command{}, err
 	}
@@ -121,14 +121,13 @@ func (c *consolidation) computeConsolidation(ctx context.Context, candidates ...
 		if len(candidates) == 1 {
 			c.recorder.Publish(deprovisioningevents.Unconsolidatable(candidates[0].Node, candidates[0].Machine, results.PodSchedulingErrors())...)
 		}
-		return Command{action: actionDoNothing}, nil
+		return Command{}, nil
 	}
 
 	// were we able to schedule all the pods on the inflight candidates?
 	if len(results.NewMachines) == 0 {
 		return Command{
 			candidates: candidates,
-			action:     actionDelete,
 		}, nil
 	}
 
@@ -137,7 +136,7 @@ func (c *consolidation) computeConsolidation(ctx context.Context, candidates ...
 		if len(candidates) == 1 {
 			c.recorder.Publish(deprovisioningevents.Unconsolidatable(candidates[0].Node, candidates[0].Machine, fmt.Sprintf("can't remove without creating %d candidates", len(results.NewMachines)))...)
 		}
-		return Command{action: actionDoNothing}, nil
+		return Command{}, nil
 	}
 
 	// get the current node price based on the offering
@@ -152,7 +151,7 @@ func (c *consolidation) computeConsolidation(ctx context.Context, candidates ...
 			c.recorder.Publish(deprovisioningevents.Unconsolidatable(candidates[0].Node, candidates[0].Machine, "can't replace with a cheaper node")...)
 		}
 		// no instance types remain after filtering by price
-		return Command{action: actionDoNothing}, nil
+		return Command{}, nil
 	}
 
 	// If the existing candidates are all spot and the replacement is spot, we don't consolidate.  We don't have a reliable
@@ -170,7 +169,7 @@ func (c *consolidation) computeConsolidation(ctx context.Context, candidates ...
 		if len(candidates) == 1 {
 			c.recorder.Publish(deprovisioningevents.Unconsolidatable(candidates[0].Node, candidates[0].Machine, "can't replace a spot node with a spot node")...)
 		}
-		return Command{action: actionDoNothing}, nil
+		return Command{}, nil
 	}
 
 	// We are consolidating a node from OD -> [OD,Spot] but have filtered the instance types by cost based on the
@@ -184,7 +183,6 @@ func (c *consolidation) computeConsolidation(ctx context.Context, candidates ...
 
 	return Command{
 		candidates:   candidates,
-		action:       actionReplace,
 		replacements: results.NewMachines,
 	}, nil
 }

pkg/controllers/deprovisioning/controller.go

@@ -158,7 +158,7 @@ func (c *Controller) deprovision(ctx context.Context, deprovisioner Deprovisione
 	if err != nil {
 		return false, fmt.Errorf("computing deprovisioning decision, %w", err)
 	}
-	if cmd.action == actionDoNothing {
+	if cmd.Action() == noOpAction {
 		return false, nil
 	}
 
@@ -171,11 +171,11 @@ func (c *Controller) deprovision(ctx context.Context, deprovisioner Deprovisione
 }
 
 func (c *Controller) executeCommand(ctx context.Context, d Deprovisioner, command Command) error {
-	deprovisioningActionsPerformedCounter.With(prometheus.Labels{"action": fmt.Sprintf("%s/%s", d, command.action)}).Add(1)
-	logging.FromContext(ctx).Infof("deprovisioning via %s %s", d, command)
+	deprovisioningActionsPerformedCounter.With(prometheus.Labels{"action": fmt.Sprintf("%s/%s", d, command.Action())}).Inc()
+	logging.FromContext(ctx).Infof("deprovisioning via %s %s", d, command.Action())
 
-	reason := fmt.Sprintf("%s/%s", d, command.action)
-	if command.action == actionReplace {
+	reason := fmt.Sprintf("%s/%s", d, command.Action())
+	if command.Action() == replaceAction {
 		if err := c.launchReplacementMachines(ctx, command, reason); err != nil {
 			// If we failed to launch the replacement, don't deprovision.  If this is some permanent failure,
 			// we don't want to disrupt workloads with no way to provision new nodes for them.

pkg/controllers/deprovisioning/drift.go

@@ -71,7 +71,6 @@ func (d *Drift) ComputeCommand(ctx context.Context, nodes ...*Candidate) (Comman
 	}); len(empty) > 0 {
 		return Command{
 			candidates: empty,
-			action:     actionDelete,
 		}, nil
 	}
 
@@ -92,16 +91,14 @@ func (d *Drift) ComputeCommand(ctx context.Context, nodes ...*Candidate) (Comman
 		if len(results.NewMachines) == 0 {
 			return Command{
 				candidates: []*Candidate{candidate},
-				action:     actionDelete,
 			}, nil
 		}
 		return Command{
 			candidates:   []*Candidate{candidate},
-			action:       actionReplace,
 			replacements: results.NewMachines,
 		}, nil
 	}
-	return Command{action: actionDoNothing}, nil
+	return Command{}, nil
 }
 
 // String is the string representation of the deprovisioner

pkg/controllers/deprovisioning/emptiness.go

@@ -67,12 +67,8 @@ func (e *Emptiness) ComputeCommand(_ context.Context, candidates ...*Candidate)
 		return cn.Machine.DeletionTimestamp.IsZero() && len(cn.pods) == 0
 	})
 
-	if len(emptyCandidates) == 0 {
-		return Command{action: actionDoNothing}, nil
-	}
 	return Command{
 		candidates: emptyCandidates,
-		action:     actionDelete,
 	}, nil
 }
 

pkg/controllers/deprovisioning/emptymachineconsolidation.go

@@ -43,7 +43,7 @@ func NewEmptyMachineConsolidation(clk clock.Clock, cluster *state.Cluster, kubeC
 // ComputeCommand generates a deprovisioning command given deprovisionable machines
 func (c *EmptyMachineConsolidation) ComputeCommand(ctx context.Context, candidates ...*Candidate) (Command, error) {
 	if c.cluster.Consolidated() {
-		return Command{action: actionDoNothing}, nil
+		return Command{}, nil
 	}
 	candidates, err := c.sortAndFilterCandidates(ctx, candidates)
 	if err != nil {
@@ -53,12 +53,11 @@ func (c *EmptyMachineConsolidation) ComputeCommand(ctx context.Context, candidat
 	// select the entirely empty nodes
 	emptyCandidates := lo.Filter(candidates, func(n *Candidate, _ int) bool { return len(n.pods) == 0 })
 	if len(emptyCandidates) == 0 {
-		return Command{action: actionDoNothing}, nil
+		return Command{}, nil
 	}
 
 	cmd := Command{
 		candidates: emptyCandidates,
-		action:     actionDelete,
 	}
 
 	// empty machine consolidation doesn't use Validation as we get to take advantage of cluster.IsNodeNominated.  This

pkg/controllers/deprovisioning/expiration.go

@@ -92,7 +92,6 @@ func (e *Expiration) ComputeCommand(ctx context.Context, nodes ...*Candidate) (C
 	}); len(empty) > 0 {
 		return Command{
 			candidates: empty,
-			action:     actionDelete,
 		}, nil
 	}
 
@@ -115,11 +114,10 @@ func (e *Expiration) ComputeCommand(ctx context.Context, nodes ...*Candidate) (C
 			With("delay", time.Since(node.GetExpirationTime(candidates[0].Node, candidates[0].provisioner))).Infof("triggering termination for expired node after TTL")
 		return Command{
 			candidates:   []*Candidate{candidate},
-			action:       actionReplace,
 			replacements: results.NewMachines,
 		}, nil
 	}
-	return Command{action: actionDoNothing}, nil
+	return Command{}, nil
 }
 
 // String is the string representation of the deprovisioner

pkg/controllers/deprovisioning/multimachineconsolidation.go

@@ -41,7 +41,7 @@ func NewMultiMachineConsolidation(clk clock.Clock, cluster *state.Cluster, kubeC
 
 func (m *MultiMachineConsolidation) ComputeCommand(ctx context.Context, candidates ...*Candidate) (Command, error) {
 	if m.cluster.Consolidated() {
-		return Command{action: actionDoNothing}, nil
+		return Command{}, nil
 	}
 	candidates, err := m.sortAndFilterCandidates(ctx, candidates)
 	if err != nil {
@@ -54,7 +54,7 @@ func (m *MultiMachineConsolidation) ComputeCommand(ctx context.Context, candidat
 	if err != nil {
 		return Command{}, err
 	}
-	if cmd.action == actionDoNothing {
+	if cmd.Action() == noOpAction {
 		return cmd, nil
 	}
 
@@ -75,14 +75,14 @@ func (m *MultiMachineConsolidation) ComputeCommand(ctx context.Context, candidat
 func (m *MultiMachineConsolidation) firstNMachineConsolidationOption(ctx context.Context, candidates []*Candidate, max int) (Command, error) {
 	// we always operate on at least two machines at once, for single machines standard consolidation will find all solutions
 	if len(candidates) < 2 {
-		return Command{action: actionDoNothing}, nil
+		return Command{}, nil
 	}
 	min := 1
 	if len(candidates) <= max {
 		max = len(candidates) - 1
 	}
 
-	lastSavedCommand := Command{action: actionDoNothing}
+	lastSavedCommand := Command{}
 	// binary search to find the maximum number of machines we can terminate
 	for min <= max {
 		mid := (min + max) / 2
@@ -96,14 +96,13 @@ func (m *MultiMachineConsolidation) firstNMachineConsolidationOption(ctx context
 
 		// ensure that the action is sensical for replacements, see explanation on filterOutSameType for why this is
 		// required
-		if action.action == actionReplace {
+		instanceTypeFiltered := false
+		if action.Action() == replaceAction {
 			action.replacements[0].InstanceTypeOptions = filterOutSameType(action.replacements[0], candidatesToConsolidate)
-			if len(action.replacements[0].InstanceTypeOptions) == 0 {
-				action.action = actionDoNothing
-			}
+			instanceTypeFiltered = len(action.replacements[0].InstanceTypeOptions) == 0
 		}
 
-		if action.action == actionReplace || action.action == actionDelete {
+		if !instanceTypeFiltered {
 			// we can consolidate machines [0,mid]
 			lastSavedCommand = action
 			min = mid + 1

pkg/controllers/deprovisioning/singlemachineconsolidation.go

@@ -42,7 +42,7 @@ func NewSingleMachineConsolidation(clk clock.Clock, cluster *state.Cluster, kube
 // nolint:gocyclo
 func (c *SingleMachineConsolidation) ComputeCommand(ctx context.Context, candidates ...*Candidate) (Command, error) {
 	if c.cluster.Consolidated() {
-		return Command{action: actionDoNothing}, nil
+		return Command{}, nil
 	}
 	candidates, err := c.sortAndFilterCandidates(ctx, candidates)
 	if err != nil {
@@ -57,7 +57,7 @@ func (c *SingleMachineConsolidation) ComputeCommand(ctx context.Context, candida
 			logging.FromContext(ctx).Errorf("computing consolidation %s", err)
 			continue
 		}
-		if cmd.action == actionDoNothing {
+		if cmd.Action() == noOpAction {
 			continue
 		}
 
@@ -70,9 +70,9 @@ func (c *SingleMachineConsolidation) ComputeCommand(ctx context.Context, candida
 			return Command{}, fmt.Errorf("command is no longer valid, %s", cmd)
 		}
 
-		if cmd.action == actionReplace || cmd.action == actionDelete {
+		if cmd.Action() != noOpAction {
 			return cmd, nil
 		}
 	}
-	return Command{action: actionDoNothing}, nil
+	return Command{}, nil
 }

pkg/controllers/deprovisioning/types.go

@@ -136,38 +136,31 @@ func (c *Candidate) lifetimeRemaining(clock clock.Clock) float64 {
 	return remaining
 }
 
-type action byte
+type Command struct {
+	candidates   []*Candidate
+	replacements []*scheduling.Machine
+}
 
 const (
-	actionDelete action = iota
-	actionReplace
-	actionDoNothing
+	noOpAction = "no-op"
+	replaceAction = "replace"
+	deleteAction = "delete"
 )
 
-func (a action) String() string {
-	switch a {
-	// Deprovisioning action with no replacement machines
-	case actionDelete:
-		return "delete"
-	// Deprovisioning action with replacement machines
-	case actionReplace:
-		return "replace"
-	case actionDoNothing:
-		return "do nothing"
+func (o Command) Action() string {
+	switch {
+	case len(o.candidates) > 0 && len(o.replacements) == 0:
+		return replaceAction
+	case len(o.candidates) > 0 && len(o.replacements) == 0:
+		return deleteAction
 	default:
-		return fmt.Sprintf("unknown (%d)", a)
+		return noOpAction
 	}
 }
 
-type Command struct {
-	candidates   []*Candidate
-	action       action
-	replacements []*scheduling.Machine
-}
-
 func (o Command) String() string {
 	var buf bytes.Buffer
-	fmt.Fprintf(&buf, "%s, terminating %d machines ", o.action, len(o.candidates))
+	fmt.Fprintf(&buf, "%s, terminating %d machines ", o.Action(), len(o.candidates))
 	for i, old := range o.candidates {
 		if i != 0 {
 			fmt.Fprint(&buf, ", ")