rewrite step jump logic

Signed-off-by: AiRanthem <zhongtianyun.zty@alibaba-inc.com>

Author: AiRanthem

api/v1beta1/rollout_types.go

@@ -459,7 +459,24 @@ type BlueGreenStatus struct {
 	UpdatedReadyReplicas int32 `json:"updatedReadyReplicas"`
 }
 
+// UnifiedStatus is a data structure used to unify BlueGreenStatus and CanaryStatus, allowing the controller to operate
+// the current release status in a unified manner without considering the release form.
+type UnifiedStatus struct {
+	*CommonStatus
+	// UpdatedRevision is the pointer of BlueGreenStatus.UpdatedRevision or CanaryStatus.CanaryRevision
+	UpdatedRevision *string `json:"updatedRevision"`
+	// UpdatedReplicas is the pointer of BlueGreenStatus.UpdatedReplicas or CanaryStatus.CanaryReplicas
+	UpdatedReplicas *int32 `json:"updatedReplicas"`
+	// UpdatedReadyReplicas is the pointer of BlueGreenStatus.UpdatedReadyReplicas or CanaryStatus.CanaryReadyReplicas
+	UpdatedReadyReplicas *int32 `json:"updatedReadyReplicas"`
+}
+
+func (u UnifiedStatus) IsNil() bool {
+	return u.CommonStatus == nil || u.UpdatedReplicas == nil || u.UpdatedReadyReplicas == nil || u.UpdatedRevision == nil
+}
+
 // GetSubStatus returns the ethier canary or bluegreen status
+// Deprecated: use
 func (r *RolloutStatus) GetSubStatus() *CommonStatus {
 	if r.CanaryStatus == nil && r.BlueGreenStatus == nil {
 		return nil
@@ -470,6 +487,22 @@ func (r *RolloutStatus) GetSubStatus() *CommonStatus {
 	return &r.BlueGreenStatus.CommonStatus
 }
 
+func (r *RolloutStatus) GetUnifiedStatus() UnifiedStatus {
+	status := UnifiedStatus{}
+	if r.CanaryStatus != nil {
+		status.CommonStatus = &r.CanaryStatus.CommonStatus
+		status.UpdatedReadyReplicas = &r.CanaryStatus.CanaryReadyReplicas
+		status.UpdatedReplicas = &r.CanaryStatus.CanaryReplicas
+		status.UpdatedRevision = &r.CanaryStatus.CanaryRevision
+	} else if r.BlueGreenStatus != nil {
+		status.CommonStatus = &r.BlueGreenStatus.CommonStatus
+		status.UpdatedReadyReplicas = &r.BlueGreenStatus.UpdatedReadyReplicas
+		status.UpdatedReplicas = &r.BlueGreenStatus.UpdatedReplicas
+		status.UpdatedRevision = &r.BlueGreenStatus.UpdatedRevision
+	}
+	return status
+}
+
 func (r *RolloutStatus) IsSubStatusEmpty() bool {
 	return r.CanaryStatus == nil && r.BlueGreenStatus == nil
 }

pkg/controller/batchrelease/labelpatch/patcher.go

@@ -124,7 +124,9 @@ func (r *realPatcher) patchPodBatchLabel(pods []*corev1.Pod, ctx *batchcontext.B
 			klog.InfoS("Pod batchID is not a number, skip patching", "pod", klog.KObj(pod), "rollout", r.logKey)
 			continue
 		}
-		plannedUpdatedReplicasForBatches[podBatchID-1]--
+		if podBatchID > 0 && podBatchID <= len(plannedUpdatedReplicasForBatches) {
+			plannedUpdatedReplicasForBatches[podBatchID-1]--
+		}
 	}
 	klog.InfoS("updatedButUnpatchedPods amount calculated", "amount", len(updatedButUnpatchedPods),
 		"rollout", r.logKey, "plan", plannedUpdatedReplicasForBatches)

pkg/controller/rollout/rollout_bluegreen.go

@@ -19,7 +19,6 @@ package rollout
 import (
 	"context"
 	"fmt"
-	"reflect"
 	"time"
 
 	corev1 "k8s.io/api/core/v1"
@@ -248,31 +247,10 @@ func (m *blueGreenReleaseManager) doCanaryPaused(c *RolloutContext) (bool, error
 }
 
 func (m *blueGreenReleaseManager) doCanaryJump(c *RolloutContext) (jumped bool) {
-	bluegreenStatus := c.NewStatus.BlueGreenStatus
-	// since we forbid adding or removing steps, currentStepIndex should always be valid
-	currentStep := c.Rollout.Spec.Strategy.BlueGreen.Steps[bluegreenStatus.CurrentStepIndex-1]
-	// nextIndex=-1 means the release is done, nextIndex=0 is not used
-	if nextIndex := bluegreenStatus.NextStepIndex; nextIndex != util.NextBatchIndex(c.Rollout, bluegreenStatus.CurrentStepIndex) && nextIndex > 0 {
-		currentIndexBackup := bluegreenStatus.CurrentStepIndex
-		currentStepStateBackup := bluegreenStatus.CurrentStepState
-		// update the current and next stepIndex
-		bluegreenStatus.CurrentStepIndex = nextIndex
-		bluegreenStatus.NextStepIndex = util.NextBatchIndex(c.Rollout, nextIndex)
-		nextStep := c.Rollout.Spec.Strategy.BlueGreen.Steps[nextIndex-1]
-		// compare next step and current step to decide the state we should go
-		if reflect.DeepEqual(nextStep.Replicas, currentStep.Replicas) {
-			bluegreenStatus.CurrentStepState = v1beta1.CanaryStepStateTrafficRouting
-		} else {
-			bluegreenStatus.CurrentStepState = v1beta1.CanaryStepStateInit
-		}
-		bluegreenStatus.LastUpdateTime = &metav1.Time{Time: time.Now()}
-		klog.Infof("rollout(%s/%s) step(%d->%d) state from(%s -> %s)",
-			c.Rollout.Namespace, c.Rollout.Name,
-			currentIndexBackup, bluegreenStatus.CurrentStepIndex,
-			currentStepStateBackup, bluegreenStatus.CurrentStepState)
-		return true
+	if c.Workload == nil {
+		return false
 	}
-	return false
+	return doStepJump(c.Rollout, c.NewStatus, c.Rollout.Spec.Strategy.BlueGreen.Steps, int(c.Workload.Replicas))
 }
 
 // cleanup after rollout is completed or finished

pkg/controller/rollout/rollout_canary.go

@@ -19,7 +19,6 @@ package rollout
 import (
 	"context"
 	"fmt"
-	"reflect"
 	"time"
 
 	corev1 "k8s.io/api/core/v1"
@@ -313,34 +312,48 @@ func (m *canaryReleaseManager) doCanaryPaused(c *RolloutContext) (bool, error) {
 	return false, nil
 }
 
-func (m *canaryReleaseManager) doCanaryJump(c *RolloutContext) (jumped bool) {
-	canaryStatus := c.NewStatus.CanaryStatus
-	// since we forbid adding or removing steps, currentStepIndex should always be valid
-	currentStep := c.Rollout.Spec.Strategy.Canary.Steps[canaryStatus.CurrentStepIndex-1]
-	// nextIndex=-1 means the release is done, nextIndex=0 is not used
-	if nextIndex := canaryStatus.NextStepIndex; nextIndex != util.NextBatchIndex(c.Rollout, canaryStatus.CurrentStepIndex) && nextIndex > 0 {
-		currentIndexBackup := canaryStatus.CurrentStepIndex
-		currentStepStateBackup := canaryStatus.CurrentStepState
+// doStepJump implements the common logic for both canary and bluegreen rollout strategies
+// to handle step jumping based on NextStepIndex.
+func doStepJump(rollout *v1beta1.Rollout, newStatus *v1beta1.RolloutStatus, steps []v1beta1.CanaryStep, workloadReplicas int) (jumped bool) {
+	status := newStatus.GetUnifiedStatus()
+	if status.IsNil() {
+		klog.InfoS("doStepJump skipped: unified status is nil")
+		return false
+	}
+	klog.InfoS("will do step jump", "steps", len(steps), "updatedReplicas", *status.UpdatedReplicas,
+		"nextStepIndex", status.NextStepIndex, "rollout", klog.KObj(rollout))
+	if nextIndex := status.NextStepIndex; nextIndex != util.NextBatchIndex(rollout, status.CurrentStepIndex) &&
+		nextIndex > 0 && nextIndex <= int32(len(steps)) {
+		currentIndexBackup := status.CurrentStepIndex
+		currentStepStateBackup := status.CurrentStepState
 		// update the current and next stepIndex
-		canaryStatus.CurrentStepIndex = nextIndex
-		canaryStatus.NextStepIndex = util.NextBatchIndex(c.Rollout, nextIndex)
-		nextStep := c.Rollout.Spec.Strategy.Canary.Steps[nextIndex-1]
+		status.CurrentStepIndex = nextIndex
+		status.NextStepIndex = util.NextBatchIndex(rollout, nextIndex)
+		nextStep := steps[nextIndex-1]
 		// compare next step and current step to decide the state we should go
-		if reflect.DeepEqual(nextStep.Replicas, currentStep.Replicas) {
-			canaryStatus.CurrentStepState = v1beta1.CanaryStepStateTrafficRouting
+		nextStepReplicas, _ := intstr.GetScaledValueFromIntOrPercent(nextStep.Replicas, workloadReplicas, true)
+		if int32(nextStepReplicas) == *status.UpdatedReplicas {
+			status.CurrentStepState = v1beta1.CanaryStepStateTrafficRouting
 		} else {
-			canaryStatus.CurrentStepState = v1beta1.CanaryStepStateInit
+			status.CurrentStepState = v1beta1.CanaryStepStateInit
 		}
-		canaryStatus.LastUpdateTime = &metav1.Time{Time: time.Now()}
-		klog.Infof("rollout(%s/%s) step(%d->%d) state from(%s -> %s)",
-			c.Rollout.Namespace, c.Rollout.Name,
-			currentIndexBackup, canaryStatus.CurrentStepIndex,
-			currentStepStateBackup, canaryStatus.CurrentStepState)
+		status.LastUpdateTime = &metav1.Time{Time: time.Now()}
+		klog.InfoS("step jumped", "rollout", klog.KObj(rollout),
+			"oldCurrentIndex", currentIndexBackup, "newCurrentIndex", status.CurrentStepIndex,
+			"oldCurrentStepState", currentStepStateBackup, "newCurrentStepState", status.CurrentStepState)
 		return true
 	}
+	klog.InfoS("step not jumped")
 	return false
 }
 
+func (m *canaryReleaseManager) doCanaryJump(c *RolloutContext) (jumped bool) {
+	if c.Workload == nil {
+		return false
+	}
+	return doStepJump(c.Rollout, c.NewStatus, c.Rollout.Spec.Strategy.Canary.Steps, int(c.Workload.Replicas))
+}
+
 // cleanup after rollout is completed or finished
 func (m *canaryReleaseManager) doCanaryFinalising(c *RolloutContext) (bool, error) {
 	canaryStatus := c.NewStatus.CanaryStatus