Define unresolved PVC admission behavior by Hrishikeshkashyap · Pull Request #645 · Azure/local-csi-driver

Hrishikesh Kashyap (Hrishikeshkashyap) · 2026-10-13T15:12:41Z

A missing PVC previously caused the webhook to allow the entire Pod early,
discarding placement constraints from other resolved volumes.

Changes

Preserve Pod-before-PVC behavior: skip missing ordinary claims and continue
processing other volumes.
Confirm PVC cache misses with an uncached API read before treating claims
as missing.
Derive generic-ephemeral claim names as <pod-name>-<volume-name>.
Preserve resolved placement constraints alongside missing, generic-ephemeral,
and unbound claims.
Surface PVC lookup errors.

Tests

Focused Linux webhook unit tests, manager cross-build, go vet, spelling,
and whitespace checks passed. Regression coverage includes both volume orders,
Pod-before-PVC, cache-miss/API-hit, API failures, and cached-claim reads.

Integration tests were not run locally because envtest binaries are unavailable.

Limitation

Creating a missing PVC later does not rerun Pod admission, so the webhook
cannot retroactively add that claim's placement constraints to an existing Pod.

Hrishikesh Kashyap (Hrishikeshkashyap) · 2026-10-14T09:55:23Z

The External E2E failure appears capacity-related: the test Pod remained Pending because the single Kind node reported insufficient free storage. The regular amd64/arm64 E2E and CSI sanity checks passed. Could someone with repository permissions rerun the failed job?
Trivy is flagging CVE-2026-78669 in golang.org/x/net v0.59.0; versions before v0.60.0 are affected.
The same check is failing on #643, #645, and #646, and upstream main is still on v0.59.0.

Copilot

🟡 Changes recommended

Stale PVC cache misses can incorrectly reject Pods whose claims already exist.

1 open finding

Avoid rejecting PVCs missing from the informer cache · New

What changed in this PR

Defines unresolved-claim admission behavior while preserving placement constraints from resolved volumes.

Changes:

Denies Pods referencing missing ordinary PVCs.
Corrects generic-ephemeral claim naming and continues processing unresolved claims.
Adds regression tests and documents the behavior.

File	Description
internal/webhook/hyperconverged/handler.go	Updates claim resolution and admission handling.
internal/webhook/hyperconverged/handler_unit_test.go	Tests missing, ephemeral, and unbound claims.
internal/webhook/hyperconverged/handler_test.go	Tests missing-claim denial in both volume orders.
docs/design/webhooks.md	Documents unresolved-claim behavior.

🧠 Review effort: Balanced

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

Copilot

🟡 Changes recommended

Generic-ephemeral PVC ownership needs validation, and the new integration test needs cache synchronization.

2 open findings

1 resolved since last review

Avoid rejecting PVCs missing from the informer cache

🧠 Review effort: Balanced

Copilot

🟢 Approval recommended

No blocking issues were identified; regression coverage addresses the changed behavior, though integration tests remain unrun locally.

0 open findings

2 resolved since last review

🧠 Review effort: Balanced

Copilot

🟢 Approval recommended

No blocking issues remain, and regression coverage addresses the changed admission behavior.

No open issues 🎉

_{Review effort: Balanced}

Copilot

🟡 Changes recommended

The new continuation path can weaken application-required affinity, and a diagnostic-only lookup can unnecessarily reject Pod admission.

2 open issues

_{Review effort: Balanced}

Hari (hasethuraman2)

Looks good to me. It is good to fail the admission when the pod has a PVC that is not resolvable at the time of pod creation.

jmclong (jmclong)

Verified the rebase includes #643 and reviewed the missing-claim handling, PVC API-reader fallback, and error handling. Focused webhook unit tests pass; CI checks are green. The previously requested changes are addressed.

Reject Pods that reference missing ordinary PVCs so placement is recalculated on a later CREATE. Preserve generic ephemeral and unbound WFFC claims while continuing to enforce resolved durability volumes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Verify ordinary missing claims, generated ephemeral claim naming, and unbound claims preserve resolved durability placement. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Document admission behavior for ordinary, generated ephemeral, and initially unbound claims. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Preserve Pod-before-PVC behavior while enforcing placement from resolved claims. Confirm PVC cache misses with the API reader and surface lookup failures. Update regression tests and correct the unresolved-claim documentation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

🟢 Approval recommended

No active review issues were identified in the webhook’s PVC placement-constraint changes.

No open issues 🎉

2 resolved since last review

_{Review effort: Balanced}

Hrishikesh Kashyap (Hrishikeshkashyap) requested review from a team, Simon Croome (croomes), jmclong (jmclong) and Lana Andreasyan (landreasyan) as code owners October 13, 2026 15:12

Hrishikesh Kashyap (Hrishikeshkashyap) force-pushed the fix/claim-resolution-admission branch from 9674f6a to 22f8517 Compare October 13, 2026 15:17

Hrishikesh Kashyap (Hrishikeshkashyap) mentioned this pull request Oct 13, 2026

Fix durability volume placement handling #641

Closed

6 of 9 tasks

jmclong (jmclong) requested changes Oct 15, 2026

View reviewed changes

Comment thread internal/webhook/hyperconverged/handler.go Outdated

jmclong (jmclong) requested a balanced review from Copilot October 15, 2026 13:22

Copilot started reviewing on behalf of jmclong (jmclong) October 15, 2026 13:23

View session

Copilot AI reviewed Oct 15, 2026

View reviewed changes

Comment thread internal/webhook/hyperconverged/handler.go Outdated

Hrishikesh Kashyap (Hrishikeshkashyap) force-pushed the fix/claim-resolution-admission branch from 22f8517 to 7acc4ea Compare October 16, 2026 09:44

Hrishikesh Kashyap (Hrishikeshkashyap) requested review from jmclong (jmclong) and a balanced review from Copilot October 16, 2026 10:00

Copilot started reviewing on behalf of Hrishikesh Kashyap (Hrishikeshkashyap) October 16, 2026 10:04

View session

Copilot AI reviewed Oct 16, 2026

View reviewed changes

Comment thread internal/webhook/hyperconverged/handler.go

Comment thread internal/webhook/hyperconverged/handler_test.go

Hrishikesh Kashyap (Hrishikeshkashyap) force-pushed the fix/claim-resolution-admission branch from 7acc4ea to 498689c Compare October 16, 2026 12:26

Hrishikesh Kashyap (Hrishikeshkashyap) requested a balanced review from Copilot October 16, 2026 12:50

Copilot started reviewing on behalf of Hrishikesh Kashyap (Hrishikeshkashyap) October 16, 2026 12:50

View session

Copilot AI reviewed Oct 16, 2026

View reviewed changes

jmclong (jmclong) requested a balanced review from Copilot October 20, 2026 14:28

Copilot started reviewing on behalf of jmclong (jmclong) October 20, 2026 14:29

View session

Copilot AI reviewed Oct 20, 2026

View reviewed changes

Hari (hasethuraman) reviewed Oct 21, 2026

View reviewed changes

Comment thread internal/webhook/hyperconverged/handler.go Outdated

Hari (hasethuraman) reviewed Oct 21, 2026

View reviewed changes

Comment thread internal/webhook/hyperconverged/handler.go Outdated

Hrishikesh Kashyap (Hrishikeshkashyap) force-pushed the fix/claim-resolution-admission branch 2 times, most recently from ea2f4e6 to e506468 Compare October 22, 2026 14:11

jmclong (jmclong) requested a balanced review from Copilot October 22, 2026 14:28

Copilot started reviewing on behalf of jmclong (jmclong) October 22, 2026 14:30

View session

Copilot AI reviewed Oct 22, 2026

View reviewed changes

Comment thread internal/webhook/hyperconverged/handler.go

Comment thread internal/webhook/hyperconverged/handler.go Outdated

Hrishikesh Kashyap (Hrishikeshkashyap) force-pushed the fix/claim-resolution-admission branch 2 times, most recently from 3be2d79 to 1646c6e Compare October 27, 2026 16:31

Hari (hasethuraman2) approved these changes Oct 28, 2026

View reviewed changes

Hari (hasethuraman) previously approved these changes Oct 28, 2026

View reviewed changes

jmclong (jmclong) previously approved these changes Oct 28, 2026

View reviewed changes

Hrishikesh Kashyap (Hrishikeshkashyap) and others added 4 commits October 28, 2026 21:48

test(webhook): cover claim resolution policy

18b1645

Verify ordinary missing claims, generated ephemeral claim naming, and unbound claims preserve resolved durability placement. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

docs(webhook): explain unresolved claim handling

bbe82a0

Document admission behavior for ordinary, generated ephemeral, and initially unbound claims. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Hrishikesh Kashyap (Hrishikeshkashyap) dismissed stale reviews from jmclong (jmclong) and Hari (hasethuraman) via 5061a66 October 28, 2026 16:21

Hrishikesh Kashyap (Hrishikeshkashyap) force-pushed the fix/claim-resolution-admission branch from 1646c6e to 5061a66 Compare October 28, 2026 16:21

jmclong (jmclong) requested a balanced review from Copilot October 28, 2026 17:16

Copilot started reviewing on behalf of jmclong (jmclong) October 28, 2026 17:17

View session

Copilot AI reviewed Oct 28, 2026

View reviewed changes

Conversation

Hrishikesh Kashyap (Hrishikeshkashyap) commented Oct 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Tests

Limitation

Uh oh!

Hrishikesh Kashyap (Hrishikeshkashyap) commented Oct 14, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Hari (hasethuraman2) left a comment

Choose a reason for hiding this comment

Uh oh!

jmclong (jmclong) left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Hrishikesh Kashyap (Hrishikeshkashyap) commented Oct 13, 2026 •

edited

Loading