diff --git a/api/v1alpha1/rollout_types.go b/api/v1alpha1/rollout_types.go index f2f58b1c..bad45a4a 100644 --- a/api/v1alpha1/rollout_types.go +++ b/api/v1alpha1/rollout_types.go @@ -198,11 +198,6 @@ const ( ProgressingReasonCancelling = "Cancelling" ProgressingReasonPaused = "Paused" - // Disabling condition - RolloutConditionDisabling RolloutConditionType = "Disabling" - // Disabling reason - DisablingReasonFinalising = "InDisabling" - // RolloutConditionSucceeded indicates whether rollout is succeeded or failed. RolloutConditionSucceeded RolloutConditionType = "Succeeded" diff --git a/pkg/controller/rollout/rollout_event_handler.go b/pkg/controller/rollout/rollout_event_handler.go index 4efad7a8..6dc2930d 100755 --- a/pkg/controller/rollout/rollout_event_handler.go +++ b/pkg/controller/rollout/rollout_event_handler.go @@ -92,7 +92,7 @@ func (w *enqueueRequestForWorkload) getRolloutForWorkload(key types.NamespacedNa continue } - if targetRef.Kind == gvk.Kind && targetGV.Group == gvk.Group && targetRef.Name == key.Name && rollout.Status.Phase != rolloutv1alpha1.RolloutPhaseDisabled { + if targetRef.Kind == gvk.Kind && targetGV.Group == gvk.Group && targetRef.Name == key.Name { return &rollout, nil } } diff --git a/pkg/controller/rollout/rollout_status.go b/pkg/controller/rollout/rollout_status.go index 6fc757f2..d6702125 100755 --- a/pkg/controller/rollout/rollout_status.go +++ b/pkg/controller/rollout/rollout_status.go @@ -50,22 +50,19 @@ func (r *RolloutReconciler) calculateRolloutStatus(rollout *v1alpha1.Rollout) (r return false, newStatus, nil } - if rollout.Spec.Disabled && newStatus.Phase != v1alpha1.RolloutPhaseDisabling { + if rollout.Spec.Disabled && newStatus.Phase != v1alpha1.RolloutPhaseDisabled && newStatus.Phase != v1alpha1.RolloutPhaseDisabling { // if rollout in progressing, indicates a working rollout is disabled, then the rollout should be finalized if newStatus.Phase == v1alpha1.RolloutPhaseProgressing { newStatus.Phase = v1alpha1.RolloutPhaseDisabling newStatus.Message = "Disabling rollout, release resources" } else { - *newStatus = v1alpha1.RolloutStatus{ - ObservedGeneration: rollout.Generation, - Phase: v1alpha1.RolloutPhaseDisabled, - Message: "Rollout is disabled", - } - return false, newStatus, nil + newStatus.Phase = v1alpha1.RolloutPhaseDisabled + newStatus.ObservedGeneration = rollout.Generation + newStatus.Message = "Rollout is disabled" } } - if newStatus.Phase == "" || newStatus.Phase == v1alpha1.RolloutPhaseDisabled { + if newStatus.Phase == "" { newStatus.Phase = v1alpha1.RolloutPhaseInitial } // get ref workload @@ -74,10 +71,12 @@ func (r *RolloutReconciler) calculateRolloutStatus(rollout *v1alpha1.Rollout) (r klog.Errorf("rollout(%s/%s) get workload failed: %s", rollout.Namespace, rollout.Name, err.Error()) return false, nil, err } else if workload == nil { - newStatus = &v1alpha1.RolloutStatus{ - ObservedGeneration: rollout.Generation, - Phase: v1alpha1.RolloutPhaseInitial, - Message: "Workload Not Found", + if !rollout.Spec.Disabled { + newStatus = &v1alpha1.RolloutStatus{ + ObservedGeneration: rollout.Generation, + Phase: v1alpha1.RolloutPhaseInitial, + Message: "Workload Not Found", + } } klog.Infof("rollout(%s/%s) workload not found, and reset status be Initial", rollout.Namespace, rollout.Name) return false, newStatus, nil @@ -138,9 +137,11 @@ func (r *RolloutReconciler) calculateRolloutStatus(rollout *v1alpha1.Rollout) (r } newStatus.Message = "workload deployment is completed" } - case v1alpha1.RolloutPhaseDisabling: - cond := util.NewRolloutCondition(v1alpha1.RolloutConditionDisabling, corev1.ConditionTrue, v1alpha1.DisablingReasonFinalising, "Rollout is disabled and releasing resources") - util.SetRolloutCondition(newStatus, *cond) + case v1alpha1.RolloutPhaseDisabled: + if !rollout.Spec.Disabled { + newStatus.Phase = v1alpha1.RolloutPhaseHealthy + newStatus.Message = "rollout is healthy" + } } return false, newStatus, nil } @@ -227,7 +228,6 @@ func (r *RolloutReconciler) reconcileRolloutTerminating(rollout *v1alpha1.Rollou } func (r *RolloutReconciler) reconcileRolloutDisabling(rollout *v1alpha1.Rollout, newStatus *v1alpha1.RolloutStatus) (*time.Time, error) { - cond := util.GetRolloutCondition(rollout.Status, v1alpha1.RolloutConditionDisabling) workload, err := r.finder.GetWorkloadForRef(rollout) if err != nil { klog.Errorf("rollout(%s/%s) get workload failed: %s", rollout.Namespace, rollout.Name, err.Error()) @@ -238,12 +238,10 @@ func (r *RolloutReconciler) reconcileRolloutDisabling(rollout *v1alpha1.Rollout, if err != nil { return nil, err } else if done { - klog.Infof("rollout(%s/%s) is disabling, and state from(%s) -> to(%s)", rollout.Namespace, rollout.Name, cond.Reason, v1alpha1.RolloutPhaseDisabled) - *newStatus = v1alpha1.RolloutStatus{ - ObservedGeneration: rollout.Generation, - Phase: v1alpha1.RolloutPhaseDisabled, - Message: "Rollout is disabled", - } + klog.Infof("rollout(%s/%s) is disabled", rollout.Namespace, rollout.Name) + newStatus.Phase = v1alpha1.RolloutPhaseDisabled + newStatus.ObservedGeneration = rollout.Generation + newStatus.Message = "Rollout is disabled" } else { // Incomplete, recheck expectedTime := time.Now().Add(time.Duration(defaultGracePeriodSeconds) * time.Second) diff --git a/test/e2e/rollout_test.go b/test/e2e/rollout_test.go index 7581fd51..420406f5 100644 --- a/test/e2e/rollout_test.go +++ b/test/e2e/rollout_test.go @@ -5510,21 +5510,14 @@ var _ = SIGDescribe("Rollout", func() { rollout3.Spec.Disabled = true rollout2.SetNamespace(namespace) Expect(k8sClient.Create(context.TODO(), rollout2)).Should(HaveOccurred()) - // wait for reconciling time.Sleep(3 * time.Second) Expect(GetObject(rollout1.Name, rollout1)).NotTo(HaveOccurred()) Expect(rollout1.Status.Phase).Should(Equal(v1alpha1.RolloutPhaseInitial)) - }) - It("Disable a rolling rollout", func() { - By("Create an enabled rollout") - rollout1 := rollout.DeepCopy() - rollout1.Spec.Disabled = false + By("Create workload") deploy := &apps.Deployment{} Expect(ReadYamlToObject("./test_data/rollout/deployment_disabled.yaml", deploy)).ToNot(HaveOccurred()) - - CreateObject(rollout1) CreateObject(deploy) WaitDeploymentAllPodsReady(deploy) Expect(GetObject(rollout1.Name, rollout1)).NotTo(HaveOccurred()) @@ -5534,22 +5527,30 @@ var _ = SIGDescribe("Rollout", func() { Expect(GetObject(deploy.Name, deploy)).NotTo(HaveOccurred()) newEnvs := mergeEnvVar(deploy.Spec.Template.Spec.Containers[0].Env, v1.EnvVar{Name: "VERSION", Value: "version-2"}) deploy.Spec.Template.Spec.Containers[0].Env = newEnvs - UpdateDeployment(deploy) WaitRolloutCanaryStepPaused(rollout1.Name, 1) + Expect(GetObject(rollout1.Name, rollout1)).NotTo(HaveOccurred()) + Expect(rollout1.Status.CanaryStatus.CanaryReplicas).Should(BeNumerically("==", 2)) + Expect(rollout1.Status.CanaryStatus.CanaryReadyReplicas).Should(BeNumerically("==", 2)) + Expect(GetObject(deploy.Name, deploy)).NotTo(HaveOccurred()) + Expect(deploy.Spec.Paused).Should(BeTrue()) By("Disable a rolling rollout") rollout1.Spec.Disabled = true UpdateRollout(rollout1) // wait for reconciling time.Sleep(5 * time.Second) - key := types.NamespacedName{Namespace: namespace, Name: rollout1.Name} + + By("Rolling should be resumed") + Expect(GetObject(deploy.Name, deploy)).NotTo(HaveOccurred()) + Expect(deploy.Spec.Paused).Should(BeFalse()) + By("Batchrelease should be deleted") + key := types.NamespacedName{Namespace: namespace, Name: rollout1.Name} Expect(k8sClient.Get(context.TODO(), key, &v1alpha1.BatchRelease{})).Should(HaveOccurred()) Expect(GetObject(rollout1.Name, rollout1)).NotTo(HaveOccurred()) Expect(rollout1.Status.Phase).Should(Equal(v1alpha1.RolloutPhaseDisabled)) }) - }) })