diff --git a/lua_configuration/convert_test_case_to_lua_object.go b/lua_configuration/convert_test_case_to_lua_object.go index 686bea18..d97e959a 100644 --- a/lua_configuration/convert_test_case_to_lua_object.go +++ b/lua_configuration/convert_test_case_to_lua_object.go @@ -81,7 +81,7 @@ func objectToTable(path string) error { rollout := testCase.Rollout trafficRouting := testCase.TrafficRouting if rollout != nil { - steps := rollout.Spec.Strategy.Canary.Steps + steps := rollout.Spec.Strategy.GetSteps() for i, step := range steps { var weight *int32 if step.TrafficRoutingStrategy.Traffic != nil { @@ -92,7 +92,7 @@ func objectToTable(path string) error { weight = utilpointer.Int32(-1) } var canaryService string - stableService := rollout.Spec.Strategy.Canary.TrafficRoutings[0].Service + stableService := rollout.Spec.Strategy.GetTrafficRouting()[0].Service canaryService = fmt.Sprintf("%s-canary", stableService) data := &custom.LuaData{ Data: custom.Data{ diff --git a/pkg/controller/batchrelease/context/context.go b/pkg/controller/batchrelease/context/context.go index 5a801249..2c428180 100644 --- a/pkg/controller/batchrelease/context/context.go +++ b/pkg/controller/batchrelease/context/context.go @@ -73,20 +73,20 @@ func (bc *BatchContext) Log() string { // IsBatchReady return nil if the batch is ready func (bc *BatchContext) IsBatchReady() error { if bc.UpdatedReplicas < bc.DesiredUpdatedReplicas { - return fmt.Errorf("current batch not ready: updated replicas not satified") + return fmt.Errorf("current batch not ready: updated replicas not satisfied, UpdatedReplicas %d < DesiredUpdatedReplicas %d", bc.UpdatedReplicas, bc.DesiredUpdatedReplicas) } unavailableToleration := allowedUnavailable(bc.FailureThreshold, bc.UpdatedReplicas) if unavailableToleration+bc.UpdatedReadyReplicas < bc.DesiredUpdatedReplicas { - return fmt.Errorf("current batch not ready: updated ready replicas not satified") + return fmt.Errorf("current batch not ready: updated ready replicas not satisfied, allowedUnavailable + UpdatedReadyReplicas %d < DesiredUpdatedReplicas %d", unavailableToleration+bc.UpdatedReadyReplicas, bc.DesiredUpdatedReplicas) } if bc.DesiredUpdatedReplicas > 0 && bc.UpdatedReadyReplicas == 0 { - return fmt.Errorf("current batch not ready: no updated ready replicas") + return fmt.Errorf("current batch not ready: no updated ready replicas, DesiredUpdatedReplicas %d > 0 and UpdatedReadyReplicas %d = 0", bc.DesiredUpdatedReplicas, bc.UpdatedReadyReplicas) } if !batchLabelSatisfied(bc.Pods, bc.RolloutID, bc.PlannedUpdatedReplicas) { - return fmt.Errorf("current batch not ready: pods with batch label not satified") + return fmt.Errorf("current batch not ready: pods with batch label not satisfied, RolloutID %s, PlannedUpdatedReplicas %d", bc.RolloutID, bc.PlannedUpdatedReplicas) } return nil } diff --git a/pkg/controller/rollout/rollout_canary.go b/pkg/controller/rollout/rollout_canary.go index 206bcbff..c69ccb96 100644 --- a/pkg/controller/rollout/rollout_canary.go +++ b/pkg/controller/rollout/rollout_canary.go @@ -87,6 +87,13 @@ func (m *canaryReleaseManager) runCanary(c *RolloutContext) error { } } switch canaryStatus.CurrentStepState { + // before CanaryStepStateUpgrade, handle some special cases, to prevent traffic loss + case v1beta1.CanaryStepStateInit: + // placeholder for the later traffic modification Pull Request + canaryStatus.NextStepIndex = util.NextBatchIndex(c.Rollout, canaryStatus.CurrentStepIndex) + canaryStatus.CurrentStepState = v1beta1.CanaryStepStateUpgrade + fallthrough + case v1beta1.CanaryStepStateUpgrade: klog.Infof("rollout(%s/%s) run canary strategy, and state(%s)", c.Rollout.Namespace, c.Rollout.Name, v1beta1.CanaryStepStateUpgrade) done, err := m.doCanaryUpgrade(c) @@ -144,7 +151,8 @@ func (m *canaryReleaseManager) runCanary(c *RolloutContext) error { if len(c.Rollout.Spec.Strategy.Canary.Steps) > int(canaryStatus.CurrentStepIndex) { canaryStatus.LastUpdateTime = &metav1.Time{Time: time.Now()} canaryStatus.CurrentStepIndex++ - canaryStatus.CurrentStepState = v1beta1.CanaryStepStateUpgrade + canaryStatus.NextStepIndex = util.NextBatchIndex(c.Rollout, canaryStatus.CurrentStepIndex) + canaryStatus.CurrentStepState = v1beta1.CanaryStepStateInit klog.Infof("rollout(%s/%s) canary step from(%d) -> to(%d)", c.Rollout.Namespace, c.Rollout.Name, canaryStatus.CurrentStepIndex-1, canaryStatus.CurrentStepIndex) } else { klog.Infof("rollout(%s/%s) canary run all steps, and completed", c.Rollout.Namespace, c.Rollout.Name) @@ -201,15 +209,12 @@ func (m *canaryReleaseManager) doCanaryMetricsAnalysis(c *RolloutContext) (bool, } func (m *canaryReleaseManager) doCanaryPaused(c *RolloutContext) (bool, error) { + if m.doCanaryJump(c) { + return false, nil + } canaryStatus := c.NewStatus.CanaryStatus currentStep := c.Rollout.Spec.Strategy.Canary.Steps[canaryStatus.CurrentStepIndex-1] steps := len(c.Rollout.Spec.Strategy.Canary.Steps) - // If it is the last step, and 100% of pods, then return true - if int32(steps) == canaryStatus.CurrentStepIndex { - if currentStep.Replicas != nil && currentStep.Replicas.StrVal == "100%" { - return true, nil - } - } cond := util.GetRolloutCondition(*c.NewStatus, v1beta1.RolloutConditionProgressing) // need manual confirmation if currentStep.Pause.Duration == nil { @@ -232,6 +237,46 @@ func (m *canaryReleaseManager) doCanaryPaused(c *RolloutContext) (bool, error) { return false, nil } +func (m *canaryReleaseManager) doCanaryJump(c *RolloutContext) (jumped bool) { + canaryStatus := c.NewStatus.CanaryStatus + currentStep := c.Rollout.Spec.Strategy.Canary.Steps[canaryStatus.CurrentStepIndex-1] + nextIndex := canaryStatus.NextStepIndex + if nextIndex != util.NextBatchIndex(c.Rollout, canaryStatus.CurrentStepIndex) && nextIndex != 0 || nextIndex < 0 { + /* + * The NextStepIndex should typically be assigned a non-negative integer. + * However, assigning it a negative value acts as a sentinel to trigger an APPROVE. + * For instance, if currentStepIndex is 2 and nextStepIndex is 3, + * directly jumping to step 3 by patching nextStepIndex should not be possible since the value remains unchanged. + * By permitting the assignment of a negative value, such as -1, we can jump to step 3, which essentially + * means to approve current batch. + * Might be useful someday in the future. + */ + if nextIndex < 0 { + nextIndex = util.NextBatchIndex(c.Rollout, canaryStatus.CurrentStepIndex) + } + currentIndexBackup := canaryStatus.CurrentStepIndex + canaryStatus.CurrentStepIndex = nextIndex + canaryStatus.NextStepIndex = util.NextBatchIndex(c.Rollout, nextIndex) + nextStep := c.Rollout.Spec.Strategy.Canary.Steps[nextIndex-1] + // if the Replicas between currentStep and nextStep is same, we can jump to + // the TrafficRouting step; otherwise, we should start from the Init step + if reflect.DeepEqual(nextStep.Replicas, currentStep.Replicas) { + canaryStatus.LastUpdateTime = &metav1.Time{Time: time.Now()} + canaryStatus.CurrentStepState = v1beta1.CanaryStepStateTrafficRouting + klog.Infof("rollout(%s/%s) step(%d) state from(%s) -> to(%s)", c.Rollout.Namespace, c.Rollout.Name, + canaryStatus.CurrentStepIndex, v1beta1.CanaryStepStatePaused, canaryStatus.CurrentStepState) + } else { + canaryStatus.LastUpdateTime = &metav1.Time{Time: time.Now()} + canaryStatus.CurrentStepState = v1beta1.CanaryStepStateInit + klog.Infof("rollout(%s/%s) step(%d) state from(%s) -> to(%s)", c.Rollout.Namespace, c.Rollout.Name, + canaryStatus.CurrentStepIndex, v1beta1.CanaryStepStatePaused, v1beta1.CanaryStepStateInit) + } + klog.Infof("rollout(%s/%s) canary step from(%d) -> to(%d)", c.Rollout.Namespace, c.Rollout.Name, currentIndexBackup, canaryStatus.CurrentStepIndex) + return true + } + return false +} + // cleanup after rollout is completed or finished func (m *canaryReleaseManager) doCanaryFinalising(c *RolloutContext) (bool, error) { // when CanaryStatus is nil, which means canary action hasn't started yet, don't need doing cleanup diff --git a/pkg/controller/rollout/rollout_progressing.go b/pkg/controller/rollout/rollout_progressing.go index 42b7effc..50f825b4 100644 --- a/pkg/controller/rollout/rollout_progressing.go +++ b/pkg/controller/rollout/rollout_progressing.go @@ -91,7 +91,7 @@ func (r *RolloutReconciler) reconcileRolloutProgressing(rollout *v1beta1.Rollout StableRevision: rolloutContext.Workload.StableRevision, CurrentStepIndex: 1, NextStepIndex: util.NextBatchIndex(rollout, 1), - CurrentStepState: v1beta1.CanaryStepStateUpgrade, + CurrentStepState: v1beta1.CanaryStepStateInit, LastUpdateTime: &metav1.Time{Time: time.Now()}, }, CanaryRevision: rolloutContext.Workload.CanaryRevision, @@ -171,7 +171,7 @@ func (r *RolloutReconciler) reconcileRolloutProgressing(rollout *v1beta1.Rollout func (r *RolloutReconciler) doProgressingInitializing(c *RolloutContext) (bool, error) { // Traffic routing - if len(c.Rollout.Spec.Strategy.Canary.TrafficRoutings) > 0 { + if c.Rollout.Spec.Strategy.HasTrafficRoutings() { if err := r.trafficRoutingManager.InitializeTrafficRouting(newTrafficRoutingContext(c)); err != nil { return false, err } @@ -230,14 +230,15 @@ func (r *RolloutReconciler) handleRolloutPaused(rollout *v1beta1.Rollout, newSta func (r *RolloutReconciler) handleContinuousRelease(c *RolloutContext) error { r.Recorder.Eventf(c.Rollout, corev1.EventTypeNormal, "Progressing", "workload continuous publishing canaryRevision, then restart publishing") klog.Infof("rollout(%s/%s) workload continuous publishing canaryRevision from(%s) -> to(%s), then restart publishing", - c.Rollout.Namespace, c.Rollout.Name, c.NewStatus.CanaryStatus.CanaryRevision, c.Workload.CanaryRevision) + c.Rollout.Namespace, c.Rollout.Name, c.NewStatus.GetCanaryRevision(), c.Workload.CanaryRevision) done, err := r.doProgressingReset(c) if err != nil { klog.Errorf("rollout(%s/%s) doProgressingReset failed: %s", c.Rollout.Namespace, c.Rollout.Name, err.Error()) return err } else if done { - c.NewStatus.CanaryStatus = nil + // clear SubStatus + c.NewStatus.Clear() progressingStateTransition(c.NewStatus, corev1.ConditionTrue, v1alpha1.ProgressingReasonInitializing, "Workload is continuous release") klog.Infof("rollout(%s/%s) workload is continuous publishing, reset complete", c.Rollout.Namespace, c.Rollout.Name) } else { @@ -250,7 +251,7 @@ func (r *RolloutReconciler) handleContinuousRelease(c *RolloutContext) error { } func (r *RolloutReconciler) handleRollbackDirectly(rollout *v1beta1.Rollout, workload *util.Workload, newStatus *v1beta1.RolloutStatus) error { - newStatus.CanaryStatus.CanaryRevision = workload.CanaryRevision + newStatus.SetCanaryRevision(workload.CanaryRevision) r.Recorder.Eventf(rollout, corev1.EventTypeNormal, "Progressing", "workload has been rollback, then rollout is canceled") klog.Infof("rollout(%s/%s) workload has been rollback directly, then rollout canceled", rollout.Namespace, rollout.Name) progressingStateTransition(newStatus, corev1.ConditionTrue, v1alpha1.ProgressingReasonCancelling, "The workload has been rolled back and the rollout process will be cancelled") @@ -259,11 +260,12 @@ func (r *RolloutReconciler) handleRollbackDirectly(rollout *v1beta1.Rollout, wor func (r *RolloutReconciler) handleRollbackInBatches(rollout *v1beta1.Rollout, workload *util.Workload, newStatus *v1beta1.RolloutStatus) error { // restart from the beginning - newStatus.CanaryStatus.CurrentStepIndex = 1 - newStatus.CanaryStatus.CanaryRevision = workload.CanaryRevision - newStatus.CanaryStatus.CurrentStepState = v1beta1.CanaryStepStateUpgrade - newStatus.CanaryStatus.LastUpdateTime = &metav1.Time{Time: time.Now()} - newStatus.CanaryStatus.RolloutHash = rollout.Annotations[util.RolloutHashAnnotation] + newStatus.GetSubStatus().CurrentStepIndex = 1 + newStatus.GetSubStatus().NextStepIndex = util.NextBatchIndex(rollout, 1) + newStatus.SetCanaryRevision(workload.CanaryRevision) + newStatus.GetSubStatus().CurrentStepState = v1beta1.CanaryStepStateInit + newStatus.GetSubStatus().LastUpdateTime = &metav1.Time{Time: time.Now()} + newStatus.GetSubStatus().RolloutHash = rollout.Annotations[util.RolloutHashAnnotation] klog.Infof("rollout(%s/%s) workload has been rollback in batches, then restart from beginning", rollout.Namespace, rollout.Name) return nil } @@ -275,23 +277,37 @@ func (r *RolloutReconciler) handleRolloutPlanChanged(c *RolloutContext) error { return err } // canary step configuration change causes current step index change - c.NewStatus.CanaryStatus.CurrentStepIndex = newStepIndex - c.NewStatus.CanaryStatus.CurrentStepState = v1beta1.CanaryStepStateUpgrade - c.NewStatus.CanaryStatus.LastUpdateTime = &metav1.Time{Time: time.Now()} - c.NewStatus.CanaryStatus.RolloutHash = c.Rollout.Annotations[util.RolloutHashAnnotation] - klog.Infof("rollout(%s/%s) canary step configuration change, and stepIndex(%d) state(%s)", - c.Rollout.Namespace, c.Rollout.Name, c.NewStatus.CanaryStatus.CurrentStepIndex, c.NewStatus.CanaryStatus.CurrentStepState) + // if not changed, we should use a special value to notify jump + if c.NewStatus.GetSubStatus().NextStepIndex == newStepIndex { + c.NewStatus.GetSubStatus().NextStepIndex = -1 + } else { + c.NewStatus.GetSubStatus().NextStepIndex = newStepIndex + } + // directly jump to step paused to process jump + c.NewStatus.GetSubStatus().CurrentStepState = v1beta1.CanaryStepStatePaused + c.NewStatus.GetSubStatus().LastUpdateTime = &metav1.Time{Time: time.Now()} + c.NewStatus.GetSubStatus().RolloutHash = c.Rollout.Annotations[util.RolloutHashAnnotation] + klog.Infof("rollout(%s/%s) canary step configuration change, and NextStepIndex(%d) state(%s)", + c.Rollout.Namespace, c.Rollout.Name, c.NewStatus.GetSubStatus().NextStepIndex, c.NewStatus.GetSubStatus().CurrentStepState) return nil } func (r *RolloutReconciler) handleNormalRolling(c *RolloutContext) error { // check if canary is done - if c.NewStatus.CanaryStatus.CurrentStepState == v1beta1.CanaryStepStateCompleted { + if c.NewStatus.GetSubStatus().CurrentStepState == v1beta1.CanaryStepStateCompleted { klog.Infof("rollout(%s/%s) progressing rolling done", c.Rollout.Namespace, c.Rollout.Name) progressingStateTransition(c.NewStatus, corev1.ConditionTrue, v1alpha1.ProgressingReasonFinalising, "Rollout has been completed and some closing work is being done") return nil } - return r.canaryManager.runCanary(c) + // modifying NextStepIndex to 0 is not allowed + if c.NewStatus.GetSubStatus().NextStepIndex == 0 { + c.NewStatus.GetSubStatus().NextStepIndex = util.NextBatchIndex(c.Rollout, c.Rollout.Status.GetSubStatus().CurrentStepIndex) + } + releaseManager, err := r.getReleaseManager(c.Rollout) + if err != nil { + return err + } + return releaseManager.runCanary(c) } // name is rollout name, tr is trafficRouting name @@ -351,30 +367,41 @@ func (r *RolloutReconciler) finalizeTrafficRouting(namespace, name, tr string) e *********************************************************************** */ + +func (r *RolloutReconciler) getReleaseManager(rollout *v1beta1.Rollout) (ReleaseManager, error) { + if rollout.Spec.Strategy.IsCanaryStragegy() { + return r.canaryManager, nil + } else if rollout.Spec.Strategy.IsBlueGreenRelease() { + // placeholder for upcoming PR + // return r.blueGreenManager, nil + } + return nil, fmt.Errorf("unknown rolling style: %s, and thus cannot call corresponding release manager", rollout.Spec.Strategy.GetRollingStyle()) +} + func isRolloutPaused(rollout *v1beta1.Rollout) bool { return rollout.Spec.Strategy.Paused } func isRolloutPlanChanged(rollout *v1beta1.Rollout) bool { status := &rollout.Status - return status.CanaryStatus.RolloutHash != "" && status.CanaryStatus.RolloutHash != rollout.Annotations[util.RolloutHashAnnotation] + return status.GetSubStatus().RolloutHash != "" && status.GetSubStatus().RolloutHash != rollout.Annotations[util.RolloutHashAnnotation] } func isContinuousRelease(rollout *v1beta1.Rollout, workload *util.Workload) bool { status := &rollout.Status - return status.CanaryStatus.CanaryRevision != "" && workload.CanaryRevision != status.CanaryStatus.CanaryRevision && !workload.IsInRollback + return status.GetCanaryRevision() != "" && workload.CanaryRevision != status.GetCanaryRevision() && !workload.IsInRollback } func isRollingBackDirectly(rollout *v1beta1.Rollout, workload *util.Workload) bool { status := &rollout.Status inBatch := util.IsRollbackInBatchPolicy(rollout, workload.Labels) - return workload.IsInRollback && workload.CanaryRevision != status.CanaryStatus.CanaryRevision && !inBatch + return workload.IsInRollback && workload.CanaryRevision != status.GetCanaryRevision() && !inBatch } func isRollingBackInBatches(rollout *v1beta1.Rollout, workload *util.Workload) bool { status := &rollout.Status inBatch := util.IsRollbackInBatchPolicy(rollout, workload.Labels) - return workload.IsInRollback && workload.CanaryRevision != status.CanaryStatus.CanaryRevision && inBatch + return workload.IsInRollback && workload.CanaryRevision != status.GetCanaryRevision() && inBatch } // 1. modify network api(ingress or gateway api) configuration, and route 100% traffic to stable pods @@ -400,16 +427,35 @@ func (r *RolloutReconciler) doProgressingReset(c *RolloutContext) (bool, error) } func (r *RolloutReconciler) recalculateCanaryStep(c *RolloutContext) (int32, error) { - batch, err := r.canaryManager.fetchBatchRelease(c.Rollout.Namespace, c.Rollout.Name) + releaseManager, err := r.getReleaseManager(c.Rollout) + if err != nil { + return 0, err + } + batch, err := releaseManager.fetchBatchRelease(c.Rollout.Namespace, c.Rollout.Name) if errors.IsNotFound(err) { return 1, nil } else if err != nil { return 0, err } currentReplicas, _ := intstr.GetScaledValueFromIntOrPercent(&batch.Spec.ReleasePlan.Batches[*batch.Spec.ReleasePlan.BatchPartition].CanaryReplicas, int(c.Workload.Replicas), true) - var stepIndex int32 - for i := range c.Rollout.Spec.Strategy.Canary.Steps { - step := c.Rollout.Spec.Strategy.Canary.Steps[i] + var stepIndex, currentIndex int32 + if c.NewStatus != nil { + currentIndex = c.NewStatus.GetSubStatus().CurrentStepIndex - 1 + } + steps := append([]int{}, int(currentIndex)) + // we don't distinguish between the changes in Replicas and Traffic + // Whatever the change is, we recalculate the step. + // we put the current step index first for retrieval, so that if Traffic is the only change, + // usually we will get the target step index same as current step index + for i := 0; i < len(c.Rollout.Spec.Strategy.GetSteps()); i++ { + if i == int(currentIndex) { + continue + } + steps = append(steps, i) + } + + for _, i := range steps { + step := c.Rollout.Spec.Strategy.GetSteps()[i] var desiredReplicas int desiredReplicas, _ = intstr.GetScaledValueFromIntOrPercent(step.Replicas, int(c.Workload.Replicas), true) stepIndex = int32(i + 1) @@ -417,6 +463,7 @@ func (r *RolloutReconciler) recalculateCanaryStep(c *RolloutContext) (int32, err break } } + klog.Infof("RolloutPlan Change detected, rollout(%s/%s) currentStepIndex %d, jumps to %d", c.Rollout.Namespace, c.Rollout.Name, currentIndex+1, stepIndex) return stepIndex, nil } @@ -429,7 +476,11 @@ func (r *RolloutReconciler) doFinalising(c *RolloutContext) (bool, error) { return false, err } } - done, err := r.canaryManager.doCanaryFinalising(c) + releaseManager, err := r.getReleaseManager(c.Rollout) + if err != nil { + return false, err + } + done, err := releaseManager.doCanaryFinalising(c) if err != nil { klog.Errorf("rollout(%s/%s) Progressing failed: %s", c.Rollout.Namespace, c.Rollout.Name, err.Error()) return false, err @@ -467,7 +518,7 @@ func setRolloutSucceededCondition(status *v1beta1.RolloutStatus, condStatus core } func newTrafficRoutingContext(c *RolloutContext) *trafficrouting.TrafficRoutingContext { - currentStep := c.Rollout.Spec.Strategy.Canary.Steps[c.NewStatus.CanaryStatus.CurrentStepIndex-1] + currentStep := c.Rollout.Spec.Strategy.GetSteps()[c.NewStatus.GetSubStatus().CurrentStepIndex-1] var revisionLabelKey string if c.Workload != nil { revisionLabelKey = c.Workload.RevisionLabelKey @@ -475,13 +526,13 @@ func newTrafficRoutingContext(c *RolloutContext) *trafficrouting.TrafficRoutingC return &trafficrouting.TrafficRoutingContext{ Key: fmt.Sprintf("Rollout(%s/%s)", c.Rollout.Namespace, c.Rollout.Name), Namespace: c.Rollout.Namespace, - ObjectRef: c.Rollout.Spec.Strategy.Canary.TrafficRoutings, + ObjectRef: c.Rollout.Spec.Strategy.GetTrafficRouting(), Strategy: currentStep.TrafficRoutingStrategy, OwnerRef: *metav1.NewControllerRef(c.Rollout, rolloutControllerKind), RevisionLabelKey: revisionLabelKey, - StableRevision: c.NewStatus.CanaryStatus.StableRevision, - CanaryRevision: c.NewStatus.CanaryStatus.PodTemplateHash, - LastUpdateTime: c.NewStatus.CanaryStatus.LastUpdateTime, - DisableGenerateCanaryService: c.Rollout.Spec.Strategy.Canary.DisableGenerateCanaryService, + StableRevision: c.NewStatus.GetSubStatus().StableRevision, + CanaryRevision: c.NewStatus.GetSubStatus().PodTemplateHash, + LastUpdateTime: c.NewStatus.GetSubStatus().LastUpdateTime, + DisableGenerateCanaryService: c.Rollout.Spec.Strategy.DisableGenerateCanaryService(), } } diff --git a/pkg/controller/rollout/rollout_progressing_test.go b/pkg/controller/rollout/rollout_progressing_test.go index 696f5dbd..6513faff 100644 --- a/pkg/controller/rollout/rollout_progressing_test.go +++ b/pkg/controller/rollout/rollout_progressing_test.go @@ -67,12 +67,11 @@ func TestReconcileRolloutProgressing(t *testing.T) { s.CanaryStatus.StableRevision = "pod-template-hash-v1" s.CanaryStatus.CanaryRevision = "6f8cc56547" s.CanaryStatus.CurrentStepIndex = 1 - // s.CanaryStatus.NextStepIndex will be initialized as 0 in ReconcileRolloutProgressing + // s.CanaryStatus.NextStepIndex will be initialized as 0 in ReconcileRolloutProgressing. // util.NextBatchIndex(rollout, s.CanaryStatus.CurrentStepIndex), which is 2 here. - // However the ReconcileRolloutProgressing won't update it, and thus expected value of it - // in the next cases will be 0 (zero in zero out), without update. This is as expected s.CanaryStatus.NextStepIndex = 2 - s.CanaryStatus.CurrentStepState = v1beta1.CanaryStepStateUpgrade + // now the first step is no longer StepStateUpgrade, it is StepStateInit now + s.CanaryStatus.CurrentStepState = v1beta1.CanaryStepStateInit return s }, expectTr: func() *v1alpha1.TrafficRouting { @@ -105,7 +104,7 @@ func TestReconcileRolloutProgressing(t *testing.T) { s.CanaryStatus.CanaryRevision = "6f8cc56547" s.CanaryStatus.CurrentStepIndex = 1 s.CanaryStatus.NextStepIndex = 2 - s.CanaryStatus.CurrentStepState = v1beta1.CanaryStepStateUpgrade + s.CanaryStatus.CurrentStepState = v1beta1.CanaryStepStateInit cond := util.GetRolloutCondition(*s, v1beta1.RolloutConditionProgressing) cond.Reason = v1alpha1.ProgressingReasonInRolling util.SetRolloutCondition(s, *cond) @@ -146,6 +145,7 @@ func TestReconcileRolloutProgressing(t *testing.T) { obj.Status.CanaryStatus.StableRevision = "pod-template-hash-v1" obj.Status.CanaryStatus.CanaryRevision = "6f8cc56547" obj.Status.CanaryStatus.CurrentStepIndex = 1 + obj.Status.CanaryStatus.NextStepIndex = 2 obj.Status.CanaryStatus.CurrentStepState = v1beta1.CanaryStepStateUpgrade cond := util.GetRolloutCondition(obj.Status, v1beta1.RolloutConditionProgressing) cond.Reason = v1alpha1.ProgressingReasonInRolling @@ -160,6 +160,7 @@ func TestReconcileRolloutProgressing(t *testing.T) { s.CanaryStatus.CanaryRevision = "6f8cc56547" s.CanaryStatus.PodTemplateHash = "pod-template-hash-v2" s.CanaryStatus.CurrentStepIndex = 1 + s.CanaryStatus.NextStepIndex = 2 s.CanaryStatus.CurrentStepState = v1beta1.CanaryStepStateUpgrade cond := util.GetRolloutCondition(*s, v1beta1.RolloutConditionProgressing) cond.Reason = v1alpha1.ProgressingReasonInRolling @@ -216,6 +217,7 @@ func TestReconcileRolloutProgressing(t *testing.T) { s.CanaryStatus.CanaryRevision = "6f8cc56547" s.CanaryStatus.PodTemplateHash = "pod-template-hash-v2" s.CanaryStatus.CurrentStepIndex = 4 + s.CanaryStatus.NextStepIndex = 0 s.CanaryStatus.CurrentStepState = v1beta1.CanaryStepStateCompleted cond := util.GetRolloutCondition(*s, v1beta1.RolloutConditionProgressing) cond.Reason = v1alpha1.ProgressingReasonFinalising @@ -274,6 +276,7 @@ func TestReconcileRolloutProgressing(t *testing.T) { s.CanaryStatus.CanaryRevision = "6f8cc56547" s.CanaryStatus.PodTemplateHash = "pod-template-hash-v2" s.CanaryStatus.CurrentStepIndex = 4 + s.CanaryStatus.NextStepIndex = 0 s.CanaryStatus.CurrentStepState = v1beta1.CanaryStepStateCompleted cond := util.GetRolloutCondition(*s, v1beta1.RolloutConditionProgressing) cond.Reason = v1alpha1.ProgressingReasonFinalising @@ -334,6 +337,7 @@ func TestReconcileRolloutProgressing(t *testing.T) { s.CanaryStatus.CanaryRevision = "6f8cc56547" s.CanaryStatus.PodTemplateHash = "pod-template-hash-v2" s.CanaryStatus.CurrentStepIndex = 4 + s.CanaryStatus.NextStepIndex = 0 s.CanaryStatus.CurrentStepState = v1beta1.CanaryStepStateCompleted cond2 := util.GetRolloutCondition(*s, v1beta1.RolloutConditionProgressing) cond2.Reason = v1alpha1.ProgressingReasonFinalising @@ -383,6 +387,7 @@ func TestReconcileRolloutProgressing(t *testing.T) { s.CanaryStatus.CanaryRevision = "6f8cc56547" s.CanaryStatus.PodTemplateHash = "pod-template-hash-v2" s.CanaryStatus.CurrentStepIndex = 4 + s.CanaryStatus.NextStepIndex = 0 s.CanaryStatus.CurrentStepState = v1beta1.CanaryStepStateCompleted cond2 := util.GetRolloutCondition(*s, v1beta1.RolloutConditionProgressing) cond2.Reason = v1alpha1.ProgressingReasonCompleted diff --git a/pkg/controller/rollout/rollout_releaseManager.go b/pkg/controller/rollout/rollout_releaseManager.go new file mode 100644 index 00000000..25cb199b --- /dev/null +++ b/pkg/controller/rollout/rollout_releaseManager.go @@ -0,0 +1,12 @@ +package rollout + +import ( + "github.com/openkruise/rollouts/api/v1beta1" +) + +type ReleaseManager interface { + runCanary(c *RolloutContext) error + doCanaryFinalising(c *RolloutContext) (bool, error) + fetchBatchRelease(ns, name string) (*v1beta1.BatchRelease, error) + removeBatchRelease(c *RolloutContext) (bool, error) +} diff --git a/pkg/controller/rollout/rollout_status.go b/pkg/controller/rollout/rollout_status.go index 015bca14..a3629413 100755 --- a/pkg/controller/rollout/rollout_status.go +++ b/pkg/controller/rollout/rollout_status.go @@ -91,10 +91,10 @@ func (r *RolloutReconciler) calculateRolloutStatus(rollout *v1beta1.Rollout) (re // update workload generation to canaryStatus.ObservedWorkloadGeneration // rollout is a target ref bypass, so there needs to be a field to identify the rollout execution process or results, // which version of deployment is targeted, ObservedWorkloadGeneration that is to compare with the workload generation - if newStatus.CanaryStatus != nil && newStatus.CanaryStatus.CanaryRevision != "" && - newStatus.CanaryStatus.CanaryRevision == workload.CanaryRevision { - newStatus.CanaryStatus.ObservedRolloutID = getRolloutID(workload) - newStatus.CanaryStatus.ObservedWorkloadGeneration = workload.Generation + if !newStatus.IsSubStatusEmpty() && newStatus.GetCanaryRevision() != "" && + newStatus.GetCanaryRevision() == workload.CanaryRevision { + newStatus.GetSubStatus().ObservedRolloutID = getRolloutID(workload) + newStatus.GetSubStatus().ObservedWorkloadGeneration = workload.Generation } switch newStatus.Phase { @@ -110,7 +110,7 @@ func (r *RolloutReconciler) calculateRolloutStatus(rollout *v1beta1.Rollout) (re cond := util.NewRolloutCondition(v1beta1.RolloutConditionProgressing, corev1.ConditionTrue, v1alpha1.ProgressingReasonInitializing, "Rollout is in Progressing") util.SetRolloutCondition(newStatus, *cond) util.RemoveRolloutCondition(newStatus, v1beta1.RolloutConditionSucceeded) - } else if newStatus.CanaryStatus == nil { + } else if newStatus.IsSubStatusEmpty() { // The following logic is to make PaaS be able to judge whether the rollout is ready // at the first deployment of the Rollout/Workload. For example: generally, a PaaS // platform can use the following code to judge whether the rollout progression is completed: @@ -167,15 +167,30 @@ func (r *RolloutReconciler) calculateRolloutStatus(rollout *v1beta1.Rollout) (re // rolloutHash mainly records the step batch information, when the user step changes, // the current batch can be recalculated func (r *RolloutReconciler) calculateRolloutHash(rollout *v1beta1.Rollout) error { - canary := rollout.Spec.Strategy.Canary.DeepCopy() - canary.FailureThreshold = nil - canary.Steps = nil - for i := range rollout.Spec.Strategy.Canary.Steps { - step := rollout.Spec.Strategy.Canary.Steps[i].DeepCopy() - step.Pause = v1beta1.RolloutPause{} - canary.Steps = append(canary.Steps, *step) + var data string + if rollout.Spec.Strategy.IsCanaryStragegy() { + canary := rollout.Spec.Strategy.Canary.DeepCopy() + canary.FailureThreshold = nil + canary.Steps = nil + for i := range rollout.Spec.Strategy.Canary.Steps { + step := rollout.Spec.Strategy.Canary.Steps[i].DeepCopy() + step.Pause = v1beta1.RolloutPause{} + canary.Steps = append(canary.Steps, *step) + } + data = util.DumpJSON(canary) + } else if rollout.Spec.Strategy.IsBlueGreenRelease() { + blueGreen := rollout.Spec.Strategy.BlueGreen.DeepCopy() + blueGreen.FailureThreshold = nil + blueGreen.Steps = nil + for i := range rollout.Spec.Strategy.BlueGreen.Steps { + step := rollout.Spec.Strategy.BlueGreen.Steps[i].DeepCopy() + step.Pause = v1beta1.RolloutPause{} + blueGreen.Steps = append(blueGreen.Steps, *step) + } + data = util.DumpJSON(blueGreen) + } else { + return fmt.Errorf("unknown rolling style: %s", rollout.Spec.Strategy.GetRollingStyle()) } - data := util.DumpJSON(canary) hash := rand.SafeEncodeString(util.EncodeHash(data)) if rollout.Annotations[util.RolloutHashAnnotation] == hash { return nil diff --git a/pkg/util/constant.go b/pkg/util/constant.go index 02ff7226..abfe2d98 100644 --- a/pkg/util/constant.go +++ b/pkg/util/constant.go @@ -27,7 +27,8 @@ const ( // BatchReleaseControlAnnotation is controller info about batchRelease when rollout BatchReleaseControlAnnotation = "batchrelease.rollouts.kruise.io/control-info" // InRolloutProgressingAnnotation marks workload as entering the rollout progressing process - //and does not allow paused=false during this process + // and does not allow paused=false during this process. However, blueGreen is an exception, + // which allows paused=false during progressing. InRolloutProgressingAnnotation = "rollouts.kruise.io/in-progressing" // RolloutHashAnnotation record observed rollout spec hash RolloutHashAnnotation = "rollouts.kruise.io/hash" diff --git a/pkg/util/controller_finder.go b/pkg/util/controller_finder.go index dc15ba1b..886b8191 100644 --- a/pkg/util/controller_finder.go +++ b/pkg/util/controller_finder.go @@ -87,7 +87,7 @@ func NewControllerFinder(c client.Client) *ControllerFinder { func (r *ControllerFinder) GetWorkloadForRef(rollout *rolloutv1beta1.Rollout) (*Workload, error) { workloadRef := rollout.Spec.WorkloadRef - if rollout.Spec.Strategy.Canary.EnableExtraWorkloadForCanary { + if rollout.Spec.Strategy.GetRollingStyle() == rolloutv1beta1.CanaryRollingStyle { for _, finder := range append(r.canaryStyleFinders(), r.partitionStyleFinders()...) { workload, err := finder(rollout.Namespace, &workloadRef) if workload != nil || err != nil { diff --git a/pkg/util/rollout_utils.go b/pkg/util/rollout_utils.go index 464370c7..7386ab70 100644 --- a/pkg/util/rollout_utils.go +++ b/pkg/util/rollout_utils.go @@ -47,7 +47,7 @@ type RolloutState struct { func IsRollbackInBatchPolicy(rollout *rolloutv1beta1.Rollout, labels map[string]string) bool { // currently, only support the case of no traffic routing - if len(rollout.Spec.Strategy.Canary.TrafficRoutings) > 0 { + if rollout.Spec.Strategy.HasTrafficRoutings() { return false } workloadRef := rollout.Spec.WorkloadRef