[Hrishikesh Kashyap (Hrishikeshkashyap)]

Pods can reference availability and durability volumes in any order, and multiple durability volumes may resolve to different nodes. Placement must not depend on volume order or permit a node that does not hold every durability volume.

Changes

• Give durability placement precedence over availability preferences.
• Intersect node sets across durability volumes.
• Deny admission when durability placement is missing or has no common node.
• Document multi-volume placement behavior.

Tests

Existing handler_unit_test.go now covers availability-only aggregation, both mixed-mode orders, matching and conflicting durability nodes in both orders, and missing durability placement.

The focused Linux webhook tests pass.

[Copilot]

🟡 Changes recommended

Early admission returns and required-affinity merging can still bypass durability placement constraints.

2 open findings

• Injected durability affinity bypasses existing required node constraints · New
• Availability PV prematurely terminates node-affinity aggregation · New

What changed in this PR

Updates Pod placement to prioritize durability volumes and require a common storage node across them.

Changes:

• Intersects durability node sets and rejects missing or conflicting placement.
• Adds multi-volume placement tests.
• Documents durability precedence.

File	Description
internal/​webhook/​hyperconverged/​handler.go	Aggregates placement constraints and adds admission checks.
internal/​webhook/​hyperconverged/​handler_unit_test.go	Tests volume ordering, conflicts, and missing placement.
docs/​design/​webhooks.md	Explains multi-volume placement rules.
docs/​architecture.md	Documents durability precedence and common-node requirements.

🧠 Review effort: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[jmclong (jmclong)]

Potential upgrade impact: durability PVCs resolved to different nodes

A concrete case is a Pod that mounts two durability-mode PVCs, for example db-data and db-wal. If their PVs resolve to different nodes (db-data to node-a, db-wal to node-b), the new intersection is empty and the webhook denies Pod admission with durability volumes have no common storage node. Before this change, the combined node list could allow scheduling on either node without ensuring both durability volumes were present there.

This can happen when the PVCs were provisioned or bound independently—for example, created at different times or for different consumers, or backed by existing PVs—and then later mounted by the same workload. Delayed binding (WaitForFirstConsumer) can help when the StorageClass/provisioner supports it and the PVCs are provisioned together for the same consumer, but it does not make already-bound PV locations common. The webhook's placement decision is based on each PV's selected-node annotation, falling back to its initial-node volume attribute.

The practical risk is that a rollout, reschedule, or recovery that needs a replacement Pod can stall because the Pod is rejected at admission; this does not by itself evict an already-running Pod. Customers can mitigate by checking all durability PVs referenced by a workload before upgrading or restarting it, ensuring those PVs resolve to a common node, and—if they do not—migrating/reprovisioning the data or otherwise reconciling placement before proceeding. Also worth noting for mixed-mode Pods: when any durability volume is present, this code applies only the durability node set, so availability volumes' preferred-node affinity is not retained.

[Hrishikesh Kashyap (Hrishikeshkashyap)]

Potential upgrade impact: durability PVCs resolved to different nodes

A concrete case is a Pod that mounts two durability-mode PVCs, for example db-data and db-wal. If their PVs resolve to different nodes (db-data to node-a, db-wal to node-b), the new intersection is empty and the webhook denies Pod admission with durability volumes have no common storage node. Before this change, the combined node list could allow scheduling on either node without ensuring both durability volumes were present there.

This can happen when the PVCs were provisioned or bound independently—for example, created at different times or for different consumers, or backed by existing PVs—and then later mounted by the same workload. Delayed binding (WaitForFirstConsumer) can help when the StorageClass/provisioner supports it and the PVCs are provisioned together for the same consumer, but it does not make already-bound PV locations common. The webhook's placement decision is based on each PV's selected-node annotation, falling back to its initial-node volume attribute.

The practical risk is that a rollout, reschedule, or recovery that needs a replacement Pod can stall because the Pod is rejected at admission; this does not by itself evict an already-running Pod. Customers can mitigate by checking all durability PVs referenced by a workload before upgrading or restarting it, ensuring those PVs resolve to a common node, and—if they do not—migrating/reprovisioning the data or otherwise reconciling placement before proceeding. Also worth noting for mixed-mode Pods: when any durability volume is present, this code applies only the durability node set, so availability volumes' preferred-node affinity is not retained.

The common-node requirement is intentional: the previous behavior could allow scheduling without ensuring all durability volumes were present on the selected node.

I’ve added a concise upgrade warning advising users to verify that their durability PVs resolve to a common storage node before upgrading or restarting affected workloads. It also clarifies that replacement Pod creation can be denied, running Pods are not evicted, and WaitForFirstConsumer does not relocate already-bound PVs.

Availability-volume preferences remain omitted for mixed-mode Pods in this PR; the durability restriction takes precedence. Preserving those preferences alongside the required constraint could be handled separately.

[Copilot]

🟡 Changes recommended

The acknowledged prerequisite fix in #643 for composing existing required affinity remains outstanding.

0 open findings

2 resolved since last review

• Injected durability affinity bypasses existing required node constraints
• Availability PV prematurely terminates node-affinity aggregation

🧠 Review effort: Balanced

[Copilot]

🔵 Needs a closer look

Empty-node validation remains incomplete, and safe affinity composition still depends on integrating and verifying #643.

1 open issue

• Reject empty resolved node names in durability placement · New

_{Review effort: Balanced}

[Hari (hasethuraman)]

What if durabilityNodes has two different node names? Lana Andreasyan (@landreasyan) what is your suggestion in that case? Should we better fail?

[Hrishikesh Kashyap (Hrishikeshkashyap)]

Metadata-derived placements are intersected, so different owner nodes result in admission denial. With the topology fallback, multiple allowed nodes are valid; we preserve and AND the selectors across volumes. If no node satisfies them, Kubernetes leaves the Pod Pending rather than the webhook choosing a node.

[Hari (hasethuraman)]

Are we going to implement this? Can we check if any pod in a state that can go un-scheduled post upgrade ?

[Hrishikesh Kashyap (Hrishikeshkashyap)]

A pre-upgrade check could be a kubectl/jq script, or a dry-run mode in the webhook that logs would-be denials. I'd think we should be tracking that as a follow-up issue rather than expanding this PR.

[Copilot]

🟡 Changes recommended

Valid topology-pinned durability volumes without selected-node metadata can now prevent replacement Pod creation.

1 open issue

• Rejects topology-pinned PVs without selected-node metadata · New

1 resolved since last review

• Reject empty resolved node names in durability placement

_{Review effort: Balanced}

[jmclong (jmclong)]

[P2] Avoid materializing the Cartesian product of retained PV topology selectors.

This fallback is reachable for durability PVs with retained spec.nodeAffinity but no selected-node metadata. Each two-term selector doubles the Pod selector terms. Using the production admission helper and applying its actual JSON patch, 12 PVs with identical two-term selectors produced 4,096 terms and a 3,485,834-byte patch. Each output term also copies the accumulated requirements, so webhook CPU/memory and the mutated Pod size grow along with the term count. Identical selectors do not need these combinations, and merely deduplicating identical inputs would not bound the general case.

A smaller fix is to leave retained PV topology on the PV rather than copy it into Pod affinity: the scheduler already checks the node affinity of every bound PV (checkBoundClaims in the VolumeBinding plugin). Keep intersecting metadata-derived owner nodes and compose that restriction with every existing nonempty Pod selector term using #643. Preserve denial for genuinely missing placement and conflicting metadata-derived owners. Topology conflicts can remain Pending, as intended here, without enumerating every combination or introducing a Node LIST.

Please add a regression using multiple topology-only PVs with two OR terms each, asserting bounded Pod patch size while retaining the metadata-owner and application-affinity constraints.

[Hrishikesh Kashyap (Hrishikeshkashyap)]

Fixed in the latest update. Retained PV topology stays on the PV and is enforced by Kubernetes’ scheduler; the webhook no longer expands it into Pod affinity terms. Metadata-derived placement is still intersected, and the Pod’s required affinity is composed with that constraint. Added a 12-PV regression that verifies bounded patch size.

[jmclong (jmclong)]

Verified the topology-selector growth fix and the bounded-patch regression, including preservation of application affinity and metadata-derived ownership constraints. Focused webhook unit tests pass; CI checks are green.