Repository navigation
Fill basic project information - #17
Merged
Merged
Conversation
Signed-off-by: Andrei Kvapil <[email protected]>
Kirill Ilin (sircthulhu)
approved these changes
Mar 13, 2024
Sergey Shevchenko (sergeyshevch)
approved these changes
Mar 13, 2024
Sergey Shevchenko (sergeyshevch)
left a comment
Contributor
There was a problem hiding this comment.
LGTM
György Gaál (gaalw)
approved these changes
Mar 13, 2024
Kirill-Garbar
approved these changes
Mar 13, 2024
| * Documentation | ||
| * Issue Triage | ||
| * Answering questions on Slack or Github Discussions | ||
| * Web design |
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]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
demo: https://github.com/aenix-io/etcd-operator/tree/basic-readme
fixes #10