handle snapshot clone with no target size specified

also add more validation to some snapshot clone tests

Signed-off-by: Michael Henriksen <mhenriks@redhat.com>

pkg/controller/datavolume/snapshot-clone-controller.go

@@ -194,6 +194,21 @@ func (r *SnapshotCloneReconciler) syncSnapshotClone(log logr.Logger, req reconci
 	}
 
 	if pvc == nil {
+		if datavolume.Spec.Storage != nil {
+			done, err := r.detectCloneSize(log, &syncRes)
+			if err != nil {
+				return syncRes, err
+			}
+			if !done {
+				if syncRes.result == nil {
+					syncRes.result = &reconcile.Result{}
+				}
+				syncRes.result.RequeueAfter = sourceInUseRequeueDuration
+				// I think pending is more accurate but doing scheduled to be consistent with PVC controller
+				return syncRes, r.syncCloneStatusPhase(&syncRes, cdiv1.CloneScheduled, nil)
+			}
+		}
+
 		pvcModifier := r.updateAnnotations
 		if syncRes.usePopulator {
 			if isCrossNamespaceClone(datavolume) {
@@ -249,6 +264,38 @@ func (r *SnapshotCloneReconciler) syncSnapshotClone(log logr.Logger, req reconci
 	return syncRes, syncErr
 }
 
+func (r *SnapshotCloneReconciler) detectCloneSize(log logr.Logger, syncState *dvSyncState) (bool, error) {
+	pvcSpec := syncState.pvcSpec
+	requestedSize := pvcSpec.Resources.Requests[corev1.ResourceStorage]
+	if !requestedSize.IsZero() {
+		log.V(3).Info("requested size is set, skipping size detection", "size", requestedSize)
+		return true, nil
+	}
+
+	datavolume := syncState.dvMutated
+	nn := types.NamespacedName{Namespace: datavolume.Spec.Source.Snapshot.Namespace, Name: datavolume.Spec.Source.Snapshot.Name}
+	snapshot := &snapshotv1.VolumeSnapshot{}
+	if err := r.client.Get(context.TODO(), nn, snapshot); err != nil {
+		if k8serrors.IsNotFound(err) {
+			log.V(3).Info("snapshot source does not exist", "namespace", nn.Namespace, "name", nn.Name)
+			return false, nil
+		}
+
+		return false, err
+	}
+
+	if snapshot.Status == nil || snapshot.Status.RestoreSize == nil {
+		log.V(3).Info("snapshot source does not have restoreSize", "namespace", nn.Namespace, "name", nn.Name)
+		return false, nil
+	}
+
+	pvcSpec.Resources.Requests[corev1.ResourceStorage] = *snapshot.Status.RestoreSize
+
+	log.V(3).Info("set pvc request size", "size", pvcSpec.Resources.Requests[corev1.ResourceStorage])
+
+	return true, nil
+}
+
 func (r *SnapshotCloneReconciler) validateAndInitLegacyClone(syncState *dvSyncState) (bool, error) {
 	// Check if source snapshot exists and do proper validation before attempting to clone
 	if done, err := r.validateCloneAndSourceSnapshot(syncState); err != nil {

pkg/controller/datavolume/snapshot-clone-controller_test.go

@@ -251,7 +251,7 @@ var _ = Describe("All DataVolume Tests", func() {
 					if sourceNamespace != dv.Namespace {
 						dv.Finalizers = append(dv.Finalizers, crossNamespaceFinalizer)
 					}
-					snapshot := createSnapshotInVolumeSnapshotClass("test-snap", "source-ns", &expectedSnapshotClass, nil, nil, true)
+					snapshot := createSnapshotInVolumeSnapshotClass("test-snap", sourceNamespace, &expectedSnapshotClass, nil, nil, true)
 					reconciler = createSnapshotCloneReconciler(storageClass, csiDriver, dv, snapshot)
 					result, err := reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}})
 					Expect(err).ToNot(HaveOccurred())
@@ -287,6 +287,29 @@ var _ = Describe("All DataVolume Tests", func() {
 					Entry("with different namespace", "source-ns"),
 				)
 
+				It("should handle size omitted", func() {
+					dv := newCloneFromSnapshotDataVolume("test-dv")
+					vm := corev1.PersistentVolumeFilesystem
+					dv.Spec.Storage = &cdiv1.StorageSpec{
+						AccessModes: dv.Spec.PVC.AccessModes,
+						VolumeMode:  &vm,
+					}
+					dv.Spec.PVC = nil
+					snapshot := createSnapshotInVolumeSnapshotClass("test-snap", dv.Namespace, &expectedSnapshotClass, nil, nil, true)
+					reconciler = createSnapshotCloneReconciler(storageClass, csiDriver, dv, snapshot)
+					result, err := reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}})
+					Expect(err).ToNot(HaveOccurred())
+					Expect(result.Requeue).To(BeFalse())
+					Expect(result.RequeueAfter).To(BeZero())
+					dv = &cdiv1.DataVolume{}
+					err = reconciler.client.Get(context.TODO(), types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}, dv)
+					Expect(err).ToNot(HaveOccurred())
+					pvc := &corev1.PersistentVolumeClaim{}
+					err = reconciler.client.Get(context.TODO(), types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}, pvc)
+					Expect(err).ToNot(HaveOccurred())
+					Expect(pvc.Spec.Resources.Requests[corev1.ResourceStorage]).To(Equal(*snapshot.Status.RestoreSize))
+				})
+
 				It("should add cloneType annotation", func() {
 					dv := newCloneFromSnapshotDataVolume("test-dv")
 					anno := map[string]string{

tests/cloner_test.go

@@ -2715,10 +2715,16 @@ var _ = Describe("all clone tests", func() {
 					By("Check host assisted clone is taking place")
 					pvc, err := f.K8sClient.CoreV1().PersistentVolumeClaims(targetNs.Name).Get(context.TODO(), dvName, metav1.GetOptions{})
 					Expect(err).ToNot(HaveOccurred())
+					// non csi
 					if pvc.Spec.DataSourceRef == nil {
 						suffix := "-host-assisted-source-pvc"
 						Expect(pvc.Annotations[controller.AnnCloneRequest]).To(HaveSuffix(suffix))
 						Expect(pvc.Spec.DataSource).To(BeNil())
+					} else {
+						dv, err := f.CdiClient.CdiV1beta1().DataVolumes(targetNs.Name).Get(context.TODO(), dvName, metav1.GetOptions{})
+						Expect(err).ToNot(HaveOccurred())
+						cloneType := utils.GetCloneType(f.CdiClient, dv)
+						Expect(cloneType).To(Equal(string(cdiv1.CloneStrategyHostAssisted)))
 					}
 				}