Basic EtcdCluster custom resource (#9) - #15

Fixes from comments #15 
- Fix API group name for custom resources
- remove unnecessary comment lines

* api: add StorageMedium enum and PodUID status

Add the API surface for memory-backed storage:

- StorageMedium enum on EtcdClusterSpec, mirrored to EtcdMemberSpec and
  Status.Observed. "" means PVC (default, unchanged behaviour); "Memory"
  means tmpfs emptyDir whose lifetime is bound to the Pod.
- Status.PodUID on EtcdMember, used by a later commit to detect Pod loss
  (tmpfs gone) for memory-backed members and trigger replacement.

No behavioural change in this commit — fields are added but not yet read
by either controller. The kubebuilder Enum marker is the only validation;
deeper rules (StorageMedium immutability, replicas=0 ban, storage size
required) belong in an admission webhook (issue cozystack#15) and are not added
to the controller.

Documented operational caveats (no PDB / anti-affinity / resource limits
yet) reference issue cozystack#16 from the field godoc so a user reading the CRD
documentation lands on the right tracking issue.

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

* controllers/cluster: propagate StorageMedium

Echo EtcdCluster.spec.storageMedium onto new EtcdMember CRs at both
seed-create and learner-create sites. Snapshot it into status.observed
during the locking-pattern update; include it in the spec-vs-observed
equality check so a mid-flight medium edit is locked out like any other
spec change.

No behavioural branching on StorageMedium yet — the member controller's
volume-source switch lands in a follow-up commit.

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

* controllers/member: tmpfs volume + skip PVC + track PodUID

For StorageMedium=Memory:

- buildPod emits EmptyDir{Medium:Memory, SizeLimit:&storage} instead of a
  PVC-backed volume. SizeLimit caps tmpfs allocation; container memory
  limits and other production hardening are tracked in cozystack#16.
- ensurePVC short-circuits, leaving Status.PVCName empty. The PVC owner
  check still applies to PVC-backed members.
- updateStatus only writes Status.PVCName for PVC-backed members.

Pod-UID tracking (used by the upcoming Pod-loss detection commit):
ensurePod records Pod.UID into Status.PodUID on both find-existing and
create-new paths; updateStatus keeps it in sync on each reconcile;
ensurePodAbsent (the dormant path) clears it so the deliberate-absence
state is distinguishable from a Pod-loss event.

No callers of Status.PodUID yet — that wiring lands next.

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

* controllers/member: self-delete memory member on pod loss

When a memory-backed member's recorded Pod is gone or has been replaced
by a Pod with a different UID, the tmpfs is gone with it — the data
cannot be recovered. Recreating a fresh Pod would silently fail to
rejoin the cluster (member ID is in raft state but the WAL is empty),
leaving the member permanently wedged.

Self-delete the EtcdMember CR instead. The existing finalizer flow
removes the member from etcd via MemberRemove (when peers are reachable)
and GC takes the rest. The cluster controller's existing scale-up gap-
fill creates a replacement EtcdMember with a fresh GenerateName and
fresh etcd member ID.

If quorum is already lost across multiple simultaneous Pod losses,
MemberRemove will fail and the surviving members stay in Terminating
until quorum returns — the correct outcome for memory clusters past the
point of no return.

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

* controllers: isBroken predicate fires on memory-member pod loss

Replace the stub with a real check for memory-backed members: a member
whose Pod UID was recorded but whose Pod is no longer observed is
broken (the tmpfs is gone with the Pod and the data cannot be
recovered).

The member controller already self-deletes such members in the next
reconcile pass; this predicate's job is to surface the count on
EtcdCluster.status.brokenMembers during the brief observation window
before that delete fires, so operators get a stable signal even when
the CR is in the process of being torn down.

PVC-backed auto-replacement stays a stub — that's a richer policy
question (corruption, irrecoverable crashloop) that this PR doesn't
decide.

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

* controllers: tests for memory-backed storage

Cover the new mechanics:

- buildPod with StorageMedium=Memory emits EmptyDir{Medium:Memory,
  SizeLimit:storage} and zero PVC volumes. Negative guard: an empty
  StorageMedium still produces a PVC-backed volume so the default path
  is not silently flipped.
- ensurePVC short-circuits for memory members (no PVC created;
  Status.PVCName stays empty).
- ensurePod captures Pod.UID into Status.PodUID via the Get path (the
  Create path can't be tested reliably — controller-runtime's fake
  client does not auto-assign UIDs and the steady-state reconcile uses
  Get anyway).
- A reconcile against a memory member with a recorded PodUID and no live
  Pod self-deletes the EtcdMember and does NOT recreate the Pod.
- A reconcile against a memory member with a stable Pod (UID matches)
  is left alone — otherwise the reconciler would churn itself to death.
- updateStatus leaves Status.PVCName empty for memory members so the
  cluster controller's Paused message can't claim "data preserved on
  PVC data-X" when there is no PVC.
- isBroken table-test pins the predicate across the four relevant
  cases: memory-with-lost-Pod (broken), memory-with-Pod (healthy),
  memory-no-UID-yet (not broken on first reconcile), and PVC-backed
  (stub stays false).
- Cluster controller propagates StorageMedium onto the seed EtcdMember
  and into Status.Observed.StorageMedium; specEqualsObserved treats a
  medium mismatch as "spec differs" so the locking pattern protects
  against a mid-flight flip.

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

* docs: memory-backed storage

- README: bullet under "What's supported", flagging the wedge with
  replicas=0 and the missing production defaults.
- concepts.md: new "Storage" section with the PVC vs Memory trade-off
  table, the Pod-loss detection model, and the known limitations
  (PDB/anti-affinity/limits not auto-emitted yet → cozystack#16; replicas=0
  combination wedges until webhook → cozystack#15). The "no automatic
  broken-member replacement" bullet now correctly notes that memory
  members do auto-replace; only PVC-backed isBroken stays a stub.
- operations.md: Memory-backed clusters section with verification
  recipe (jq the Pod's volume + check mount inside the container),
  the three things to pre-deploy for a production memory cluster
  (PodDisruptionBudget, anti-affinity, container memory limits via
  LimitRange), how Pod-loss auto-replacement plays out, and an
  explicit "don't pause" warning.
- installation.md: a sample memory-backed EtcdCluster manifest with
  pointers to concepts/operations for the caveats.

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

* Address review of feat/memory-storage

Two blockers from review:

1. docs/operations.md "Broken member" section still said isBroken is a
   stub. After this PR it's a real predicate for memory members (even
   though it doesn't observably surface — see below). Reword to qualify
   "PVC-backed members" and point at the Memory-backed clusters section
   for the auto-replacement story.

2. docs/concepts.md and controllers/helpers.go both promised that
   Status.BrokenMembers would surface a brief non-zero window during
   memory pod-loss. The controller flow makes this not true in
   practice: the member-controller's loss path self-deletes the
   EtcdMember in the same reconcile pass, and the cluster-controller's
   updateStatus iterates the running set (filterRunningMembers
   excludes DeletionTimestamp != nil), so the predicate is never
   consulted for the lost member. Walk the claim back rather than
   adding sub-reconcile Status writes purely for cosmetic visibility:
   the brief window would be too short for any kubectl-poll or scrape
   to catch and would not change any operator action. Update both the
   doc paragraph and the isBroken comment to be honest about the
   predicate's current placeholder role.

   Add TestBrokenMembers_StaysZeroAcrossMemoryPodLoss as the integration
   contract: with both controllers exercised across a memory pod-loss
   + member-deletion cycle, BrokenMembers stays at 0. The dedicated
   unit test for the predicate's pure logic
   (TestIsBroken_MemoryMemberWithLostPodIsBroken) stays — it pins the
   intent for the future hook the predicate exists to be.

Non-blocking note worth addressing in this PR: add a Detection latency
paragraph to docs/operations.md noting that node-NotReady failures are
gated by kube-controller-manager's --pod-eviction-timeout (default
5 min), so unannounced node failures surface as up to 5 minutes of
degraded quorum before auto-replacement kicks in. Tuning that timeout
is outside the operator's control; documenting the budget matters
because the rest of the runbook leads readers to expect ~seconds.

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

* controllers: honest Paused message for memory-backed clusters

When a user pauses a memory-backed cluster (replicas: 0 — not yet
gated by an admission webhook, see cozystack#15), the scale-down code path runs
unchanged: the survivor is flipped to spec.dormant=true and its Pod is
deleted. findDormantMember returns the survivor and the existing
Paused branch publishes:

  Available=False/Paused: cluster is paused (spec.replicas=0); data is
  preserved on PVC data-<name>

This is actively false — the PVC never existed (ensurePVC was skipped
for the memory member), the tmpfs evaporated with the Pod, and the
cluster cannot be resumed. Operators reading the condition message
(directly, via runbooks, or via dashboards keyed on
.status.conditions[?(@.type=='Available')].message) would be misled
about durability the cluster doesn't have.

Tighten the Paused branch to special-case the memory medium with a
message that names tmpfs and points the user at recreating the
cluster. The condition stays Available=False/Paused — the cluster is
paused, the wedge is real — only the message changes.

The admission webhook in cozystack#15 will supersede this guard by rejecting
replicas=0+memory at create/update time; until that lands, this is
the cheap, honest fallback.

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

* docs+test: README catchup, drop fragile three-phase test

README's "What's not supported" line was the only doc still claiming
"No automatic broken-member replacement (the predicate is stubbed)".
After the previous review pass walked back the BrokenMembers surfacing
claim and the operations / concepts docs landed on "memory members
auto-replace on Pod loss; PVC-backed isBroken stays a stub;
BrokenMembers reads 0 in practice", the README slipped through. Bring
it in line — no behaviour change.

Drop TestBrokenMembers_StaysZeroAcrossMemoryPodLoss. The contract it
pinned (BrokenMembers stays 0 across a memory pod-loss cycle) is
already covered by TestIsBroken_MemoryMemberWithLostPodIsBroken (the
predicate's truth table) plus TestReconcile_MemoryMemberDeletesSelfOn
PodLoss (the member-controller self-delete path). The three-phase test
duplicated those, depended on the current reconcile ordering, and
would break whenever isBroken is refined with better liveness gates —
which is its intended evolution per the helpers.go comment.

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

---------

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

…ozystack#18)

* api: CEL XValidation rules on EtcdClusterSpec (cozystack#15)

Apiserver-side enforcement (k8s 1.28+) for the four validation rules
the previous PR's docs flagged as "user must avoid this":

  - StorageMedium immutable. Flipping the medium on an existing cluster
    would orphan the previous PVC (or tmpfs) and the rolling-migrate
    path is not implemented. Default the field to "" so oldSelf is
    always present on Update — CEL transition rules skip when oldSelf
    is absent, which would otherwise let absent → Memory slip past
    immutability.

  - Storage > 0 when StorageMedium=Memory. Zero or unset Storage with
    Memory medium produces an unbounded tmpfs; reject at admission.

  - Replicas=0 with Memory rejected. The previously documented wedge
    (dormant flip deletes the Pod, tmpfs evaporates, resume path
    recreates the member as if data were preserved → empty data dir,
    etcd refuses to start). Apiserver now blocks the combination.

  - Storage cannot shrink. PVCs cannot shrink and tmpfs SizeLimit
    reduction does not free already-allocated memory.

Implementation entirely in +kubebuilder:validation:XValidation markers.
No webhook, no Service, no cert-manager dependency — CEL is compiled
into the CRD's x-kubernetes-validations and evaluated by the apiserver
directly. `quantity()` library required (k8s 1.28+, already the
project's documented floor).

go.mod gains github.com/blang/semver/v4 as an indirect transitive of
controller-runtime's envtest package, which the next commit needs.

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

* api: envtest harness + CEL rule tests

CEL XValidation runs apiserver-side, so the only way to test these
contracts is to spin up a real apiserver via controller-runtime's
envtest, install the project's CRDs, and observe accept/reject
behaviour on Create and Update.

TestMain skips the package entirely when KUBEBUILDER_ASSETS is unset
so dev-IDE runs (`go test ./...`) don't fail; the Makefile's `make
test` target sets it via `setup-envtest use`. Existing make test
integration continues to work; this PR adds the first envtest user.

Tests:
  - StorageMedium immutable: create PVC-backed, attempt to flip to
    Memory on Update, expect rejection.
  - Storage > 0 for Memory: create with Memory + storage=0, expect
    rejection at Create time.
  - Replicas=0 + Memory: rejected on Create and on transition 3 → 0.
  - Storage shrink: 1Gi → 512Mi rejected; 1Gi → 2Gi accepted (negative
    guard that the shrink rule isn't a no-grow rule).
  - Happy path: PVC default, Memory with non-zero storage, and PVC
    with replicas=0 (the wedge rule must only fire for Memory) all
    Create successfully — guards against rule inversion or over-
    broad CEL expressions.
Signed-off-by: Timofei Larkin <[email protected]>

* docs: walk back wedge caveats; document CEL validation

The memory-storage PR's docs introduced "this combination wedges, the
admission webhook in cozystack#15 will fix it, until then don't do this"
caveats across concepts, operations, installation, and the API
godoc. With the CEL rules now landed on the CRD itself, the apiserver
rejects those combinations at admission. Update the docs to reflect:

- concepts.md: replace "Known limitation: replicas=0 wedges" with an
  "Apiserver-enforced validation" table listing the four rules, when
  each fires, and why.
- operations.md: replace "Do not set replicas=0 on memory clusters"
  with the actual rejection-error transcript from the apiserver.
- installation.md: drop the "cozystack#15 wedge to avoid" parenthetical.
- README.md: add an "Apiserver-enforced validation" bullet under
  "What's supported"; drop the "Not safe to combine with replicas=0
  yet" qualifier since it's no longer possible.

The k8s 1.28+ floor for the quantity() CEL library is consistent with
the project's existing minimum (1.28+ already documented in
installation.md prerequisites).

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

* api: coerce storage to string before quantity() (CEL int-or-string)

CEL's quantity() function accepts only string input. spec.storage in
the CRD schema is x-kubernetes-int-or-string, so a kubectl-style YAML
like `storage: 0` arrives at the apiserver as a JSON integer rather
than a quantity string. Without explicit string() coercion CEL trips
"no such overload: quantity(int)" and the user sees that runtime error
instead of the rule's message field. Both rules that call quantity()
now wrap the field in string() first.

The typed Go API always emits Quantity values as strings, which is why
the original envtest cases passed — they couldn't reach the integer
path. The smoke test on dev4 surfaced this immediately on
`kubectl apply` of a raw YAML.

Add TestCEL_StorageMustBeNonZero_IntegerInput using
unstructured.Unstructured to send int64(0) directly; pins the
contract that the rule's human-readable message must surface for
either input form.

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

* Address review of feat/cel-validation

Two stale comments still claimed the admission gate was future work
("admission webhook in cozystack#15 will reject ...", "Until admission gates
the replicas=0+memory combination (issue cozystack#15) ..."). Both predate the
CEL rule landing in the same PR. Rewrite to describe the defensive
runtime branch correctly: the CEL rule blocks the combination on
Create/Update, but an operator upgrade onto a pre-existing dormant
memory cluster (which slipped in before the rule was installed) still
reaches the Paused-message branch — defence in depth, not future work.

Non-blocking observation: the `quantity()` CEL library was alpha in
1.28 (behind CRDValidationRatcheting) and stable in 1.29. The strict
floor for the CRD to install cleanly is 1.29, not 1.28; the previous
PR's "1.28+ tested" carry-over was technically right for alpha but
risks support requests from 1.28 users with the gate off. Tighten
README, docs/concepts.md, and docs/installation.md to "1.29+".

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

* Address review of CEL validation PR

Two of the three blockers raised were real:

1. docs/concepts.md and docs/installation.md attributed the quantity()
   gating to CRDValidationRatcheting. Wrong gate: that's KEP-4008
   (allow lingering schema violations on Update, GA in 1.33), and is
   unrelated to CEL. The actual gating story is:

     - CEL CRD validation (CustomResourceValidationExpressions): beta
       1.25, GA 1.29.
     - quantity() CEL extension: added in 1.28.

   Safe floor for both pieces GA is 1.29; that's also the floor the
   docs already settled on for unrelated reasons. Rewrite the paragraph
   in concepts.md and the installation.md prerequisite row to describe
   reality and drop the CRDValidationRatcheting red herring entirely —
   it would actively mislead a cluster admin toggling the wrong gate.

2. The Go doc comment on EtcdClusterSpec (which controller-gen carries
   through to the generated CRD YAML's `description` field) still said
   "k8s 1.28+ apiserver-enforced". Operators reading
   `kubectl explain etcdcluster.spec` would see 1.28+ while the docs
   say 1.29+. Bump the godoc to match; regenerate the CRD YAML so the
   apiserver-served schema agrees.

The reviewer also raised a fourth "blocker" about documenting the
difference between Gi (binary) and G (decimal) quantity units in case
a user is surprised that storage: 1Gi → 1G is rejected as a shrink.
Not addressed: that's standard Kubernetes resource.Quantity semantics
(every PVC, memory limit, CPU request, ... behaves identically), and
documenting it in this operator's docs sets the precedent that every
operator should re-document k8s primitives. The CEL rule's error
"spec.storage cannot be shrunk" is correct and terse; users surprised
by 1Gi vs 1G need k8s docs, not ours.

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

---------

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