Validating and defaulting webhooks and tests - #19

* api+controllers: EtcdMember.Status.IsVoter + role-label constants

Add Status.IsVoter to EtcdMember (the carrier bit between the cluster
controller's MemberList view and the member controller's Pod label
maintenance) and the matching LabelRole / RoleVoter constants in
helpers.go.

No behavioural change yet — the cluster controller writes IsVoter from
MemberList in the next commit, and the member controller propagates
that to the Pod's etcd.lllamnyp.su/role=voter label after that.

This addition keeps the controller chain clean: cluster owns CR-level
state (writes Status.IsVoter), member owns Pod-level state (reads
Status.IsVoter, patches the role label). No cluster-controller-to-Pod
patches across the layering boundary.

Signed-off-by: Timofei Larkin <[email protected]>

* controllers: write IsVoter, propagate to Pod label, emit PDB

Three pieces, layered through the existing controller chain:

1. Cluster controller writes Status.IsVoter:
   - syncIsVoter() consumes etcd's MemberList response and patches each
     EtcdMember.Status.IsVoter to match etcd's !IsLearner view. Match
     key is the peer URL (not Member.Name, which is empty until the
     etcd process announces itself post-Pod-start). Idempotent.
   - Called from promotePendingLearner — the existing site that already
     fetches MemberList in the current==desired steady-state branch.
   - Seed creation pre-stamps Status.IsVoter=true with a follow-up
     r.Status().Update(). The seed is never a learner, so this skips
     the 1-2 reconcile wait for the first MemberList round to discover
     that fact, closing the gap where the seed Pod would otherwise
     exist unprotected by the PDB.

2. Member controller propagates IsVoter to the Pod label:
   - buildPod() adds etcd.lllamnyp.su/role=voter at creation when
     Status.IsVoter is already set (the seed's pre-stamp case).
   - ensurePod() now calls reconcileRoleLabel() after the Get-existing
     branch — idempotent Patch that adds or strips the role label to
     match Status.IsVoter. Handles the steady-state case where the
     cluster controller flips IsVoter on an existing Pod (post-promote)
     and the lag is one reconcile cycle.
   - Cluster controller stays out of Pod patches entirely. The role
     label is the only cross-layer signal and it flows: MemberList →
     cluster patches CR.Status.IsVoter → member patches Pod label.

3. PDB resource:
   - reconcilePDB() emits a per-cluster PodDisruptionBudget owned by
     the EtcdCluster. Selector keys on LabelCluster + LabelRole=
     RoleVoter; only voter-labelled Pods are protected, so learner
     evictions don't consume budget and learner-only Pod kills aren't
     blocked.
   - maxUnavailable = (voterCount - 1) / 2 (integer-divided, auto-
     floored). For voterCount=0 the PDB is deleted: pre-bootstrap,
     paused, or wedged state — nothing to protect.
   - Called from the end of updateStatus, after the existing broken-
     count surface. RBAC marker for policy/v1 podisruptionbudgets and
     for etcdmembers/status (needed for syncIsVoter's patches).
   - SetupWithManager gains Owns(&policyv1.PodDisruptionBudget{}) so
     PDB drift (user/automation deletion) triggers reconcile.

Race characterisation: scale-up holds N voters in PDB while etcd has
N+1 briefly (next reconcile catches up); scale-down's MemberRemove
makes etcd N-1 while PDB still reflects N (k8s PDB controller counts
DeletionTimestamp != nil Pods as unavailable, so the budget naturally
accounts for the in-flight removal). Both windows are one reconcile
wide and quorum-preserving.

Closes part of cozystack#16 (PDB).

Signed-off-by: Timofei Larkin <[email protected]>

* controllers: tests for IsVoter, role label, and PDB

Cluster controller:
- pdbMaxUnavailable table: 0-7 voters covering the floor (1,2 → 0;
  3,4 → 1; 5,6 → 2; 7 → 3).
- reconcilePDB creates a PDB whose Selector restricts to LabelRole=
  RoleVoter (the central correctness property — without it, learner
  evictions would consume budget).
- reconcilePDB updates MaxUnavailable in-place when voter count
  changes (3 → 5 transitions the budget 1 → 2).
- reconcilePDB deletes the PDB when voterCount drops to 0; idempotent
  on subsequent calls.
- syncIsVoter patches Status.IsVoter from a MemberList snapshot, by
  peer URL (not Name — Name is empty in the post-MemberAdd window).
  Covers four cases: up-patch, down-patch, no-op when already
  correct, and skip when member is not yet in etcd's list.
- Bootstrap pre-stamps Status.IsVoter=true on the seed (end-to-end
  through Reconcile, not just a unit test of the helper).

Member controller:
- ensurePod applies LabelRole=RoleVoter when Status.IsVoter=true on
  an existing Pod that lacks the label (steady-state propagation).
- ensurePod strips LabelRole when Status.IsVoter=false on a Pod that
  still has a stale label (the inverse — important for the future
  case where a member could transition from voter back to learner).
- buildPod includes LabelRole=RoleVoter at create time when
  Status.IsVoter is already set (the seed pre-stamp's create-time
  benefit). Negative side: IsVoter=false omits the label entirely
  rather than emitting an empty string that would match a permissive
  selector.

Signed-off-by: Timofei Larkin <[email protected]>

* docs: PodDisruptionBudget section + walk back "no PDB yet" callouts

Concepts.md: new ## PodDisruptionBudget section before ## Conditions.
Covers the selector (LabelCluster + LabelRole=RoleVoter), the (n-1)/2
budget formula, the two-hop voter detection path (cluster controller
writes Status.IsVoter → member controller patches Pod label), the
seed pre-stamp shortcut, the two transient races (scale-up over-
permits by N→N+1 lag, scale-down's Terminating-Pod accounting), and
the zero-voters → delete-PDB behaviour.

Operations.md: new ## Draining nodes (PodDisruptionBudget) section
before ## Recipes. Shows the kubectl get pdb output, the eviction-
denied error users will see, how to identify voter Pods by label,
and why drains might block during scale events.

Walked back the "no PDB yet, tracked in cozystack#16" callouts in the same
files (and in installation.md, README.md, and the StorageMedium type
godoc). The PodDisruptionBudget is now auto-emitted on every cluster;
cozystack#16 still tracks the remaining production gaps (anti-affinity,
container memory limits).

Signed-off-by: Timofei Larkin <[email protected]>

* controllers: route MemberList around learners via IsVoter

The cluster controller's memberEndpoints helper and the member
controller's discoverMemberID peer filter both previously used
MemberReady=True as a proxy for "voter". The original assumption was
that Ready transitively implies non-learner because the member can
only become Ready after its MemberID was discovered via a voter peer.

That assumption is wrong: a learner whose Pod is Running and which
the operator successfully looked up via a voter is itself Ready=True
even though it remains a learner until MemberPromote completes.
clientv3 then round-robins MemberList across the returned endpoints
and hits "rpc not supported for learner" with non-trivial probability,
wedging promote during the scale-up window.

Status.IsVoter (introduced in this PR for the PDB selector) is the
precise signal: it's set true only when etcd's MemberList confirms
IsLearner=false. Switch both call sites to filter on IsVoter instead.
The unfiltered-fallback path is preserved for pre-bootstrap cases
where no voter has been observed yet.

Surfaced on dev4 smoke: 3-replica PVC cluster wedged after seed +
learner came up, operator's promote MemberList kept hitting the
learner endpoint. Tests in TestDiscoverMemberID_* were updated to set
Status.IsVoter on the voter fixtures, matching the new contract.
isMemberReady was dropped (no remaining callers).

Signed-off-by: Timofei Larkin <[email protected]>

* controllers/member: grant Pod patch RBAC for role label

reconcileRoleLabel was added in this PR to keep the Pod's role=voter
label in sync with Status.IsVoter, but the member controller's RBAC
marker only listed get;list;watch;create;delete for pods — patch was
missing. The smoke test on dev4 surfaced this as repeated:

  pods "pdb-test-Xxxxx" is forbidden: User "...controller-manager"
  cannot patch resource "pods" in API group ""

Add patch to the verb list, regenerate role.yaml. Tests don't exercise
RBAC (the fake client doesn't enforce it), so the smoke is what
catches this — the regression check is "operator's pods reconcile
without 'forbidden' errors", which is operationally observable.

(Plus an incidental gofmt fix to a comment alignment in
TestSyncIsVoter_PatchesFromMemberList that `make manifests` triggered.)

Signed-off-by: Timofei Larkin <[email protected]>

* api+controllers: expose /scale on EtcdMember for PDB

The PodDisruptionBudget controller traverses each protected Pod's
controllerRef looking for the /scale subresource on the parent — and
fails closed when it's missing:

  status:
    conditions:
    - reason: SyncFailed
      message: etcdmembers.lllamnyp.su does not implement the scale
               subresource
    expectedPods: 0
    disruptionsAllowed: 0

With expectedPods=0 the PDB doesn't actually gate any disruption,
even though it advertises maxUnavailable=1. The smoke test on dev4
caught this — kubectl get pdb showed the right MaxUnavailable but
ALLOWED DISRUPTIONS stuck at 0 with the SyncFailed condition above.

Each EtcdMember represents exactly one Pod, so /scale's contract is
degenerate: spec.replicas locked to 1 (validation Min=Max=1), status
.replicas pinned to 1, status.selector exposes a label expression
matching the one Pod. The /scale subresource exists only to satisfy
the PDB controller's traversal; it's not a tunable knob.

Cluster controller doesn't need to set spec.replicas explicitly — the
+kubebuilder:default=1 marker means the apiserver fills it in on
EtcdMember Create. Member controller writes the status fields in
updateStatus. Existing TestUpdateStatus_NoChurnInSteadyState was
updated to pre-set Status.Replicas/Selector so the test fixture
matches what a steady-state member looks like.

Signed-off-by: Timofei Larkin <[email protected]>

* docs+test: tighten PDB doc accuracy from review

Five reader-facing accuracy fixes flagged in /branch-review of PR cozystack#19:

1. docs/concepts.md (Transient races, scale-up arm): the quorum
   formula in the worked example used the ceiling-form
   `⌈(N+1)/2⌉+1`, which coincidentally matches the floor-form for
   even M but understates quorum for odd M (M=3 ceiling says 3,
   actual is 2). Etcd's write quorum is `⌊M/2⌋+1`; rewrite the
   parenthetical to use the floor form and state the M=N+1 case
   directly.

2. docs/concepts.md (Transient races, scale-down arm): the doc said
   the k8s PDB controller "counts DeletionTimestamp != nil Pods as
   already unavailable". That's not the mechanism — the PDB
   controller's `currentHealthy` counts Pods whose `Ready` condition
   is True, regardless of DeletionTimestamp. The Terminating Pod
   stops counting toward the budget's healthy total once kubelet
   flips Ready to False at the start of graceful shutdown. End
   conclusion is unchanged; the explanation is corrected.

3. docs/concepts.md (Where the role=voter label comes from): "Two
   reconcile hops" undercounted by one. Cluster writes IsVoter (hop
   1) → member patches Pod label (hop 2) → cluster's next pass picks
   up the new label via reconcilePDB (hop 3). State the full
   end-to-end path and the cycle count.

4. docs/operations.md (Why drains might block during scale events):
   "millisecond-wide race window" was wrong. Steady-state
   RequeueAfter is 30s, so the race window is one reconcile cycle
   wide (up to ~30s in the worst case). Soften the wording to match.

5. controllers/etcdmember_controller_test.go: rename
   TestDiscoverMemberID_ExcludesNonReadyPeers →
   TestDiscoverMemberID_ExcludesNonVoterPeers and rewrite its
   docstring. The test's substance was updated to use Status.IsVoter
   in the previous commit; the name and docstring still described
   the older Ready-based filter.

Signed-off-by: Timofei Larkin <[email protected]>

---------

Signed-off-by: Timofei Larkin <[email protected]>