feat: quorum-aware PodDisruptionBudget per EtcdCluster

config/rbac/role.yaml

@@ -87,3 +87,15 @@ rules:
   - get
   - patch
   - update
+- apiGroups:
+  - policy
+  resources:
+  - poddisruptionbudgets
+  verbs:
+  - create
+  - delete
+  - get
+  - list
+  - patch
+  - update
+  - watch

docs/api-references/docs.md

@@ -153,6 +153,24 @@ _Appears in:_
 | `labels` _object (keys:string, values:string)_ |  |  |  |
 
 
+#### PodSpec
+
+
+
+
+
+
+
+_Appears in:_
+- [PodTemplate](#podtemplate)
+
+| Field | Description | Default | Validation |
+| --- | --- | --- | --- |
+| `affinity` _[Affinity](https://kubernetes.io/docs/reference/generated/kubernetes-api/v/#affinity-v1-core)_ |  |  |  |
+| `nodeSelector` _object (keys:string, values:string)_ |  |  |  |
+| `tolerations` _[Toleration](https://kubernetes.io/docs/reference/generated/kubernetes-api/v/#toleration-v1-core) array_ |  |  |  |
+
+
 #### PodTemplate
 
 
@@ -167,6 +185,7 @@ _Appears in:_
 | Field | Description | Default | Validation |
 | --- | --- | --- | --- |
 | `metadata` _[PodMetadata](#podmetadata)_ | Refer to Kubernetes API documentation for fields of `metadata`. |  |  |
+| `spec` _[PodSpec](#podspec)_ |  |  |  |
 
 
 #### ProviderAutoConfig

docs/pod-disruption-budget.md

@@ -0,0 +1,33 @@
+# PodDisruptionBudget
+
+The operator always creates a PodDisruptionBudget for every `EtcdCluster`,
+named after the cluster and selecting all its etcd pods. `minAvailable` is
+sized so that voluntary evictions (node drains, cluster-autoscaler
+scale-down, managed node-pool upgrades) can never break etcd quorum, and is
+re-tightened ahead of scale operations. There is no spec field to disable or
+override it; manual edits are reverted on the next reconcile, and a
+same-named PDB not created by the operator is left untouched (the operator
+then manages none).
+
+## Sizing
+
+`minAvailable = pods - (voters - quorum)`, computed from the live member
+list (learners and not-yet-removed pods count as pods but not voters):
+
+| Cluster size | Quorum | minAvailable | Evictions allowed |
+| --- | --- | --- | --- |
+| 1 | 1 | 1 | 0 |
+| 2 | 2 | 2 | 0 |
+| 3 | 2 | 2 | 1 |
+| 5 | 3 | 3 | 2 |
+| 7 | 4 | 4 | 3 |
+
+## Sizes 1 and 2 block drains by design
+
+A 1- or 2-member cluster cannot lose any member without losing quorum, so
+its PDB permanently disallows all voluntary evictions. `kubectl drain` and
+autoscaler scale-down will wedge on these pods with a generic
+"cannot evict pod as it would violate the pod's disruption budget" error.
+To move such a pod, either scale the cluster to 3 first, or delete the pod
+directly (`kubectl delete pod` bypasses the eviction API; expect downtime
+while the StatefulSet recreates it).

internal/controller/etcdcluster_controller.go

@@ -25,6 +25,7 @@ import (
 	certv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1"
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
+	policyv1 "k8s.io/api/policy/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -66,6 +67,7 @@ type reconcileState struct {
 // +kubebuilder:rbac:groups=operator.etcd.io,resources=etcdclusters/status,verbs=get;update;patch
 // +kubebuilder:rbac:groups=operator.etcd.io,resources=etcdclusters/finalizers,verbs=update
 // +kubebuilder:rbac:groups=apps,resources=statefulsets,verbs=get;list;watch;create;update;patch;delete
+// +kubebuilder:rbac:groups=policy,resources=poddisruptionbudgets,verbs=get;list;watch;create;update;patch;delete
 // +kubebuilder:rbac:groups=core,resources=services,verbs=get;list;watch;create;update;patch;delete
 // +kubebuilder:rbac:groups=core,resources=configmaps,verbs=get;list;watch;create;update;patch;delete
 // +kubebuilder:rbac:groups="",resources=events,verbs=create;patch;get;list;update
@@ -105,11 +107,32 @@ func (r *EtcdClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 		return res, err
 	}
 
-	if err = r.performHealthChecks(ctx, state); err != nil {
-		return ctrl.Result{}, err
+	// Reconcile the PDB even when the health check errors: healthCheck still
+	// returns the member list for unhealthy clusters, which is exactly when
+	// disruption protection matters most. A PDB write failure is logged but
+	// never blocks health handling or cluster reconciliation.
+	healthErr := r.performHealthChecks(ctx, state)
+	var stsReplicas int32
+	if state.sts != nil && state.sts.Spec.Replicas != nil {
+		stsReplicas = *state.sts.Spec.Replicas
+	}
+	pdbErr := reconcilePodDisruptionBudget(
+		ctx, log.FromContext(ctx), r.Client, state.cluster, state.memberListResp, stsReplicas, r.Scheme,
+	)
+	if pdbErr != nil {
+		log.FromContext(ctx).Error(pdbErr, "Failed to reconcile PodDisruptionBudget")
+	}
+	if healthErr != nil {
+		return ctrl.Result{}, healthErr
 	}
 
-	return r.reconcileClusterState(ctx, state)
+	res, err = r.reconcileClusterState(ctx, state)
+	if err == nil && res.IsZero() && pdbErr != nil {
+		// Nothing else requeues; surface the PDB failure for backoff. Assign
+		// to err so the deferred status/metrics closure observes it too.
+		err = pdbErr
+	}
+	return res, err
 }
 
 // fetchAndValidateState retrieves the EtcdCluster and its StatefulSet and ensures
@@ -586,7 +609,8 @@ func (r *EtcdClusterReconciler) SetupWithManager(mgr ctrl.Manager) error {
 		For(&ecv1alpha1.EtcdCluster{}).
 		Owns(&appsv1.StatefulSet{}).
 		Owns(&corev1.Service{}).
-		Owns(&corev1.ConfigMap{})
+		Owns(&corev1.ConfigMap{}).
+		Owns(&policyv1.PodDisruptionBudget{})
 
 	// Conditionally watch cert-manager Certificate resources if CRDs are installed
 	// This allows the controller to react to Certificate status changes when using cert-manager provider

internal/controller/poddisruptionbudget.go

@@ -0,0 +1,155 @@
+/*
+Copyright 2024.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package controller
+
+import (
+	"context"
+	"fmt"
+
+	"github.com/go-logr/logr"
+	policyv1 "k8s.io/api/policy/v1"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/util/intstr"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
+
+	ecv1alpha1 "go.etcd.io/etcd-operator/api/v1alpha1"
+	clientv3 "go.etcd.io/etcd/client/v3"
+)
+
+func pdbNameForEtcdCluster(ec *ecv1alpha1.EtcdCluster) string {
+	return ec.Name
+}
+
+func votingMemberCount(resp *clientv3.MemberListResponse) int {
+	if resp == nil {
+		return 0
+	}
+	count := 0
+	for _, m := range resp.Members {
+		if !m.IsLearner {
+			count++
+		}
+	}
+	return count
+}
+
+// evictableVoterCount is how many voters can be lost while keeping quorum.
+func evictableVoterCount(voting int) int {
+	if voting <= 0 {
+		return 0
+	}
+	quorum := voting/2 + 1
+	return voting - quorum
+}
+
+// pdbMinAvailable sizes minAvailable for a selector matching every cluster
+// pod. A label selector cannot tell voters from learners (or from the pod of
+// a just-removed member), so the budget must assume every permitted eviction
+// lands on a voter: minAvailable = total pods - evictable voters.
+//
+// The loop reconciles the PDB before mutating membership, so pending scale
+// steps are priced in ahead of time:
+//   - scale-in: RemoveMember runs before the StatefulSet shrinks, leaving the
+//     removed member's pod matching the selector; size for the post-removal
+//     voter set and keep the stricter value.
+//   - scale-out: a learner (and its pod) is added before the PDB is next
+//     updated; reserve the extra pod now.
+func pdbMinAvailable(total, voting, desiredSize int) int32 {
+	minAvailable := total - evictableVoterCount(voting)
+	switch {
+	case total > desiredSize:
+		if postRemoval := total - evictableVoterCount(voting-1); postRemoval > minAvailable {
+			minAvailable = postRemoval
+		}
+	case total < desiredSize:
+		minAvailable++
+	}
+	return int32(minAvailable)
+}
+
+// reconcilePodDisruptionBudget keeps a PDB sized so voluntary evictions can
+// never break quorum (see pdbMinAvailable). With no observed members it
+// leaves any existing PDB alone: a stale-but-protective PDB during an outage
+// beats deleting it, and minAvailable 0 protects nothing.
+//
+// stsReplicas floors the pod total: after RemoveMember but before the
+// StatefulSet shrinks (a crash can freeze that state), the removed member's
+// pod still matches the selector while it is gone from the member list, and
+// sizing off members alone would re-widen the eviction budget over a smaller
+// voter set.
+func reconcilePodDisruptionBudget(
+	ctx context.Context,
+	logger logr.Logger,
+	c client.Client,
+	ec *ecv1alpha1.EtcdCluster,
+	memberListResp *clientv3.MemberListResponse,
+	stsReplicas int32,
+	scheme *runtime.Scheme,
+) error {
+	voting := votingMemberCount(memberListResp)
+	if voting == 0 {
+		return nil
+	}
+	total := len(memberListResp.Members)
+	if int(stsReplicas) > total {
+		total = int(stsReplicas)
+	}
+
+	pdb := &policyv1.PodDisruptionBudget{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      pdbNameForEtcdCluster(ec),
+			Namespace: ec.Namespace,
+		},
+	}
+
+	// Never adopt a PDB this operator did not create: patching it would
+	// clobber a user-managed spec, and SetControllerReference fails forever
+	// when another controller owns it. Skipping keeps the permanent name
+	// collision out of the requeue path.
+	existing := &policyv1.PodDisruptionBudget{}
+	switch err := c.Get(ctx, client.ObjectKeyFromObject(pdb), existing); {
+	case err == nil:
+		if !metav1.IsControlledBy(existing, ec) {
+			logger.Info("Skipping PodDisruptionBudget: existing object is not controlled by this EtcdCluster",
+				"name", pdb.Name, "namespace", pdb.Namespace)
+			return nil
+		}
+	case !apierrors.IsNotFound(err):
+		return fmt.Errorf("failed to get PodDisruptionBudget %s/%s: %w", pdb.Namespace, pdb.Name, err)
+	}
+
+	minAvailable := intstr.FromInt32(pdbMinAvailable(total, voting, ec.Spec.Size))
+	result, err := controllerutil.CreateOrPatch(ctx, c, pdb, func() error {
+		pdb.Spec.MinAvailable = &minAvailable
+		pdb.Spec.MaxUnavailable = nil // apiserver rejects specs with both fields set
+		pdb.Spec.Selector = &metav1.LabelSelector{MatchLabels: etcdPodLabels(ec)}
+		return controllerutil.SetControllerReference(ec, pdb, scheme)
+	})
+	if err != nil {
+		return err
+	}
+
+	if result == controllerutil.OperationResultCreated || result == controllerutil.OperationResultUpdated {
+		logger.Info("PodDisruptionBudget reconciled",
+			"name", pdb.Name, "namespace", pdb.Namespace, "result", result,
+			"minAvailable", minAvailable.IntValue())
+	}
+	return nil
+}