From f3b29f3f32df312b85e23083fa3d68598298701e Mon Sep 17 00:00:00 2001 From: yunbo Date: Tue, 11 Jun 2024 15:30:40 +0800 Subject: [PATCH] restore the enableExtra field in BR Signed-off-by: yunbo --- api/v1alpha1/batchrelease_plan_types.go | 3 ++ api/v1alpha1/conversion.go | 3 ++ api/v1alpha1/rollout_types.go | 10 +++-- api/v1beta1/batchrelease_plan_types.go | 3 ++ api/v1beta1/rollout_types.go | 13 +++--- .../rollouts.kruise.io_batchreleases.yaml | 14 +++++++ .../bases/rollouts.kruise.io_rollouts.yaml | 42 +++++++++++-------- pkg/controller/rollout/rollout_canary.go | 13 +++--- pkg/controller/rollout/rollout_canary_test.go | 8 +++- pkg/controller/rollout/rollout_progressing.go | 33 ++++++--------- pkg/controller/rollout/rollout_status.go | 32 ++++++-------- pkg/util/rollout_utils.go | 7 +--- 12 files changed, 103 insertions(+), 78 deletions(-) diff --git a/api/v1alpha1/batchrelease_plan_types.go b/api/v1alpha1/batchrelease_plan_types.go index 7514b3cb..55a5da76 100644 --- a/api/v1alpha1/batchrelease_plan_types.go +++ b/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 diff --git a/api/v1alpha1/conversion.go b/api/v1alpha1/conversion.go index 6872c916..28ad4a9c 100644 --- 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 index df3fa933..d67a5388 100644 --- 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 index bf1940be..8bcd7222 100644 --- a/api/v1beta1/batchrelease_plan_types.go +++ b/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 diff --git a/api/v1beta1/rollout_types.go b/api/v1beta1/rollout_types.go index 89f7a2bc..2cd5f800 100644 --- 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 index ec704f93..55b461bf 100644 --- a/config/crd/bases/rollouts.kruise.io_batchreleases.yaml +++ b/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 diff --git a/config/crd/bases/rollouts.kruise.io_rollouts.yaml b/config/crd/bases/rollouts.kruise.io_rollouts.yaml index f257c70f..845225f8 100644 --- 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/rollout/rollout_canary.go b/pkg/controller/rollout/rollout_canary.go index 206bcbff..6f623610 100644 --- 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 index 573849cf..53a5830c 100644 --- 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,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() diff --git a/pkg/controller/rollout/rollout_progressing.go b/pkg/controller/rollout/rollout_progressing.go index 42b7effc..2388eb46 100644 --- 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, } } diff --git a/pkg/controller/rollout/rollout_status.go b/pkg/controller/rollout/rollout_status.go index 015bca14..0e33522e 100755 --- a/pkg/controller/rollout/rollout_status.go +++ b/pkg/controller/rollout/rollout_status.go @@ -123,32 +123,24 @@ func (r *RolloutReconciler) calculateRolloutStatus(rollout *v1beta1.Rollout) (re // But at the first deployment of Rollout/Workload, CanaryStatus isn't set due to no rollout progression, // and PaaS platform cannot judge whether the deployment is completed base on the code above. So we have // to update the status just like the rollout was completed. + commonStatus := v1beta1.CommonStatus{ + ObservedRolloutID: getRolloutID(workload), + ObservedWorkloadGeneration: workload.Generation, + PodTemplateHash: workload.PodTemplateHash, + StableRevision: workload.StableRevision, + CurrentStepIndex: int32(len(rollout.Spec.Strategy.GetSteps())), + NextStepIndex: util.NextBatchIndex(rollout, int32(len(rollout.Spec.Strategy.GetSteps()))), + CurrentStepState: v1beta1.CanaryStepStateCompleted, + RolloutHash: rollout.Annotations[util.RolloutHashAnnotation], + } if rollout.Spec.Strategy.IsBlueGreenRelease() { newStatus.BlueGreenStatus = &v1beta1.BlueGreenStatus{ - CommonStatus: v1beta1.CommonStatus{ - ObservedRolloutID: getRolloutID(workload), - ObservedWorkloadGeneration: workload.Generation, - PodTemplateHash: workload.PodTemplateHash, - StableRevision: workload.StableRevision, - CurrentStepIndex: int32(len(rollout.Spec.Strategy.GetSteps())), - NextStepIndex: util.NextBatchIndex(rollout, int32(len(rollout.Spec.Strategy.GetSteps()))), - CurrentStepState: v1beta1.CanaryStepStateCompleted, - RolloutHash: rollout.Annotations[util.RolloutHashAnnotation], - }, + CommonStatus: commonStatus, UpdatedRevision: workload.CanaryRevision, } } else { newStatus.CanaryStatus = &v1beta1.CanaryStatus{ - CommonStatus: v1beta1.CommonStatus{ - ObservedRolloutID: getRolloutID(workload), - ObservedWorkloadGeneration: workload.Generation, - PodTemplateHash: workload.PodTemplateHash, - StableRevision: workload.StableRevision, - CurrentStepIndex: int32(len(rollout.Spec.Strategy.GetSteps())), - NextStepIndex: util.NextBatchIndex(rollout, int32(len(rollout.Spec.Strategy.GetSteps()))), - CurrentStepState: v1beta1.CanaryStepStateCompleted, - RolloutHash: rollout.Annotations[util.RolloutHashAnnotation], - }, + CommonStatus: commonStatus, CanaryRevision: workload.CanaryRevision, } } diff --git a/pkg/util/rollout_utils.go b/pkg/util/rollout_utils.go index 324bfe04..927be3a1 100644 --- a/pkg/util/rollout_utils.go +++ b/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 }