Skip to content

Fill basic project information - #17

Merged
Andrei Kvapil (kvaps) merged 1 commit into
mainfrom
basic-readme
Mar 13, 2024
Merged

Andrei Kvapil (kvaps) merged 1 commit into
mainfrom
basic-readme

Conversation

@kvaps

@kvaps Andrei Kvapil (kvaps) commented Mar 13, 2024 •

Copy link
Copy Markdown
Member

Signed-off-by: Andrei Kvapil <[email protected]>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Comment thread CONTRIBUTING.md
* Documentation
* Issue Triage
* Answering questions on Slack or Github Discussions
* Web design

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Web design? :)

@kvaps
Andrei Kvapil (kvaps) merged commit b984cad into main Mar 13, 2024
Terry Tang (saymololy) pushed a commit to saymololy/etcd-operator that referenced this pull request Nov 2, 2026
* 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]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fill basic project information

5 participants