last

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() == noOpAction {
+	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())}).Inc()
-	logging.FromContext(ctx).Infof("deprovisioning via %s %s", d, command.action())
+	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() == replaceAction {
+	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/multimachineconsolidation.go

@@ -54,7 +54,7 @@ func (m *MultiMachineConsolidation) ComputeCommand(ctx context.Context, candidat
 	if err != nil {
 		return Command{}, err
 	}
-	if cmd.action() == noOpAction {
+	if cmd.Action() == noOpAction {
 		return cmd, nil
 	}
 
@@ -97,7 +97,7 @@ func (m *MultiMachineConsolidation) firstNMachineConsolidationOption(ctx context
 		// ensure that the action is sensical for replacements, see explanation on filterOutSameType for why this is
 		// required
 		instanceTypeFiltered := false
-		if action.action() == replaceAction {
+		if action.Action() == replaceAction {
 			action.replacements[0].InstanceTypeOptions = filterOutSameType(action.replacements[0], candidatesToConsolidate)
 			instanceTypeFiltered = len(action.replacements[0].InstanceTypeOptions) == 0
 		}

pkg/controllers/deprovisioning/singlemachineconsolidation.go

@@ -57,7 +57,7 @@ func (c *SingleMachineConsolidation) ComputeCommand(ctx context.Context, candida
 			logging.FromContext(ctx).Errorf("computing consolidation %s", err)
 			continue
 		}
-		if cmd.action() == noOpAction {
+		if cmd.Action() == noOpAction {
 			continue
 		}
 
@@ -70,7 +70,7 @@ func (c *SingleMachineConsolidation) ComputeCommand(ctx context.Context, candida
 			return Command{}, fmt.Errorf("command is no longer valid, %s", cmd)
 		}
 
-		if cmd.action() != noOpAction {
+		if cmd.Action() != noOpAction {
 			return cmd, nil
 		}
 	}

pkg/controllers/deprovisioning/types.go

@@ -56,6 +56,9 @@ type Candidate struct {
 func NewCandidate(ctx context.Context, kubeClient client.Client, recorder events.Recorder, clk clock.Clock, node *state.StateNode,
 	provisionerMap map[string]*v1alpha5.Provisioner, provisionerToInstanceTypes map[string]map[string]*cloudprovider.InstanceType) (*Candidate, error) {
 
+	if node.Node == nil || node.Machine == nil {
+		return nil, fmt.Errorf("state node doesn't contain both a node and a machine")
+	}
 	// check whether the node has all the labels we need
 	for _, label := range []string{
 		v1alpha5.LabelCapacityType,
@@ -101,9 +104,6 @@ func NewCandidate(ctx context.Context, kubeClient client.Client, recorder events
 		recorder.Publish(deprovisioningevents.Blocked(node.Node, node.Machine, "machine is nominated")...)
 		return nil, fmt.Errorf("state node is nominated")
 	}
-	if node.Node == nil || node.Machine == nil {
-		return nil, fmt.Errorf("state node doesn't contain both a node and a machine")
-	}
 
 	pods, err := node.Pods(ctx, kubeClient)
 	if err != nil {
@@ -147,19 +147,20 @@ const (
 	deleteAction = "delete"
 )
 
-func (o Command) action() string {
-	if len(o.candidates) == 0 {
-		return noOpAction
-	}
-	if len(o.replacements) > 0 {
+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 noOpAction
 	}
-	return deleteAction
 }
 
 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, ", ")