restore the enableExtra field in BR

Signed-off-by: yunbo <[email protected]>

api/v1alpha1/batchrelease_plan_types.go

@@ -56,6 +56,9 @@ type ReleasePlan struct {
 	PatchPodTemplateMetadata *PatchPodTemplateMetadata `json:"patchPodTemplateMetadata,omitempty"`
 	// RollingStyle can be "Canary", "Partiton" or "BlueGreen"
 	RollingStyle RollingStyleType `json:"rollingStyle,omitempty"`
+	// When user verifies that the canary version is ready, we will remove the canary deployment and release the deployment workload-demo in full.
+	// Current only support k8s native deployment
+	EnableExtraWorkloadForCanary bool `json:"enableExtraWorkloadForCanary"`
 }
 
 type FinalizingPolicyType string

api/v1alpha1/conversion.go

@@ -357,6 +357,8 @@ func (src *BatchRelease) ConvertTo(dst conversion.Hub) error {
 			obj.Spec.ReleasePlan.RollingStyle = v1beta1.BlueGreenRollingStyle
 		}
 
+		obj.Spec.ReleasePlan.EnableExtraWorkloadForCanary = srcSpec.ReleasePlan.EnableExtraWorkloadForCanary
+
 		// status
 		obj.Status = v1beta1.BatchReleaseStatus{
 			StableRevision:           src.Status.StableRevision,
@@ -434,6 +436,7 @@ func (dst *BatchRelease) ConvertFrom(src conversion.Hub) error {
 		}
 		dst.Annotations[RolloutStyleAnnotation] = strings.ToLower(string(srcV1beta1.Spec.ReleasePlan.RollingStyle))
 		dst.Spec.ReleasePlan.RollingStyle = RollingStyleType(srcV1beta1.Spec.ReleasePlan.RollingStyle)
+		dst.Spec.ReleasePlan.EnableExtraWorkloadForCanary = srcV1beta1.Spec.ReleasePlan.EnableExtraWorkloadForCanary
 
 		// status
 		dst.Status = BatchReleaseStatus{

api/v1alpha1/rollout_types.go

@@ -244,11 +244,15 @@ type CanaryStatus struct {
 	CanaryReadyReplicas int32 `json:"canaryReadyReplicas"`
 	// NextStepIndex defines the next step of the rollout is on.
 	// In normal case, NextStepIndex is equal to CurrentStepIndex + 1
-	// If the current step is the last step, NextStepIndex is equal to 0
-	// Before the release, NextStepIndex is also equal to 0
-	// It is allowed to modify NextStepIndex by design,
+	// If the current step is the last step, NextStepIndex is equal to -1
+	// Before the release, NextStepIndex is also equal to -1
+	// 0 is not used and won't appear in any case
+	// It is allowed to patch NextStepIndex by design,
 	// e.g. if CurrentStepIndex is 2, user can patch NextStepIndex to 3 (if exists) to
 	// achieve batch jump, or patch NextStepIndex to 1 to implement a re-execution of step 1
+	// Patching it with a non-positive value is meaningless, which will be corrected
+	// in the next reconciliation
+	// achieve batch jump, or patch NextStepIndex to 1 to implement a re-execution of step 1
 	NextStepIndex int32 `json:"nextStepIndex"`
 	// +optional
 	CurrentStepIndex int32             `json:"currentStepIndex"`

api/v1beta1/batchrelease_plan_types.go

@@ -56,6 +56,9 @@ type ReleasePlan struct {
 	PatchPodTemplateMetadata *PatchPodTemplateMetadata `json:"patchPodTemplateMetadata,omitempty"`
 	// RollingStyle can be "Canary", "Partiton" or "BlueGreen"
 	RollingStyle RollingStyleType `json:"rollingStyle,omitempty"`
+	// When user verifies that the canary version is ready, we will remove the canary deployment and release the deployment workload-demo in full.
+	// Current only support k8s native deployment
+	EnableExtraWorkloadForCanary bool `json:"enableExtraWorkloadForCanary"`
 }
 
 type FinalizingPolicyType string

api/v1beta1/rollout_types.go

@@ -350,17 +350,19 @@ type CommonStatus struct {
 	StableRevision string `json:"stableRevision,omitempty"`
 	// pod template hash is used as service selector label
 	PodTemplateHash string `json:"podTemplateHash"`
-	// CurrentStepIndex defines the current step of the rollout is on. If the current step index is null, the
-	// controller will execute the rollout.
+	// CurrentStepIndex defines the current step of the rollout is on.
 	// +optional
 	CurrentStepIndex int32 `json:"currentStepIndex"`
 	// NextStepIndex defines the next step of the rollout is on.
 	// In normal case, NextStepIndex is equal to CurrentStepIndex + 1
-	// If the current step is the last step, NextStepIndex is equal to 0
-	// Before the release, NextStepIndex is also equal to 0
-	// It is allowed to modify NextStepIndex by design,
+	// If the current step is the last step, NextStepIndex is equal to -1
+	// Before the release, NextStepIndex is also equal to -1
+	// 0 is not used and won't appear in any case
+	// It is allowed to patch NextStepIndex by design,
 	// e.g. if CurrentStepIndex is 2, user can patch NextStepIndex to 3 (if exists) to
 	// achieve batch jump, or patch NextStepIndex to 1 to implement a re-execution of step 1
+	// Patching it with a non-positive value is useless and meaningless, which will be corrected
+	// in the next reconciliation
 	NextStepIndex int32 `json:"nextStepIndex"`
 	// FinalisingStep the step of finalising
 	FinalisingStep   FinalisingStepType `json:"finalisingStep"`
@@ -464,6 +466,7 @@ func (r *RolloutStatus) SetCanaryReadyReplicas(replicas int32) {
 type CanaryStepState string
 
 const (
+	// the first step, handle some special cases before step upgrade, to prevent traffic loss
 	CanaryStepStateInit            CanaryStepState = "BeforeStepUpgrade"
 	CanaryStepStateUpgrade         CanaryStepState = "StepUpgrade"
 	CanaryStepStateTrafficRouting  CanaryStepState = "StepTrafficRouting"

config/crd/bases/rollouts.kruise.io_batchreleases.yaml

@@ -88,6 +88,11 @@ spec:
                       - canaryReplicas
                       type: object
                     type: array
+                  enableExtraWorkloadForCanary:
+                    description: When user verifies that the canary version is ready,
+                      we will remove the canary deployment and release the deployment
+                      workload-demo in full. Current only support k8s native deployment
+                    type: boolean
                   failureThreshold:
                     anyOf:
                     - type: integer
@@ -124,6 +129,8 @@ spec:
                   rolloutID:
                     description: RolloutID indicates an id for each rollout progress
                     type: string
+                required:
+                - enableExtraWorkloadForCanary
                 type: object
               targetReference:
                 description: TargetRef contains the GVK and name of the workload that
@@ -348,6 +355,11 @@ spec:
                       - canaryReplicas
                       type: object
                     type: array
+                  enableExtraWorkloadForCanary:
+                    description: When user verifies that the canary version is ready,
+                      we will remove the canary deployment and release the deployment
+                      workload-demo in full. Current only support k8s native deployment
+                    type: boolean
                   failureThreshold:
                     anyOf:
                     - type: integer
@@ -384,6 +396,8 @@ spec:
                   rolloutID:
                     description: RolloutID indicates an id for each rollout progress
                     type: string
+                required:
+                - enableExtraWorkloadForCanary
                 type: object
               workloadRef:
                 description: WorkloadRef contains enough information to let you identify

config/crd/bases/rollouts.kruise.io_rollouts.yaml

@@ -450,10 +450,14 @@ spec:
                     description: NextStepIndex defines the next step of the rollout
                       is on. In normal case, NextStepIndex is equal to CurrentStepIndex
                       + 1 If the current step is the last step, NextStepIndex is equal
-                      to 0 Before the release, NextStepIndex is also equal to 0 It
-                      is allowed to modify NextStepIndex by design, e.g. if CurrentStepIndex
-                      is 2, user can patch NextStepIndex to 3 (if exists) to achieve
-                      batch jump, or patch NextStepIndex to 1 to implement a re-execution
+                      to -1 Before the release, NextStepIndex is also equal to -1
+                      0 is not used and won't appear in any case It is allowed to
+                      patch NextStepIndex by design, e.g. if CurrentStepIndex is 2,
+                      user can patch NextStepIndex to 3 (if exists) to achieve batch
+                      jump, or patch NextStepIndex to 1 to implement a re-execution
+                      of step 1 Patching it with a non-positive value is meaningless,
+                      which will be corrected in the next reconciliation achieve batch
+                      jump, or patch NextStepIndex to 1 to implement a re-execution
                       of step 1
                     format: int32
                     type: integer
@@ -1250,8 +1254,7 @@ spec:
                 properties:
                   currentStepIndex:
                     description: CurrentStepIndex defines the current step of the
-                      rollout is on. If the current step index is null, the controller
-                      will execute the rollout.
+                      rollout is on.
                     format: int32
                     type: integer
                   currentStepState:
@@ -1268,11 +1271,13 @@ spec:
                     description: NextStepIndex defines the next step of the rollout
                       is on. In normal case, NextStepIndex is equal to CurrentStepIndex
                       + 1 If the current step is the last step, NextStepIndex is equal
-                      to 0 Before the release, NextStepIndex is also equal to 0 It
-                      is allowed to modify NextStepIndex by design, e.g. if CurrentStepIndex
-                      is 2, user can patch NextStepIndex to 3 (if exists) to achieve
-                      batch jump, or patch NextStepIndex to 1 to implement a re-execution
-                      of step 1
+                      to -1 Before the release, NextStepIndex is also equal to -1
+                      0 is not used and won't appear in any case It is allowed to
+                      patch NextStepIndex by design, e.g. if CurrentStepIndex is 2,
+                      user can patch NextStepIndex to 3 (if exists) to achieve batch
+                      jump, or patch NextStepIndex to 1 to implement a re-execution
+                      of step 1 Patching it with a non-positive value is useless and
+                      meaningless, which will be corrected in the next reconciliation
                     format: int32
                     type: integer
                   observedRolloutID:
@@ -1337,8 +1342,7 @@ spec:
                     type: string
                   currentStepIndex:
                     description: CurrentStepIndex defines the current step of the
-                      rollout is on. If the current step index is null, the controller
-                      will execute the rollout.
+                      rollout is on.
                     format: int32
                     type: integer
                   currentStepState:
@@ -1355,11 +1359,13 @@ spec:
                     description: NextStepIndex defines the next step of the rollout
                       is on. In normal case, NextStepIndex is equal to CurrentStepIndex
                       + 1 If the current step is the last step, NextStepIndex is equal
-                      to 0 Before the release, NextStepIndex is also equal to 0 It
-                      is allowed to modify NextStepIndex by design, e.g. if CurrentStepIndex
-                      is 2, user can patch NextStepIndex to 3 (if exists) to achieve
-                      batch jump, or patch NextStepIndex to 1 to implement a re-execution
-                      of step 1
+                      to -1 Before the release, NextStepIndex is also equal to -1
+                      0 is not used and won't appear in any case It is allowed to
+                      patch NextStepIndex by design, e.g. if CurrentStepIndex is 2,
+                      user can patch NextStepIndex to 3 (if exists) to achieve batch
+                      jump, or patch NextStepIndex to 1 to implement a re-execution
+                      of step 1 Patching it with a non-positive value is useless and
+                      meaningless, which will be corrected in the next reconciliation
                     format: int32
                     type: integer
                   observedRolloutID:

pkg/controller/rollout/rollout_canary.go

@@ -359,12 +359,13 @@ func createBatchRelease(rollout *v1beta1.Rollout, rolloutID string, batch int32,
 				Name:       rollout.Spec.WorkloadRef.Name,
 			},
 			ReleasePlan: v1beta1.ReleasePlan{
-				Batches:                  batches,
-				RolloutID:                rolloutID,
-				BatchPartition:           utilpointer.Int32Ptr(batch),
-				FailureThreshold:         rollout.Spec.Strategy.Canary.FailureThreshold,
-				PatchPodTemplateMetadata: rollout.Spec.Strategy.Canary.PatchPodTemplateMetadata,
-				RollingStyle:             rollout.Spec.Strategy.GetRollingStyle(),
+				Batches:                      batches,
+				RolloutID:                    rolloutID,
+				BatchPartition:               utilpointer.Int32Ptr(batch),
+				FailureThreshold:             rollout.Spec.Strategy.Canary.FailureThreshold,
+				PatchPodTemplateMetadata:     rollout.Spec.Strategy.Canary.PatchPodTemplateMetadata,
+				RollingStyle:                 rollout.Spec.Strategy.GetRollingStyle(),
+				EnableExtraWorkloadForCanary: rollout.Spec.Strategy.Canary.EnableExtraWorkloadForCanary,
 			},
 		},
 	}

pkg/controller/rollout/rollout_canary_test.go

@@ -99,6 +99,7 @@ func TestRunCanary(t *testing.T) {
 					},
 				}
 				br.Spec.ReleasePlan.BatchPartition = utilpointer.Int32(0)
+				br.Spec.ReleasePlan.EnableExtraWorkloadForCanary = true
 				br.Spec.ReleasePlan.RollingStyle = v1beta1.CanaryRollingStyle
 				return br
 			},
@@ -158,6 +159,7 @@ func TestRunCanary(t *testing.T) {
 					},
 				}
 				br.Spec.ReleasePlan.BatchPartition = utilpointer.Int32(0)
+				br.Spec.ReleasePlan.EnableExtraWorkloadForCanary = true
 				br.Spec.ReleasePlan.RollingStyle = v1beta1.CanaryRollingStyle
 				br.Status = v1beta1.BatchReleaseStatus{
 					ObservedGeneration: 1,
@@ -206,13 +208,17 @@ func TestRunCanary(t *testing.T) {
 					},
 				}
 				br.Spec.ReleasePlan.BatchPartition = utilpointer.Int32(0)
+				br.Spec.ReleasePlan.EnableExtraWorkloadForCanary = true
 				br.Spec.ReleasePlan.RollingStyle = v1beta1.CanaryRollingStyle
 				return br
 			},
 		},
 	}
 
-	for _, cs := range cases {
+	for i, cs := range cases {
+		if i != 1 {
+			continue
+		}
 		t.Run(cs.name, func(t *testing.T) {
 			deps, rss := cs.getObj()
 			rollout, br := cs.getRollout()

pkg/util/rollout_utils.go

@@ -165,15 +165,12 @@ func EncodeHash(data string) string {
 	return fmt.Sprintf("%x", sha256.Sum256([]byte(data)))
 }
 
+// calculate the next batch index
 func NextBatchIndex(rollout *rolloutv1beta1.Rollout, CurrentStepIndex int32) int32 {
 	if rollout == nil {
-		return 0
+		return -1
 	}
 	allSteps := int32(len(rollout.Spec.Strategy.GetSteps()))
-	// In initilization, CurrentStep is set to Completed, and NextStepIndex is set to 0
-	// when Current Step is the last step (Progeessing or Completed), we set NextStepIndex to 0
-	// Patching NextStepIndex to 0 is meaningless for user, if doing so, nothing happen
-	// and step jump won't happen
 	if CurrentStepIndex >= allSteps {
 		return -1
 	}