address minor comments

Signed-off-by: Britania Rodriguez Reyes <britaniar@microsoft.com>

Author: britaniar

pkg/controllers/updaterun/controller.go

@@ -176,7 +176,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req runtime.Request) (runtim
 			return runtime.Result{}, r.recordUpdateRunSucceeded(ctx, updateRun)
 		}
 
-		return r.updateUpdateRunStatus(ctx, updateRun, waitTime, execErr, state, runObjRef)
+		return r.handleIncompleteUpdateRun(ctx, updateRun, waitTime, execErr, state, runObjRef)
 	case placementv1beta1.StateStop:
 		// Stop the updateRun.
 		klog.V(2).InfoS("Stopping the updateRun", "state", state, "updatingStageIndex", updatingStageIndex, "updateRun", runObjRef)
@@ -191,7 +191,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req runtime.Request) (runtim
 			return runtime.Result{}, r.recordUpdateRunStopped(ctx, updateRun)
 		}
 
-		return r.updateUpdateRunStatus(ctx, updateRun, waitTime, stopErr, state, runObjRef)
+		return r.handleIncompleteUpdateRun(ctx, updateRun, waitTime, stopErr, state, runObjRef)
 
 	default:
 		// Initialize, Run, or Stop are the only supported states.
@@ -202,7 +202,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req runtime.Request) (runtim
 	return runtime.Result{}, nil
 }
 
-func (r *Reconciler) updateUpdateRunStatus(ctx context.Context, updateRun placementv1beta1.UpdateRunObj, waitTime time.Duration, err error, state placementv1beta1.State, runObjRef klog.ObjectRef) (runtime.Result, error) {
+func (r *Reconciler) handleIncompleteUpdateRun(ctx context.Context, updateRun placementv1beta1.UpdateRunObj, waitTime time.Duration, err error, state placementv1beta1.State, runObjRef klog.ObjectRef) (runtime.Result, error) {
 	// The execution or stopping is not finished yet or it encounters a retriable error.
 	// We need to record the status and requeue.
 	if updateErr := r.recordUpdateRunStatus(ctx, updateRun); updateErr != nil {

pkg/controllers/updaterun/execution.go

@@ -224,7 +224,7 @@ func (r *Reconciler) executeUpdatingStage(
 				}
 			}
 			markClusterUpdatingStarted(clusterStatus, updateRun.GetGeneration())
-			markStageUpdatingStarted(updatingStageStatus, updateRun.GetGeneration())
+			markStageUpdatingProgressStarted(updatingStageStatus, updateRun.GetGeneration())
 			// Need to continue as we need to process at most maxConcurrency number of clusters in parallel.
 			continue
 		}
@@ -328,7 +328,7 @@ func (r *Reconciler) executeDeleteStage(
 		existingDeleteStageClusterMap[existingDeleteStageStatus.Clusters[i].ClusterName] = &existingDeleteStageStatus.Clusters[i]
 	}
 	// Mark the delete stage as started in case it's not.
-	markStageUpdatingStarted(updateRunStatus.DeletionStageStatus, updateRun.GetGeneration())
+	markStageUpdatingProgressStarted(updateRunStatus.DeletionStageStatus, updateRun.GetGeneration())
 	for _, binding := range toBeDeletedBindings {
 		bindingSpec := binding.GetBindingSpec()
 		curCluster, exist := existingDeleteStageClusterMap[bindingSpec.TargetCluster]
@@ -698,8 +698,8 @@ func markUpdateRunWaiting(updateRun placementv1beta1.UpdateRunObj, message strin
 	})
 }
 
-// markStageUpdatingStarted marks the stage updating status as started in memory.
-func markStageUpdatingStarted(stageUpdatingStatus *placementv1beta1.StageUpdatingStatus, generation int64) {
+// markStageUpdatingProgressStarted marks the stage updating status as started in memory.
+func markStageUpdatingProgressStarted(stageUpdatingStatus *placementv1beta1.StageUpdatingStatus, generation int64) {
 	if stageUpdatingStatus.StartTime == nil {
 		stageUpdatingStatus.StartTime = &metav1.Time{Time: time.Now()}
 	}

pkg/controllers/updaterun/stop.go

@@ -79,7 +79,7 @@ func (r *Reconciler) stopUpdatingStage(
 	for i := 0; i < len(updatingStageStatus.Clusters); i++ {
 		clusterStatus := &updatingStageStatus.Clusters[i]
 		clusterStartedCond := meta.FindStatusCondition(clusterStatus.Conditions, string(placementv1beta1.ClusterUpdatingConditionStarted))
-		if clusterStartedCond == nil || condition.IsConditionStatusFalse(clusterStartedCond, updateRun.GetGeneration()) {
+		if !condition.IsConditionStatusTrue(clusterStartedCond, updateRun.GetGeneration()) {
 			// Cluster has not started updating therefore no need to do anything.
 			continue
 		}
@@ -168,7 +168,7 @@ func (r *Reconciler) stopDeleteStage(
 				klog.ErrorS(unexpectedErr, "The binding should be deleting before we mark a cluster deleting", "clusterStatus", curCluster, "updateRun", updateRunRef)
 				return false, fmt.Errorf("%w: %s", errStagedUpdatedAborted, unexpectedErr.Error())
 			}
-			return false, nil
+			continue
 		}
 	}