Skip to content

fix(controllers): extend crash-loop self-heal to memory members - #352

Merged
Timofei Larkin (lllamnyp) merged 1 commit into
cozystack:mainfrom
kjvalencik:kj/memory-crashloop-selfheal
Aug 3, 2026
Merged

fix(controllers): extend crash-loop self-heal to memory members#352
Timofei Larkin (lllamnyp) merged 1 commit into
cozystack:mainfrom
kjvalencik:kj/memory-crashloop-selfheal

Conversation

@kjvalencik

@kjvalencik K.J. Valencik (kjvalencik) commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

The crash-loop self-heal (#336) excluded memory-medium members on the theory that the pod-loss self-heal already covers them. That check only fires when the Pod is gone or replaced (Status.PodUID mismatch); a memory member whose Pod is alive but whose etcd can never start keeps the same Pod UID forever and is invisible to it. The operator therefore left such a member crash-looping unrecovered forever.

The live failure mode: a replacement learner is created with --initial-cluster baked into its immutable Pod spec; if membership changes again before the learner's first successful boot, etcd fatals with error validating peerURLs ...: member count is unequal on every restart. With max-learners=1 the wedged learner also blocks all further member replacement, so the cluster stays degraded until a human (or a node rotation that happens to kill the Pod) intervenes.

Drop the medium exclusion. Every other guard stays: non-bootstrap, etcdContainerStuck (not ready, restart threshold, not OOMKilled, Pod not terminating) and the quorum gate. Replacing a stuck memory member is strictly milder than the already-covered PVC case — its data dies with the Pod anyway.

Includes a regression test (TestUpdateStatus_ReplacesStuckMemoryMember) and doc updates in docs/concepts.md / docs/operations.md.

Related: #351

Summary by CodeRabbit

  • Bug Fixes

    • Extended crash-loop self-healing to memory-backed members, enabling automatic replacement when cluster quorum is retained.
  • Documentation

    • Clarified that crash-loop replacement applies to both memory-backed and PVC-backed members.
    • Documented behavior for stale membership, lost data directories, and memory members without PVC cleanup.

@gemini-code-assist

Copy link
Copy Markdown

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

@coderabbitai

coderabbitai Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 781ab2c1-2a76-46cb-81ec-4bd0c7aea594

📥 Commits

Reviewing files that changed from the base of the PR and between fe2b15e and 0ecaf81.

📒 Files selected for processing (4)
  • controllers/etcdmember_controller.go
  • controllers/etcdmember_controller_test.go
  • docs/concepts.md
  • docs/operations.md
🚧 Files skipped from review as they are similar to previous changes (4)
  • controllers/etcdmember_controller.go
  • controllers/etcdmember_controller_test.go
  • docs/operations.md
  • docs/concepts.md

📝 Walkthrough

Walkthrough

Crash-loop self-healing now applies to eligible memory-backed members as well as PVC-backed members. A regression test covers replacement when quorum remains available. Documentation describes the expanded behavior.

Changes

Crash-loop recovery

Layer / File(s) Summary
Memory member self-healing and regression coverage
controllers/etcdmember_controller.go, controllers/etcdmember_controller_test.go
The controller allows replacement of persistently crash-looping non-bootstrap members without requiring PVC storage. The test verifies deletion of a stuck memory-backed member when the cluster retains quorum.
Recovery documentation
docs/concepts.md, docs/operations.md
The documentation covers memory-backed and PVC-backed members, quorum protection, replacement learners, and PVC cleanup only when a PVC exists.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

Suggested labels: bugfix

Suggested reviewers: androndo, lllamnyp

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: extending crash-loop self-healing to memory members.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added controllers documentation Improvements or additions to documentation labels Jul 31, 2026

@coderabbitai coderabbitai Bot left a comment

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.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/concepts.md`:
- Around line 160-166: Document that crash-loop self-healing applies only when
EtcdMember.Spec.Bootstrap is false: qualify the trigger and replacement flow in
docs/concepts.md lines 160-166, and qualify the crash-loop replacement exception
in both storage rows at docs/concepts.md lines 130-131. Keep bootstrap members
excluded from this recovery behavior.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 88795c74-1f40-4e63-95a9-ba3ac4d66c3f

📥 Commits

Reviewing files that changed from the base of the PR and between 9c5d896 and ffe7e26.

📒 Files selected for processing (4)
  • controllers/etcdmember_controller.go
  • controllers/etcdmember_controller_test.go
  • docs/concepts.md
  • docs/operations.md

Comment thread docs/concepts.md
@lllamnyp

Copy link
Copy Markdown
Member

Hi K.J. Valencik (@kjvalencik) , thanks for your amazing work here and on #351. Could you please run git commit --amend --signoff --no-edit && git push --force-with-lease on this PR so the DCO check passes?

The crash-loop self-heal (cozystack#336) excluded memory-medium members on the
theory that the pod-loss self-heal already covers them. That check only
fires when the Pod is gone or replaced (Status.PodUID mismatch); a
memory member whose Pod is alive but whose etcd can never start keeps
the same Pod UID forever and is invisible to it.

The live failure mode: a replacement learner is created with
--initial-cluster baked into its immutable Pod spec; if membership
changes again before the learner's first successful boot, etcd fatals
with "error validating peerURLs ...: member count is unequal" on every
restart. With max-learners=1 the wedged learner also blocks all further
member replacement, so the cluster stays degraded until a human (or a
node rotation that happens to kill the Pod) intervenes.

Drop the medium exclusion. Every other guard stays: non-bootstrap,
etcdContainerStuck (not ready, restart threshold, not OOMKilled, Pod not
terminating) and the quorum gate. Replacing a stuck memory member is
strictly milder than the already-covered PVC case — its data dies with
the Pod anyway.

Related: cozystack#351
Signed-off-by: K.J. Valencik <kjvalencik@gmail.com>
@kjvalencik

Copy link
Copy Markdown
Contributor Author

Done, thanks!

@lllamnyp
Timofei Larkin (lllamnyp) merged commit 24f3892 into cozystack:main Aug 3, 2026
12 of 14 checks passed
myasnikovdaniil added a commit to cozystack/cozystack that referenced this pull request Aug 4, 2026
## What this PR does

Bumps the cozystack etcd-operator packages from **v0.5.3 to v0.5.4**.

`v0.5.4` is a controller bug-fix release — no API, RBAC or values
changes:

- fix(controllers): derive `--initial-cluster-state` from phase, not
from the seed
([cozystack/etcd-operator#355](cozystack/etcd-operator#355))
- fix(controllers): stop exempting the bootstrap seed from self-heal
([cozystack/etcd-operator#354](cozystack/etcd-operator#354))
- fix(controllers): extend crash-loop self-heal to memory members
([cozystack/etcd-operator#352](cozystack/etcd-operator#352))
- fix(controllers): switch the PDB from `maxUnavailable` to
`minAvailable`
([cozystack/etcd-operator#351](cozystack/etcd-operator#351))

Changes in this repo:

- `packages/system/etcd-operator/Chart.yaml` — `appVersion: v0.5.3 →
v0.5.4`. The manager image tag defaults to `.Chart.AppVersion`
(`values.yaml` keeps `tag: ""`), so this reimages the controller to
`ghcr.io/cozystack/etcd-operator:v0.5.4`.
- `packages/system/etcd-operator/Makefile`,
`packages/system/etcd-operator-crds/Makefile` — `ETCD_OPERATOR_REF:
v0.5.3 → v0.5.4`.
- `packages/system/etcd-operator-crds/templates/etcdmembers.yaml` —
re-vendored at v0.5.4 via `make update`. Description-only change to the
`/scale` `replicas`/`selector` field docs, tracking the PDB
`minAvailable` fix. `etcdclusters` and `etcdsnapshots` are
byte-identical to v0.5.3.
- `templates/rbac.yaml` intentionally left as-is:
`manager-role-rules.yaml` is byte-identical between v0.5.3 and v0.5.4.

**Upgrade path (PDB switch, cozystack/etcd-operator#351).** This is the
one change that rewrites live objects: the operator moves each
EtcdCluster's PodDisruptionBudget from `maxUnavailable` to
`minAvailable`. Setting both fields is invalid, but upstream
`reconcilePDB` handles the migration — it treats a surviving
pre-migration `maxUnavailable` as divergence and explicitly clears it
(`MaxUnavailable = nil`) before writing `MinAvailable`, so existing
clusters are reconciled cleanly on upgrade rather than wedging their
PDB.

Verified locally: `helm lint` and `helm template` pass for both
packages; rendered manager image resolves to
`ghcr.io/cozystack/etcd-operator:v0.5.4`. These `packages/system/*`
packages have no `generate:` target and no `values.schema.json`, so
there is nothing for `make generate` to regenerate.

### Screenshots

Not applicable — no UI changes.

### Downstream repositories

Walked the trigger map in `docs/agents/contributing.md` file-by-file
against the diff:

- The diff touches only `packages/system/etcd-operator*` — no
`packages/apps/**` or `packages/extra/**` add/rename/remove, no
`packages/core/platform` or `installer` values, no Talos bump, no
asset-name or dev-tooling change → **website / ansible-cozystack** not
reached.
- The CRD edit is description-only, inside etcd-operator's own
`etcd-operator.cozystack.io` CRDs — not the provider's hand-typed
`Package`/`Plan`/`RestoreJob` types, and no new managed app →
**terraform-provider-cozystack** not reached.
- No `hack/` change, no `packages/system/<name>-rd/cozyrds/**`, no
`ApplicationDefinition` CRD / `chartRef.kind` enum change, no
`cozyhr`/`package.mk` contract change, no telemetry-metric or
proxy-label rename → **ccp / talm / cozyhr / cozy-proxy /
telemetry-server / examples / external-apps-example** not reached.

- [x] No downstream repository is affected by this change
- [ ] [cozystack/website](https://github.com/cozystack/website) -
follow-up:
- [ ]
[cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack)
- follow-up:
- [ ]
[cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack)
- follow-up:
- [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up:
- [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up:
- [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) -
follow-up:
- [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) -
follow-up:
- [ ]
[cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server)
- follow-up:
- [ ]
[cozystack/external-apps-example](https://github.com/cozystack/external-apps-example)
- follow-up:
- [ ] [cozystack/examples](https://github.com/cozystack/examples) -
follow-up:

### Release note

```release-note
chore(etcd-operator): bump etcd-operator to v0.5.4 (controller bug-fixes: PDB switched to minAvailable, crash-loop self-heal extended to memory members, --initial-cluster-state derived from phase, seed no longer exempt from self-heal)
```


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

- **Updates**
  - Updated the etcd operator to version v0.5.4.
- Clarified scale-related resource descriptions, including replica
counts, selectors, and disruption budget behavior.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Ksenia Fokina (just-ksenos) pushed a commit to just-ksenos/cozystack that referenced this pull request Aug 4, 2026
Bump the cozystack etcd-operator packages from v0.5.3 to v0.5.4. The
release is a controller bug-fix set with no API, RBAC or values changes:

- fix(controllers): derive --initial-cluster-state from phase, not from
  the seed (cozystack/etcd-operator#355)
- fix(controllers): stop exempting the bootstrap seed from self-heal
  (cozystack/etcd-operator#354)
- fix(controllers): extend crash-loop self-heal to memory members
  (cozystack/etcd-operator#352)
- fix(controllers): switch the PDB from maxUnavailable to minAvailable
  (cozystack/etcd-operator#351)

Adaptations:
- etcd-operator/Chart.yaml: appVersion v0.5.3 -> v0.5.4 (the manager image
  tag defaults to .Chart.AppVersion, so this reimages the controller).
- etcd-operator/Makefile, etcd-operator-crds/Makefile: ETCD_OPERATOR_REF
  v0.5.3 -> v0.5.4.
- etcd-operator-crds/templates/etcdmembers.yaml: re-vendored at v0.5.4 via
  `make update`; description-only change to the /scale replicas/selector
  fields tracking the PDB minAvailable fix. etcdclusters/etcdsnapshots
  unchanged.
- templates/rbac.yaml left as-is: manager-role-rules.yaml is byte-identical
  between v0.5.3 and v0.5.4.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

controllers documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants