[release-v1.43] Fix smart clone request size update (#2289)

* Fix smart clone request size update

In case the pvc got an actual size as the expected
size there wasn't an update of the pvc spec with the
actual user request size, which left the pvc spec with
a smaller size then the user requested(the data size).
This caused a discrepancy when trying to restore such pvc,
which restored a pvc with the small size instead of the user request
size.

Signed-off-by: Shelly Kagan <skagan@redhat.com>

* review fixes

Signed-off-by: Shelly Kagan <skagan@redhat.com>

* cherry pick fix

Signed-off-by: Shelly Kagan <skagan@redhat.com>

* Fix smart clone update request size condition

Signed-off-by: Shelly Kagan <skagan@redhat.com>

pkg/controller/datavolume-controller.go

@@ -1334,10 +1334,11 @@ func (r *DatavolumeReconciler) expand(log logr.Logger,
 	}
 
 	expansionRequired := actualSize.Cmp(requestedSize) < 0
+	updateRequestSizeRequired := currentSize.Cmp(requestedSize) < 0
 
 	log.V(3).Info("Expand sizes", "req", requestedSize, "cur", currentSize, "act", actualSize, "exp", expansionRequired)
 
-	if expansionRequired && requestedSize.Cmp(currentSize) != 0 {
+	if updateRequestSizeRequired {
 		pvc.Spec.Resources.Requests[corev1.ResourceStorage] = requestedSize
 		if err := r.client.Update(context.TODO(), pvc); err != nil {
 			return false, err

pkg/controller/datavolume-controller_test.go

@@ -1397,6 +1397,60 @@ var _ = Describe("All DataVolume Tests", func() {
 			Entry("snapshot", cdiv1.CloneStrategySnapshot),
 		)
 
+		DescribeTable("After smart clone", func(actualSize resource.Quantity, currentSize resource.Quantity, expectedDvPhase cdiv1.DataVolumePhase) {
+			strategy := cdiv1.CloneStrategySnapshot
+			controller := true
+
+			dv := newCloneDataVolume("test-dv")
+			scName := "testsc"
+			sc := createStorageClassWithProvisioner(scName, map[string]string{
+				AnnDefaultStorageClass: "true",
+			}, "csi-plugin")
+			accessMode := []corev1.PersistentVolumeAccessMode{corev1.ReadOnlyMany}
+			storageProfile := createStorageProfileWithCloneStrategy(scName,
+				[]cdiv1.ClaimPropertySet{{AccessModes: accessMode, VolumeMode: &blockMode}},
+				&strategy)
+			snapshotClassName := "snap-class"
+			snapClass := createSnapshotClass(snapshotClassName, nil, "csi-plugin")
+
+			srcPvc := createPvcInStorageClass("test", metav1.NamespaceDefault, &scName, nil, nil, corev1.ClaimBound)
+			targetPvc := createPvcInStorageClass("test-dv", metav1.NamespaceDefault, &scName, nil, nil, corev1.ClaimBound)
+			targetPvc.OwnerReferences = append(targetPvc.OwnerReferences, metav1.OwnerReference{
+				Kind:       "DataVolume",
+				Controller: &controller,
+				Name:       "test-dv",
+				UID:        dv.UID,
+			})
+			targetPvc.Spec.Resources.Requests[corev1.ResourceStorage] = currentSize
+			targetPvc.Status.Capacity[corev1.ResourceStorage] = actualSize
+			targetPvc.SetAnnotations(make(map[string]string))
+			targetPvc.GetAnnotations()[AnnCloneOf] = "true"
+
+			reconciler = createDatavolumeReconciler(dv, srcPvc, targetPvc, storageProfile, sc, snapClass, createVolumeSnapshotContentCrd(), createVolumeSnapshotClassCrd(), createVolumeSnapshotCrd())
+
+			By("Reconcile")
+			result, err := reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}})
+			Expect(err).To(Not(HaveOccurred()))
+			Expect(result).To(Not(BeNil()))
+
+			By(fmt.Sprintf("Verifying that dv phase is now in %s", expectedDvPhase))
+			dv = &cdiv1.DataVolume{}
+			err = reconciler.client.Get(context.TODO(), types.NamespacedName{Name: "test-dv", Namespace: metav1.NamespaceDefault}, dv)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(dv.Status.Phase).To(Equal(expectedDvPhase))
+
+			By("Verifying that pvc request size as expected")
+			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(resource.MustParse("1G")))
+
+		},
+			Entry("Should expand pvc when actual and current differ then the requested size", resource.MustParse("500M"), resource.MustParse("500M"), cdiv1.ExpansionInProgress),
+			Entry("Should update request size when current size differ then the requested size and actual size is bigger then both", resource.MustParse("2G"), resource.MustParse("500M"), cdiv1.ExpansionInProgress),
+			Entry("Should update request size when current size differ from requested size", resource.MustParse("1G"), resource.MustParse("500M"), cdiv1.ExpansionInProgress),
+			Entry("Should complete clone in case all sizes match", resource.MustParse("1G"), resource.MustParse("1G"), cdiv1.Succeeded),
+		)
 	})
 
 	var _ = Describe("CSI clone", func() {