docs: document act-as-daemonset mode (OP-345) - #2748
Conversation
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
|
Claude finished @kristina-solovyova's task in 6m 34s —— View job Review: docs for act-as-daemonset mode
I treated this as a factual-accuracy review rather than a prose one, since the value of a doc like this is entirely in whether it matches the operator. I checked every number, formula, event, policy name, severity, default and transition rule against the source. What checked outEssentially all of it, including the parts that would have been easy to get wrong:
Findings
#1 is the one that matters. #2 is a genuine gap rather than a wording issue. The doc's best-effort caveat blames only partial drive-signing, but the validator reads hugepages as Details, code references and suggested diffs are in the 5 inline comments. One thing I could not verifyThe verbatim CEL message quoted at Nothing here blocks; #1 is a two-minute fix and the rest are polish on a doc that is, on the evidence, unusually well checked against its implementation. |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| </file> | ||
| <file relpath="operator/deployment/act-as-daemonset.md"> | ||
| <tags>act as daemonset, daemonset mode, auto full drives, AutoFullDrives, implicit sizing mode, mode detection, no flag, empty dynamicTemplate, both-or-neither, computeContainers driveContainers set together, CEL rejection, full drives, exclusive drives, per-node sizing, heterogeneous nodes, node-pinned drive container, drives decoupled from cores, numDrives override, numDrives pin, driveCores pin, drive core limit 19, maxCoresPerContainer, signed full drives, weka.io/weka-full-drives, hugepages budget, compute hugepages ceiling, hugepagesTlcRatio, computeMaxHugepagesMiB, capacity-based hugepages, compute sizing, computeToDriveCoreRatio, compute 1:1 floor, compute shortfall coverage, hard infeasibility, whole plan infeasible, one bad node blocks the cluster, RejectedNodes, mode flip rejection, one-way mode switch, changing sizing mode on a live cluster, adopting the daemonset mode, supported mode switches, cluster_auto_full_drives_pin_exceeds_node_drives, cluster_auto_full_drives_compute_hugepages, cluster_sizing_mode_flip, cluster_auto_full_drives_min_nodes, form-cluster minimum nodes, MinContainersNotReady, node selector sets container count, admission policy summary, AutoFullDrivesInfeasible, AutoFullDrivesDrivesStranded, AutoFullDrivesGrowthDetected, CapacityGrowthApplied, UnschedulableDriveContainer, UnschedulableComputeContainer, unschedulable planner container GC, AutoFullDrivesNodeIneligible, node eligibility, cordoned nodes, NotReady nodes, untolerated taints, driveHugepages override, computeHugepages override, FORM_CLUSTER_MIN_COMPUTE_CONTAINERS, drive cores never traded for compute, pod restart, expand-only reconciliation, drive-only growth is free, QLC drives excluded, QLCDrivesSkipped, upgrade note, nodeSelector scope</tags> | ||
| <summary>Deployment guide for the implicit "act as daemonset" sizing mode (internally auto full drives / AutoFullDrives): an exclusive full-drives mode selected by NOT setting container counts — active iff computeContainers, driveContainers, clusterCapacity, containerCapacity and driveCapacity are all unset — that creates one node-pinned drive container per eligible node and auto-derives numDrives, drive cores, hugepages and memory from that node's own signed full drives. There is no flag; an empty or absent dynamicTemplate is the mode. Covers the nine-row mode-detection decision table and the both-or-neither CEL rule rejecting exactly one of computeContainers/driveContainers (with the verbatim admission message); the comparison with explicit container counts and the drive-sharing modes; prerequisites (signing drives before cluster creation); the drives/cores decoupling — a container always takes ALL its node's signed drives (or the numDrives pin) while driveCores = pin else min(drives, 19), so numDrives == driveCores is NO LONGER an invariant, a driveCores pin below the drive count is lossless, and a pin above it is infeasible; numDrives as a per-node largest-drives override (above a node's signed count is infeasible, below strands the rest as an expected Normal event); the per-container 19-core limit capping cores only; the hugepages budget (1664 MiB/drive core, 3064 MiB/compute core floor); compute sizing from a configurable ratio (default 2:1 full-drives) with a hard 1:1 floor and how shortfalls are covered; the compute-hugepages ceiling as the practical limit — because all drives are claimed the capacity-based term is fixed and cannot be shrunk by capping cores — worked end to end on an 8-node lab fleet (8 x 6 x 14307 GiB = 686736 GiB claimed, 87902 MiB capacity share per container at 8 compute containers vs 50016 MiB free, needing 18 compute-eligible nodes) with four remedies (more compute nodes, raise hugepagesTlcRatio, lower computeMaxHugepagesMiB, pin numDrives lower) and admission enforcement; hard infeasibility when ANY node cannot fit a container sized for all its drives (nothing created anywhere, every offending node named with the binding dimension, and one bad node blocking the whole cluster called out); the one-way mode-switch rule on a live cluster enforced by cluster_sizing_mode_flip (Error in both modes, UPDATE-only, covering every sizing-mode pair since nothing else guards the capacity transitions) — explicit container counts to daemonset is ALLOWED, the running drive containers being adopted via the pod's node and grown in place to that node's full drive set with a pod recreation owed where cores rose, while daemonset to counts and every capacity transition except drive-sharing to clusterCapacity are rejected with reverting as the remedy; the note that all the other policies run on updates too, so the switch still faces the min-nodes, compute-hugepages and pin-exceeds gates; continuous expand-only reconciliation where drive-only growth is always free and needs no pod restart; the pod-restart caveat for core changes; the upgrade note that existing containers gain drives while cores stay put; QLC exclusion from full-drives signing; the full AutoFullDrives* event table with types and throttles, plus the container-level UnschedulableDriveContainer/UnschedulableComputeContainer reaps, gated on a confirmed PodScheduled=False/Unschedulable condition rather than age alone so a pod merely Pending during a slow drivers build is left alone; and troubleshooting for unsigned drives, stranded drives, fewer cores than drives, the two admission rejections, infeasibility, never-scheduled drive and compute containers (both reaped on the same terms, since an unscheduled compute container's cores are counted but never served — the reap requires a node-pinned container whose pod NEVER bound and a scheduler verdict standing longer than the timeout, timed from the verdict rather than from container creation), and nodes barred from NEW placement as cordoned, NotReady or carrying an untolerated taint (a Normal per-node AutoFullDrivesNodeIneligible event; existing containers there keep running, stay charged and still grow, and their unclaimed drives still count toward the plan's reported fleet total).</summary> |
There was a problem hiding this comment.
The summary contradicts the doc it indexes, in five places. summary.xml is what agents/tooling read instead of the 1029-line file, so a wrong claim here is worse than no claim — it lands the reader on the opposite conclusion from the one the doc works hard to establish.
"the hugepages budget (1664 MiB/drive core, 3064 MiB/compute core floor)"— the doc's Hugepages budget exists specifically to retire the flat 1664/core figure: "it held only while drives and cores were locked together."hugepages.go:19-21says the same. The summary asserts the very number the doc corrects."with four remedies (more compute nodes, raise hugepagesTlcRatio, lower computeMaxHugepagesMiB, pin numDrives lower)"— the doc lists six, and the omitted [DEVOPS-1595] Install weka on underlying VM #4 is "PindriveCoreslower … the one remedy that costs no capacity at all". Dropping it loses the mode's headline lever. Add envtest #6 (computeCores) is missing too."drive-only growth is always free and needs no pod restart"— the doc says the opposite twice: "Drive-only growth is cheap, but not free" (200 MiB/drive), and the restart is owed because "the pod's hugepages limit is immutable and enforced". Confirmed insteps_planner_apply.go:324-336— the AFD drives-only branch emits a WarningCapacityGrowthApplied, unlike the clusterCapacity branch which does sayno restart required."the upgrade note that existing containers gain drives while cores stay put"— the section is titled "existing clusters gain drives, and cores unless pinned", and its table shows a 3 → 6 core rise."the nine-row mode-detection decision table"— it has 7 rows.
Points 3 and 4 look like they were written against an earlier draft and not re-synced.
| **Treat it as best-effort, not a guarantee.** The check projects from the drives signed **at the time | ||
| you apply**, and it skips nodes carrying no `weka.io/weka-full-drives` annotation yet — deliberately, | ||
| so that creating a cluster before any drives are signed is not blocked. The gap is the partial case: a | ||
| cluster applied while signing is still in progress is measured against only the nodes annotated so | ||
| far, so a claim that will not fit the finished fleet can pass admission and surface later as | ||
| `AutoFullDrivesInfeasible` at runtime. Sign the whole fleet before applying if you want admission to |
There was a problem hiding this comment.
The best-effort caveat names only the partial-signing gap, but there's a second one that will bite more often on a shared fleet: the admission check reads node hugepages as allocatable, so foreign pods are not subtracted at all. From internal/validation/cluster_auto_full_drives_compute_hugepages.go:51-54:
"Node headroom is otherwise read as ALLOCATABLE hugepages-2Mi … so foreign pods are not subtracted. That over-states what is free on a busy node … this only fires when the requirement does not fit even an idle fleet."
The runtime planner, by contrast, charges against every scheduled non-terminal pod — which this doc states plainly under Hugepages budget ("a foreign workload's CPU/hugepages/memory requests reduce believed availability the same way a WekaContainer's do"). So the two models genuinely differ, and a fleet that is fine when idle but carries other workloads can pass admission and still land on AutoFullDrivesInfeasible. Given how careful the rest of the section is about which check owns what, worth one sentence here.
| **Treat it as best-effort, not a guarantee.** The check projects from the drives signed **at the time | |
| you apply**, and it skips nodes carrying no `weka.io/weka-full-drives` annotation yet — deliberately, | |
| so that creating a cluster before any drives are signed is not blocked. The gap is the partial case: a | |
| cluster applied while signing is still in progress is measured against only the nodes annotated so | |
| far, so a claim that will not fit the finished fleet can pass admission and surface later as | |
| `AutoFullDrivesInfeasible` at runtime. Sign the whole fleet before applying if you want admission to | |
| **Treat it as best-effort, not a guarantee.** The check projects from the drives signed **at the time | |
| you apply**, and it skips nodes carrying no `weka.io/weka-full-drives` annotation yet — deliberately, | |
| so that creating a cluster before any drives are signed is not blocked. The gap is the partial case: a | |
| cluster applied while signing is still in progress is measured against only the nodes annotated so | |
| far, so a claim that will not fit the finished fleet can pass admission and surface later as | |
| `AutoFullDrivesInfeasible` at runtime. Sign the whole fleet before applying if you want admission to | |
| see the real number. | |
| It also reads each node's hugepages as **allocatable**, without subtracting foreign pods — so it | |
| measures an *idle* fleet, and only fires when the requirement does not fit even then. The runtime | |
| planner charges against every scheduled, non-terminal pod (see [Hugepages | |
| budget](#hugepages-budget)), so a fleet that is busy rather than idle can clear admission and still | |
| go infeasible on the first reconcile. Check `weka-capacity explore-nodes` for the figures the planner | |
| will actually see. |
| **An update to a live cluster is rejected as a mode flip.** | ||
| Expected for every mode change but two: adding `computeContainers`/`driveContainers` to a live | ||
| daemonset cluster, or setting or unsetting `clusterCapacity`/`containerCapacity`/`driveCapacity` in | ||
| any other combination, is rejected once drive containers exist. Revert the change. The two | ||
| transitions that should be **accepted** are unsetting *both* container counts to adopt the daemonset |
There was a problem hiding this comment.
This sentence is garbled: it opens "Expected for every mode change but two:" and then, after the colon, lists the rejections rather than the two exceptions — which only arrive in the following sentence. First read, adding computeContainers/driveContainers to a live daemonset cluster parses as one of the two allowed switches, which is exactly backwards (modeSwitchSupported in internal/validation/cluster_sizing_mode_flip.go:118-131 allows only counts→daemonset and drive-sharing→clusterCapacity).
| **An update to a live cluster is rejected as a mode flip.** | |
| Expected for every mode change but two: adding `computeContainers`/`driveContainers` to a live | |
| daemonset cluster, or setting or unsetting `clusterCapacity`/`containerCapacity`/`driveCapacity` in | |
| any other combination, is rejected once drive containers exist. Revert the change. The two | |
| transitions that should be **accepted** are unsetting *both* container counts to adopt the daemonset | |
| Only two mode changes are supported once drive containers exist: unsetting *both* container counts to | |
| adopt the daemonset mode, and moving a drive-sharing cluster to `clusterCapacity`. Everything else is | |
| rejected — adding `computeContainers`/`driveContainers` to a live daemonset cluster, and setting or | |
| unsetting `clusterCapacity`/`containerCapacity`/`driveCapacity` in any other combination. Revert the | |
| change. If you made one of the two supported switches and it was *still* rejected, the policy is | |
| misfiring and it is worth reporting. Note that unsetting only one count |
| **warning** in relaxed. The same policy covers a `driveCores` pin above a node's effective drive | ||
| count; it deliberately stays silent on a pin *below* it, which is lossless. |
There was a problem hiding this comment.
Minor precision point, in a section that is otherwise scrupulous about which check owns what: the policy's driveCores leg is skipped entirely when numDrives is pinned, so it doesn't cover that combination. cluster_auto_full_drives_pin_exceeds_node_drives.go:108-110:
// Effective drive count is the pin when set, else everything the node signed. Skipped
// entirely under a numDrives pin — CEL owns the numDrives >= driveCores comparison.
if driveCores > 0 && numDrives <= 0 && driveCores > signed {So with both pinned, driveCores > numDrives is caught by the CRD's CEL rule (which the doc already documents two paragraphs down) and driveCores > signed cannot occur independently, since CEL bounds it by numDrives ≤ signed. Suggest scoping the sentence:
| **warning** in relaxed. The same policy covers a `driveCores` pin above a node's effective drive | |
| count; it deliberately stays silent on a pin *below* it, which is lossless. | |
| **warning** in relaxed. The same policy covers a `driveCores` pin above a node's signed drive count | |
| when `numDrives` is unset — with `numDrives` pinned, the CRD's `numDrives >= driveCores` CEL rule | |
| owns that comparison instead; it deliberately stays silent on a pin *below* it, which is lossless. |
| 5. **Smaller compute containers do not rescue it.** Even at one core each, the capacity-based term | ||
| alone gives `87,902 + 1,700 + 64 = 89,666` MiB — still above the node's *entire* 60,000 MiB, before | ||
| the drive container takes its share. The binding term is claimed capacity, not cores, which is why | ||
| there is nothing for the operator to trade: only the container **count** in the denominator moves |
There was a problem hiding this comment.
Nit on the hypothetical: it holds the count at 8 while dropping cores to 1, but that pair isn't reachable — remedy 6 says a computeCores pin derives the count as ceil(requiredComputeCores / computeCores), so computeCores: 1 would demand 96 containers, at which point capacityBased is 703,217 / 96 ≈ 7,325 MiB and does fit. The point you're making (at a fixed count, cores are not what binds) is right and worth making — it just reads as a contradiction against remedy 6 for anyone who checks. One clause fixes it:
| 5. **Smaller compute containers do not rescue it.** Even at one core each, the capacity-based term | |
| alone gives `87,902 + 1,700 + 64 = 89,666` MiB — still above the node's *entire* 60,000 MiB, before | |
| the drive container takes its share. The binding term is claimed capacity, not cores, which is why | |
| there is nothing for the operator to trade: only the container **count** in the denominator moves | |
| 5. **Smaller compute containers do not rescue it.** Hold the count at 8 and shrink the containers: even | |
| at one core each, the capacity-based term alone gives `87,902 + 1,700 + 64 = 89,666` MiB — still above | |
| the node's *entire* 60,000 MiB, before |
b0c56ee to
ac1d906
Compare
c932080 to
4b14958
Compare
|
Claude finished @kristina-solovyova's task in 1m 57s —— View job Re-review after synchronize: no content change
This push was a stack rebase, not a doc change. The single commit I re-read the four flagged locations and they are verbatim unchanged:
I also re-checked that the rebase didn't move the ground under the doc: Open findings (unchanged, all in the inline comments above)
Nothing new to add — the suggested diffs in the 5 inline comments still apply cleanly. #1 remains the only one I'd call worth doing before merge, since • branch |
ac1d906 to
367a3a0
Compare
4b14958 to
13adcac
Compare
Re-review after synchronize: no content change (2nd rebase)
Another stack rebase, not a doc change. The commit is now I confirmed this at the blob level rather than by eye — the three files carrying open findings hash to exactly what my previous reviews were anchored to:
Same hashes ⇒ the flagged lines are unchanged; no need to re-read them. I also re-checked that the replay didn't move the ground under the doc. Open findings (unchanged, all in the inline comments above)
Nothing new. The suggested diffs in the 5 inline comments still apply cleanly. #1 is the only one I'd do before merge — View job run • branch |
13adcac to
a8f3d2a
Compare
367a3a0 to
28c8b8e
Compare
a8f3d2a to
b9e4652
Compare
|
Claude finished @kristina-solovyova's task in 1m 30s —— View job Re-review after synchronize: no content change (3rd rebase)
Another stack replay, not a doc change. The commit is now The three files carrying open findings hash to exactly what my previous reviews were anchored to:
Same hashes ⇒ the flagged lines are unchanged; no need to re-read them. Per-file diff stats also match ( One stat did move: the PR total is now +1283/−70, down from +1324/−70. Deletions are unchanged and no per-file count in the doc tree moved, so this is the base branch having absorbed ~41 lines of pure additions during the replay — not content dropped from this PR. I re-read the whole non-doc remainder of the diff ( I also re-checked that the replay didn't move the ground under the doc. Open findings (unchanged, all in the inline comments above)
Nothing new. The suggested diffs in the 5 inline comments still apply cleanly. #1 is the only one I'd do before merge — --- • branch |
28c8b8e to
5694b4a
Compare
b9e4652 to
18f869e
Compare
|
Claude finished @kristina-solovyova's task in 6m 57s —— View job Re-review after synchronize — content did change this time
Unlike the previous three pushes, this one edited the docs. What the edit didThe theme is stripping historical framing — which is exactly the rule this PR adds to
Also improved: the min-nodes failure modes are now scoped to "In relaxed mode, where the policy only warns" and put in the present tense, which is more accurate than the old "was completely silent" — in strict mode those failures can't occur, because the apply is rejected. The design rationale that came out of the worked example survives in condensed form in the One bad node callout, so nothing load-bearing was lost. Re-verification: everything still reproducesI re-checked the current text against the code rather than assuming round one still held:
FindingsTwo new, both outside the docs — in the part of the diff I passed over in round one:
A is the one I'd fix. The base branch already has a short form of that rule at lines 67-68; this PR appends an expanded form at 70-77, separated by the unrelated B: The five earlier findings are all still openI checked each location in the current file — three of the five paragraphs were touched by this push but none of the substance changed:
#1 is still the only one I'd call worth doing before merge: One small inconsistency I'll flag without opening a finding: Still unverified, as in every previous round: the CEL message quoted verbatim at Nothing here blocks. On the evidence of two independent verification passes, this doc is unusually well-checked against its implementation — every figure I could recompute, recomputed. --- • branch |
| - Never swallow errors unless explicitly asked for | ||
| - Comments should be short, explain what the code does and why it is the way it is — nothing else. | ||
| Do NOT write comments about: | ||
| - History or changelog ("an earlier revision did X", "previously this was Y", | ||
| "renamed from Z", "added in PR #123") — that belongs in git or Jira tasks, not the source | ||
| - The process of writing the code ("as requested", "TODO: I chose this because", | ||
| self-justification, apologies, notes addressed to a reviewer) | ||
| - Anything restating the line below it, or narrating obvious mechanics |
There was a problem hiding this comment.
This leaves two copies of the same rule in ## Code Style. The base branch already carries a short form of it two bullets up (line 67-68):
- Comments should be short, explain what the code does and why it is the way it is — nothing else.
Do NOT write comments about: history or changelog, anything restating the line below it, or narrating obvious mechanics
The block added here restates that verbatim in its first line and then expands the "Do NOT" list. It also lands after the unrelated - Never swallow errors bullet, so the two copies aren't even adjacent — a reader hits the short rule, an unrelated rule, then the long rule.
This matters more than a normal duplicate because CLAUDE.md is loaded into every agent's context on every session: the duplication is pure token cost, and the two copies can now drift independently (they already differ — only the long one covers "the process of writing the code").
Looks like the intent was to replace the short line, not append beside it. Note that the expanded version is what the rest of this PR is written against — the doc edits in this push strip "previously / a change in behaviour / the genuinely new option" framing out of act-as-daemonset.md, which is exactly the new "History or changelog" bullet in action.
| - Never swallow errors unless explicitly asked for | |
| - Comments should be short, explain what the code does and why it is the way it is — nothing else. | |
| Do NOT write comments about: | |
| - History or changelog ("an earlier revision did X", "previously this was Y", | |
| "renamed from Z", "added in PR #123") — that belongs in git or Jira tasks, not the source | |
| - The process of writing the code ("as requested", "TODO: I chose this because", | |
| self-justification, apologies, notes addressed to a reviewer) | |
| - Anything restating the line below it, or narrating obvious mechanics | |
| - Comments should be short, explain what the code does and why it is the way it is — nothing else. | |
| Do NOT write comments about: | |
| - History or changelog ("an earlier revision did X", "previously this was Y", | |
| "renamed from Z", "added in PR #123") — that belongs in git or Jira tasks, not the source | |
| - The process of writing the code ("as requested", "TODO: I chose this because", | |
| self-justification, apologies, notes addressed to a reviewer) | |
| - Anything restating the line below it, or narrating obvious mechanics | |
| A reader who has never seen the diff should find every comment useful. |
…and drop the now-redundant short form at lines 67-68, keeping - Never swallow errors where it is.
| ## Drive-container sizing modes | ||
|
|
||
| Mode is derived from which `spec.dynamicTemplate` fields are set; `plannerSizingMode` | ||
| (`steps_planner_apply.go`) is the single detection site. | ||
|
|
||
| | Mode | Family | Planner | Notes | | ||
| |------|--------|---------|-------| | ||
| | explicit counts (`computeContainers`+`driveContainers`, +`numDrives`/`driveCores`) | exclusive | none (static template) | uniform shape, scheduler-placed | | ||
| | `numDrives`+`driveCapacity` | drive-sharing | none (cores derived in `allocator.getDriveCores`) | TLC-only | | ||
| | `containerCapacity` | drive-sharing | none (cores derived) | split by `driveTypesRatio` | | ||
| | `clusterCapacity` | drive-sharing | `PlanCapacity` | whole-cluster target, FD-aware, grows | | ||
| | **daemonset** (auto full drives) | exclusive | `PlanAutoFullDrives` | active iff counts + all 3 capacity fields unset (`UsesAutoFullDrives()`); 1 node-pinned container per node taking all its signed drives, expand-only | | ||
|
|
||
| CEL both-or-neither: with no capacity field, `computeContainers`/`driveContainers` must be both set or both unset. | ||
|
|
||
| Event reasons/severities/throttling: `planner_events.go` (`plannerEventSpecs` table). | ||
|
|
||
| Core sizing formulas: `internal/capacityplanner/{cores,hugepages}.go` — drive/compute core arithmetic | ||
| (`FullDriveCores`, `RequiredComputeCores`) and hugepages (`DriveContainerHugepagesMiB`, | ||
| `ComputeContainerHugepagesMiB`). | ||
|
|
||
| Explicit `dynamicTemplate` overrides (cores, hugepages) are enforced by admission validators, not | ||
| auto-calculation. | ||
|
|
||
| Validators (`internal/validation/`, severities in `internal/admission/defaults.go`): | ||
|
|
||
| - `cluster_auto_full_drives_pin_exceeds_node_drives` — pinned cores/drives exceed node's signed drives | ||
| - `cluster_auto_full_drives_compute_hugepages` — projected compute hugepages exceed node headroom | ||
| - `cluster_auto_full_drives_min_nodes` — role selector matches fewer nodes than min container counts | ||
| - `cluster_sizing_mode_flip` — derived mode changed (UPDATE only) while drive containers exist | ||
| - `cluster_compute_drive_cores_floor` / `cluster_drive_compute_core_ratio` — compute:drive core ratio floor/advisory | ||
| - `cluster_cores_per_container_limit` — pinned cores above `maxCoresPerContainer` | ||
|
|
||
| Docs: `doc/operator/deployment/act-as-daemonset.md`, `cluster-capacity.md`. |
There was a problem hiding this comment.
This block pushes the file past the size budget stated in this PR's own CLAUDE.md:
Files should stay under 3KB. If a file grows too large, split into subdirectories.
.ainav/controllers/wekacluster.md goes 1,544 → 4,263 bytes — 2.8× the guideline, and the added section is ~2.4KB of it. (.ainav/index.md also crosses over, 3,186 → 3,594; .ainav/config/index.md was already over and grows 4,265 → 5,006.)
The content itself is good and the mode table is genuinely useful — the issue is only where it lives. The file already uses the extract-and-link pattern for exactly this situation (wekacluster-drive-planning.md, management-proxy.md), so the natural fix is a sibling:
| `steps_planner_apply.go` | Build/apply for both planner modes → [wekacluster-sizing-modes.md](wekacluster-sizing-modes.md) |
with the mode table, the CEL note, the formula pointers and the validator list moving into .ainav/controllers/wekacluster-sizing-modes.md. That keeps wekacluster.md near its original size and puts the detail one hop away, which is what the 3-hop navigation model in CLAUDE.md asks for.
Not blocking — but this is the file agents load first for wekacluster, so it's the one where the budget is worth holding.
18f869e to
c528458
Compare
5694b4a to
1e55292
Compare
|
Claude Code is working… I'll analyze this and get back to you. |
|
Claude finished @kristina-solovyova's task in 1m 21s —— View job Re-review on
|
| # | Where | Severity | Status |
|---|---|---|---|
| 1 | doc/summary.xml:31 — index summary contradicts the doc in five places |
Main | open |
| A | CLAUDE.md:70-77 — second copy of the "Comments should be short" rule |
Worth fixing | open |
| 2 | act-as-daemonset.md:451 — best-effort caveat omits the foreign-pod blind spot |
Worth fixing | open |
| 3 | act-as-daemonset.md:918 — garbled "every mode change but two" sentence |
Minor | open |
| 4 | act-as-daemonset.md:190 — driveCores leg is skipped under a numDrives pin |
Minor | open |
| B | .ainav/controllers/wekacluster.md:39 — file at 4.3KB, past the 3KB guideline |
Minor | open |
| 5 | act-as-daemonset.md:371 — "one core each" conflicts with remedy 6 |
Nit | open |
Nothing new to add; the suggested diffs in the inline comments still apply cleanly. #1 remains the only one I'd do before merge — summary.xml is what tooling reads instead of the 1008-line file, and two of its claims are the inverse of the doc's. Still non-blocking.
Also still unverified, as in every round: the CEL message quoted verbatim at act-as-daemonset.md:48-52 and the numDrives >= driveCores rule live in pkg/weka-k8s-api, a replace target not populated in this checkout.
One aside, not a finding: this CI checkout has an uncommitted working-tree edit to CLAUDE.md that deletes the 8 lines this PR adds — i.e. it resolves finding A in the opposite direction (dropping the expanded rule, keeping the short one). It is not part of the PR and I left it alone; flagging only in case it points at something in the runner setup.
• branch 08-17-docs_document_act-as-daemonset_mode_op-345_
c528458 to
b6b214a
Compare
1e55292 to
c96a3a8
Compare
|
Claude finished @kristina-solovyova's task in 3m 8s —— View job Re-review after synchronize: docs unchanged, but the diff shape moved
The docs did not change; the stack was replayed. Commit is now The one thing that did move:
|
| # | Where | Severity | Status |
|---|---|---|---|
| 1 | doc/summary.xml:31 — index summary contradicts the doc in five places |
Main | open |
| A | CLAUDE.md:68 + :71 — two copies of the "Comments should be short" rule |
Worth fixing | open |
| 2 | act-as-daemonset.md:451 — best-effort caveat omits the foreign-pod blind spot |
Worth fixing | open |
| 3 | act-as-daemonset.md:918 — garbled "every mode change but two" sentence |
Minor | open |
| 4 | act-as-daemonset.md:186 — driveCores leg is skipped under a numDrives pin |
Minor | open |
| B | .ainav/controllers/wekacluster.md — 4,263 bytes, past the 3KB guideline |
Minor | open |
| 5 | act-as-daemonset.md:371 — "one core each" conflicts with remedy 6 |
Nit | open |
I confirmed each against the committed tree, not from memory: all five phrases (nine-row, 1664 MiB/drive core, four remedies, drive-only growth is always free and needs no pod restart, gain drives while cores stay put) are still in summary.xml; git show HEAD:CLAUDE.md still has the rule at both line 68 and line 71; wekacluster.md is still 4,263 bytes and .ainav/index.md 3,594. Findings 2 and 4 are re-confirmed by the code itself — cluster_auto_full_drives_compute_hugepages.go:52 still says "so foreign pods are not subtracted", and cluster_auto_full_drives_pin_exceeds_node_drives.go:112 still gates the driveCores leg on numDrives <= 0.
#1 stays the only one I'd do before merge: summary.xml is what tooling reads instead of the 1008-line file, and two of its claims are the inverse of the doc's. Non-blocking.
Still unverified, as in every round: the CEL message quoted verbatim at act-as-daemonset.md:48-52 and the numDrives >= driveCores rule live in pkg/weka-k8s-api, a replace target not populated in this checkout.
Aside, not a finding, same as last round: this CI checkout carries an uncommitted working-tree edit to CLAUDE.md that deletes the 8 lines this PR adds — resolving finding A in the opposite direction. Not part of the PR; I left it alone.
--- • branch 08-17-docs_document_act-as-daemonset_mode_op-345_
Graphite Automations"Add anton/matt/sergey/kristina as reviwers on operator PRs" took an action on this PR • (08/19/26)2 reviewers were added to this PR based on Anton Bykov's automation. |
c96a3a8 to
c9d54cd
Compare
b6b214a to
8f3d30b
Compare
doc/operator/deployment/act-as-daemonset.md covers how the mode is selected, what gets auto-calculated, why compute hugepages are the practical ceiling, heterogeneous fleets, the allowed sizing-mode transitions, expand-only reconciliation, the upgrade effects on existing clusters, and the admission checks and events with their troubleshooting. cluster-capacity.md picks up the shared capacityPlannerConstraints knobs, maxCoresPerContainer and the new unschedulable-container events. Also refreshes drive-sharing.md, the summary.xml index, the generated api_dump, and the .ainav navigation entries. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
c9d54cd to
ea7b646
Compare
8f3d30b to
c168aa3
Compare
1 participant
doc/operator/deployment/act-as-daemonset.md covers how the mode is selected, what
gets auto-calculated, why compute hugepages are the practical ceiling, heterogeneous
fleets, the allowed sizing-mode transitions, expand-only reconciliation, the upgrade
effects on existing clusters, and the admission checks and events with their
troubleshooting.
cluster-capacity.md picks up the shared capacityPlannerConstraints knobs,
maxCoresPerContainer and the new unschedulable-container events. Also refreshes
drive-sharing.md, the summary.xml index, the generated api_dump, and the .ainav
navigation entries.
Co-Authored-By: Claude Opus 5 (1M context) noreply@anthropic.com