From 0bb2e298833bd395343271d7f23c084dcdb5d1f1 Mon Sep 17 00:00:00 2001 From: Kirill Date: Sun, 31 Mar 2024 16:55:36 +0500 Subject: [PATCH] fix: use status condition for setting cluster configmap state (#94) Before this fix only current statefulset readiness status defined cluster state in configmap. --- api/v1alpha1/etcdcluster_types.go | 18 ++--- internal/controller/etcdcluster_controller.go | 30 ++++++-- internal/controller/factory/condition.go | 16 ++++- internal/controller/factory/condition_test.go | 53 ++++++++++++++ internal/controller/factory/configMap.go | 14 ++-- internal/controller/factory/configmap_test.go | 70 ++++++++++++------- 6 files changed, 152 insertions(+), 49 deletions(-) diff --git a/api/v1alpha1/etcdcluster_types.go b/api/v1alpha1/etcdcluster_types.go index 7cce4eeb..a9d3c7df 100644 --- 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 index 3bea9e07..8c3761eb 100644 --- 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 index 690cedcf..f340623c 100644 --- 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 index 4c5908c3..4cabc59e 100644 --- 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 index 37dc2d3a..ebe1eb6c 100644 --- 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 index 010e42ff..a3bb3220 100644 --- 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() {