zedkube: reconcile the longhorn disk reservation instead of latching it - #6241
Conversation
andrewd-zededa
added
the
next-17.0.x-rc
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #6241 +/- ##
==========================================
+ Coverage 22.93% 23.30% +0.36%
==========================================
Files 510 520 +10
Lines 93473 95205 +1732
==========================================
+ Hits 21440 22184 +744
- Misses 70292 71095 +803
- Partials 1741 1926 +185 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Hey @andrewd-zededa hope all is well :) Its not clear to me why we even need to set storage.longhorn.disk.reserved.gigabytes in first place ? |
|
Claude finished @rene's task in 2m 21s —— View job Code review
I reviewed the reconcile rewrite, the error-throttling helper, and the new Two minor points, posted inline:
No correctness, security, or performance blockers found. Steady-state cost is one Get per interval as intended. |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| if updated { | ||
| log.Noticef("applyLonghornDiskReserved: set reserved space to %d GB on node %s", | ||
| reservedGB, z.nodeName) | ||
| } |
There was a problem hiding this comment.
The failure path is now carefully throttled (one Error, then one every 30 min), but the success Notice is not. In steady state this is silent (an Update only fires on drift), so it's fine for the normal case. However, if something outside EVE persistently re-writes the reservation back — Longhorn or another controller fighting this reconcile — every kubeCfgTimer tick would issue an Update and emit an identical Notice forever, the same spam scenario the error path was hardened against. Worth either throttling this the same way or consciously accepting that a repeated repair is a signal worth logging each time.
storage.longhorn.disk.reserved.gigabytes is applied once and then never looked at again: a single in-memory flag gates the apply, and the only things that clear it are a change to the config value or a zedbox restart. Longhorn deletes and recreates its node object whenever the node leaves and rejoins the cluster, and the recreated object comes back carrying Longhorn's own default reservation of 30% of the disk. A latched apply never notices, so the EVE value silently stops being enforced. On a cluster whose disks are already near the over-provisioning budget that difference is what stops any new replica from being scheduled, and the only recovery is a manual kubectl patch. Drop the flag and reconcile on every kubeCfgTimer tick. Steady state is one Get per interval and an Update only when a disk differs, so drift is corrected on the next tick wherever it comes from. The bool returned by SetLonghornNodeDiskReserved changes meaning from "stop retrying" to "an Update was issued", which gives zedkube something worth logging: a Notice naming the node each time it repairs the reservation. Because the reconcile now runs forever with no success latch, a persistent failure would log identically every minute, so errors are throttled to one Error on first sight and one every thirty minutes while unchanged. Second, stop inferring tie-breaker nodes from the Longhorn Schedulable condition. That condition is not a property of the node's role: Longhorn drives it off the Kubernetes cordon, so it reads False for any cordon, including the boot-time cordon every node passes through before nodeOnBootHealthStatusWatcher uncordons it and the cordon applied during every drain. Returning "applied" for that state means an ordinary storage node that happens to be cordoned when the reconcile runs is treated as a tie-breaker and skipped. Test the controller's designation instead: EdgeNodeClusterConfig already carries TieBreakerNodeID from the EVE API, and zedkube already holds both that config and the local node UUID. The new IsTieBreakerNode fails closed, so a cluster with no designated tie-breaker treats no node as one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Andrew Durbin <andrewd@zededa.com>
b5e1ba3 to
6fc3d43
Compare
Hi @zedi-pramodh, the default reserved space configured by longhorn leads to 30% of the persist vault disk unusable for scheduling replicas. This setting allows for per cluster configuring, allowing for multiple storage users of vault. |
4 participants
Description
storage.longhorn.disk.reserved.gigabytes is applied once and then never looked at again: a single in-memory flag gates the apply, and the only things that clear it are a change to the config value or a zedbox restart. Longhorn deletes and recreates its node object whenever the node leaves and rejoins the cluster, and the recreated object comes back carrying Longhorn's own default reservation of 30% of the disk. A latched apply never notices, so the EVE value silently stops being enforced. On a cluster whose disks are already near the over-provisioning budget that difference is what stops any new replica from being scheduled, and the only recovery is a manual kubectl patch.
Drop the flag and reconcile on every kubeCfgTimer tick. Steady state is one Get per interval and an Update only when a disk differs, so drift is corrected on the next tick wherever it comes from. The bool returned by SetLonghornNodeDiskReserved changes meaning from "stop retrying" to "an Update was issued", which gives zedkube something worth logging: a Notice naming the node each time it repairs the reservation. Because the reconcile now runs forever with no success latch, a persistent failure would log identically every minute, so errors are throttled to one Error on first sight and one every thirty minutes while unchanged.
Second, stop inferring tie-breaker nodes from the Longhorn Schedulable condition. That condition is not a property of the node's role: Longhorn drives it off the Kubernetes cordon, so it reads False for any cordon, including the boot-time cordon every node passes through before nodeOnBootHealthStatusWatcher uncordons it and the cordon applied during every drain. Returning "applied" for that state means an ordinary storage node that happens to be cordoned when the reconcile runs is treated as a tie-breaker and skipped. Test the controller's designation instead: EdgeNodeClusterConfig already carries TieBreakerNodeID from the EVE API, and zedkube already holds both that config and the local node UUID. The new IsTieBreakerNode fails closed, so a cluster with no designated tie-breaker treats no node as one.
PR dependencies
None
How to test and validate this PR
while true; do kubectl -n longhorn-system get nodes.longhorn.io <nodename> -o jsonpath='{.spec.disks.*.storageReserved}{"\n"}'; date; sleep 10; doneto wait for the value to setChangelog notes
Detect and fix config drift of longhorn node storage reserved
PR Backports
Checklist
And the last but not least:
check them.
Please, check the boxes above after submitting the PR in interactive mode.