Merge pull request #6 from Kuromesi/disabled

disabled rollout

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"
 

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
 		}
 	}

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)

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))
 		})
-
 	})
 })