update against review

Signed-off-by: YZ775 <yuzuki-mimura@cybozu.co.jp>
cybozu-go · Jul 27, 2023 · 4e1672d · 4e1672d
1 parent 818b3b3
commit 4e1672d
Show file tree

Hide file tree

Showing 2 changed files with 72 additions and 18 deletions.
diff --git a/controllers/mysqlcluster_controller.go b/controllers/mysqlcluster_controller.go
@@ -239,7 +239,7 @@ func (r *MySQLClusterReconciler) reconcileV1(ctx context.Context, req ctrl.Reque
 		log.Error(err, "failed to reconcile stateful set")
 		return ctrl.Result{}, err
 	}
-	if err := r.UpdateStatusByStatefulSet(ctx, req, cluster); err != nil {
+	if err := r.UpdateStatusByStatefulSet(ctx, cluster); err != nil {
 		return ctrl.Result{}, err
 	}
 
@@ -1866,7 +1866,7 @@ func (r *MySQLClusterReconciler) finalizeV1(ctx context.Context, cluster *mocov1
 	return nil
 }
 
-func (r *MySQLClusterReconciler) UpdateStatusByStatefulSet(ctx context.Context, req ctrl.Request, cluster *mocov1beta2.MySQLCluster) error {
+func (r *MySQLClusterReconciler) UpdateStatusByStatefulSet(ctx context.Context, cluster *mocov1beta2.MySQLCluster) error {
 	log := crlog.FromContext(ctx)
 
 	var sts appsv1.StatefulSet
@@ -1875,14 +1875,11 @@ func (r *MySQLClusterReconciler) UpdateStatusByStatefulSet(ctx context.Context,
 		return fmt.Errorf("failed to get StatefulSet %s/%s: %w", cluster.Namespace, cluster.PrefixedName(), err)
 	}
 
-	needUpdate := false
 	updated := metav1.ConditionFalse
 	reason := "UpdateInProgress"
 	message := "the current state is UpdateInProgress"
 	if sts.Spec.UpdateStrategy.Type == appsv1.RollingUpdateStatefulSetStrategyType && sts.Spec.UpdateStrategy.RollingUpdate != nil {
 		if sts.Spec.Replicas != nil && sts.Spec.UpdateStrategy.RollingUpdate.Partition != nil {
-			updated = metav1.ConditionFalse
-			reason = "UpdateInProgress"
 			message = "UpdateInProgress, Partition is enabled, waiting for the all pods to be updated"
 			if sts.Status.UpdatedReplicas < (*sts.Spec.Replicas - *sts.Spec.UpdateStrategy.RollingUpdate.Partition) {
 				message = "UpdateInProgress, Partition is enabled, waiting for partitioned roll out to finish"
@@ -1897,6 +1894,7 @@ func (r *MySQLClusterReconciler) UpdateStatusByStatefulSet(ctx context.Context,
 
 	currentConditionUpToDate := meta.FindStatusCondition(cluster.Status.Conditions, mocov1beta2.ConditionUpToDate)
 	// if current status and new status are different, update status
+	needUpdate := false
 	if currentConditionUpToDate == nil || currentConditionUpToDate.Status != updated || currentConditionUpToDate.Message != message {
 		needUpdate = true
 	}

diff --git a/controllers/mysqlcluster_controller_test.go b/controllers/mysqlcluster_controller_test.go
@@ -1657,6 +1657,69 @@ var _ = Describe("MySQLCluster reconciler", func() {
 		}).Should(Succeed())
 	})
 
+	It("should sets ConditionUpToDate to be false when status of StatefulSet is empty", func() {
+		cluster := testNewMySQLCluster("test")
+		err := k8sClient.Create(ctx, cluster)
+		Expect(err).NotTo(HaveOccurred())
+		var sts *appsv1.StatefulSet
+		Eventually(func() error {
+			sts = &appsv1.StatefulSet{}
+			if err := k8sClient.Get(ctx, client.ObjectKey{Namespace: "test", Name: "moco-test"}, sts); err != nil {
+				return err
+			}
+			return nil
+		}).Should(Succeed())
+
+		By("setting sts status to be ready")
+		sts.Status.Replicas = 3
+		sts.Status.ReadyReplicas = 3
+		sts.Status.AvailableReplicas = 3
+		sts.Status.CurrentRevision = "hoge"
+		sts.Status.UpdateRevision = "hoge"
+		sts.Status.CurrentReplicas = 3
+		sts.Status.UpdatedReplicas = 3
+		sts.Status.ObservedGeneration = sts.Generation
+		err = k8sClient.Status().Update(ctx, sts)
+		Expect(err).NotTo(HaveOccurred())
+
+		By("waiting condition to be updated")
+		Eventually(func() error {
+			cluster2 := &mocov1beta2.MySQLCluster{}
+			if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(cluster), cluster2); err != nil {
+				return err
+			}
+			conditionUpToDate := meta.FindStatusCondition(cluster2.Status.Conditions, mocov1beta2.ConditionUpToDate)
+			if conditionUpToDate == nil {
+				return fmt.Errorf("not yet updated")
+			}
+			if conditionUpToDate.Status != metav1.ConditionTrue {
+				return fmt.Errorf("not yet updated")
+			}
+			return nil
+		}).Should(Succeed())
+
+		By("setting sts status to be empty")
+		sts.Status = appsv1.StatefulSetStatus{}
+		err = k8sClient.Status().Update(ctx, sts)
+		Expect(err).NotTo(HaveOccurred())
+
+		By("checking condition is false")
+		Eventually(func() error {
+			cluster2 := &mocov1beta2.MySQLCluster{}
+			if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(cluster), cluster2); err != nil {
+				return err
+			}
+			conditionUpToDate := meta.FindStatusCondition(cluster2.Status.Conditions, mocov1beta2.ConditionUpToDate)
+			if conditionUpToDate == nil {
+				return fmt.Errorf("condition does not exists")
+			}
+			if conditionUpToDate.Status != metav1.ConditionFalse {
+				return fmt.Errorf("condition is not false")
+			}
+			return nil
+		}).Should(Succeed())
+	})
+
 	It("should not sets ConditionUpToDate to be true when StatefulSet is not ready", func() {
 		cluster := testNewMySQLCluster("test")
 		err := k8sClient.Create(ctx, cluster)
@@ -1721,9 +1784,6 @@ var _ = Describe("MySQLCluster reconciler", func() {
 			if conditionUpToDate == nil {
 				return fmt.Errorf("not yet updated")
 			}
-			if err != nil {
-				return err
-			}
 			if conditionUpToDate.Status != metav1.ConditionFalse {
 				return fmt.Errorf("not yet updated")
 			}
@@ -1832,13 +1892,11 @@ var _ = Describe("MySQLCluster reconciler", func() {
 			if conditionUpToDate.Message != "UpdateInProgress, Partition is enabled, waiting for partitioned roll out to finish" {
 				return fmt.Errorf("not yet updated: %s", conditionUpToDate.Message)
 			}
+			if conditionUpToDate.Status != metav1.ConditionFalse {
+				return fmt.Errorf("status is not false")
+			}
 			return nil
 		}).Should(Succeed())
-		cluster = &mocov1beta2.MySQLCluster{}
-		err = k8sClient.Get(ctx, client.ObjectKey{Namespace: "test", Name: "test"}, cluster)
-		Expect(err).NotTo(HaveOccurred())
-		conditionUpToDate := meta.FindStatusCondition(cluster.Status.Conditions, mocov1beta2.ConditionUpToDate)
-		Expect(conditionUpToDate.Status).To(Equal(metav1.ConditionFalse))
 
 		By("setting sts status to be partitioned roll out completed")
 		sts.Status.Replicas = 3
@@ -1865,12 +1923,10 @@ var _ = Describe("MySQLCluster reconciler", func() {
 			if conditionUpToDate.Message != "UpdateInProgress, Partition is enabled, waiting for the all pods to be updated" {
 				return fmt.Errorf("not yet updated: %s", conditionUpToDate.Message)
 			}
+			if conditionUpToDate.Status != metav1.ConditionFalse {
+				return fmt.Errorf("status is not false")
+			}
 			return nil
 		}).Should(Succeed())
-		cluster = &mocov1beta2.MySQLCluster{}
-		err = k8sClient.Get(ctx, client.ObjectKey{Namespace: "test", Name: "test"}, cluster)
-		Expect(err).NotTo(HaveOccurred())
-		conditionUpToDate = meta.FindStatusCondition(cluster.Status.Conditions, mocov1beta2.ConditionUpToDate)
-		Expect(conditionUpToDate.Status).To(Equal(metav1.ConditionFalse))
 	})
 })