feat: quorum-aware PodDisruptionBudget per EtcdCluster#399
Conversation
The operator now maintains a PDB named after the cluster, with minAvailable set to the quorum of observed voting members (learners excluded) and the same label selector as the StatefulSet pods. The PDB is reconciled even when health checks fail, since healthCheck still returns the member list; with no observed members any existing PDB is left untouched rather than weakened. Includes regenerated config/rbac/role.yaml plus generate/api-docs output that was stale on main. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Xavier Lange <xrlange@gmail.com>
Size minAvailable as total members minus evictable voters, since the label selector cannot distinguish voters from learners: learner pods no longer widen the eviction budget. Pending scale steps are priced in before membership mutations run, closing the windows where a removed member's lingering pod or an incoming learner pod would otherwise permit quorum-breaking evictions. Never adopt a PDB the operator did not create: an existing object without our controller ownerRef is skipped instead of patched (or wedging on SetControllerReference). PDB write failures are logged and retried without masking health-check errors or blocking cluster state reconciliation. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Xavier Lange <xrlange@gmail.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: xrl The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Hi @xrl. Thanks for your PR. I'm waiting for a etcd-io member to verify that this patch is reasonable to test. If it is, they should reply with Regular contributors should join the org to skip this step. Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
After RemoveMember but before the StatefulSet shrinks, the removed member's pod still matches the PDB selector; sizing off the member list alone dropped minAvailable and let a node drain evict a live voter of the already-shrunk voter set. Signed-off-by: Xavier Lange <xrlange@gmail.com> (cherry picked from commit 6f0f407)
Returning pdbErr directly bypassed the captured err the deferred closure reads, so ObserveReconcile recorded success and Degraded stayed False while the loop was error-backing-off. Signed-off-by: Xavier Lange <xrlange@gmail.com> (cherry picked from commit 2da8b58)
Covers the sizing table and that sizes 1-2 block node drains by design, since the eviction error does not point back at the operator. Signed-off-by: Xavier Lange <xrlange@gmail.com> (cherry picked from commit c1184d5)
1 participant
What
The operator now maintains a quorum-aware
PodDisruptionBudgetfor everyEtcdCluster— named after the cluster, owned by it (ownerRef, so GC'd on delete), selecting the StatefulSet pods via the sameapp/controllerlabels as the headless Service.Without a PDB, a node drain or any voluntary eviction can take down enough members at once to break quorum. Kubernetes-native etcd operators are expected to guard against this out of the box; today a
kubectl drainacross two nodes of a 3-member cluster is a quorum loss.Sizing (
minAvailable)A label selector cannot tell voters from learners (or from the pod of a just-removed member), so the budget assumes every permitted eviction lands on a voter:
Pending scale steps are priced in before membership mutations run, closing the transition windows:
Reconciliation behavior
healthCheckstill returns the member list for unhealthy clusters, which is exactly when disruption protection matters most. A PDB write failure is logged and surfaced for backoff only when nothing else requeues; it never masks health-check errors or blocks cluster-state reconciliation.minAvailable: 0protects nothing.SetControllerReferencewould wedge the requeue path forever.SetupWithManageraddsOwns(&policyv1.PodDisruptionBudget{}), so drift (manual edits/deletes) is repaired on the next event.Files
internal/controller/poddisruptionbudget.go— sizing math +reconcilePodDisruptionBudget(CreateOrPatch).internal/controller/etcdcluster_controller.go— wire intoReconcile,Ownswatch,policyRBAC marker.internal/controller/utils.go— extract sharedetcdPodLabels(StatefulSet / headless Service / PDB select on one set).config/rbac/role.yaml,docs/api-references/docs.md,api/v1alpha1/zz_generated.deepcopy.go— regenerated viamake generate manifests api-docs(includes output that was stale on main).internal/controller/poddisruptionbudget_test.go— envtest + table tests.Review
An adversarial review pass produced 5 findings, all blocking/important ones resolved (second commit): learner pods widening the eviction budget, the scale-in window where a removed member's lingering pod permits a quorum-breaking eviction, the symmetric scale-out window, adoption of a foreign same-name PDB (wedging on
SetControllerReference), and PDB write errors masking health-check errors.Testing
TestPDBMinAvailable,TestVotingMemberCount): steady 1/2/3/5, learner mixes, pending scale-in/out including multi-step (5→2).TestReconcilePodDisruptionBudget): create, resize on membership change, learner handling, pending-scale sizing, no-members no-op, foreign-PDB non-adoption — all against a real apiserver, consistent with the repo's controller-helper testing pattern.make testandmake verifypass.Descoped follow-ups
Trimmed to keep this PR #382/#384-sized: (1) no user-facing API field (e.g.
spec.podDisruptionBudgettemplate/override a la ECK) — operator-managed PDB only, so zero CRD/deepcopy/api-docs churn; (2) no PDB during bootstrap before the first etcd member is observed (minAvailable 0 protects nothing; PDB appears on the first loop that sees a member); (3) no deletion/GC logic — ownerRef garbage collection covers cluster deletion, and the reconciler deliberately never deletes a PDB on transient member-list failures; (4) no maxUnavailable mode, no unhealthyPodEvictionPolicy; (5) no e2e/kind test (test/e2e) — envtest-only coverage, consistent with the repo's controller-helper testing pattern; (6) no status surfacing of the PDB in EtcdClusterStatus.🤖 Generated with Claude Code
PR series — operability fixes, TLS & EtcdMirror
Small single-purpose PRs from live kind-cluster testing of the operator. Each stands alone unless an After is listed. → = this PR.
events.k8s.ioRBAC so operator Events are actually recordedmembers[]/leaderIDfrom one health snapshot (consistent leader)altNames.ipAddressesinto certificatesvalidityDuration(365d,100d12h) as documentedDegradedcondition (was empty status)--max-concurrent-reconciles)podTemplate.specscheduling fields: topologySpread, resources, priorityClass, schedulerNameconfig/surfacespec.tls.{peer,client}surfaces (breaking alpha API)TLSReadycondition + TLS lifecycle EventsPeerCANotSharedEtcdMirrorCRD: one-way cross-cluster replication API (types + CEL)pkg/mirroragentreplication engine (fenced checkpoint-in-target)mirror-agentbinary: config/TLS-reload/statusz/metrics + kind e2e/metricsendpointEtcdBackupCR → object storage (S3/GCS)🟢 ready · ⚪ draft