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

chart/templates/role.yaml

@@ -37,7 +37,7 @@ rules:
     resources: ["configmaps", "secrets", "services", "pods", "pods/attach", "pods/portforward", "pods/exec", "persistentvolumeclaims"]
     verbs: ["create", "delete", "patch", "update", "get", "list", "watch"]
   - apiGroups: [""]
-    resources: ["pods/status", "pods/ephemeralcontainers"]
+    resources: ["pods/status", "pods/ephemeralcontainers", "persistentvolumeclaims/status"]
     verbs: ["patch", "update"]
   {{- end }}
   {{- if ge (.Capabilities.KubeVersion.Minor|int) 35 }}

chart/tests/role_test.yaml

@@ -111,6 +111,12 @@ tests:
       - equal:
           path: metadata.namespace
           value: my-namespace
+      - contains:
+          path: rules
+          content:
+            apiGroups: [""]
+            resources: ["pods/status", "pods/ephemeralcontainers", "persistentvolumeclaims/status"]
+            verbs: ["patch", "update"]
 
   - it: multi-namespace mode
     set:

pkg/snapshot/volumes/csi/restoreinprogress.go

@@ -176,7 +176,12 @@ func (r *Restorer) reconcileInProgressPVC(ctx context.Context, requestObj runtim
 	originalPVC := &volumeRestoreRequest.PersistentVolumeClaim
 	_, err := r.kubeClient.CoreV1().PersistentVolumeClaims(originalPVC.Namespace).Get(ctx, originalPVC.Name, metav1.GetOptions{})
 	if err == nil {
-		// existing PVC found
+		// existing PVC found — skip restore to avoid data loss; user must delete the
+		// PVC and re-run the restore to recover data from the snapshot.
+		r.logger.Infof(
+			"PersistentVolumeClaim %s/%s already exists; skipping volume restore. "+
+				"Data from the snapshot was NOT applied. Delete the PersistentVolumeClaim and re-run restore to recover snapshot data.",
+			originalPVC.Namespace, originalPVC.Name)
 		status.Phase = snapshotapi.VolumeSnapshotPhaseSkipped
 		return status, nil
 	} else if !kerrors.IsNotFound(err) {
@@ -370,9 +375,10 @@ func (r *Restorer) inProgressPVCReconcileFinished(requestObj runtime.Object, vol
 			err,
 		}
 	case snapshotapi.VolumeSnapshotPhaseSkipped:
-		eventType = corev1.EventTypeNormal
+		eventType = corev1.EventTypeWarning
 		reason = "VolumeRestoreSkipped"
-		messageFmt = "Skipped restoring PersistentVolumeClaim %s/%s"
+		messageFmt = "Skipped restoring PersistentVolumeClaim %s/%s: a PersistentVolumeClaim with this name already exists. " +
+			"Snapshot data was NOT applied. Delete the PersistentVolumeClaim and re-run the restore to recover data from the snapshot."
 		args = []interface{}{
 			volumeRestoreRequest.PersistentVolumeClaim.Namespace,
 			volumeRestoreRequest.PersistentVolumeClaim.Name,

pkg/snapshot/volumes/csi/restoreinprogress_test.go

@@ -0,0 +1,113 @@
+package csi
+
+import (
+	"strings"
+	"testing"
+
+	snapshotapi "github.com/loft-sh/api/v4/pkg/snapshot"
+	"github.com/loft-sh/vcluster/pkg/snapshot/volumes"
+	"github.com/loft-sh/vcluster/pkg/util/loghelper"
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/client-go/tools/events"
+)
+
+func TestInProgressPVCReconcileFinishedSkippedEmitsWarning(t *testing.T) {
+	fakeRecorder := events.NewFakeRecorder(10)
+	restorer := &Restorer{
+		snapshotHandler: snapshotHandler{
+			eventRecorder: fakeRecorder,
+			logger:        loghelper.New("restore-skip-event-test"),
+		},
+	}
+
+	requestObj := &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: "req", Namespace: "host-ns"}}
+	volumeRestoreRequest := volumes.RestoreRequest{
+		PersistentVolumeClaim: corev1.PersistentVolumeClaim{
+			ObjectMeta: metav1.ObjectMeta{Name: "data", Namespace: "app-ns"},
+		},
+	}
+	volumeRestoreStatus := volumes.RestoreStatus{Phase: snapshotapi.VolumeSnapshotPhaseSkipped}
+
+	restorer.inProgressPVCReconcileFinished(requestObj, volumeRestoreRequest, volumeRestoreStatus, nil)
+
+	select {
+	case event := <-fakeRecorder.Events:
+		if !strings.HasPrefix(event, "Warning ") {
+			t.Errorf("expected Warning event for skipped restore, got: %s", event)
+		}
+		if !strings.Contains(event, "VolumeRestoreSkipped") {
+			t.Errorf("expected reason VolumeRestoreSkipped, got: %s", event)
+		}
+		if !strings.Contains(event, "app-ns/data") {
+			t.Errorf("expected event to reference the PVC namespace/name, got: %s", event)
+		}
+		for _, fragment := range []string{
+			"already exists",
+			"NOT applied",
+			"Delete the PersistentVolumeClaim",
+			"re-run the restore",
+		} {
+			if !strings.Contains(event, fragment) {
+				t.Errorf("expected event message to contain %q, got: %s", fragment, event)
+			}
+		}
+	default:
+		t.Fatal("expected an event to be recorded for skipped restore, got none")
+	}
+}
+
+func TestInProgressPVCReconcileFinishedCompletedAndFailedUnchanged(t *testing.T) {
+	cases := []struct {
+		name           string
+		status         volumes.RestoreStatus
+		expectedPrefix string
+		expectedReason string
+	}{
+		{
+			name:           "completed",
+			status:         volumes.RestoreStatus{Phase: snapshotapi.VolumeSnapshotPhaseCompleted},
+			expectedPrefix: "Normal ",
+			expectedReason: "VolumeRestored",
+		},
+		{
+			name:           "failed",
+			status:         volumes.RestoreStatus{Phase: snapshotapi.VolumeSnapshotPhaseFailed},
+			expectedPrefix: "Warning ",
+			expectedReason: "VolumeRestoreFailed",
+		},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			fakeRecorder := events.NewFakeRecorder(10)
+			restorer := &Restorer{
+				snapshotHandler: snapshotHandler{
+					eventRecorder: fakeRecorder,
+					logger:        loghelper.New("restore-event-regression-test"),
+				},
+			}
+
+			requestObj := &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: "req", Namespace: "host-ns"}}
+			volumeRestoreRequest := volumes.RestoreRequest{
+				PersistentVolumeClaim: corev1.PersistentVolumeClaim{
+					ObjectMeta: metav1.ObjectMeta{Name: "data", Namespace: "app-ns"},
+				},
+			}
+
+			restorer.inProgressPVCReconcileFinished(requestObj, volumeRestoreRequest, tc.status, nil)
+
+			select {
+			case event := <-fakeRecorder.Events:
+				if !strings.HasPrefix(event, tc.expectedPrefix) {
+					t.Errorf("expected prefix %q, got: %s", tc.expectedPrefix, event)
+				}
+				if !strings.Contains(event, tc.expectedReason) {
+					t.Errorf("expected reason %q, got: %s", tc.expectedReason, event)
+				}
+			default:
+				t.Fatalf("expected an event to be recorded for %s phase, got none", tc.name)
+			}
+		})
+	}
+}