restore the enableExtra field in BR

Signed-off-by: yunbo <[email protected]>
openkruise · Jun 11, 2024 · 6894467 · 6894467
1 parent 3b03486
commit 6894467
Show file tree

Hide file tree

Showing 13 changed files with 118 additions and 78 deletions.
diff --git a/api/v1alpha1/batchrelease_plan_types.go b/api/v1alpha1/batchrelease_plan_types.go
@@ -56,6 +56,14 @@ type ReleasePlan struct {
 	PatchPodTemplateMetadata *PatchPodTemplateMetadata `json:"patchPodTemplateMetadata,omitempty"`
 	// RollingStyle can be "Canary", "Partiton" or "BlueGreen"
 	RollingStyle RollingStyleType `json:"rollingStyle,omitempty"`
+	// EnableExtraWorkloadForCanary indicates whether to create extra workload for canary
+	// True corresponds to RollingStyle "Canary".
+	// False corresponds to RollingStyle "Partiton".
+	// Ignored in BlueGreen-style.
+	// This field is about to deprecate, use RollingStyle instead.
+	// If both of them are set, controller will only consider this
+	// filed when RollingStyle is empty
+	EnableExtraWorkloadForCanary bool `json:"enableExtraWorkloadForCanary"`
 }
 
 type FinalizingPolicyType string

diff --git a/api/v1alpha1/conversion.go b/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{

diff --git a/api/v1alpha1/rollout_types.go b/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"`

diff --git a/api/v1beta1/batchrelease_plan_types.go b/api/v1beta1/batchrelease_plan_types.go
@@ -56,6 +56,14 @@ type ReleasePlan struct {
 	PatchPodTemplateMetadata *PatchPodTemplateMetadata `json:"patchPodTemplateMetadata,omitempty"`
 	// RollingStyle can be "Canary", "Partiton" or "BlueGreen"
 	RollingStyle RollingStyleType `json:"rollingStyle,omitempty"`
+	// EnableExtraWorkloadForCanary indicates whether to create extra workload for canary
+	// True corresponds to RollingStyle "Canary".
+	// False corresponds to RollingStyle "Partiton".
+	// Ignored in BlueGreen-style.
+	// This field is about to deprecate, use RollingStyle instead.
+	// If both of them are set, controller will only consider this
+	// filed when RollingStyle is empty
+	EnableExtraWorkloadForCanary bool `json:"enableExtraWorkloadForCanary"`
 }
 
 type FinalizingPolicyType string

diff --git a/api/v1beta1/rollout_types.go b/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"

diff --git a/config/crd/bases/rollouts.kruise.io_batchreleases.yaml b/config/crd/bases/rollouts.kruise.io_batchreleases.yaml
@@ -88,6 +88,14 @@ spec:
                       - canaryReplicas
                       type: object
                     type: array
+                  enableExtraWorkloadForCanary:
+                    description: EnableExtraWorkloadForCanary indicates whether to
+                      create extra workload for canary True corresponds to RollingStyle
+                      "Canary". False corresponds to RollingStyle "Partiton". Ignored
+                      in BlueGreen-style. This field is about to deprecate, use RollingStyle
+                      instead. If both of them are set, controller will only consider
+                      this filed when RollingStyle is empty
+                    type: boolean
                   failureThreshold:
                     anyOf:
                     - type: integer
@@ -124,6 +132,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 +358,14 @@ spec:
                       - canaryReplicas
                       type: object
                     type: array
+                  enableExtraWorkloadForCanary:
+                    description: EnableExtraWorkloadForCanary indicates whether to
+                      create extra workload for canary True corresponds to RollingStyle
+                      "Canary". False corresponds to RollingStyle "Partiton". Ignored
+                      in BlueGreen-style. This field is about to deprecate, use RollingStyle
+                      instead. If both of them are set, controller will only consider
+                      this filed when RollingStyle is empty
+                    type: boolean
                   failureThreshold:
                     anyOf:
                     - type: integer
@@ -384,6 +402,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

diff --git a/config/crd/bases/rollouts.kruise.io_rollouts.yaml b/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:

diff --git a/pkg/controller/batchrelease/batchrelease_executor.go b/pkg/controller/batchrelease/batchrelease_executor.go
@@ -197,8 +197,10 @@ func (r *Executor) getReleaseController(release *v1beta1.BatchRelease, newStatus
 		Namespace: release.Namespace,
 		Name:      targetRef.Name,
 	}
-
 	rollingStyle := release.Spec.ReleasePlan.RollingStyle
+	if len(rollingStyle) == 0 && release.Spec.ReleasePlan.EnableExtraWorkloadForCanary {
+		rollingStyle = v1beta1.CanaryRollingStyle
+	}
 	klog.Infof("BatchRelease(%v) using %s-style release controller for this batch release", klog.KObj(release), rollingStyle)
 	switch rollingStyle {
 	case v1beta1.BlueGreenRollingStyle:

diff --git a/pkg/controller/rollout/rollout_canary.go b/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,
 			},
 		},
 	}

diff --git a/pkg/controller/rollout/rollout_canary_test.go b/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,6 +208,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
 			},

diff --git a/pkg/controller/rollout/rollout_progressing.go b/pkg/controller/rollout/rollout_progressing.go
@@ -68,32 +68,25 @@ func (r *RolloutReconciler) reconcileRolloutProgressing(rollout *v1beta1.Rollout
 		klog.Infof("rollout(%s/%s) is Progressing, and in reason(%s)", rollout.Namespace, rollout.Name, cond.Reason)
 		// clear and create
 		newStatus.Clear()
+		commonStatus := v1beta1.CommonStatus{
+			ObservedWorkloadGeneration: rolloutContext.Workload.Generation,
+			RolloutHash:                rolloutContext.Rollout.Annotations[util.RolloutHashAnnotation],
+			ObservedRolloutID:          getRolloutID(rolloutContext.Workload),
+			StableRevision:             rolloutContext.Workload.StableRevision,
+			CurrentStepIndex:           1,
+			NextStepIndex:              util.NextBatchIndex(rollout, 1),
+			CurrentStepState:           v1beta1.CanaryStepStateInit,
+			LastUpdateTime:             &metav1.Time{Time: time.Now()},
+		}
 		if rollout.Spec.Strategy.IsBlueGreenRelease() {
 			newStatus.BlueGreenStatus = &v1beta1.BlueGreenStatus{
-				CommonStatus: v1beta1.CommonStatus{
-					ObservedWorkloadGeneration: rolloutContext.Workload.Generation,
-					RolloutHash:                rolloutContext.Rollout.Annotations[util.RolloutHashAnnotation],
-					ObservedRolloutID:          getRolloutID(rolloutContext.Workload),
-					StableRevision:             rolloutContext.Workload.StableRevision,
-					CurrentStepIndex:           1,
-					NextStepIndex:              util.NextBatchIndex(rollout, 1),
-					CurrentStepState:           v1beta1.CanaryStepStateInit,
-					LastUpdateTime:             &metav1.Time{Time: time.Now()},
-				},
+				CommonStatus:    commonStatus,
 				UpdatedRevision: rolloutContext.Workload.CanaryRevision,
 			}
 		} else {
+			commonStatus.CurrentStepState = v1beta1.CanaryStepStateUpgrade
 			newStatus.CanaryStatus = &v1beta1.CanaryStatus{
-				CommonStatus: v1beta1.CommonStatus{
-					ObservedWorkloadGeneration: rolloutContext.Workload.Generation,
-					RolloutHash:                rolloutContext.Rollout.Annotations[util.RolloutHashAnnotation],
-					ObservedRolloutID:          getRolloutID(rolloutContext.Workload),
-					StableRevision:             rolloutContext.Workload.StableRevision,
-					CurrentStepIndex:           1,
-					NextStepIndex:              util.NextBatchIndex(rollout, 1),
-					CurrentStepState:           v1beta1.CanaryStepStateUpgrade,
-					LastUpdateTime:             &metav1.Time{Time: time.Now()},
-				},
+				CommonStatus:   commonStatus,
 				CanaryRevision: rolloutContext.Workload.CanaryRevision,
 			}
 		}