Repository navigation
[Backport release-1.6] fix(linstor): add opt-in graceful satellite shutdown on Talos - #4338
europrinter (yankawai) wants to merge 1 commit into
Conversation
Backport of cozystack#4292 to release-1.6. Cozystack's Talos configuration removes the systemd DRBD shutdown guard without a replacement. talos.gracefulShutdown.enabled, disabled by default, releases unused Secondary DRBD resources before the satellite stops so their backing devices, including ZFS zvols, can be released during normal shutdown. The hook skips Primary resources and open devices, rechecks each candidate, and stops on ambiguous status, command failure or timeout. It uses no force, demotion, Kubernetes API calls or extra permissions. Three differences from main, all from the same cause: release-1.6 has no drbd.enabled flag, since the DRBD-less-substrate switch landed after 1.6 was cut. The hook is therefore gated on talos.gracefulShutdown.enabled alone, the values block adds only that key, and the case asserting the hook is omitted on a substrate without DRBD is left out along with the flag it sets. The README line naming both flags names talos.enabled instead. Nothing else differs. The automated backport in cozystack#4320 stopped at the same values.yaml hunk and pushed the conflict markers as its only commit, which is why this exists. Verified on this branch: 11 Helm tests and the 20 behavioural Python tests pass. Assisted-by: LLM Signed-off-by: Yan Bondarenko <[email protected]>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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. Comment |
github-actions
Bot
added
area/release
Aleksei Sviridkin (lexfrei)
left a comment
Aleksei Sviridkin (lexfrei)
left a comment
There was a problem hiding this comment.
europrinter (@yankawai) LGTM. The backport matches #4292 apart from the three differences you listed, and an existing 1.6 install renders exactly as before.
Business context: brings the opt-in Talos satellite preStop hook from #4292 to the 1.6 line.
The rest of the change is byte-identical to main: the hook script, its Python tests and the Makefile line. All three differences come from one missing flag, drbd.enabled. It arrived on main with 06ee08f, after 1.6 was cut. Keeping main's gate would break the chart here: with that line restored, 5 of the 11 Helm cases fail with nil pointer evaluating interface {}.enabled. The values hunk only adds keys. The 1.6 image pins stay, and nothing from main's drbd or strictTopology blocks comes along.
Default renders don't change. origin/release-1.6 and this head produce the same objects on both variants of the chart, Talos and generic Linux. With talos.gracefulShutdown.enabled=true only LinstorSatelliteConfiguration/cozystack-talos changes, and it's the same object main renders for that value.
The hook's dependencies are in the 1.6 satellite image. The pinned piraeus-server:v1.6.3 has Python 3.11.2 and drbd-utils 9.34.3, and its drbdsetup status accepts --json. All 20 Python tests pass inside that image. Piraeus operator and Talos are on the same versions as main.
The Helm test count is lower than main's for a plain reason. Main has 22 cases, and 10 of them live in csi-topology_test.yaml and satellites_test.yaml, which cover features this branch doesn't have. The last missing one is the DRBD case you dropped. Forcing the template gate to true turns the two default-lifecycle cases red, so the remaining tests still guard the default.
CI hasn't run yet. The Pull Request, Pre-Commit Checks and API Review Gate workflows are waiting for maintainer approval with no jobs started, and only the label, size and DCO bots have reported.
Two things that don't block:
- The commit author is
Aleksei Sviridkin <[email protected]>, not you. Picking the #4292 merge commit carried its author over, while the three commits on main are yours. This branch keeps PR commits under a merge commit, so blame here would credit me for your code.git commit --amend --author "Yan Bondarenko <[email protected]>"fixes it. - Merges into release-1.6 copy the PR body into the merge commit message, as 8a6ce81 shows. The last paragraph of the body is a request to maintainers and would end up in branch history. A PR comment is a better place for it.
…utdown on Talos (#4401) This reopens a fork backport from a branch in this repository so its CI can run. The commit is the one from #4338 by @yankawai, unchanged. On release-1.6 the pull-request CI pushes the images it builds, and a run from a fork has no registry credentials, so #4338 stopped at the build jobs and never reached e2e. The description below is the author's. ## What this PR does Manual backport of #4292 to `release-1.6`. The automated backport, #4320, stopped on a conflict in `packages/system/linstor/values.yaml` and pushed the markers as its only commit, so it is red on DCO and would ship conflict markers in a values file if merged. The conflict is `drbd.enabled`, which does not exist on this line: the DRBD-less-substrate switch landed on main after 1.6 was cut, and the cherry-pick pulled its values block in with the hunk it needed. Everything else in the change applies unchanged, so three differences follow from that one cause: - the preStop hook is gated on `talos.gracefulShutdown.enabled` alone rather than on it and `drbd.enabled`; - the values block adds `talos.gracefulShutdown` only; - the case asserting the hook is omitted on a Talos substrate without DRBD goes with the flag it sets, and the README line that named both flags names `talos.enabled`. Verified on this branch: 11 Helm tests and the 20 behavioural Python tests pass. The file set and line counts match #4292 apart from those three points. Feature summary, unchanged from #4292: Cozystack's Talos configuration removes the systemd DRBD shutdown guard without a replacement. `talos.gracefulShutdown.enabled`, disabled by default, releases unused Secondary DRBD resources before the satellite stops so their backing devices, including ZFS zvols, can be released during normal shutdown. The hook skips Primary resources and open devices, rechecks each candidate, and stops on ambiguous status, command failure or timeout. It never forces, demotes, calls the Kubernetes API or asks for extra permissions. It is opt-in because preStop also runs on ordinary satellite restarts, and Piraeus has one DaemonSet per node with no cluster-wide rollout barrier. ### Screenshots Not a UI change. ### Downstream repositories Walked the trigger map against the diff: this touches one system chart, its hook script and its tests. No downstream repository restates the linstor chart values. - [x] No downstream repository is affected by this change ### Release note ```release-note fix(linstor): add `talos.gracefulShutdown.enabled`, off by default, so a satellite releases unused Secondary DRBD resources before it stops and their backing devices, ZFS zvols included, can be released during a normal Talos shutdown. Enable it only where satellite and node restarts are serialized: the hook also runs on ordinary pod restarts, and Piraeus has one DaemonSet per node. ```
3 participants
What this PR does
Manual backport of #4292 to
release-1.6.The automated backport, #4320, stopped on a conflict in
packages/system/linstor/values.yamland pushed the markers as its only commit, so it is red on DCO and would ship conflict markers in a values file if merged.The conflict is
drbd.enabled, which does not exist on this line: the DRBD-less-substrate switch landed on main after 1.6 was cut, and the cherry-pick pulled its values block in with the hunk it needed. Everything else in the change applies unchanged, so three differences follow from that one cause:talos.gracefulShutdown.enabledalone rather than on it anddrbd.enabled;talos.gracefulShutdownonly;talos.enabled.Verified on this branch: 11 Helm tests and the 20 behavioural Python tests pass. The file set and line counts match #4292 apart from those three points.
Feature summary, unchanged from #4292: Cozystack's Talos configuration removes the systemd DRBD shutdown guard without a replacement.
talos.gracefulShutdown.enabled, disabled by default, releases unused Secondary DRBD resources before the satellite stops so their backing devices, including ZFS zvols, can be released during normal shutdown. The hook skips Primary resources and open devices, rechecks each candidate, and stops on ambiguous status, command failure or timeout. It never forces, demotes, calls the Kubernetes API or asks for extra permissions. It is opt-in because preStop also runs on ordinary satellite restarts, and Piraeus has one DaemonSet per node with no cluster-wide rollout barrier.Close #4320 in favour of this one, or tell me to take a different route and I will.
Screenshots
Not a UI change.
Downstream repositories
Walked the trigger map against the diff: this touches one system chart, its hook script and its tests. No downstream repository restates the linstor chart values.
Release note