Fixes for etcd status fields

api/v1alpha1/types_etcd.go

@@ -376,12 +376,14 @@ type EtcdStatus struct {
 	// +optional
 	Conditions []Condition `json:"conditions,omitempty"`
 	// ServiceName is the name of the etcd service.
+	// Deprecated: this field will be removed in the future.
 	// +optional
 	ServiceName *string `json:"serviceName,omitempty"`
 	// LastError represents the last occurred error.
 	// +optional
 	LastError *string `json:"lastError,omitempty"`
-	// Cluster size is the size of the etcd cluster.
+	// Cluster size is the current size of the etcd cluster.
+	// Deprecated: this field will not be populated with any value and will be removed in the future.
 	// +optional
 	ClusterSize *int32 `json:"clusterSize,omitempty"`
 	// CurrentReplicas is the current replica count for the etcd cluster.
@@ -397,10 +399,12 @@ type EtcdStatus struct {
 	// +optional
 	Ready *bool `json:"ready,omitempty"`
 	// UpdatedReplicas is the count of updated replicas in the etcd cluster.
+	// Deprecated: this field will be removed in the future.
 	// +optional
 	UpdatedReplicas int32 `json:"updatedReplicas,omitempty"`
 	// LabelSelector is a label query over pods that should match the replica count.
 	// It must match the pod template's labels.
+	// Deprecated: this field will be removed in the future.
 	// +optional
 	LabelSelector *metav1.LabelSelector `json:"labelSelector,omitempty"`
 	// Members represents the members of the etcd cluster

charts/druid/charts/crds/templates/10-crd-druid.gardener.cloud_etcds.yaml

@@ -1701,7 +1701,9 @@ spec:
             description: EtcdStatus defines the observed state of Etcd.
             properties:
               clusterSize:
-                description: Cluster size is the size of the etcd cluster.
+                description: 'Cluster size is the current size of the etcd cluster.
+                  Deprecated: this field will not be populated with any value and
+                  will be removed in the future.'
                 format: int32
                 type: integer
               conditions:
@@ -1762,8 +1764,9 @@ spec:
                     type: string
                 type: object
               labelSelector:
-                description: LabelSelector is a label query over pods that should
-                  match the replica count. It must match the pod template's labels.
+                description: 'LabelSelector is a label query over pods that should
+                  match the replica count. It must match the pod template''s labels.
+                  Deprecated: this field will be removed in the future.'
                 properties:
                   matchExpressions:
                     description: matchExpressions is a list of label selector requirements.
@@ -1867,11 +1870,13 @@ spec:
                 format: int32
                 type: integer
               serviceName:
-                description: ServiceName is the name of the etcd service.
+                description: 'ServiceName is the name of the etcd service. Deprecated:
+                  this field will be removed in the future.'
                 type: string
               updatedReplicas:
-                description: UpdatedReplicas is the count of updated replicas in the
-                  etcd cluster.
+                description: 'UpdatedReplicas is the count of updated replicas in
+                  the etcd cluster. Deprecated: this field will be removed in the
+                  future.'
                 format: int32
                 type: integer
             type: object

config/crd/bases/10-crd-druid.gardener.cloud_etcds.yaml

@@ -1701,7 +1701,9 @@ spec:
             description: EtcdStatus defines the observed state of Etcd.
             properties:
               clusterSize:
-                description: Cluster size is the size of the etcd cluster.
+                description: 'Cluster size is the current size of the etcd cluster.
+                  Deprecated: this field will not be populated with any value and
+                  will be removed in the future.'
                 format: int32
                 type: integer
               conditions:
@@ -1762,8 +1764,9 @@ spec:
                     type: string
                 type: object
               labelSelector:
-                description: LabelSelector is a label query over pods that should
-                  match the replica count. It must match the pod template's labels.
+                description: 'LabelSelector is a label query over pods that should
+                  match the replica count. It must match the pod template''s labels.
+                  Deprecated: this field will be removed in the future.'
                 properties:
                   matchExpressions:
                     description: matchExpressions is a list of label selector requirements.
@@ -1867,11 +1870,13 @@ spec:
                 format: int32
                 type: integer
               serviceName:
-                description: ServiceName is the name of the etcd service.
+                description: 'ServiceName is the name of the etcd service. Deprecated:
+                  this field will be removed in the future.'
                 type: string
               updatedReplicas:
-                description: UpdatedReplicas is the count of updated replicas in the
-                  etcd cluster.
+                description: 'UpdatedReplicas is the count of updated replicas in
+                  the etcd cluster. Deprecated: this field will be removed in the
+                  future.'
                 format: int32
                 type: integer
             type: object

controllers/custodian/reconciler.go

@@ -113,11 +113,6 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 func (r *Reconciler) updateEtcdStatus(ctx context.Context, logger logr.Logger, etcd *druidv1alpha1.Etcd, sts *appsv1.StatefulSet) error {
 	logger.Info("Updating etcd status with statefulset information")
 
-	// Bootstrap is a special case which is handled by the etcd controller.
-	if !inBootstrap(etcd) && len(etcd.Status.Members) != 0 {
-		etcd.Status.ClusterSize = pointer.Int32(int32(len(etcd.Status.Members)))
-	}
-
 	if sts != nil {
 		etcd.Status.Etcd = &druidv1alpha1.CrossVersionObjectReference{
 			APIVersion: sts.APIVersion,
@@ -143,11 +138,3 @@ func (r *Reconciler) updateEtcdStatus(ctx context.Context, logger logr.Logger, e
 
 	return r.Client.Status().Update(ctx, etcd)
 }
-
-func inBootstrap(etcd *druidv1alpha1.Etcd) bool {
-	if etcd.Status.ClusterSize == nil {
-		return true
-	}
-	return len(etcd.Status.Members) == 0 ||
-		(len(etcd.Status.Members) < int(etcd.Spec.Replicas) && etcd.Spec.Replicas == *etcd.Status.ClusterSize)
-}

controllers/etcd/etcd_suite_test.go

@@ -0,0 +1,13 @@
+package etcd
+
+import (
+	"testing"
+
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+)
+
+func TestEtcdController(t *testing.T) {
+	RegisterFailHandler(Fail)
+	RunSpecs(t, "Etcd Controller Suite")
+}

controllers/etcd/reconciler.go

@@ -398,25 +398,11 @@ func isPeerTLSChangedToEnabled(peerTLSEnabledStatusFromMembers bool, configMapVa
 	return configMapValues.PeerUrlTLS != nil
 }
 
-func bootstrapReset(etcd *druidv1alpha1.Etcd) {
-	etcd.Status.Members = nil
-	etcd.Status.ClusterSize = pointer.Int32(etcd.Spec.Replicas)
-}
-
-func clusterInBootstrap(etcd *druidv1alpha1.Etcd) bool {
-	return etcd.Status.Replicas == 0 ||
-		(etcd.Spec.Replicas > 1 && etcd.Status.Replicas == 1)
-}
-
 func (r *Reconciler) updateEtcdErrorStatus(ctx context.Context, etcd *druidv1alpha1.Etcd, result reconcileResult) error {
 	lastErrStr := fmt.Sprintf("%v", result.err)
 	etcd.Status.LastError = &lastErrStr
 	etcd.Status.ObservedGeneration = &etcd.Generation
 	if result.sts != nil {
-		if clusterInBootstrap(etcd) {
-			// Reset members in bootstrap phase to ensure dependent conditions can be calculated correctly.
-			bootstrapReset(etcd)
-		}
 		ready, _ := druidutils.IsStatefulSetReady(etcd.Spec.Replicas, result.sts)
 		etcd.Status.Ready = &ready
 		etcd.Status.Replicas = pointer.Int32Deref(result.sts.Spec.Replicas, 0)
@@ -426,10 +412,6 @@ func (r *Reconciler) updateEtcdErrorStatus(ctx context.Context, etcd *druidv1alp
 }
 
 func (r *Reconciler) updateEtcdStatus(ctx context.Context, etcd *druidv1alpha1.Etcd, result reconcileResult) error {
-	if clusterInBootstrap(etcd) {
-		// Reset members in bootstrap phase to ensure dependent conditions can be calculated correctly.
-		bootstrapReset(etcd)
-	}
 	if result.sts != nil {
 		ready, _ := druidutils.IsStatefulSetReady(etcd.Spec.Replicas, result.sts)
 		etcd.Status.Ready = &ready

pkg/component/etcd/poddisruptionbudget/poddisruptionbudget_test.go

@@ -28,7 +28,6 @@ import (
 	policyv1 "k8s.io/api/policy/v1"
 	policyv1beta1 "k8s.io/api/policy/v1beta1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/utils/pointer"
 
 	"github.com/gardener/gardener/pkg/operation/botanist/component"
 	kutil "github.com/gardener/gardener/pkg/utils/kubernetes"
@@ -157,12 +156,9 @@ var _ = Describe("PodDisruptionBudget", func() {
 					checkV1beta1PDB(pdb, &values, namespace)
 				})
 			})
-			Context("when etcd replicas are 5 and clusterSize in etcd status is 5", func() {
+			Context("when etcd replicas are 5", func() {
 				It("should create the PDB successfully", func() {
 					etcd.Spec.Replicas = 5
-					etcd.Status = druidv1alpha1.EtcdStatus{
-						ClusterSize: pointer.Int32(4),
-					}
 					values = GenerateValues(etcd)
 					pdbDeployer = New(cl, namespace, &values, *k8sVersion_1_20_0)
 					Expect(pdbDeployer.Deploy(ctx)).To(Succeed())
@@ -175,16 +171,15 @@ var _ = Describe("PodDisruptionBudget", func() {
 		})
 
 		Context("when pdb already exists", func() {
-			It("should update the pdb successfully", func() {
+			It("should update the pdb successfully when etcd replicas change", func() {
+				// existing PDB with min = 0
 				values = GenerateValues(etcd)
 				Expect(cl.Create(ctx, defaultPDB)).To(Succeed())
 				Expect(cl.Get(ctx, kutil.Key(namespace, values.Name), pdb)).To(Succeed())
 				Expect(pdb.Spec.MinAvailable.IntVal).To(BeNumerically("==", 0))
 
+				// Post updating the etcd replicas
 				etcd.Spec.Replicas = 5
-				etcd.Status = druidv1alpha1.EtcdStatus{
-					ClusterSize: pointer.Int32(4),
-				}
 				values = GenerateValues(etcd)
 				pdbDeployer = New(cl, namespace, &values, *k8sVersion_1_20_0)
 				Expect(pdbDeployer.Deploy(ctx)).To(Succeed())
@@ -206,7 +201,7 @@ var _ = Describe("PodDisruptionBudget", func() {
 			values = GenerateValues(etcd)
 		})
 		Context("when PDB does not exist", func() {
-			It("should destroy successfully", func() {
+			It("should destroy successfully ignoring the not-found error", func() {
 				pdbDeployer = New(cl, namespace, &values, *k8sVersion_1_20_0)
 				Expect(pdbDeployer.Destroy(ctx)).To(Succeed())
 				Expect(cl.Get(ctx, kutil.Key(namespace, values.Name), pdb)).To(BeNotFoundError())
@@ -233,9 +228,6 @@ var _ = Describe("PodDisruptionBudget", func() {
 				Expect(pdb.APIVersion).To(Equal("policy/v1beta1"))
 
 				etcd.Spec.Replicas = 5
-				etcd.Status = druidv1alpha1.EtcdStatus{
-					ClusterSize: pointer.Int32(4),
-				}
 				values = GenerateValues(etcd)
 				pdbDeployer = New(cl, namespace, &values, *k8sVersion_1_25_0)
 				Expect(pdbDeployer.Deploy(ctx)).To(Succeed())
@@ -248,7 +240,6 @@ var _ = Describe("PodDisruptionBudget", func() {
 
 				Expect(cl.Delete(ctx, pdb1)).To(Succeed())
 				Expect(cl.Delete(ctx, pdb)).To(Succeed())
-
 			})
 		})
 	})

pkg/component/etcd/poddisruptionbudget/values_helper.go

@@ -41,7 +41,7 @@ func GenerateValues(etcd *druidv1alpha1.Etcd) Values {
 // CalculatePDBMinAvailable calculates the minimum available value for the PDB
 func CalculatePDBMinAvailable(etcd *druidv1alpha1.Etcd) int {
 	// do not enable for single node cluster
-	if etcd.Spec.Replicas < 2 {
+	if etcd.Spec.Replicas <= 1 {
 		return 0
 	}
 

pkg/health/condition/check_all_members.go

@@ -35,20 +35,30 @@ func (a *allMembersReady) Check(_ context.Context, etcd druidv1alpha1.Etcd) Resu
 
 	result := &result{
 		conType: druidv1alpha1.ConditionTypeAllMembersReady,
-		status:  druidv1alpha1.ConditionTrue,
-		reason:  "AllMembersReady",
-		message: "All members are ready",
+		status:  druidv1alpha1.ConditionFalse,
+		reason:  "NotAllMembersReady",
+		message: "At least one member is not ready",
 	}
 
-	for _, member := range etcd.Status.Members {
-		if member.Status != druidv1alpha1.EtcdMemberStatusReady {
-			result.status = druidv1alpha1.ConditionFalse
-			result.reason = "NotAllMembersReady"
-			result.message = "At least one member is not ready"
+	if int32(len(etcd.Status.Members)) < etcd.Spec.Replicas {
+		// not all members are registered yet
+		return result
+	}
 
-			return result
+	// If we are here this means that all members have registered. Check if any member
+	// has a not-ready status. If there is at least one then set the overall status as false.
+	ready := true
+	for _, member := range etcd.Status.Members {
+		ready = ready && member.Status == druidv1alpha1.EtcdMemberStatusReady
+		if !ready {
+			break
 		}
 	}
+	if ready {
+		result.status = druidv1alpha1.ConditionTrue
+		result.reason = "AllMembersReady"
+		result.message = "All members are ready"
+	}
 
 	return result
 }

pkg/health/condition/check_all_members_test.go

@@ -40,6 +40,9 @@ var _ = Describe("AllMembersReadyCheck", func() {
 		Context("when members in status", func() {
 			It("should return that all members are ready", func() {
 				etcd := druidv1alpha1.Etcd{
+					Spec: druidv1alpha1.EtcdSpec{
+						Replicas: 3,
+					},
 					Status: druidv1alpha1.EtcdStatus{
 						Members: []druidv1alpha1.EtcdMemberStatus{
 							readyMember,
@@ -58,6 +61,9 @@ var _ = Describe("AllMembersReadyCheck", func() {
 
 			It("should return that members are not ready", func() {
 				etcd := druidv1alpha1.Etcd{
+					Spec: druidv1alpha1.EtcdSpec{
+						Replicas: 3,
+					},
 					Status: druidv1alpha1.EtcdStatus{
 						Members: []druidv1alpha1.EtcdMemberStatus{
 							readyMember,
@@ -73,11 +79,35 @@ var _ = Describe("AllMembersReadyCheck", func() {
 				Expect(result.ConditionType()).To(Equal(druidv1alpha1.ConditionTypeAllMembersReady))
 				Expect(result.Status()).To(Equal(druidv1alpha1.ConditionFalse))
 			})
+
+			It("should return all members are not ready when number of members registered are less than spec replicas", func() {
+				etcd := druidv1alpha1.Etcd{
+					Spec: druidv1alpha1.EtcdSpec{
+						Replicas: 3,
+					},
+					Status: druidv1alpha1.EtcdStatus{
+						Members: []druidv1alpha1.EtcdMemberStatus{
+							readyMember,
+							readyMember,
+						},
+					},
+				}
+				check := AllMembersCheck(nil)
+
+				result := check.Check(context.TODO(), etcd)
+
+				Expect(result.ConditionType()).To(Equal(druidv1alpha1.ConditionTypeAllMembersReady))
+				Expect(result.Status()).To(Equal(druidv1alpha1.ConditionFalse))
+				Expect(result.Reason()).To(Equal("NotAllMembersReady"))
+			})
 		})
 
 		Context("when no members in status", func() {
 			It("should return that readiness is unknown", func() {
 				etcd := druidv1alpha1.Etcd{
+					Spec: druidv1alpha1.EtcdSpec{
+						Replicas: 3,
+					},
 					Status: druidv1alpha1.EtcdStatus{
 						Members: []druidv1alpha1.EtcdMemberStatus{},
 					},

pkg/health/condition/check_ready.go

@@ -18,21 +18,12 @@ import (
 	"context"
 
 	druidv1alpha1 "github.com/gardener/etcd-druid/api/v1alpha1"
-	"github.com/gardener/etcd-druid/pkg/utils"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
 type readyCheck struct{}
 
 func (r *readyCheck) Check(_ context.Context, etcd druidv1alpha1.Etcd) Result {
-	if etcd.Status.ClusterSize == nil {
-		return &result{
-			conType: druidv1alpha1.ConditionTypeReady,
-			status:  druidv1alpha1.ConditionUnknown,
-			reason:  "ClusterSizeUnknown",
-			message: "Cannot determine readiness of cluster since no cluster size has been calculated",
-		}
-	}
 
 	// TODO: remove this case as soon as leases are completely supported by etcd-backup-restore
 	if len(etcd.Status.Members) == 0 {
@@ -45,7 +36,7 @@ func (r *readyCheck) Check(_ context.Context, etcd druidv1alpha1.Etcd) Result {
 	}
 
 	var (
-		size         = utils.Max(int(*etcd.Status.ClusterSize), len(etcd.Status.Members))
+		size         = len(etcd.Status.Members)
 		quorum       = size/2 + 1
 		readyMembers = 0
 	)