diff --git a/chart/templates/role.yaml b/chart/templates/role.yaml index 90dbcd05d2..66fe1d6229 100644 --- a/chart/templates/role.yaml +++ b/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 }} diff --git a/chart/tests/role_test.yaml b/chart/tests/role_test.yaml index 3433b5d080..e26027d1fd 100644 --- a/chart/tests/role_test.yaml +++ b/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: diff --git a/pkg/snapshot/volumes/csi/restoreinprogress.go b/pkg/snapshot/volumes/csi/restoreinprogress.go index 5d0bfd22fc..801941f5e9 100644 --- a/pkg/snapshot/volumes/csi/restoreinprogress.go +++ b/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, diff --git a/pkg/snapshot/volumes/csi/restoreinprogress_test.go b/pkg/snapshot/volumes/csi/restoreinprogress_test.go new file mode 100644 index 0000000000..ac9e1426b3 --- /dev/null +++ b/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) + } + }) + } +}