GHA docker publish pipeline - #4

Architectural change — Observed/ProgressDeadline locking pattern
================================================================
The cluster controller now reconciles toward EtcdCluster.status.observed,
not directly against spec. Spec changes are noticed but not acted on
until the in-flight target is met (or the deadline expires). This
addresses two distinct race conditions the reviewer flagged:

  - cozystack#1 (scale-up immediately followed by scale-down): in the old code,
    a user reverting spec.replicas after MemberAdd but before the new
    pod's MemberID is populated would orphan the etcd member. The
    locking pattern means the second scale doesn't take effect until
    the first one has settled.

  - cozystack#5 (mid-bootstrap replica change): if spec.replicas changed between
    the first and last bootstrap member's creation, the --initial-cluster
    flags would diverge and etcd couldn't form. observed.replicas locks
    the bootstrap target size at the first reconcile.

User-facing escape hatch: status.progressDeadline. Patch it to a past
time via `kubectl patch --subresource=status` to abort a stuck reconcile
(documented in README).

Specific reviewer fixes
=======================
  cozystack#1: removeMemberFromEtcd now falls back to MemberList lookup by name
      (or peer URL) when status.MemberID is empty. Belt-and-suspenders
      with the locking pattern.

  cozystack#2: ensurePVC refuses to reuse a same-named PVC owned by a different
      EtcdMember UID. Same-named scale-down/up cycles can't bind the
      new pod to the prior member's data dir.

  cozystack#3: MemberReady=True is now gated on status.MemberID being populated.
      A pod that's PodReady but whose memberID we haven't learned yet
      reports Ready=False, Reason=DiscoveringMemberID.

  cozystack#4: tryDiscoverCluster logs etcd-client and MemberList errors and
      sets Available=False, Reason=ClusterUnreachable on the cluster
      so users can see what's blocking the bootstrap.

  cozystack#6: isBroken predicate is wired into the steady-state path; the
      result is exposed as status.brokenMembers. The predicate still
      always returns false (auto-replacement policy is not yet decided)
      but it has a tested call site instead of being dead code.

  cozystack#7: README is no longer kubebuilder boilerplate. Documents the
      v1alpha2 model, the locking pattern, the abort-stuck-reconcile
      recipe, and supported/unsupported features.

  cozystack#8: config/samples/kustomization.yaml added so `kubectl apply -k
      config/samples/` works as advertised.

Recommended (non-blocking) items also addressed:
  - cluster-controller RBAC trimmed to get;list;watch on etcdclusters
    (only status uses update;patch).
  - ensureService documents that drift on existing Services is
    intentionally not reconciled.

Test infrastructure
===================
Introduced an EtcdClusterClient interface and EtcdClientFactory so
tests can inject a fake etcd. SetupWithManager defaults the factory
to DefaultEtcdClientFactory in production. All 13 new unit tests use
controller-runtime's fake client + an in-memory fakeEtcd to drive the
reconcilers and assert on K8s + etcd state, with no envtest dependency.

Verified end-to-end on a live cluster (bootstrap → scale 3→5 → scale
5→3 with consistent etcd member list → cluster deletion).

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Signed-off-by: Timofei Larkin <[email protected]>

…edGeneration

cozystack#1 (scaleUp not crash-safe): MemberAdd is idempotent now. Before calling
it, scaleUp does a MemberList and checks whether a member with our peerURL
is already registered (the symptom of a crash between MemberAdd and
Create(EtcdMember) on a previous reconcile). If so, skip MemberAdd and
proceed to creating the CR. MemberAdd stays in the cluster controller.

cozystack#2 (bootstrap consistency on resume): added a test asserting that when
bootstrap() resumes with some EtcdMembers already present, the
InitialCluster value computed for the missing members exactly matches the
one stored on the existing CRs. Belt-and-suspenders for the locking
pattern; previously implicit, now pinned.

cozystack#3 (concurrent scale-down race): Reconcile checks whether any EtcdMember
already has DeletionTimestamp set before deciding to issue another delete.
If one is in flight, requeue and wait. The cluster controller stays the
decision-maker; the EtcdMember finalizer remains the actuator that calls
MemberRemove. This eliminates the race where two finalizers run
MemberRemove against stale snapshots of the etcd cluster.

cozystack#4 (Status().Update errors swallowed): tryDiscoverCluster now checks the
return value. Conflicts requeue; other errors propagate so
controller-runtime's backoff applies.

cozystack#5 (status churn while parked): two changes work together. handleDeadline
Exceeded now returns ctrl.Result{} (no requeue) once it has set the
terminal conditions — the watch on the EtcdCluster resource is enough to
wake us up if the user edits spec to recover. setClusterCondition returns
a "changed" bool by comparing against the existing condition; callers
skip Status().Update when nothing changed. ResourceVersion stays stable
across no-op terminal reconciles (verified by test).

cozystack#7 (storage doc drift): README's "What's not supported" now calls out
that spec.storage edits don't resize existing PVCs. Filed cozystack#2 to track
the resize feature.

cozystack#8 (ObservedGeneration on conditions): setClusterCondition stamps
cluster.Generation; setMemberCondition (new helper) stamps
member.Generation. All cluster-side conditions now go through
setClusterCondition; all member-side conditions through
setMemberCondition.

UID alignment: pod runs as 65532 (distroless nonroot convention; matches
the operator's own pod and kubebuilder default) instead of an arbitrary
60000. The official quay.io/coreos/etcd image runs as root by default;
fsGroup ensures the data dir is writable as 65532.

Tests: 7 new unit tests pinning each fix. All 21 tests pass.
Verified end-to-end on dev4 — bootstrap completes with conditions
carrying observedGeneration=1 and Progressing=False/Reconciled.

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Signed-off-by: Timofei Larkin <[email protected]>

Bootstrap simplification (architectural)
========================================
Bootstrap now creates a single seed member (cluster-0) with
--initial-cluster listing only itself. Subsequent members join via
MemberAdd once ClusterID is latched. This eliminates the entire
"multiple bootstrapping members must agree on --initial-cluster"
coordination hazard and shrinks the surface area for the reviewer's
historical cozystack#5 (mid-bootstrap replica change) to zero.

Concrete reviewer fixes
=======================
  cozystack#1: in-flight-deletion gate is hoisted out of the scaleDown branch
      and now applies to scaleUp as well. Both directions must wait
      for any EtcdMember with DeletionTimestamp to fully GC before
      issuing another etcd-cluster mutation — concurrent MemberAdd and
      MemberRemove racing across stale snapshots was the symmetric
      hazard.

  cozystack#2: tryDiscoverCluster only latches ClusterID when MemberList
      returns exactly one member matching the seed by name or peer
      URL. Anything else (stale response, propagation lag, spurious
      member) is treated as "not yet definitive" and retried.

  cozystack#3: removeMemberFromEtcd errors out when other EtcdMembers exist on
      the CR side but none have a PodName recorded (transient state).
      Previously returned nil and silently let the finalizer clear,
      orphaning the etcd-side member.

  cozystack#4: handleDeletion propagates non-NotFound errors from the
      owner-cluster Get instead of treating apiserver flakes as
      "cluster alive". controller-runtime applies backoff.

  cozystack#5: README's BootstrapFailed recovery section tells the user to
      wait until prior EtcdMembers and PVCs have GC'd before
      re-applying. The tightened pvcOwnedBy refuses to reuse a PVC
      owned by a stale UID, so a fast re-apply otherwise sticks.

Recommended items
=================
  - deriveClusterToken now includes namespace + UID so two
    same-named clusters across namespaces never share a token.
    Existing clusters preserved via status.clusterToken snapshot.
  - Cluster ID and member ID now formatted as %016x for parity with
    etcdctl's output and so leading zeros don't get stripped.

Followup issues
===============
  - cozystack#5: flip Spec.Bootstrap=false once joined, to prevent phantom-
    cluster bootstrap if the seed pod restarts with a fresh PVC.
    Deferred because it requires a Pod-recreate-on-spec-change
    branch in ensurePod that we'll need for version upgrades
    anyway.

Tests
=====
30 unit tests pass. New tests:
  - TestBootstrap_CreatesSingleSeedMember
  - TestBootstrap_Idempotent
  - TestObservedSpec_LocksMidFlight (replaces the now-obsolete
    LocksReplicasMidBootstrap)
  - TestScaleUp_WaitsForInFlightDeletion
  - TestTryDiscoverCluster_RejectsPartialMembership
  - TestTryDiscoverCluster_LatchesOnValidResponse
  - TestRemoveMemberFromEtcd_PeerWithEmptyPodNameRetries
  - TestHandleDeletion_TransientGetErrorReturnsError

Verified end-to-end on dev4: single-node bootstrap → scale-up to 3 →
scale-up to 5 → scale-down to 3 → delete, etcd member list stays
consistent across every transition, clusterID zero-padded, conditions
carry observedGeneration=1.

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Signed-off-by: Timofei Larkin <[email protected]>

…status

Architectural change — learner-mode scale-up
============================================
Scale-up now adds members as learners (MemberAddAsLearner) and promotes
them (MemberPromote) once etcd reports they have caught up on the raft
log. Voting quorum does not shift until promotion, so an unreachable
joiner can never destabilise the existing cluster (reviewer blocker cozystack#1
on the previous round).

The flow is now:
  1. Reconcile gates on every existing EtcdMember reporting Ready=True
     before starting another scale-up step. allMembersReady() returns
     false → requeue 5s.
  2. scaleUp first asks etcd for the member list. If any member is a
     learner, MemberPromote — on success, Requeue; on
     ErrLearnerNotReady, RequeueAfter 5s (just wait).
  3. Otherwise, MemberAddAsLearner + create EtcdMember CR. Crash-safety
     check still skips MemberAddAsLearner when the peerURL is already
     in etcd's list (handles operator restart between add and CR
     creation).
  4. Critical subtlety found via live smoke: the iteration that adds
     the *last* learner reaches current==desired before that learner
     gets promoted. The steady-state branch now calls
     promotePendingLearner too — same logic, no extra add. Without
     this the final member stays a learner forever.

Other reviewer blockers
=======================
  cozystack#2 — discoverMemberID falls back to PeerURL matching when etcd
       reports a member with empty Name (the window between
       MemberAddAsLearner and the joiner reporting in). Mirrors what
       resolveMemberID already did.
  cozystack#3 — Reconcile short-circuits on cluster.DeletionTimestamp. No more
       creating Services for a Terminating cluster.
  cozystack#4 — setMemberCondition returns a bool, mirroring setClusterCondition.
       updateStatus tracks "changed" across PodName/PVCName/MemberID
       and condition writes; if nothing changed, Status().Update is
       skipped. ResourceVersion stable across no-op steady-state
       reconciles (verified by test).
  cozystack#5 — gofmt -w on previously drifted files.
  cozystack#6 — README documents the learner-add-then-promote flow.

EtcdClusterClient interface gains MemberAddAsLearner / MemberPromote.
fakeEtcd tracks IsLearner state and exposes addLearnerCalls /
promoteCalls for assertion.

Tests
=====
44 unit tests, six new:
  - TestScaleUp_WaitsForAllMembersReady (cozystack#1 gate)
  - TestScaleUp_AddsAsLearner (cozystack#1 happy path)
  - TestScaleUp_PromotesExistingLearner (cozystack#1 second half)
  - TestReconcile_PromotesLearnerWhenCurrentEqualsDesired (post-final
    add — caught only by live smoke testing)
  - TestReconcile_ShortCircuitsOnDeletion (cozystack#3)
  - TestDiscoverMemberID_FallsBackToPeerURL (cozystack#2)
  - TestUpdateStatus_NoChurnInSteadyState (cozystack#4)

Verified end-to-end on dev4: 5-member cluster bootstraps, learners
appear briefly in etcdctl member list with IS LEARNER=true, then get
promoted. Final steady state has IS LEARNER=false for all 5.

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Signed-off-by: Timofei Larkin <[email protected]>