Skip to content

Commit a809542

Browse files
committed
fix comments
1 parent eede6d9 commit a809542

File tree

3 files changed

+24
-34
lines changed

3 files changed

+24
-34
lines changed

pkg/controllers/updaterun/controller_integration_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -300,7 +300,7 @@ func generateInitializationFailedMetric(updateRun *placementv1beta1.ClusterStage
300300
func generateProgressingMetric(updateRun *placementv1beta1.ClusterStagedUpdateRun) *prometheusclientmodel.Metric {
301301
return &prometheusclientmodel.Metric{
302302
Label: generateMetricsLabels(updateRun, string(placementv1beta1.StagedUpdateRunConditionProgressing),
303-
string(metav1.ConditionTrue), condition.UpdateRunStartedReason),
303+
string(metav1.ConditionTrue), condition.UpdateRunProgressingReason),
304304
Gauge: &prometheusclientmodel.Gauge{
305305
Value: ptr.To(float64(time.Now().UnixNano()) / 1e9),
306306
},
@@ -627,7 +627,7 @@ func generateTrueCondition(obj client.Object, condType any) metav1.Condition {
627627
case placementv1beta1.StagedUpdateRunConditionInitialized:
628628
reason = condition.UpdateRunInitializeSucceededReason
629629
case placementv1beta1.StagedUpdateRunConditionProgressing:
630-
reason = condition.UpdateRunStartedReason
630+
reason = condition.UpdateRunProgressingReason
631631
case placementv1beta1.StagedUpdateRunConditionSucceeded:
632632
reason = condition.UpdateRunSucceededReason
633633
}

pkg/controllers/updaterun/execution.go

Lines changed: 20 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -59,9 +59,10 @@ func (r *Reconciler) execute(
5959
updatingStageIndex int,
6060
toBeUpdatedBindings, toBeDeletedBindings []*placementv1beta1.ClusterResourceBinding,
6161
) (bool, time.Duration, error) {
62-
// Before the execute returns, mark the update run as started in memory if it's not marked as waiting or stuck.
63-
// If the update run actually succeeds or fails, it will be remarked later in controller.go.
64-
defer markUpdateRunStartedIfNotWaitingOrStuck(updateRun)
62+
// Mark updateRun as progressing if it's not already marked as waiting or stuck.
63+
// This avoids triggering an unnecessary in-memory transition from stuck (waiting) -> progressing -> stuck (waiting),
64+
// which would update the lastTransitionTime even though the status hasn't effectively changed.
65+
markUpdateRunProgressingIfNotWaitingOrStuck(updateRun)
6566

6667
if updatingStageIndex < len(updateRun.Status.StagesStatus) {
6768
updatingStage := &updateRun.Status.StagesStatus[updatingStageIndex]
@@ -181,7 +182,7 @@ func (r *Reconciler) executeUpdatingStage(
181182
finished, updateErr := checkClusterUpdateResult(binding, clusterStatus, updatingStageStatus, updateRun)
182183
if finished {
183184
finishedClusterCount++
184-
unmarkUpdateRunStuck(updateRun)
185+
markUpdateRunProgressing(updateRun)
185186
continue
186187
} else {
187188
// If cluster update has been running for more than "updateRunStuckThreshold", mark the update run as stuck.
@@ -206,7 +207,7 @@ func (r *Reconciler) executeUpdatingStage(
206207
return 0, err
207208
}
208209
if approved {
209-
unmarkUpdateRunWaiting(updateRun)
210+
markUpdateRunProgressing(updateRun)
210211
markStageUpdatingSucceeded(updatingStageStatus, updateRun.Generation)
211212
// No need to wait to get to the next stage.
212213
return 0, nil
@@ -464,21 +465,26 @@ func checkClusterUpdateResult(
464465
return false, nil
465466
}
466467

467-
// markUpdateRunStartedIfNotWaitingOrStuck marks the update run as started in memory if it's not marked as waiting or stuck already.
468-
func markUpdateRunStartedIfNotWaitingOrStuck(updateRun *placementv1beta1.ClusterStagedUpdateRun) {
468+
// markUpdateRunProgressing marks the update run as progressing in memory.
469+
func markUpdateRunProgressing(updateRun *placementv1beta1.ClusterStagedUpdateRun) {
470+
meta.SetStatusCondition(&updateRun.Status.Conditions, metav1.Condition{
471+
Type: string(placementv1beta1.StagedUpdateRunConditionProgressing),
472+
Status: metav1.ConditionTrue,
473+
ObservedGeneration: updateRun.Generation,
474+
Reason: condition.UpdateRunProgressingReason,
475+
Message: "The update run is making progress",
476+
})
477+
}
478+
479+
// markUpdateRunProgressingIfNotWaitingOrStuck marks the update run as proegressing in memory if it's not marked as waiting or stuck already.
480+
func markUpdateRunProgressingIfNotWaitingOrStuck(updateRun *placementv1beta1.ClusterStagedUpdateRun) {
469481
progressingCond := meta.FindStatusCondition(updateRun.Status.Conditions, string(placementv1beta1.StagedUpdateRunConditionProgressing))
470482
if condition.IsConditionStatusFalse(progressingCond, updateRun.Generation) &&
471483
(progressingCond.Reason == condition.UpdateRunWaitingReason || progressingCond.Reason == condition.UpdateRunStuckReason) {
472484
// The updateRun is waiting or stuck, no need to mark it as started.
473485
return
474486
}
475-
meta.SetStatusCondition(&updateRun.Status.Conditions, metav1.Condition{
476-
Type: string(placementv1beta1.StagedUpdateRunConditionProgressing),
477-
Status: metav1.ConditionTrue,
478-
ObservedGeneration: updateRun.Generation,
479-
Reason: condition.UpdateRunStartedReason,
480-
Message: "The stages started updating",
481-
})
487+
markUpdateRunProgressing(updateRun)
482488
}
483489

484490
// markUpdateRunStuck marks the updateRun as stuck in memory.
@@ -492,14 +498,6 @@ func markUpdateRunStuck(updateRun *placementv1beta1.ClusterStagedUpdateRun, stag
492498
})
493499
}
494500

495-
// unmarkUpdateRunStuck unmarks the updateRun as stuck in memory.
496-
func unmarkUpdateRunStuck(updateRun *placementv1beta1.ClusterStagedUpdateRun) {
497-
progressingCond := meta.FindStatusCondition(updateRun.Status.Conditions, string(placementv1beta1.StagedUpdateRunConditionProgressing))
498-
if condition.IsConditionStatusFalse(progressingCond, updateRun.Generation) && progressingCond.Reason == condition.UpdateRunStuckReason {
499-
meta.RemoveStatusCondition(&updateRun.Status.Conditions, string(placementv1beta1.StagedUpdateRunConditionProgressing))
500-
}
501-
}
502-
503501
// markUpdateRunWaiting marks the updateRun as waiting in memory.
504502
func markUpdateRunWaiting(updateRun *placementv1beta1.ClusterStagedUpdateRun, stageName string) {
505503
meta.SetStatusCondition(&updateRun.Status.Conditions, metav1.Condition{
@@ -511,14 +509,6 @@ func markUpdateRunWaiting(updateRun *placementv1beta1.ClusterStagedUpdateRun, st
511509
})
512510
}
513511

514-
// unmarkUpdateRunWaiting unmarks the updateRun as waiting in memory.
515-
func unmarkUpdateRunWaiting(updateRun *placementv1beta1.ClusterStagedUpdateRun) {
516-
progressingCond := meta.FindStatusCondition(updateRun.Status.Conditions, string(placementv1beta1.StagedUpdateRunConditionProgressing))
517-
if condition.IsConditionStatusFalse(progressingCond, updateRun.Generation) && progressingCond.Reason == condition.UpdateRunWaitingReason {
518-
meta.RemoveStatusCondition(&updateRun.Status.Conditions, string(placementv1beta1.StagedUpdateRunConditionProgressing))
519-
}
520-
}
521-
522512
// markStageUpdatingStarted marks the stage updating status as started in memory.
523513
func markStageUpdatingStarted(stageUpdatingStatus *placementv1beta1.StageUpdatingStatus, generation int64) {
524514
if stageUpdatingStatus.StartTime == nil {

pkg/utils/condition/condition.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -144,8 +144,8 @@ const (
144144
// UpdateRunInitializeFailedReason is the reason string of condition if the update run is failed to initialize.
145145
UpdateRunInitializeFailedReason = "UpdateRunInitializedFailed"
146146

147-
// UpdateRunStartedReason is the reason string of condition if the staged update run has started.
148-
UpdateRunStartedReason = "UpdateRunStarted"
147+
// UpdateRunProgressingReason is the reason string of condition if the staged update run is progressing.
148+
UpdateRunProgressingReason = "UpdateRunProgressing"
149149

150150
// UpdateRunFailedReason is the reason string of condition if the staged update run failed.
151151
UpdateRunFailedReason = "UpdateRunFailed"

0 commit comments

Comments
 (0)