Repository navigation
fix(linstor): plunger demotes idle-Primary DRBD resources - #2539
Conversation
DRBD auto-promotes a resource to Primary during the provisioning path (e.g. mkfs.ext4 against a freshly created replica), but auto-demote sometimes fails to fire on close. The resource then stays Primary on a node that has no consumer, and CSI on the legitimate target node fails NodePublishVolume with "failed to set source device readwrite". The only recovery is an explicit drbdadm secondary on the stuck node. Add a per-satellite plunger handler that detects local Primaries where every device has open == false and demotes them. Both the detection and the action are local to the satellite, so no peer coordination is needed. open:no makes drbdadm secondary safe; if a real consumer is about to arrive, auto-promote will redo it. Upstream tracker: piraeusdatastore/piraeus-operator#942 Co-Authored-By: Claude <[email protected]> Signed-off-by: Andrei Kvapil <[email protected]>
github-actions
Bot
added
area/storage
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughIntroduces a new DRBD remediation function that automatically demotes idle Primary resources to Secondary state. The function parses JSON status output, identifies Primary resources with no open devices on the current node, and attempts demotion via Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.Comment |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a recurring issue where DRBD resources fail to automatically demote from Primary after provisioning, causing subsequent mount failures on target nodes. By introducing a cleanup mechanism in the satellite sidecar, the system can now safely identify and demote these idle resources, ensuring that legitimate consumers can successfully publish volumes. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on Gemini (@gemini-code-assist) comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
dosubot
Bot
added
the
size/M
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces the drbd_fix_idle_primary function to the plunger-satellite.sh script to automatically demote DRBD resources that remain in the Primary state without active consumers, addressing an issue where auto-promote can block volume publishing on other nodes. Feedback suggests a performance optimization to fetch the DRBD status JSON once in the main loop and pass it as an argument to both drbd_fix_idle_primary and drbd_fix_stalled_sync to avoid redundant, expensive calls to drbdsetup.
| drbd_fix_idle_primary() { | ||
| local json | ||
| json="$(drbd_status_json)" | ||
| [ -n "$json" ] || return 0 | ||
|
|
||
| printf '%s' "$json" | jq -r ' | ||
| .[]? | ||
| | select(.role == "Primary") | ||
| | select(all(.devices[]?; .open == false)) | ||
| | .name | ||
| ' | while IFS= read -r res; do | ||
| [ -n "$res" ] || continue | ||
| log "Idle Primary detected: res=$res open:no -> drbdadm secondary" | ||
| drbdadm secondary "$res" || log "WARN: secondary failed for $res" | ||
| done | ||
| } |
There was a problem hiding this comment.
The drbd_fix_idle_primary function calls drbd_status_json to fetch the DRBD status. However, drbd_fix_stalled_sync (called later in the main loop) also calls the same function. Since drbdsetup status --json can be relatively expensive on systems with many resources, it would be more efficient to fetch the JSON once in the main loop and pass it as an argument to both functions. Note that while drbd_fix_stalled_sync is not fully within the diff, its call site is, and its definition is available in the file context.
|
I have a complementary fix in #2538 that targets the same root cause from the prevention side: retries These compose naturally. #2538 prevents the race in-flight, this PR is the safety net when prevention misses or when a resource ends up in idle-Primary for other reasons. We've also seen a related symptom that the plunger's local check wouldn't catch: the controller snapshot freezing in Related upstream: LINBIT/linstor-server#268, LINBIT/drbd#74. |
myasnikovdaniil
added
the
kind/backport
|
Successfully created backport PR for |
3 participants
What this PR does
Adds a per-satellite plunger handler that detects DRBD resources
stuck in
Primaryon the local node with no consumer and demotesthem to
Secondary.Background
DRBD auto-promotes a resource to
Primaryduring the provisioningpath (
mkfs.ext4against a freshly created replica, for example),but auto-demote sometimes fails to fire on close. The resource then
stays
Primaryon a node that has no consumer pod, and CSI on thelegitimate target node fails
NodePublishVolumewith:The only recovery is an explicit
drbdadm secondaryon the stucknode — there is no upstream automation for it.
This was hit deterministically in Cozystack e2e CI on freshly
provisioned PVCs without any pod migration or node failure. Upstream
tracker: piraeusdatastore/piraeus-operator#942 (and the related
piraeusdatastore/linstor-csi#172).
Implementation
In
plunger-satellite.sh(already running as a privileged sidecaron every linstor-satellite pod) add
drbd_fix_idle_primary:For every match, run
drbdadm secondary <res>.Why this is safe
open == falseon every device of the resource means no readeror writer holds the block device — the demote cannot interrupt I/O.
coordination needed.
the promotion when CSI opens the device — so even a misfire is a
no-op.
(3 nodes, 22 legitimate Primaries, 0 false positives — every
legitimate Primary has at least one
open: truevolume).Release note
Summary by CodeRabbit