Document automatic volume placement (pin, load-aware, Pod co-location) - #178
Open
boddumanohar wants to merge 4 commits into
Open
Document automatic volume placement (pin, load-aware, Pod co-location)#178boddumanohar wants to merge 4 commits into
boddumanohar wants to merge 4 commits into
Conversation
…inity, disable-smart-placement) Adds a new page covering the PVC annotations from the "Volume Placement at Creation Time" design doc that aren't documented anywhere yet: simplyblock.io/host-id (Tier 0 pin, plus its deprecated simplybk/ prefix), simplyblock.io/pod-affinity (Tier 1 co-location, marked Planned pending implementation), and simplyblock.io/disable-smart-placement (opt-out). Cross-links added from node-affinity.md and volume-migration.md. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
|
🤖: Deployment available as https://docstest.simplyblock.io/docs/volume-placement-draft |
Collaborator
|
Why would we point out tier 1 if nothing is implemented? Same as, why would we make a distinction between "tier 0" and "tier 1"? |
The previous draft followed the design doc's proposed mechanism, which turned out to be stale relative to the shipped code in simplyblock-operator. Corrections, verified against operator/internal/webhook/simplyblock_volume_placement_injector.go and csi-driver/pkg/spdk/controllerserver.go: - The canonical pin annotation is simplyblock.io/selected-storage-node; host-id and the legacy simplybk/host-id prefix are accepted but normalized into it automatically. Documented as deprecated, not as the primary annotation. - Load-aware placement for new volumes is real and already shipped, gated by volumeAutoPlacement.latencyBenchmarkEnabled (not .enabled, which only controls the continuous rebalancer) — it writes a non-pinning simplyblock.io/placement-hint annotation. Documented as present-tense, not "Planned". - Pod co-location (simplyblock.io/pod-affinity) has no EnableNodeAffinity precondition in the actual code — that cluster flag only affects SPDK's erasure-coding data plane, unrelated to this CSI-level mechanism. Removed the incorrect precondition from both the new page and the node-affinity.md cross-link. - simplyblock.io/disable-smart-placement is implemented and documented as-is. Also removed all "Planned" warnings now that every documented annotation and behavior is verified against the current code, and corrected the auto-rebalancing cross-reference in volume-migration.md to name latencyBenchmarkEnabled specifically. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
boddumanohar
changed the title
boddumanohar
marked this pull request as draft
August 3, 2026 08:40
Member
Author
|
need to do some refining, moving it to draft state for now. |
Matches the terse, table-driven format used elsewhere (e.g. quality-of-service.md, backup-recovery.md's annotation reference, reference/kubernetes/index.md): a resolution-order table, a single annotations reference table, then one compact example per annotation instead of narrative walkthroughs. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
|
🤖: Deployment available as https://docstest.simplyblock.io/docs/volume-placement-draft |
1 similar comment
Contributor
|
🤖: Deployment available as https://docstest.simplyblock.io/docs/volume-placement-draft |
boddumanohar
marked this pull request as ready for review
August 3, 2026 12:36
Tier 1 (pod-affinity co-location) is already per-PVC opt-in, the inverse of load-aware placement's cluster-wide opt-out default, so disable-smart-placement never actually affected it in a way users needed to override — not setting pod-affinity already excludes a PVC from co-location. Matches the corresponding fix in simplyblock-operator (csi-driver's coLocateEligible/disableSmartPlacement plumbing reverted to a plain pod-affinity check). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
|
🤖: Deployment available as https://docstest.simplyblock.io/docs/volume-placement-draft |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Documents
docs/usage/simplyblock-csi/volume-placement.md— how Simplyblock picks a new volume's primary storage node. This started from the "Volume Placement at Creation Time" design doc, but the design doc turned out to be stale relative to what's actually shipped insimplyblock-operator, so the content here is rewritten to match the real code, verified by reading (not just grepping)operator/internal/webhook/simplyblock_volume_placement_injector.goandcsi-driver/pkg/spdk/controllerserver.go:simplyblock.io/selected-storage-nodeis the canonical pin annotation (already documented for migration; this page notes it also works at creation time).host-idand legacysimplybk/host-idare accepted but auto-normalized into it — documented as deprecated.volumeAutoPlacement.latencyBenchmarkEnabled(not.enabled, which only controls the separate continuous rebalancer) — it stamps a non-pinningsimplyblock.io/placement-hintannotation. Documented present-tense, no "Planned" language.simplyblock.io/pod-affinity) has noEnableNodeAffinityprecondition in the actual code — that cluster flag only affects SPDK's erasure-coding data plane and is unrelated to this CSI-level mechanism. Removed that incorrect precondition, including from thenode-affinity.mdcross-link.simplyblock.io/disable-smart-placementis implemented and documented as-is.Also corrects the
volume-migration.mdAuto-Rebalancing cross-reference to namelatencyBenchmarkEnabledspecifically, since that's the flag that actually gates creation-time placement.Test plan
mkdocs buildlocally to confirm the new page renders and nav weight (40050) doesn't collideatlas-lib/kube/names.goandcontrollerserver.goonce more before merge, since this is user-facing and precision matters🤖 Generated with Claude Code