fix(vd): respect user-specified storage class when restoring from snapshot (#1417)

Fix the VirtualDisk controller to respect user-specified storageClassName when creating disks from VirtualDiskSnapshot. Previously, the controller was always using the storage class from the original disk (stored in VolumeSnapshot annotations), completely ignoring the user-specified value in spec.persistentVolumeClaim.storageClassName. Also add error message when user tries to perform cross-provider restore.

Signed-off-by: Daniil Loktev <[email protected]>
(cherry picked from commit 55e2188)

Author: loktev-d

images/virtualization-artifact/pkg/common/annotations/annotations.go

@@ -160,6 +160,9 @@ const (
 	// AnnAccessMode is the annotation for indicating that access mode.  (USED IN STORAGE sds controllers)
 	AnnAccessModes           = AnnAPIGroupV + "/access-mode"
 	AnnAccessModesDeprecated = "accessModes"
+	// AnnStorageProvisioner is the annotation for indicating storage provisioner
+	AnnStorageProvisioner           = "volume.kubernetes.io/storage-provisioner"
+	AnnStorageProvisionerDeprecated = "volume.beta.kubernetes.io/storage-provisioner"
 
 	// AppLabel is the app name label.
 	AppLabel = "app"

images/virtualization-artifact/pkg/controller/vd/internal/source/step/create_pvc_from_vdsnapshot_step.go

@@ -24,6 +24,7 @@ import (
 
 	vsv1 "github.com/kubernetes-csi/external-snapshotter/client/v6/apis/volumesnapshot/v1"
 	corev1 "k8s.io/api/core/v1"
+	storagev1 "k8s.io/api/storage/v1"
 	k8serrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
@@ -103,6 +104,21 @@ func (s CreatePVCFromVDSnapshotStep) Take(ctx context.Context, vd *v1alpha2.Virt
 		return &reconcile.Result{}, nil
 	}
 
+	if err := s.validateStorageClassCompatibility(ctx, vd, vdSnapshot, vs); err != nil {
+		vd.Status.Phase = v1alpha2.DiskFailed
+		s.cb.
+			Status(metav1.ConditionFalse).
+			Reason(vdcondition.ProvisioningFailed).
+			Message(err.Error())
+		s.recorder.Event(
+			vd,
+			corev1.EventTypeWarning,
+			v1alpha2.ReasonDataSourceSyncFailed,
+			err.Error(),
+		)
+		return &reconcile.Result{}, nil
+	}
+
 	pvc := s.buildPVC(vd, vs)
 
 	err = s.client.Create(ctx, pvc)
@@ -158,10 +174,16 @@ func (s CreatePVCFromVDSnapshotStep) AddOriginalMetadata(vd *v1alpha2.VirtualDis
 }
 
 func (s CreatePVCFromVDSnapshotStep) buildPVC(vd *v1alpha2.VirtualDisk, vs *vsv1.VolumeSnapshot) *corev1.PersistentVolumeClaim {
-	storageClassName := vs.Annotations[annotations.AnnStorageClassName]
-	if storageClassName == "" {
-		storageClassName = vs.Annotations[annotations.AnnStorageClassNameDeprecated]
+	var storageClassName string
+	if vd.Spec.PersistentVolumeClaim.StorageClass != nil && *vd.Spec.PersistentVolumeClaim.StorageClass != "" {
+		storageClassName = *vd.Spec.PersistentVolumeClaim.StorageClass
+	} else {
+		storageClassName = vs.Annotations[annotations.AnnStorageClassName]
+		if storageClassName == "" {
+			storageClassName = vs.Annotations[annotations.AnnStorageClassNameDeprecated]
+		}
 	}
+
 	volumeMode := vs.Annotations[annotations.AnnVolumeMode]
 	if volumeMode == "" {
 		volumeMode = vs.Annotations[annotations.AnnVolumeModeDeprecated]
@@ -219,3 +241,51 @@ func (s CreatePVCFromVDSnapshotStep) buildPVC(vd *v1alpha2.VirtualDisk, vs *vsv1
 		Spec: spec,
 	}
 }
+
+func (s CreatePVCFromVDSnapshotStep) validateStorageClassCompatibility(ctx context.Context, vd *v1alpha2.VirtualDisk, vdSnapshot *v1alpha2.VirtualDiskSnapshot, vs *vsv1.VolumeSnapshot) error {
+	if vd.Spec.PersistentVolumeClaim.StorageClass == nil || *vd.Spec.PersistentVolumeClaim.StorageClass == "" {
+		return nil
+	}
+
+	targetSCName := *vd.Spec.PersistentVolumeClaim.StorageClass
+
+	var targetSC storagev1.StorageClass
+	err := s.client.Get(ctx, types.NamespacedName{Name: targetSCName}, &targetSC)
+	if err != nil {
+		return fmt.Errorf("cannot fetch target storage class %q: %w", targetSCName, err)
+	}
+
+	if vs.Spec.Source.PersistentVolumeClaimName == nil || *vs.Spec.Source.PersistentVolumeClaimName == "" {
+		// Can't determine original PVC, skip validation
+		return nil
+	}
+
+	pvcName := *vs.Spec.Source.PersistentVolumeClaimName
+
+	var originalPVC corev1.PersistentVolumeClaim
+	err = s.client.Get(ctx, types.NamespacedName{Name: pvcName, Namespace: vdSnapshot.Namespace}, &originalPVC)
+	if err != nil {
+		return fmt.Errorf("cannot fetch original PVC %q: %w", pvcName, err)
+	}
+
+	originalProvisioner := originalPVC.Annotations[annotations.AnnStorageProvisioner]
+	if originalProvisioner == "" {
+		originalProvisioner = originalPVC.Annotations[annotations.AnnStorageProvisionerDeprecated]
+	}
+
+	if originalProvisioner == "" {
+		// Can't determine original provisioner, skip validation
+		return nil
+	}
+
+	if targetSC.Provisioner != originalProvisioner {
+		return fmt.Errorf(
+			"cannot restore snapshot to storage class %q: incompatible storage providers. "+
+				"Original snapshot was created by %q, target storage class uses %q. "+
+				"Cross-provider snapshot restore is not supported",
+			targetSCName, originalProvisioner, targetSC.Provisioner,
+		)
+	}
+
+	return nil
+}