Correct the cloneStrategy on StorageProfile (#1951)

Moves the cloneStrategy up one level from claimPropertySet to
StorageProfile.spec and StorageProfile.status.

Signed-off-by: Bartosz Rybacki <brybacki@redhat.com>

doc/storageprofile.md

@@ -19,12 +19,13 @@ apiVersion: cdi.kubevirt.io/v1beta1
 kind: StorageProfile
 metadata: 
   name: hostpath-provisioner
-spec: 
+spec:
   claimPropertySets: 
   - accessModes:
     - ReadWriteOnce
     volumeMode: 
       Filesystem
+  cloneStrategy: snapshot
 status:
     storageClass: hostpath-provisioner
     provisioner: kubevirt.io/hostpath-provisioner
@@ -32,12 +33,14 @@ status:
     - accessModes: 
       - ReadWriteOnce
       volumeMode: Filesystem
+    cloneStrategy: snapshot
 ```
 
 Current version supports the following parameters:
-- `accessMode` - contains the desired access modes the volume should have
-- `volumeMode` -  defines what type of volume is required by the claim
 - `cloneStrategy` - defines the preferred method for performing a CDI clone
+- `claimPropertySets` contains a list of `claimPropertySet` (currently only one supported) 
+  - `accessMode` - contains the desired access modes the volume should have
+  - `volumeMode` - defines what type of volume is required by the claim
 
 Values for accessModes and volumeMode are exactly the same as for PVC: `accessModes` is a list of `[ReadWriteMany|ReadWriteOnce|ReadOnlyMany]`
 and `volumeMode` is a single value `Filesystem` or `Block`. The value for `cloneStrategy` ca be one of: `copy`,`snapshot`,`csi-clone`. 

pkg/apis/core/v1beta1/openapi_generated.go

@@ -14809,13 +14809,6 @@ func schema_pkg_apis_core_v1beta1_ClaimPropertySet(ref common.ReferenceCallback)
 							Format:      "",
 						},
 					},
-					"cloneStrategy": {
-						SchemaProps: spec.SchemaProps{
-							Description: "CloneStrategy defines the preferred method for performing a CDI clone",
-							Type:        []string{"string"},
-							Format:      "",
-						},
-					},
 				},
 			},
 		},
@@ -16396,6 +16389,13 @@ func schema_pkg_apis_core_v1beta1_StorageProfileSpec(ref common.ReferenceCallbac
 				Description: "StorageProfileSpec defines specification for StorageProfile",
 				Type:        []string{"object"},
 				Properties: map[string]spec.Schema{
+					"cloneStrategy": {
+						SchemaProps: spec.SchemaProps{
+							Description: "CloneStrategy defines the preferred method for performing a CDI clone",
+							Type:        []string{"string"},
+							Format:      "",
+						},
+					},
 					"claimPropertySets": {
 						SchemaProps: spec.SchemaProps{
 							Description: "ClaimPropertySets is a provided set of properties applicable to PVC",
@@ -16439,6 +16439,13 @@ func schema_pkg_apis_core_v1beta1_StorageProfileStatus(ref common.ReferenceCallb
 							Format:      "",
 						},
 					},
+					"cloneStrategy": {
+						SchemaProps: spec.SchemaProps{
+							Description: "CloneStrategy defines the preferred method for performing a CDI clone",
+							Type:        []string{"string"},
+							Format:      "",
+						},
+					},
 					"claimPropertySets": {
 						SchemaProps: spec.SchemaProps{
 							Description: "ClaimPropertySets computed from the spec and detected in the system",

pkg/apis/core/v1beta1/types.go

@@ -364,6 +364,8 @@ type StorageProfile struct {
 
 //StorageProfileSpec defines specification for StorageProfile
 type StorageProfileSpec struct {
+	// CloneStrategy defines the preferred method for performing a CDI clone
+	CloneStrategy *CDICloneStrategy `json:"cloneStrategy,omitempty"`
 	// ClaimPropertySets is a provided set of properties applicable to PVC
 	ClaimPropertySets []ClaimPropertySet `json:"claimPropertySets,omitempty"`
 }
@@ -374,6 +376,8 @@ type StorageProfileStatus struct {
 	StorageClass *string `json:"storageClass,omitempty"`
 	// The Storage class provisioner plugin name
 	Provisioner *string `json:"provisioner,omitempty"`
+	// CloneStrategy defines the preferred method for performing a CDI clone
+	CloneStrategy *CDICloneStrategy `json:"cloneStrategy,omitempty"`
 	// ClaimPropertySets computed from the spec and detected in the system
 	ClaimPropertySets []ClaimPropertySet `json:"claimPropertySets,omitempty"`
 }
@@ -388,8 +392,6 @@ type ClaimPropertySet struct {
 	// Value of Filesystem is implied when not included in claim spec.
 	// +optional
 	VolumeMode *corev1.PersistentVolumeMode `json:"volumeMode,omitempty" protobuf:"bytes,6,opt,name=volumeMode,casttype=PersistentVolumeMode"`
-	// CloneStrategy defines the preferred method for performing a CDI clone
-	CloneStrategy *CDICloneStrategy `json:"cloneStrategy,omitempty"`
 }
 
 //StorageProfileList provides the needed parameters to request a list of StorageProfile from the system

pkg/apis/core/v1beta1/types_swagger_generated.go

@@ -159,6 +159,7 @@ func (StorageProfile) SwaggerDoc() map[string]string {
 func (StorageProfileSpec) SwaggerDoc() map[string]string {
 	return map[string]string{
 		"":                  "StorageProfileSpec defines specification for StorageProfile",
+		"cloneStrategy":     "CloneStrategy defines the preferred method for performing a CDI clone",
 		"claimPropertySets": "ClaimPropertySets is a provided set of properties applicable to PVC",
 	}
 }
@@ -168,16 +169,16 @@ func (StorageProfileStatus) SwaggerDoc() map[string]string {
 		"":                  "StorageProfileStatus provides the most recently observed status of the StorageProfile",
 		"storageClass":      "The StorageClass name for which capabilities are defined",
 		"provisioner":       "The Storage class provisioner plugin name",
+		"cloneStrategy":     "CloneStrategy defines the preferred method for performing a CDI clone",
 		"claimPropertySets": "ClaimPropertySets computed from the spec and detected in the system",
 	}
 }
 
 func (ClaimPropertySet) SwaggerDoc() map[string]string {
 	return map[string]string{
-		"":              "ClaimPropertySet is a set of properties applicable to PVC",
-		"accessModes":   "AccessModes contains the desired access modes the volume should have.\nMore info: https://kubernetes.io/docs/concepts/storage/persistent-volumes#access-modes-1\n+optional",
-		"volumeMode":    "VolumeMode defines what type of volume is required by the claim.\nValue of Filesystem is implied when not included in claim spec.\n+optional",
-		"cloneStrategy": "CloneStrategy defines the preferred method for performing a CDI clone",
+		"":            "ClaimPropertySet is a set of properties applicable to PVC",
+		"accessModes": "AccessModes contains the desired access modes the volume should have.\nMore info: https://kubernetes.io/docs/concepts/storage/persistent-volumes#access-modes-1\n+optional",
+		"volumeMode":  "VolumeMode defines what type of volume is required by the claim.\nValue of Filesystem is implied when not included in claim spec.\n+optional",
 	}
 }
 

pkg/apis/core/v1beta1/zz_generated.deepcopy.go

@@ -365,11 +365,6 @@ func (in *ClaimPropertySet) DeepCopyInto(out *ClaimPropertySet) {
 		*out = new(v1.PersistentVolumeMode)
 		**out = **in
 	}
-	if in.CloneStrategy != nil {
-		in, out := &in.CloneStrategy, &out.CloneStrategy
-		*out = new(CDICloneStrategy)
-		**out = **in
-	}
 	return
 }
 
@@ -1330,6 +1325,11 @@ func (in *StorageProfileList) DeepCopyObject() runtime.Object {
 // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
 func (in *StorageProfileSpec) DeepCopyInto(out *StorageProfileSpec) {
 	*out = *in
+	if in.CloneStrategy != nil {
+		in, out := &in.CloneStrategy, &out.CloneStrategy
+		*out = new(CDICloneStrategy)
+		**out = **in
+	}
 	if in.ClaimPropertySets != nil {
 		in, out := &in.ClaimPropertySets, &out.ClaimPropertySets
 		*out = make([]ClaimPropertySet, len(*in))
@@ -1363,6 +1363,11 @@ func (in *StorageProfileStatus) DeepCopyInto(out *StorageProfileStatus) {
 		*out = new(string)
 		**out = **in
 	}
+	if in.CloneStrategy != nil {
+		in, out := &in.CloneStrategy, &out.CloneStrategy
+		*out = new(CDICloneStrategy)
+		**out = **in
+	}
 	if in.ClaimPropertySets != nil {
 		in, out := &in.ClaimPropertySets, &out.ClaimPropertySets
 		*out = make([]ClaimPropertySet, len(*in))

pkg/controller/datavolume-controller.go

@@ -2526,12 +2526,8 @@ func (r *DatavolumeReconciler) getPreferredCloneStrategyForStorageClass(storageC
 	if err != nil {
 		return nil, errors.Wrap(err, "cannot get StorageProfile")
 	}
-	if len(storageProfile.Status.ClaimPropertySets) > 0 {
-		cloneStrategy := storageProfile.Status.ClaimPropertySets[0].CloneStrategy
-		return cloneStrategy, nil
-	}
 
-	return nil, nil
+	return storageProfile.Status.CloneStrategy, nil
 }
 
 func getDefaultVolumeMode(c client.Client, storageClass *storagev1.StorageClass) (*corev1.PersistentVolumeMode, error) {

pkg/controller/storageprofile-controller.go

@@ -58,6 +58,8 @@ func (r *StorageProfileReconciler) reconcileStorageProfile(sc *storagev1.Storage
 
 	storageProfile.Status.StorageClass = &sc.Name
 	storageProfile.Status.Provisioner = &sc.Provisioner
+	storageProfile.Status.CloneStrategy = storageProfile.Spec.CloneStrategy
+
 	storageProfile.Status.ClaimPropertySets = storageProfile.Spec.ClaimPropertySets
 
 	var claimPropertySet *cdiv1.ClaimPropertySet

pkg/controller/util_test.go

@@ -670,10 +670,10 @@ func createStorageProfileWithCloneStrategy(name string,
 		Status: cdiv1.StorageProfileStatus{
 			StorageClass: &name,
 			ClaimPropertySets: []cdiv1.ClaimPropertySet{{
-				AccessModes:   accessModes,
-				VolumeMode:    &volumeMode,
-				CloneStrategy: cloneStrategy,
+				AccessModes: accessModes,
+				VolumeMode:  &volumeMode,
 			}},
+			CloneStrategy: cloneStrategy,
 		},
 	}
 }

pkg/operator/resources/crds_generated.go

@@ -3461,14 +3461,14 @@ spec:
                       items:
                         type: string
                       type: array
-                    cloneStrategy:
-                      description: CloneStrategy defines the preferred method for performing a CDI clone
-                      type: string
                     volumeMode:
                       description: VolumeMode defines what type of volume is required by the claim. Value of Filesystem is implied when not included in claim spec.
                       type: string
                   type: object
                 type: array
+              cloneStrategy:
+                description: CloneStrategy defines the preferred method for performing a CDI clone
+                type: string
             type: object
           status:
             description: StorageProfileStatus provides the most recently observed status of the StorageProfile
@@ -3483,14 +3483,14 @@ spec:
                       items:
                         type: string
                       type: array
-                    cloneStrategy:
-                      description: CloneStrategy defines the preferred method for performing a CDI clone
-                      type: string
                     volumeMode:
                       description: VolumeMode defines what type of volume is required by the claim. Value of Filesystem is implied when not included in claim spec.
                       type: string
                   type: object
                 type: array
+              cloneStrategy:
+                description: CloneStrategy defines the preferred method for performing a CDI clone
+                type: string
               provisioner:
                 description: The Storage class provisioner plugin name
                 type: string

tests/cloner_test.go

@@ -1770,8 +1770,8 @@ func validateCloneType(f *framework.Framework, dv *cdiv1.DataVolume) {
 
 			defaultCloneStrategy := cdiv1.CDICloneStrategy(cdiv1.CloneStrategySnapshot)
 			cloneStrategy := &defaultCloneStrategy
-			if len(spec.ClaimPropertySets) > 0 {
-				cloneStrategy = spec.ClaimPropertySets[0].CloneStrategy
+			if spec.CloneStrategy != nil {
+				cloneStrategy = spec.CloneStrategy
 			}
 
 			sc, err := f.K8sClient.StorageV1().StorageClasses().Get(context.TODO(), f.SnapshotSCName, metav1.GetOptions{})

tests/utils/storageprofile.go

@@ -52,10 +52,7 @@ func ConfigureCloneStrategy(client client.Client,
 	gomega.Eventually(func() *cdiv1.CDICloneStrategy {
 		profile, err := clientSet.CdiV1beta1().StorageProfiles().Get(context.TODO(), storageClassName, metav1.GetOptions{})
 		gomega.Expect(err).ToNot(gomega.HaveOccurred())
-		if len(profile.Status.ClaimPropertySets) > 0 {
-			return profile.Status.ClaimPropertySets[0].CloneStrategy
-		}
-		return nil
+		return profile.Status.CloneStrategy
 	}, time.Second*30, time.Second).Should(gomega.Equal(&cloneStrategy))
 
 	return nil
@@ -63,12 +60,7 @@ func ConfigureCloneStrategy(client client.Client,
 
 func updateCloneStrategy(originalProfileSpec *cdiv1.StorageProfileSpec, cloneStrategy cdiv1.CDICloneStrategy) *cdiv1.StorageProfileSpec {
 	newProfileSpec := originalProfileSpec.DeepCopy()
-
-	if len(newProfileSpec.ClaimPropertySets) == 0 {
-		newProfileSpec.ClaimPropertySets = []cdiv1.ClaimPropertySet{{CloneStrategy: &cloneStrategy}}
-	} else {
-		newProfileSpec.ClaimPropertySets[0].CloneStrategy = &cloneStrategy
-	}
+	newProfileSpec.CloneStrategy = &cloneStrategy
 
 	return newProfileSpec
 }