Default virt storage class

Signed-off-by: Alex Kalenyuk <akalenyu@redhat.com>

pkg/controller/common/util.go

@@ -27,6 +27,7 @@ import (
 	"net/http"
 	"reflect"
 	"regexp"
+	"sort"
 	"strconv"
 	"strings"
 	"sync"
@@ -211,6 +212,8 @@ const (
 
 	//AnnDefaultStorageClass is the annotation indicating that a storage class is the default one.
 	AnnDefaultStorageClass = "storageclass.kubernetes.io/is-default-class"
+	// AnnDefaultVirtStorageClass is the annotation indicating that a storage class is the default one for virtualization purposes
+	AnnDefaultVirtStorageClass = "storageclass.kubevirt.io/is-default-class"
 
 	// AnnOpenShiftImageLookup is the annotation for OpenShift image stream lookup
 	AnnOpenShiftImageLookup = "alpha.image.policy.openshift.io/resolve-names"
@@ -434,22 +437,50 @@ func GetVolumeMode(pvc *corev1.PersistentVolumeClaim) corev1.PersistentVolumeMod
 	return util.ResolveVolumeMode(pvc.Spec.VolumeMode)
 }
 
-// GetStorageClassByName looks up the storage class based on the name. If no storage class is found returns nil
+// GetStorageClassByName looks up the storage class based on the name
+// If name is nil, it looks for the following, in this order:
+// default kubevirt storage class (if the caller is interested) storageclass.kubevirt.io/is-default-class
+// default k8s storage class storageclass.kubernetes.io/is-default-class
+// If no storage class is found, returns nil
 func GetStorageClassByName(ctx context.Context, client client.Client, name *string) (*storagev1.StorageClass, error) {
+	if name == nil {
+		return GetFallbackStorageClass(ctx, client, false)
+	}
+
 	// look up storage class by name
-	if name != nil {
-		storageClass := &storagev1.StorageClass{}
-		if err := client.Get(ctx, types.NamespacedName{Name: *name}, storageClass); err != nil {
-			if k8serrors.IsNotFound(err) {
-				return nil, nil
-			}
-			klog.V(3).Info("Unable to retrieve storage class", "storage class name", *name)
-			return nil, errors.Errorf("unable to retrieve storage class %s", *name)
+	storageClass := &storagev1.StorageClass{}
+	if err := client.Get(ctx, types.NamespacedName{Name: *name}, storageClass); err != nil {
+		if k8serrors.IsNotFound(err) {
+			return nil, nil
 		}
-		return storageClass, nil
+		klog.V(3).Info("Unable to retrieve storage class", "storage class name", *name)
+		return nil, errors.Errorf("unable to retrieve storage class %s", *name)
 	}
-	// No storage class found, just return nil for storage class and let caller deal with it.
-	return GetDefaultStorageClass(ctx, client)
+
+	return storageClass, nil
+}
+
+// GetStorageClassByNameWithVirtFallback looks up the storage class based on the name
+// If name is nil, it looks for the following, in this order:
+// default kubevirt storage class (if the caller is interested) storageclass.kubevirt.io/is-default-class
+// default k8s storage class storageclass.kubernetes.io/is-default-class
+// If no storage class is found, returns nil
+func GetStorageClassByNameWithVirtFallback(ctx context.Context, client client.Client, name *string, considerVirtSc bool) (*storagev1.StorageClass, error) {
+	if name == nil {
+		return GetFallbackStorageClass(ctx, client, considerVirtSc)
+	}
+
+	// look up storage class by name
+	storageClass := &storagev1.StorageClass{}
+	if err := client.Get(ctx, types.NamespacedName{Name: *name}, storageClass); err != nil {
+		if k8serrors.IsNotFound(err) {
+			return nil, nil
+		}
+		klog.V(3).Info("Unable to retrieve storage class", "storage class name", *name)
+		return nil, errors.Errorf("unable to retrieve storage class %s", *name)
+	}
+
+	return storageClass, nil
 }
 
 // GetDefaultStorageClass returns the default storage class or nil if none found
@@ -460,14 +491,62 @@ func GetDefaultStorageClass(ctx context.Context, client client.Client) (*storage
 		return nil, errors.New("unable to retrieve storage classes")
 	}
 	for _, storageClass := range storageClasses.Items {
-		if storageClass.Annotations["storageclass.kubernetes.io/is-default-class"] == "true" {
+		if storageClass.Annotations[AnnDefaultStorageClass] == "true" {
 			return &storageClass, nil
 		}
 	}
 
 	return nil, nil
 }
 
+// GetFallbackStorageClass returns nil
+func GetFallbackStorageClass(ctx context.Context, client client.Client, considerVirtStorageClass bool) (*storagev1.StorageClass, error) {
+	storageClasses := &storagev1.StorageClassList{}
+	if err := client.List(ctx, storageClasses); err != nil {
+		klog.V(3).Info("Unable to retrieve available storage classes")
+		return nil, errors.New("unable to retrieve storage classes")
+	}
+
+	if considerVirtStorageClass {
+		virtSc := GetPlatformDefaultStorageClass(ctx, storageClasses, AnnDefaultVirtStorageClass)
+		if virtSc != nil {
+			return virtSc, nil
+		}
+	}
+	return GetPlatformDefaultStorageClass(ctx, storageClasses, AnnDefaultStorageClass), nil
+}
+
+// GetPlatformDefaultStorageClass returns the default storage class according to the provided annotation or nil if none found
+func GetPlatformDefaultStorageClass(ctx context.Context, storageClasses *storagev1.StorageClassList, defaultAnnotationKey string) *storagev1.StorageClass {
+	defaultClasses := []storagev1.StorageClass{}
+
+	for _, storageClass := range storageClasses.Items {
+		if storageClass.Annotations[defaultAnnotationKey] == "true" {
+			defaultClasses = append(defaultClasses, storageClass)
+		}
+	}
+
+	if len(defaultClasses) == 0 {
+		return nil
+	}
+
+	// Primary sort by creation timestamp, newest first
+	// Secondary sort by class name, ascending order
+	// Follows k8s behavior
+	// https://github.com/kubernetes/kubernetes/blob/731068288e112c8b5af70f676296cc44661e84f4/pkg/volume/util/storageclass.go#L58-L59
+	sort.Slice(defaultClasses, func(i, j int) bool {
+		if defaultClasses[i].CreationTimestamp.UnixNano() == defaultClasses[j].CreationTimestamp.UnixNano() {
+			return defaultClasses[i].Name < defaultClasses[j].Name
+		}
+		return defaultClasses[i].CreationTimestamp.UnixNano() > defaultClasses[j].CreationTimestamp.UnixNano()
+	})
+	if len(defaultClasses) > 1 {
+		klog.V(3).Infof("%d default StorageClasses were found, choosing: %s", len(defaultClasses), defaultClasses[0].Name)
+	}
+
+	return &defaultClasses[0]
+}
+
 // GetFilesystemOverheadForStorageClass determines the filesystem overhead defined in CDIConfig for the storageClass.
 func GetFilesystemOverheadForStorageClass(ctx context.Context, client client.Client, storageClassName *string) (cdiv1.Percent, error) {
 	cdiConfig := &cdiv1.CDIConfig{}

pkg/controller/datavolume/util.go

@@ -94,7 +94,8 @@ func renderPvcSpecVolumeModeAndAccessModes(client client.Client, recorder record
 		pvcSpec.VolumeMode = &volumeMode
 	}
 
-	storageClass, err := cc.GetStorageClassByName(context.TODO(), client, dv.Spec.Storage.StorageClassName)
+	shouldConsiderVirt := cc.GetContentType(string(dv.Spec.ContentType)) == string(cdiv1.DataVolumeKubeVirt)
+	storageClass, err := cc.GetStorageClassByNameWithVirtFallback(context.TODO(), client, dv.Spec.Storage.StorageClassName, shouldConsiderVirt)
 	if err != nil {
 		return err
 	}

tests/clone-populator_test.go

@@ -342,7 +342,7 @@ var _ = Describe("Clone Populator tests", func() {
 				}
 				sc, err := f.K8sClient.StorageV1().StorageClasses().Get(context.TODO(), utils.DefaultStorageClass.GetName(), metav1.GetOptions{})
 				Expect(err).ToNot(HaveOccurred())
-				noExpansionStorageClass, err = f.CreateVariationOfStorageClass(sc, disableVolumeExpansion)
+				noExpansionStorageClass, err = f.CreateNonDefaultVariationOfStorageClass(sc, disableVolumeExpansion)
 				Expect(err).ToNot(HaveOccurred())
 			})
 

tests/cloner_test.go

@@ -2716,7 +2716,7 @@ var _ = Describe("all clone tests", func() {
 				}
 				sc, err := f.K8sClient.StorageV1().StorageClasses().Get(context.TODO(), utils.DefaultStorageClass.GetName(), metav1.GetOptions{})
 				Expect(err).ToNot(HaveOccurred())
-				noExpansionStorageClass, err = f.CreateVariationOfStorageClass(sc, disableVolumeExpansion)
+				noExpansionStorageClass, err = f.CreateNonDefaultVariationOfStorageClass(sc, disableVolumeExpansion)
 				Expect(err).ToNot(HaveOccurred())
 			})
 

tests/datavolume_test.go

@@ -1276,7 +1276,7 @@ var _ = Describe("[vendor:cnv-qe@redhat.com][level:component]DataVolume tests",
 			Expect(err).ToNot(HaveOccurred())
 
 			// verify PVC was created
-			By("verifying pvc was created and is Bound")
+			By("verifying pvc was created")
 			pvc, err := utils.WaitForPVC(f.K8sClient, dataVolume.Namespace, dataVolume.Name)
 			Expect(err).ToNot(HaveOccurred())
 
@@ -1291,6 +1291,112 @@ var _ = Describe("[vendor:cnv-qe@redhat.com][level:component]DataVolume tests",
 			Entry("for clone DataVolume", createCloneDataVolume, fillCommand),
 		)
 
+		Context("default virt storage class", func() {
+			var defaultVirtStorageClass *storagev1.StorageClass
+			var dummyStorageClass *storagev1.StorageClass
+			var defaultStorageClass *storagev1.StorageClass
+
+			getDefaultStorageClassName := func() string {
+				return utils.DefaultStorageClass.GetName()
+			}
+			getDefaultVirtStorageClassName := func() string {
+				return defaultVirtStorageClass.GetName()
+			}
+			getDummyStorageClassName := func() string {
+				return dummyStorageClass.GetName()
+			}
+			importFunc := func() *cdiv1.DataVolume {
+				return utils.NewDataVolumeWithHTTPImportAndStorageSpec("dv-virt-sc-test-import", "1Gi", tinyCoreIsoURL())
+			}
+			importFuncPVCAPI := func() *cdiv1.DataVolume {
+				return utils.NewDataVolumeWithHTTPImport("dv-virt-sc-test-import", "1Gi", tinyCoreIsoURL())
+			}
+			importExplicitScFunc := func() *cdiv1.DataVolume {
+				dv := utils.NewDataVolumeWithHTTPImportAndStorageSpec("dv-virt-sc-test-import", "1Gi", tinyCoreIsoURL())
+				sc := getDummyStorageClassName()
+				dv.Spec.Storage.StorageClassName = &sc
+				return dv
+			}
+			uploadFunc := func() *cdiv1.DataVolume {
+				return utils.NewDataVolumeForUploadWithStorageAPI("dv-virt-sc-test-upload", "1Gi")
+			}
+			cloneFunc := func() *cdiv1.DataVolume {
+				sourcePodFillerName := fmt.Sprintf("%s-filler-pod", dataVolumeName)
+				pvcDef := utils.NewPVCDefinition(pvcName, "1Gi", nil, nil)
+				sourcePvc := f.CreateAndPopulateSourcePVC(pvcDef, sourcePodFillerName, fillCommand)
+
+				By(fmt.Sprintf("creating a new target PVC (datavolume) to clone %s", sourcePvc.Name))
+				return utils.NewDataVolumeForImageCloningAndStorageSpec("dv-virt-sc-test-clone", "1Gi", f.Namespace.Name, sourcePvc.Name, nil, nil)
+			}
+			archiveFunc := func() *cdiv1.DataVolume {
+				return utils.NewDataVolumeWithArchiveContentStorage("dv-virt-sc-test-archive", "1Gi", tarArchiveURL())
+			}
+
+			BeforeEach(func() {
+				addVirtParam := func(sc *storagev1.StorageClass) {
+					if len(sc.Parameters) == 0 {
+						sc.Parameters = map[string]string{}
+					}
+					sc.Parameters["better.for.kubevirt.io"] = "true"
+					controller.AddAnnotation(sc, controller.AnnDefaultVirtStorageClass, "true")
+				}
+				addDummyAnn := func(sc *storagev1.StorageClass) {
+					controller.AddAnnotation(sc, "dummy", "true")
+				}
+				sc, err := f.K8sClient.StorageV1().StorageClasses().Get(context.TODO(), getDefaultStorageClassName(), metav1.GetOptions{})
+				Expect(err).ToNot(HaveOccurred())
+				defaultStorageClass = sc
+				defaultVirtStorageClass, err = f.CreateNonDefaultVariationOfStorageClass(sc, addVirtParam)
+				Expect(err).ToNot(HaveOccurred())
+				dummyStorageClass, err = f.CreateNonDefaultVariationOfStorageClass(sc, addDummyAnn)
+				Expect(err).ToNot(HaveOccurred())
+			})
+
+			AfterEach(func() {
+				err := f.K8sClient.StorageV1().StorageClasses().Delete(context.TODO(), defaultVirtStorageClass.Name, metav1.DeleteOptions{})
+				Expect(err).ToNot(HaveOccurred())
+				err = f.K8sClient.StorageV1().StorageClasses().Delete(context.TODO(), dummyStorageClass.Name, metav1.DeleteOptions{})
+				Expect(err).ToNot(HaveOccurred())
+
+				if defaultStorageClass.Annotations[controller.AnnDefaultStorageClass] != "true" {
+					controller.AddAnnotation(defaultStorageClass, controller.AnnDefaultStorageClass, "true")
+					_, err = f.K8sClient.StorageV1().StorageClasses().Update(context.TODO(), defaultStorageClass, metav1.UpdateOptions{})
+					Expect(err).ToNot(HaveOccurred())
+				}
+			})
+
+			DescribeTable("Should", func(dvFunc func() *cdiv1.DataVolume, getExpectedStorageClassName func() string, removeDefault bool) {
+				var err error
+				// Default storage class exists check
+				_ = utils.GetDefaultStorageClass(f.K8sClient)
+				if removeDefault {
+					controller.AddAnnotation(defaultStorageClass, controller.AnnDefaultStorageClass, "false")
+					defaultStorageClass, err = f.K8sClient.StorageV1().StorageClasses().Update(context.TODO(), defaultStorageClass, metav1.UpdateOptions{})
+					Expect(err).ToNot(HaveOccurred())
+				}
+
+				dataVolume := dvFunc()
+				By(fmt.Sprintf("creating new datavolume %s", dataVolume.Name))
+				dataVolume, err = utils.CreateDataVolumeFromDefinition(f.CdiClient, f.Namespace.Name, dataVolume)
+				Expect(err).ToNot(HaveOccurred())
+
+				// verify PVC was created
+				By("verifying pvc was created")
+				pvc, err := utils.WaitForPVC(f.K8sClient, dataVolume.Namespace, dataVolume.Name)
+				Expect(err).ToNot(HaveOccurred())
+
+				Expect(pvc.Spec.StorageClassName).To(HaveValue(Equal(getExpectedStorageClassName())))
+			},
+				Entry("respect default virt storage class for import DataVolume", importFunc, getDefaultVirtStorageClassName, false),
+				Entry("respect default virt storage class for upload DataVolume", uploadFunc, getDefaultVirtStorageClassName, false),
+				Entry("respect default virt storage class for clone DataVolume", cloneFunc, getDefaultVirtStorageClassName, false),
+				Entry("respect default virt storage class even if no k8s default exists", importFunc, getDefaultVirtStorageClassName, true),
+				Entry("not respect default virt storage class for contenType other than kubevirt", archiveFunc, getDefaultStorageClassName, false),
+				Entry("not respect default virt storage class for PVC api", importFuncPVCAPI, getDefaultStorageClassName, false),
+				Entry("not respect default virt storage class if explicit storage class provided", importExplicitScFunc, getDummyStorageClassName, false),
+			)
+		})
+
 		It("Should handle a pre populated DV", func() {
 			By(fmt.Sprintf("initializing dataVolume marked as prePopulated %s", dataVolumeName))
 			dataVolume := utils.NewDataVolumeWithHTTPImport(dataVolumeName, "1Gi", cirrosURL())

tests/framework/framework.go

@@ -577,24 +577,28 @@ func (f *Framework) CreateWFFCVariationOfStorageClass(sc *storagev1.StorageClass
 		sc.VolumeBindingMode = &wffc
 	}
 
-	return f.CreateVariationOfStorageClass(sc, setWaitForFirstConsumer)
+	return f.CreateNonDefaultVariationOfStorageClass(sc, setWaitForFirstConsumer)
 }
 
-// CreateVariationOfStorageClass creates a variation of a storage class following mutationFunc's changes
-func (f *Framework) CreateVariationOfStorageClass(sc *storagev1.StorageClass, mutationFunc func(*storagev1.StorageClass)) (*storagev1.StorageClass, error) {
+// CreateNonDefaultVariationOfStorageClass creates a variation of a storage class following mutationFunc's changes
+func (f *Framework) CreateNonDefaultVariationOfStorageClass(sc *storagev1.StorageClass, mutationFunc func(*storagev1.StorageClass)) (*storagev1.StorageClass, error) {
 	scCopy := sc.DeepCopy()
-	mutationFunc(sc)
+	mutationFunc(scCopy)
 	if reflect.DeepEqual(sc, scCopy) {
 		return sc, nil
 	}
-	sc.ObjectMeta = metav1.ObjectMeta{
-		GenerateName: fmt.Sprintf("%s-temp-variation", sc.Name),
+	if val, ok := scCopy.Annotations[cc.AnnDefaultStorageClass]; ok && val == "true" {
+		scCopy.Annotations[cc.AnnDefaultStorageClass] = "false"
+	}
+	scCopy.ObjectMeta = metav1.ObjectMeta{
+		GenerateName: fmt.Sprintf("%s-temp-variation", scCopy.Name),
 		Labels: map[string]string{
 			"cdi.kubevirt.io/testing": "",
 		},
+		Annotations: scCopy.Annotations,
 	}
 
-	return f.K8sClient.StorageV1().StorageClasses().Create(context.TODO(), sc, metav1.CreateOptions{})
+	return f.K8sClient.StorageV1().StorageClasses().Create(context.TODO(), scCopy, metav1.CreateOptions{})
 }
 
 // UpdateCdiConfigResourceLimits sets the limits in the CDIConfig object

tests/utils/datavolume.go

@@ -969,7 +969,7 @@ func NewDataVolumeWithArchiveContent(dataVolumeName string, size string, httpURL
 					URL: httpURL,
 				},
 			},
-			ContentType: "archive",
+			ContentType: cdiv1.DataVolumeArchive,
 			PVC: &k8sv1.PersistentVolumeClaimSpec{
 				AccessModes: []k8sv1.PersistentVolumeAccessMode{k8sv1.ReadWriteOnce},
 				Resources: k8sv1.ResourceRequirements{
@@ -982,6 +982,31 @@ func NewDataVolumeWithArchiveContent(dataVolumeName string, size string, httpURL
 	}
 }
 
+// NewDataVolumeWithArchiveContentStorage initializes a DataVolume struct with 'archive' ContentType
+func NewDataVolumeWithArchiveContentStorage(dataVolumeName string, size string, httpURL string) *cdiv1.DataVolume {
+	return &cdiv1.DataVolume{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: dataVolumeName,
+		},
+		Spec: cdiv1.DataVolumeSpec{
+			Source: &cdiv1.DataVolumeSource{
+				HTTP: &cdiv1.DataVolumeSourceHTTP{
+					URL: httpURL,
+				},
+			},
+			ContentType: cdiv1.DataVolumeArchive,
+			Storage: &cdiv1.StorageSpec{
+				AccessModes: []k8sv1.PersistentVolumeAccessMode{k8sv1.ReadWriteOnce},
+				Resources: k8sv1.ResourceRequirements{
+					Requests: k8sv1.ResourceList{
+						k8sv1.ResourceName(k8sv1.ResourceStorage): resource.MustParse(size),
+					},
+				},
+			},
+		},
+	}
+}
+
 // PersistentVolumeClaimFromDataVolume creates a PersistentVolumeClaim definition so we can use PersistentVolumeClaim for various operations.
 func PersistentVolumeClaimFromDataVolume(datavolume *cdiv1.DataVolume) *corev1.PersistentVolumeClaim {
 	// This function can work with DVs that use the 'Storage' field instead of the 'PVC' field