Skip to content

fix(snapshot): unblock PVC restore and warn on silent skip (#3951)#3964

Open
Roshr2211 wants to merge 1 commit into
loft-sh:mainfrom
Roshr2211:fix/issue-3951-pvc-status-rbac
Open

fix(snapshot): unblock PVC restore and warn on silent skip (#3951)#3964
Roshr2211 wants to merge 1 commit into
loft-sh:mainfrom
Roshr2211:fix/issue-3951-pvc-status-rbac

Conversation

@Roshr2211
Copy link
Copy Markdown

Summary

Fixes #3951.

  • RBAC: add persistentvolumeclaims/status to the syncer Role so the
    Status().Update() call in patcher.ApplyObjectPatch succeeds during
    PVC sync. Without this, restoring a PVC from a vcluster snapshot
    (after deleting the existing one) leaves it stuck Pending with
    cannot update resource 'persistentvolumeclaims/status' in API group ''.
  • Louder skip: when a user runs restore without first deleting an
    existing PVC, the restorer silently marks it Skipped and emits a
    generic Normal event. Users assume the restore worked and lose the
    snapshot's data. Upgrade the event to Warning, expand the message to
    explain the cause and the required action, and add an Infof at the
    detection site so the skip appears in normal logs. Skip semantics
    are preserved as a safety default — replacing a bound PVC would
    destroy live data.

Verification

  • Live RBAC check via impersonation: `PUT /api/v1/namespaces/X/persistentvolumeclaims/Y/status`
    as the syncer ServiceAccount. Before: 403 Forbidden matching the exact error from PVC restore from vcluster snapshot fails #3951. After: 200 OK,
    `status.phase=Bound` set, `subresource=status` in managedFields.
  • Real-running vcluster on rancher-desktop with the fixed image: PVC created in vcluster
    syncs Bound to host with no `persistentvolumeclaims/status` errors in syncer logs.
  • `helm unittest chart`: 208/208 pass, including the new contains-assertion in
    `chart/tests/role_test.yaml`.
  • `go test ./pkg/snapshot/...`: pass, including new unit tests for
    `inProgressPVCReconcileFinished` covering the Skipped / Completed / Failed branches.

Test plan

  • Existing chart tests pass
  • New chart test pins the rule's resource list
  • New Go unit test asserts the Skipped event is Warning with actionable message;
    Completed/Failed branches unchanged
  • Live impersonated PUT to `/status` succeeds with the fixed Role
  • PVC sync works in a running vcluster pod built from this branch

)

Add persistentvolumeclaims/status to the syncer Role so virtual->host
status sync no longer fails with 'cannot update resource
persistentvolumeclaims/status'.

Make the skip path louder: emit a Warning event explaining that the
existing PVC blocked the restore and snapshot data was not applied.
Skip semantics preserved as a safety default.
@Roshr2211 Roshr2211 requested review from a team as code owners May 28, 2026 17:51
@mfranczy
Copy link
Copy Markdown
Contributor

mfranczy commented Jun 1, 2026

@codex review

@chatgpt-codex-connector
Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Chef's kiss.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

PVC restore from vcluster snapshot fails

2 participants