fix(vi): respect storageClassName when creating from snapshot (#1533)

Fix the VirtualImage controller to respect user-specified storageClassName when creating images 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]>

Author: loktev-d

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

@@ -39,6 +39,7 @@ import (
 	"github.com/deckhouse/virtualization-controller/pkg/controller/service"
 	vdsupplements "github.com/deckhouse/virtualization-controller/pkg/controller/vd/internal/supplements"
 	"github.com/deckhouse/virtualization-controller/pkg/eventrecord"
+	"github.com/deckhouse/virtualization-controller/pkg/logger"
 	"github.com/deckhouse/virtualization/api/core/v1alpha2"
 	"github.com/deckhouse/virtualization/api/core/v1alpha2/vdcondition"
 )
@@ -255,8 +256,9 @@ func (s CreatePVCFromVDSnapshotStep) validateStorageClassCompatibility(ctx conte
 		return fmt.Errorf("cannot fetch target storage class %q: %w", targetSCName, err)
 	}
 
+	log, _ := logger.GetDataSourceContext(ctx, "objectref")
 	if vs.Spec.Source.PersistentVolumeClaimName == nil || *vs.Spec.Source.PersistentVolumeClaimName == "" {
-		// Can't determine original PVC, skip validation
+		log.With("volumeSnapshot.name", vs.Name).Debug("Cannot determine original PVC from VolumeSnapshot, skipping storage class compatibility validation")
 		return nil
 	}
 
@@ -274,7 +276,7 @@ func (s CreatePVCFromVDSnapshotStep) validateStorageClassCompatibility(ctx conte
 	}
 
 	if originalProvisioner == "" {
-		// Can't determine original provisioner, skip validation
+		log.With("pvc.name", pvcName).Debug("Cannot determine original provisioner from PVC annotations, skipping storage class compatibility validation")
 		return nil
 	}
 

images/virtualization-artifact/pkg/controller/vi/internal/source/step/create_pvc_step.go

@@ -23,6 +23,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 CreatePersistentVolumeClaimStep) Take(ctx context.Context, vi *v1alpha2.
 		return &reconcile.Result{}, nil
 	}
 
+	if err := s.validateStorageClassCompatibility(ctx, vi, vdSnapshot, vs); err != nil {
+		vi.Status.Phase = v1alpha2.ImageFailed
+		s.cb.
+			Status(metav1.ConditionFalse).
+			Reason(vicondition.ProvisioningFailed).
+			Message(err.Error())
+		s.recorder.Event(
+			vi,
+			corev1.EventTypeWarning,
+			v1alpha2.ReasonDataSourceSyncFailed,
+			err.Error(),
+		)
+		return &reconcile.Result{}, nil
+	}
+
 	pvc := s.buildPVC(vi, vs)
 
 	err = s.client.Create(ctx, pvc)
@@ -124,9 +140,14 @@ func (s CreatePersistentVolumeClaimStep) Take(ctx context.Context, vi *v1alpha2.
 }
 
 func (s CreatePersistentVolumeClaimStep) buildPVC(vi *v1alpha2.VirtualImage, vs *vsv1.VolumeSnapshot) *corev1.PersistentVolumeClaim {
-	storageClassName := vs.Annotations[annotations.AnnStorageClassName]
-	if storageClassName == "" {
-		storageClassName = vs.Annotations[annotations.AnnStorageClassNameDeprecated]
+	var storageClassName string
+	if vi.Spec.PersistentVolumeClaim.StorageClass != nil && *vi.Spec.PersistentVolumeClaim.StorageClass != "" {
+		storageClassName = *vi.Spec.PersistentVolumeClaim.StorageClass
+	} else {
+		storageClassName = vs.Annotations[annotations.AnnStorageClassName]
+		if storageClassName == "" {
+			storageClassName = vs.Annotations[annotations.AnnStorageClassNameDeprecated]
+		}
 	}
 	volumeMode := vs.Annotations[annotations.AnnVolumeMode]
 	if volumeMode == "" {
@@ -185,3 +206,52 @@ func (s CreatePersistentVolumeClaimStep) buildPVC(vi *v1alpha2.VirtualImage, vs
 		Spec: spec,
 	}
 }
+
+func (s CreatePersistentVolumeClaimStep) validateStorageClassCompatibility(ctx context.Context, vi *v1alpha2.VirtualImage, vdSnapshot *v1alpha2.VirtualDiskSnapshot, vs *vsv1.VolumeSnapshot) error {
+	if vi.Spec.PersistentVolumeClaim.StorageClass == nil || *vi.Spec.PersistentVolumeClaim.StorageClass == "" {
+		return nil
+	}
+
+	targetSCName := *vi.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)
+	}
+
+	log, _ := logger.GetDataSourceContext(ctx, "objectref")
+	if vs.Spec.Source.PersistentVolumeClaimName == nil || *vs.Spec.Source.PersistentVolumeClaimName == "" {
+		log.With("volumeSnapshot.name", vs.Name).Debug("Cannot determine original PVC from VolumeSnapshot, skipping storage class compatibility 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 == "" {
+		log.With("pvc.name", pvcName).Debug("Cannot determine original provisioner from PVC annotations, skipping storage class compatibility 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
+}