fix: use status condition for setting cluster configmap state (aenix-…

…io#94) Before this fix only current statefulset readiness status defined cluster state in configmap.
aobort · Mar 31, 2024 · 0bb2e29 · 0bb2e29
1 parent e893b93
commit 0bb2e29
Show file tree

Hide file tree

Showing 6 changed files with 152 additions and 49 deletions.
diff --git a/api/v1alpha1/etcdcluster_types.go b/api/v1alpha1/etcdcluster_types.go
@@ -44,17 +44,19 @@ type EtcdCondType string
 type EtcdCondMessage string
 
 const (
-	EtcdCondTypeInitStarted         EtcdCondType = "InitializationStarted"
-	EtcdCondTypeInitComplete        EtcdCondType = "InitializationComplete"
-	EtcdCondTypeStatefulSetReady    EtcdCondType = "StatefulSetReady"
-	EtcdCondTypeStatefulSetNotReady EtcdCondType = "StatefulSetNotReady"
+	EtcdCondTypeInitStarted           EtcdCondType = "InitializationStarted"
+	EtcdCondTypeInitComplete          EtcdCondType = "InitializationComplete"
+	EtcdCondTypeWaitingForFirstQuorum EtcdCondType = "WaitingForFirstQuorum"
+	EtcdCondTypeStatefulSetReady      EtcdCondType = "StatefulSetReady"
+	EtcdCondTypeStatefulSetNotReady   EtcdCondType = "StatefulSetNotReady"
 )
 
 const (
-	EtcdInitCondNegMessage  EtcdCondMessage = "Cluster initialization started"
-	EtcdInitCondPosMessage  EtcdCondMessage = "Cluster managed resources created"
-	EtcdReadyCondNegMessage EtcdCondMessage = "Cluster StatefulSet is not Ready"
-	EtcdReadyCondPosMessage EtcdCondMessage = "Cluster StatefulSet is Ready"
+	EtcdInitCondNegMessage           EtcdCondMessage = "Cluster initialization started"
+	EtcdInitCondPosMessage           EtcdCondMessage = "Cluster managed resources created"
+	EtcdReadyCondNegMessage          EtcdCondMessage = "Cluster StatefulSet is not Ready"
+	EtcdReadyCondPosMessage          EtcdCondMessage = "Cluster StatefulSet is Ready"
+	EtcdReadyCondNegWaitingForQuorum EtcdCondMessage = "Waiting for first quorum to be established"
 )
 
 // EtcdClusterStatus defines the observed state of EtcdCluster

diff --git a/internal/controller/etcdcluster_controller.go b/internal/controller/etcdcluster_controller.go
@@ -95,11 +95,31 @@ func (r *EtcdClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 	}
 
 	// set cluster readiness condition
-	factory.SetCondition(instance, factory.NewCondition(etcdaenixiov1alpha1.EtcdConditionReady).
-		WithStatus(clusterReady).
-		WithReason(string(etcdaenixiov1alpha1.EtcdCondTypeStatefulSetReady)).
-		WithMessage(string(etcdaenixiov1alpha1.EtcdReadyCondPosMessage)).
-		Complete())
+	existingCondition := factory.GetCondition(instance, etcdaenixiov1alpha1.EtcdConditionReady)
+	if existingCondition.Reason == string(etcdaenixiov1alpha1.EtcdCondTypeWaitingForFirstQuorum) {
+		// we should change from "waiting for first quorum establishment" to "StatefulSet ready / not ready"
+		// only after sts gets ready first time
+		if clusterReady {
+			factory.SetCondition(instance, factory.NewCondition(etcdaenixiov1alpha1.EtcdConditionReady).
+				WithStatus(true).
+				WithReason(string(etcdaenixiov1alpha1.EtcdCondTypeStatefulSetReady)).
+				WithMessage(string(etcdaenixiov1alpha1.EtcdReadyCondPosMessage)).
+				Complete())
+		}
+	} else {
+		reason := etcdaenixiov1alpha1.EtcdCondTypeStatefulSetNotReady
+		message := etcdaenixiov1alpha1.EtcdReadyCondNegMessage
+		if clusterReady {
+			reason = etcdaenixiov1alpha1.EtcdCondTypeStatefulSetReady
+			message = etcdaenixiov1alpha1.EtcdReadyCondPosMessage
+		}
+
+		factory.SetCondition(instance, factory.NewCondition(etcdaenixiov1alpha1.EtcdConditionReady).
+			WithStatus(clusterReady).
+			WithReason(string(reason)).
+			WithMessage(string(message)).
+			Complete())
+	}
 	return r.updateStatus(ctx, instance)
 }
 

diff --git a/internal/controller/factory/condition.go b/internal/controller/factory/condition.go
@@ -69,8 +69,8 @@ func FillConditions(cluster *etcdaenixiov1alpha1.EtcdCluster) {
 		Complete())
 	SetCondition(cluster, NewCondition(etcdaenixiov1alpha1.EtcdConditionReady).
 		WithStatus(false).
-		WithReason(string(etcdaenixiov1alpha1.EtcdCondTypeStatefulSetNotReady)).
-		WithMessage(string(etcdaenixiov1alpha1.EtcdReadyCondNegMessage)).
+		WithReason(string(etcdaenixiov1alpha1.EtcdCondTypeWaitingForFirstQuorum)).
+		WithMessage(string(etcdaenixiov1alpha1.EtcdReadyCondNegWaitingForQuorum)).
 		Complete())
 }
 
@@ -96,3 +96,15 @@ func SetCondition(
 	}
 	cluster.Status.Conditions[idx] = condition
 }
+
+// GetCondition returns condition from cluster status conditions by type or nil if not present.
+func GetCondition(cluster *etcdaenixiov1alpha1.EtcdCluster, condType string) *metav1.Condition {
+	idx := slices.IndexFunc(cluster.Status.Conditions, func(c metav1.Condition) bool {
+		return c.Type == condType
+	})
+	if idx == -1 {
+		return nil
+	}
+
+	return &cluster.Status.Conditions[idx]
+}
diff --git a/internal/controller/factory/condition_test.go b/internal/controller/factory/condition_test.go
@@ -18,6 +18,7 @@ package factory
 
 import (
 	"slices"
+	"time"
 
 	etcdaenixiov1alpha1 "github.com/aenix-io/etcd-operator/api/v1alpha1"
 	. "github.com/onsi/ginkgo/v2"
@@ -71,4 +72,56 @@ var _ = Describe("Condition builder", func() {
 			Expect(etcdCluster.Status.Conditions[idx].LastTransitionTime).NotTo(Equal(timestamp))
 		})
 	})
+
+	Context("when retrieving conditions", func() {
+		It("should return nil if condition of such type is not present", func() {
+			etcdCluster := &etcdaenixiov1alpha1.EtcdCluster{
+				Status: etcdaenixiov1alpha1.EtcdClusterStatus{
+					Conditions: []metav1.Condition{
+						{
+							Type:               etcdaenixiov1alpha1.EtcdConditionInitialized,
+							Status:             metav1.ConditionTrue,
+							ObservedGeneration: 0,
+							LastTransitionTime: metav1.NewTime(time.Now()),
+							Reason:             string(etcdaenixiov1alpha1.EtcdCondTypeInitComplete),
+							Message:            string(etcdaenixiov1alpha1.EtcdInitCondPosMessage),
+						},
+					},
+				},
+			}
+
+			Expect(GetCondition(etcdCluster, etcdaenixiov1alpha1.EtcdConditionReady)).To(BeNil())
+		})
+
+		It("should return correct condition from the list", func() {
+			expectedCond := metav1.Condition{
+				Type:               etcdaenixiov1alpha1.EtcdConditionReady,
+				Status:             metav1.ConditionTrue,
+				ObservedGeneration: 0,
+				LastTransitionTime: metav1.NewTime(time.Now()),
+				Reason:             string(etcdaenixiov1alpha1.EtcdCondTypeStatefulSetReady),
+				Message:            string(etcdaenixiov1alpha1.EtcdReadyCondPosMessage),
+			}
+
+			etcdCluster := &etcdaenixiov1alpha1.EtcdCluster{
+				Status: etcdaenixiov1alpha1.EtcdClusterStatus{
+					Conditions: []metav1.Condition{
+						expectedCond,
+						{
+							Type:               etcdaenixiov1alpha1.EtcdConditionInitialized,
+							Status:             metav1.ConditionTrue,
+							ObservedGeneration: 0,
+							LastTransitionTime: metav1.NewTime(time.Now()),
+							Reason:             string(etcdaenixiov1alpha1.EtcdCondTypeInitComplete),
+							Message:            string(etcdaenixiov1alpha1.EtcdInitCondPosMessage),
+						},
+					},
+				},
+			}
+			foundCond := GetCondition(etcdCluster, etcdaenixiov1alpha1.EtcdConditionReady)
+			if Expect(foundCond).NotTo(BeNil()) {
+				Expect(*foundCond).To(Equal(expectedCond))
+			}
+		})
+	})
 })
diff --git a/internal/controller/factory/configMap.go b/internal/controller/factory/configMap.go
@@ -19,7 +19,6 @@ package factory
 import (
 	"context"
 	"fmt"
-	"slices"
 
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -75,13 +74,10 @@ func CreateOrUpdateClusterStateConfigMap(
 	return reconcileConfigMap(ctx, rclient, cluster.Name, configMap)
 }
 
-// isEtcdClusterReady returns true if condition "Ready" has status equal to "True", otherwise false.
+// isEtcdClusterReady returns true if condition "Ready" has progressed
+// from reason v1alpha1.EtcdCondTypeWaitingForFirstQuorum.
 func isEtcdClusterReady(cluster *etcdaenixiov1alpha1.EtcdCluster) bool {
-	idx := slices.IndexFunc(cluster.Status.Conditions, func(condition metav1.Condition) bool {
-		return condition.Type == etcdaenixiov1alpha1.EtcdConditionReady
-	})
-	if idx == -1 {
-		return false
-	}
-	return cluster.Status.Conditions[idx].Status == metav1.ConditionTrue
+	cond := GetCondition(cluster, etcdaenixiov1alpha1.EtcdConditionReady)
+	return cond != nil && (cond.Reason == string(etcdaenixiov1alpha1.EtcdCondTypeStatefulSetReady) ||
+		cond.Reason == string(etcdaenixiov1alpha1.EtcdCondTypeStatefulSetNotReady))
 }
diff --git a/internal/controller/factory/configmap_test.go b/internal/controller/factory/configmap_test.go
@@ -54,31 +54,51 @@ var _ = Describe("CreateOrUpdateClusterStateConfigMap handlers", func() {
 
 		It("should successfully ensure the configmap", func() {
 			cm := &corev1.ConfigMap{}
-
-			By("creating the configmap for initial cluster")
-			err := CreateOrUpdateClusterStateConfigMap(ctx, etcdcluster, k8sClient, k8sClient.Scheme())
-			Expect(err).NotTo(HaveOccurred())
-
-			err = k8sClient.Get(ctx, typeNamespacedName, cm)
-			cmUid := cm.UID
-			Expect(err).NotTo(HaveOccurred())
-			Expect(cm.Data["ETCD_INITIAL_CLUSTER_STATE"]).To(Equal("new"))
-
-			By("updating the configmap for initialized cluster")
-			SetCondition(etcdcluster, NewCondition(etcdaenixiov1alpha1.EtcdConditionReady).
-				WithStatus(true).Complete())
-			err = CreateOrUpdateClusterStateConfigMap(ctx, etcdcluster, k8sClient, k8sClient.Scheme())
-			Expect(err).NotTo(HaveOccurred())
-
-			err = k8sClient.Get(ctx, typeNamespacedName, cm)
-			Expect(err).NotTo(HaveOccurred())
-			Expect(cm.Data["ETCD_INITIAL_CLUSTER_STATE"]).To(Equal("existing"))
-			// Check that we are updating the same configmap
-			Expect(cm.UID).To(Equal(cmUid))
-
-			By("deleting the configmap")
-
-			Expect(k8sClient.Delete(ctx, cm)).To(Succeed())
+			var err error
+			var cmUid types.UID
+			By("creating the configmap for initial cluster", func() {
+				err = CreateOrUpdateClusterStateConfigMap(ctx, etcdcluster, k8sClient, k8sClient.Scheme())
+				Expect(err).NotTo(HaveOccurred())
+
+				err = k8sClient.Get(ctx, typeNamespacedName, cm)
+				cmUid = cm.UID
+				Expect(err).NotTo(HaveOccurred())
+				Expect(cm.Data["ETCD_INITIAL_CLUSTER_STATE"]).To(Equal("new"))
+			})
+
+			By("updating the configmap for initialized cluster", func() {
+				SetCondition(etcdcluster, NewCondition(etcdaenixiov1alpha1.EtcdConditionReady).
+					WithReason(string(etcdaenixiov1alpha1.EtcdCondTypeStatefulSetReady)).
+					WithStatus(true).
+					Complete())
+				err = CreateOrUpdateClusterStateConfigMap(ctx, etcdcluster, k8sClient, k8sClient.Scheme())
+				Expect(err).NotTo(HaveOccurred())
+
+				err = k8sClient.Get(ctx, typeNamespacedName, cm)
+				Expect(err).NotTo(HaveOccurred())
+				Expect(cm.Data["ETCD_INITIAL_CLUSTER_STATE"]).To(Equal("existing"))
+				// Check that we are updating the same configmap
+				Expect(cm.UID).To(Equal(cmUid))
+			})
+
+			By("updating the configmap back to new", func() {
+				SetCondition(etcdcluster, NewCondition(etcdaenixiov1alpha1.EtcdConditionReady).
+					WithReason(string(etcdaenixiov1alpha1.EtcdCondTypeWaitingForFirstQuorum)).
+					WithStatus(true).
+					Complete())
+				err = CreateOrUpdateClusterStateConfigMap(ctx, etcdcluster, k8sClient, k8sClient.Scheme())
+				Expect(err).NotTo(HaveOccurred())
+
+				err = k8sClient.Get(ctx, typeNamespacedName, cm)
+				Expect(err).NotTo(HaveOccurred())
+				Expect(cm.Data["ETCD_INITIAL_CLUSTER_STATE"]).To(Equal("new"))
+				// Check that we are updating the same configmap
+				Expect(cm.UID).To(Equal(cmUid))
+			})
+
+			By("deleting the configmap", func() {
+				Expect(k8sClient.Delete(ctx, cm)).To(Succeed())
+			})
 		})
 
 		It("should fail on creating the configMap with invalid owner reference", func() {