Skip to content

Commit 81959d2

Browse files
committed
set RecheckTime when minSuccessTime is defined
Signed-off-by: Qing Hao <[email protected]>
1 parent b492d8b commit 81959d2

File tree

2 files changed

+301
-81
lines changed

2 files changed

+301
-81
lines changed

pkg/apis/cluster/v1alpha1/rollout.go

Lines changed: 40 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,10 @@ type ClusterRolloutStatus struct {
5050
// Used to calculate timeout for progressing and failed status and minimum success time (i.e. soak
5151
// time) for succeeded status.
5252
LastTransitionTime *metav1.Time
53-
// TimeOutTime is the timeout time when the status is progressing or failed (optional field).
54-
TimeOutTime *metav1.Time
53+
// RecheckTime is the time when the cluster should be rechecked (optional field).
54+
// For succeeded status, tracks when the minSuccessTime (soak time) period ends.
55+
// For progressing/failed status, tracks when the timeout occurs.
56+
RecheckTime *metav1.Time
5557
}
5658

5759
// RolloutResult contains list of clusters that are timeOut, removed and required to rollOut. A
@@ -306,24 +308,24 @@ func progressivePerCluster(
306308
failureBreach = failureCount > maxFailures
307309
}
308310

309-
// Return if the list of exsiting rollout clusters has reached the target rollout size
311+
// Return if the list of existing rollout clusters has reached the target rollout size
310312
if len(rolloutClusters) >= rolloutSize {
311313
return RolloutResult{
312314
ClustersToRollout: rolloutClusters,
313315
ClustersTimeOut: timeoutClusters,
314316
MaxFailureBreach: failureBreach,
315-
RecheckAfter: minRecheckAfter(rolloutClusters, minSuccessTime),
317+
RecheckAfter: minRecheckAfter(rolloutClusters),
316318
}
317319
}
318320
}
319321

320-
// Return if the exsiting rollout clusters maxFailures is breached.
322+
// Return if the existing rollout clusters maxFailures is breached.
321323
if failureBreach {
322324
return RolloutResult{
323325
ClustersToRollout: rolloutClusters,
324326
ClustersTimeOut: timeoutClusters,
325327
MaxFailureBreach: failureBreach,
326-
RecheckAfter: minRecheckAfter(rolloutClusters, minSuccessTime),
328+
RecheckAfter: minRecheckAfter(rolloutClusters),
327329
}
328330
}
329331

@@ -352,15 +354,15 @@ func progressivePerCluster(
352354
return RolloutResult{
353355
ClustersToRollout: rolloutClusters,
354356
ClustersTimeOut: timeoutClusters,
355-
RecheckAfter: minRecheckAfter(rolloutClusters, minSuccessTime),
357+
RecheckAfter: minRecheckAfter(rolloutClusters),
356358
}
357359
}
358360
}
359361

360362
return RolloutResult{
361363
ClustersToRollout: rolloutClusters,
362364
ClustersTimeOut: timeoutClusters,
363-
RecheckAfter: minRecheckAfter(rolloutClusters, minSuccessTime),
365+
RecheckAfter: minRecheckAfter(rolloutClusters),
364366
}
365367
}
366368

@@ -430,7 +432,7 @@ func progressivePerGroup(
430432
ClustersToRollout: rolloutClusters,
431433
ClustersTimeOut: timeoutClusters,
432434
MaxFailureBreach: failureBreach,
433-
RecheckAfter: minRecheckAfter(rolloutClusters, minSuccessTime),
435+
RecheckAfter: minRecheckAfter(rolloutClusters),
434436
}
435437
}
436438
}
@@ -440,7 +442,7 @@ func progressivePerGroup(
440442
ClustersToRollout: rolloutClusters,
441443
ClustersTimeOut: timeoutClusters,
442444
MaxFailureBreach: failureBreach,
443-
RecheckAfter: minRecheckAfter(rolloutClusters, minSuccessTime),
445+
RecheckAfter: minRecheckAfter(rolloutClusters),
444446
}
445447
}
446448

@@ -470,17 +472,19 @@ func determineRolloutStatus(
470472
case Succeeded:
471473
// If the cluster succeeded but is still within the MinSuccessTime (i.e. "soak" time),
472474
// still add it to the list of rolloutClusters
473-
minSuccessTimeTime := getTimeOutTime(status.LastTransitionTime, minSuccessTime)
474-
if RolloutClock.Now().Before(minSuccessTimeTime.Time) {
475+
minSuccessTimeTime := calculateRecheckTime(status.LastTransitionTime, minSuccessTime)
476+
if minSuccessTimeTime != nil && RolloutClock.Now().Before(minSuccessTimeTime.Time) {
477+
// Set RecheckTime to track when the soak period ends
478+
status.RecheckTime = minSuccessTimeTime
475479
rolloutClusters = append(rolloutClusters, *status)
476480
}
477481

478482
return rolloutClusters, timeoutClusters
479483
case TimeOut, Skip:
480484
return rolloutClusters, timeoutClusters
481485
default: // For progressing, failed, or unknown status.
482-
timeOutTime := getTimeOutTime(status.LastTransitionTime, timeout)
483-
status.TimeOutTime = timeOutTime
486+
timeOutTime := calculateRecheckTime(status.LastTransitionTime, timeout)
487+
status.RecheckTime = timeOutTime
484488
// check if current time is before the timeout time
485489
if timeOutTime == nil || RolloutClock.Now().Before(timeOutTime.Time) {
486490
rolloutClusters = append(rolloutClusters, *status)
@@ -493,20 +497,21 @@ func determineRolloutStatus(
493497
return rolloutClusters, timeoutClusters
494498
}
495499

496-
// getTimeOutTime calculates the timeout time given a start time and duration, instantiating the
497-
// RolloutClock if a start time isn't provided.
498-
func getTimeOutTime(startTime *metav1.Time, timeout time.Duration) *metav1.Time {
499-
var timeoutTime time.Time
500-
// if timeout is not set (default to maxTimeDuration), the timeout time should not be set
501-
if timeout == maxTimeDuration {
500+
// calculateRecheckTime calculates the recheck time by adding a duration to a start time.
501+
// If startTime is nil, it uses the current time from RolloutClock.
502+
// If duration is maxTimeDuration (indicating no timeout/soak period), it returns nil.
503+
func calculateRecheckTime(startTime *metav1.Time, duration time.Duration) *metav1.Time {
504+
var recheckTime time.Time
505+
// if duration is not set (default to maxTimeDuration), the recheck time should not be set
506+
if duration == maxTimeDuration {
502507
return nil
503508
}
504509
if startTime == nil {
505-
timeoutTime = RolloutClock.Now().Add(timeout)
510+
recheckTime = RolloutClock.Now().Add(duration)
506511
} else {
507-
timeoutTime = startTime.Add(timeout)
512+
recheckTime = startTime.Add(duration)
508513
}
509-
return &metav1.Time{Time: timeoutTime}
514+
return &metav1.Time{Time: recheckTime}
510515
}
511516

512517
// calculateRolloutSize calculates the maximum portion from a total number of clusters by parsing a
@@ -598,19 +603,21 @@ func decisionGroupsToGroupKeys(decisionsGroup []clusterv1alpha1.MandatoryDecisio
598603
return result
599604
}
600605

601-
func minRecheckAfter(rolloutClusters []ClusterRolloutStatus, minSuccessTime time.Duration) *time.Duration {
602-
var minRecheckAfter *time.Duration
606+
func minRecheckAfter(rolloutClusters []ClusterRolloutStatus) *time.Duration {
607+
var minDuration *time.Duration
608+
603609
for _, r := range rolloutClusters {
604-
if r.TimeOutTime != nil {
605-
timeOut := r.TimeOutTime.Sub(RolloutClock.Now())
606-
if minRecheckAfter == nil || *minRecheckAfter > timeOut {
607-
minRecheckAfter = &timeOut
610+
// Check RecheckTime for both Progressing/Failed clusters (timeout) and Succeeded clusters (soak period)
611+
if r.RecheckTime != nil {
612+
recheckDuration := r.RecheckTime.Sub(RolloutClock.Now())
613+
// Only consider positive durations (future recheck times)
614+
if recheckDuration > 0 {
615+
if minDuration == nil || *minDuration > recheckDuration {
616+
minDuration = &recheckDuration
617+
}
608618
}
609619
}
610620
}
611-
if minSuccessTime != 0 && (minRecheckAfter == nil || minSuccessTime < *minRecheckAfter) {
612-
minRecheckAfter = &minSuccessTime
613-
}
614621

615-
return minRecheckAfter
622+
return minDuration
616623
}

0 commit comments

Comments
 (0)