Address review comments - part 6: Replace function preserveExpiryTimeSet to nil check

Author: thiyyakat

pkg/controller/machineset.go

@@ -495,7 +495,7 @@ func (c *controller) manageReplicas(ctx context.Context, allMachines []*v1alpha1
 // or if it is a candidate for auto-preservation
 // TODO@thiyyakat: find more suitable name for function
 func (c *controller) isMachineCandidateForPreservation(ctx context.Context, machineSet *v1alpha1.MachineSet, machine *v1alpha1.Machine) (bool, error) {
-	if machineutils.IsPreserveExpiryTimeSet(machine) && !machineutils.HasPreservationTimedOut(machine) {
+	if machine.Status.CurrentStatus.PreserveExpiryTime != nil && !machineutils.HasPreservationTimedOut(machine) {
 		klog.V(3).Infof("Machine %q is preserved until %v, not adding to stale machines", machine.Name, machine.Status.CurrentStatus.PreserveExpiryTime)
 		return true, nil
 	}
@@ -508,7 +508,7 @@ func (c *controller) isMachineCandidateForPreservation(ctx context.Context, mach
 			return false, nil
 		}
 	}
-	if *machineSet.Status.AutoPreserveFailedMachineCount < *machineSet.Spec.AutoPreserveFailedMachineMax {
+	if machineSet.Status.AutoPreserveFailedMachineCount != nil && machineSet.Spec.AutoPreserveFailedMachineMax != nil && *machineSet.Status.AutoPreserveFailedMachineCount < *machineSet.Spec.AutoPreserveFailedMachineMax {
 		err := c.annotateMachineForAutoPreservation(ctx, machine)
 		if err != nil {
 			return true, err
@@ -729,7 +729,7 @@ func prioritisePreservedMachines(machines []*v1alpha1.Machine) []*v1alpha1.Machi
 	preservedMachines := make([]*v1alpha1.Machine, 0, len(machines))
 	otherMachines := make([]*v1alpha1.Machine, 0, len(machines))
 	for _, mc := range machines {
-		if machineutils.IsPreserveExpiryTimeSet(mc) {
+		if mc.Status.CurrentStatus.PreserveExpiryTime != nil {
 			preservedMachines = append(preservedMachines, mc)
 		} else {
 			otherMachines = append(otherMachines, mc)

pkg/util/provider/machinecontroller/machine.go

@@ -753,8 +753,7 @@ func (c *controller) handlePreserveAnnotationsChange(oldAnnotations, newAnnotati
 		return existsInOld != existsInNew || valueOld != valueNew
 	}
 	// Special case: annotation changed from 'now' to 'when-failed'
-	isPreserved := machineutils.IsPreserveExpiryTimeSet(machine)
-	if !isPreserved {
+	if machine.Status.CurrentStatus.PreserveExpiryTime == nil {
 		return true
 	}
 	if machineutils.IsMachineFailed(machine) {
@@ -803,7 +802,7 @@ func (c *controller) manageMachinePreservation(ctx context.Context, machine *v1a
 	if !isPreserveAnnotationValueValid(preserveValue) {
 		klog.Warningf("Preserve annotation value %q on machine %s is invalid", preserveValue, machine.Name)
 		return
-	} else if preserveValue == machineutils.PreserveMachineAnnotationValueFalse || (machineutils.IsPreserveExpiryTimeSet(clone) && machineutils.HasPreservationTimedOut(clone)) {
+	} else if preserveValue == machineutils.PreserveMachineAnnotationValueFalse || (clone.Status.CurrentStatus.PreserveExpiryTime != nil && machineutils.HasPreservationTimedOut(clone)) {
 		err = c.stopMachinePreservation(ctx, clone)
 		return
 	} else if preserveValue == machineutils.PreserveMachineAnnotationValueWhenFailed {

pkg/util/provider/machinecontroller/machine_util.go

@@ -2181,7 +2181,7 @@ func (c *controller) canMarkMachineFailed(machineDeployName, machineName, namesp
 		if machine.Status.CurrentStatus.Phase != v1alpha1.MachineUnknown && machine.Status.CurrentStatus.Phase != v1alpha1.MachineRunning {
 			// since Preserved Failed machines are not replaced immediately,
 			// they need not be considered towards inProgress
-			if !machineutils.IsPreserveExpiryTimeSet(machine) {
+			if machine.Status.CurrentStatus.PreserveExpiryTime == nil {
 				inProgress++
 			}
 			switch machine.Status.CurrentStatus.Phase {
@@ -2370,9 +2370,8 @@ Utility Functions for Machine Preservation
 // preserveMachine contains logic to start the preservation of a machine and node.
 func (c *controller) preserveMachine(ctx context.Context, machine *v1alpha1.Machine, preserveValue string) error {
 	nodeName := machine.Labels[v1alpha1.NodeLabelKey]
-	isExpirySet := machineutils.IsPreserveExpiryTimeSet(machine)
 	updatedMachine := machine.DeepCopy()
-	if !isExpirySet {
+	if machine.Status.CurrentStatus.PreserveExpiryTime == nil {
 		klog.V(4).Infof("Starting preservation flow for machine %q.", machine.Name)
 		// Step 1: Add preserveExpiryTime to machine status
 		updatedMachine, err := c.setPreserveExpiryTimeOnMachine(ctx, updatedMachine)
@@ -2526,7 +2525,7 @@ func (c *controller) shouldNodeBeDrained(machine *v1alpha1.Machine, existingCond
 func (c *controller) stopMachinePreservation(ctx context.Context, machine *v1alpha1.Machine) error {
 	// removal of preserveExpiryTime is the last step of stopping preservation
 	// if preserveExpiryTime is not set, preservation is already stopped
-	if !machineutils.IsPreserveExpiryTimeSet(machine) {
+	if machine.Status.CurrentStatus.PreserveExpiryTime == nil {
 		return nil
 	}
 	nodeName := machine.Labels[v1alpha1.NodeLabelKey]

pkg/util/provider/machineutils/utils.go

@@ -153,11 +153,6 @@ func IsMachineTriggeredForDeletion(m *v1alpha1.Machine) bool {
 	return m.Annotations[MachinePriority] == "1" || m.Annotations[TriggerDeletionByMCM] == "true"
 }
 
-// IsPreserveExpiryTimeSet checks if machine is preserved by MCM
-func IsPreserveExpiryTimeSet(m *v1alpha1.Machine) bool {
-	return !m.Status.CurrentStatus.PreserveExpiryTime.IsZero()
-}
-
 // HasPreservationTimedOut checks if the Status.CurrentStatus.PreserveExpiryTime has not yet passed
 func HasPreservationTimedOut(m *v1alpha1.Machine) bool {
 	return !m.Status.CurrentStatus.PreserveExpiryTime.After(time.Now())