Host assisted clone should adjust requested storage when cloning from block to filesystem volume mode (#3900)

pkg/controller/clone-controller.go

@@ -14,7 +14,6 @@ import (
 
 	corev1 "k8s.io/api/core/v1"
 	k8serrors "k8s.io/apimachinery/pkg/api/errors"
-	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
@@ -779,11 +778,11 @@ func ValidateCanCloneSourceAndTargetSpec(ctx context.Context, c client.Client, s
 	}
 
 	// TODO: use detection pod here, then permissive should not be needed
-	sourceUsableSpace, err := getUsableSpace(ctx, c, sourcePvc)
+	sourceUsableSpace, err := cc.GetUsableSpace(ctx, c, sourcePvc)
 	if err != nil {
 		return err
 	}
-	targetUsableSpace, err := getUsableSpace(ctx, c, targetPvc)
+	targetUsableSpace, err := cc.GetUsableSpace(ctx, c, targetPvc)
 	if err != nil {
 		return err
 	}
@@ -795,21 +794,3 @@ func ValidateCanCloneSourceAndTargetSpec(ctx context.Context, c client.Client, s
 	// Can clone.
 	return nil
 }
-
-func getUsableSpace(ctx context.Context, c client.Client, pvc *corev1.PersistentVolumeClaim) (resource.Quantity, error) {
-	sizeRequest := pvc.Spec.Resources.Requests[corev1.ResourceStorage]
-	volumeMode := util.ResolveVolumeMode(pvc.Spec.VolumeMode)
-
-	if volumeMode == corev1.PersistentVolumeFilesystem {
-		fsOverhead, err := cc.GetFilesystemOverheadForStorageClass(ctx, c, pvc.Spec.StorageClassName)
-		if err != nil {
-			return resource.Quantity{}, err
-		}
-		fsOverheadFloat, _ := strconv.ParseFloat(string(fsOverhead), 64)
-		usableSpaceRaw := util.GetUsableSpace(fsOverheadFloat, sizeRequest.Value())
-
-		return *resource.NewScaledQuantity(usableSpaceRaw, 0), nil
-	}
-
-	return sizeRequest, nil
-}

pkg/controller/clone/BUILD.bazel

@@ -55,6 +55,7 @@ go_test(
     ],
     embed = [":go_default_library"],
     deps = [
+        "//pkg/common:go_default_library",
         "//pkg/controller/common:go_default_library",
         "//staging/src/kubevirt.io/containerized-data-importer-api/pkg/apis/core/v1beta1:go_default_library",
         "//vendor/github.com/kubernetes-csi/external-snapshotter/client/v6/apis/volumesnapshot/v1:go_default_library",

pkg/controller/clone/host-clone.go

@@ -195,6 +195,60 @@ func (p *HostClonePhase) createClaim(ctx context.Context) (*corev1.PersistentVol
 	}
 	cc.AddLabel(claim, cc.LabelExcludeFromVeleroBackup, "true")
 
+	if targetVolumeMode := cc.GetVolumeMode(claim); targetVolumeMode == corev1.PersistentVolumeFilesystem {
+		// It is possible when the source pvc has VolumMode 'block'
+		// and the claim has 'filesystem' in which case the filesystem overhead need to be considered
+		sourcePvc := &corev1.PersistentVolumeClaim{}
+		sourcePvcKey := client.ObjectKey{Namespace: p.Namespace, Name: p.SourceName}
+
+		if err := p.Client.Get(ctx, sourcePvcKey, sourcePvc); err != nil {
+			return nil, err
+		}
+
+		if claim.Spec.Resources.Requests != nil {
+			size := sourcePvc.Spec.Resources.Requests[corev1.ResourceStorage]
+			inflate := true
+			if sourceVolumeMode := cc.GetVolumeMode(sourcePvc); sourceVolumeMode == corev1.PersistentVolumeFilesystem {
+				// Get the datavolume associate with the source
+				if dv, err := cc.GetDVFromPVC(ctx, p.Client, sourcePvc); err == nil {
+					if dv != nil {
+						if sourceSize, err := cc.GetDVCloneSize(ctx, p.Client, dv); err == nil {
+							// If the source PVC is filesystem, just directly compare
+							targetSize := claim.Spec.Resources.Requests[corev1.ResourceStorage]
+							if targetSize.Cmp(*sourceSize) >= 0 {
+								// the target size has enough space, not to inflate
+								inflate = false
+							}
+						} else {
+							inflate = false
+						}
+					} else {
+						// can't determine the overhead, assuming size is correct
+						inflate = false
+					}
+				} else {
+					inflate = false
+				}
+			} else {
+				// If the source PVC is block, we need to account for the overhead
+				if usableSpace, err := cc.GetUsableSpace(ctx, p.Client, claim); err == nil {
+					if usableSpace.Cmp(size) >= 0 {
+						inflate = false
+					}
+				} else {
+					inflate = false
+				}
+			}
+			if inflate {
+				newUsableSpace, err := cc.InflateSizeWithOverhead(ctx, p.Client, size.Value(), &claim.Spec)
+				if err != nil {
+					return nil, err
+				}
+				claim.Spec.Resources.Requests[corev1.ResourceStorage] = newUsableSpace
+			}
+		}
+	}
+
 	if err := p.Client.Create(ctx, claim); err != nil {
 		checkQuotaExceeded(p.Recorder, p.Owner, err)
 		return nil, err

pkg/controller/clone/host-clone_test.go

@@ -23,6 +23,7 @@ import (
 	. "github.com/onsi/gomega"
 
 	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/client-go/kubernetes/scheme"
@@ -33,18 +34,37 @@ import (
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
 
 	cdiv1 "kubevirt.io/containerized-data-importer-api/pkg/apis/core/v1beta1"
+	"kubevirt.io/containerized-data-importer/pkg/common"
 	cc "kubevirt.io/containerized-data-importer/pkg/controller/common"
 )
 
 var _ = Describe("HostClonePhase test", func() {
 	log := logf.Log.WithName("host-clone-phase-test")
 
-	creatHostClonePhase := func(objects ...runtime.Object) *HostClonePhase {
+	type ResourceModifier struct {
+		modifySourcePvc  func(pvcSpec *corev1.PersistentVolumeClaimSpec)
+		modifyDesiredPvc func(pvcSpec *corev1.PersistentVolumeClaimSpec)
+	}
+
+	creatHostClonePhase := func(modifier *ResourceModifier, objects ...runtime.Object) *HostClonePhase {
 		s := scheme.Scheme
 		_ = cdiv1.AddToScheme(s)
 
 		objects = append(objects, cc.MakeEmptyCDICR())
 
+		source := &corev1.PersistentVolumeClaim{
+			ObjectMeta: metav1.ObjectMeta{
+				Namespace: "ns",
+				Name:      "source",
+			},
+		}
+
+		if modifier != nil && modifier.modifySourcePvc != nil {
+			modifier.modifySourcePvc(&source.Spec)
+		}
+
+		objects = append(objects, source)
+
 		// Create a fake client to mock API calls.
 		builder := fake.NewClientBuilder().
 			WithScheme(s).
@@ -69,11 +89,15 @@ var _ = Describe("HostClonePhase test", func() {
 			},
 		}
 
+		if modifier != nil && modifier.modifyDesiredPvc != nil {
+			modifier.modifyDesiredPvc(&desired.Spec)
+		}
+
 		return &HostClonePhase{
 			Owner:          owner,
 			OwnershipLabel: "label",
 			Namespace:      "ns",
-			SourceName:     "source",
+			SourceName:     source.Name,
 			DesiredClaim:   desired,
 			ImmediateBind:  true,
 			Preallocation:  false,
@@ -91,7 +115,7 @@ var _ = Describe("HostClonePhase test", func() {
 	}
 
 	It("should create pvc", func() {
-		p := creatHostClonePhase()
+		p := creatHostClonePhase(nil)
 
 		result, err := p.Reconcile(context.Background())
 		Expect(err).ToNot(HaveOccurred())
@@ -114,7 +138,7 @@ var _ = Describe("HostClonePhase test", func() {
 	})
 
 	It("should create pvc with priorityclass", func() {
-		p := creatHostClonePhase()
+		p := creatHostClonePhase(nil)
 		p.PriorityClassName = "priority"
 
 		result, err := p.Reconcile(context.Background())
@@ -127,6 +151,80 @@ var _ = Describe("HostClonePhase test", func() {
 		Expect(pvc.Annotations[cc.AnnPriorityClassName]).To(Equal("priority"))
 	})
 
+	It("should adjust requested size for block to filesystem volume mode clone", func() {
+		setPvcAttributes := func(pvcSpec *corev1.PersistentVolumeClaimSpec, volumeMode corev1.PersistentVolumeMode, storage string) {
+			pvcSpec.VolumeMode = &volumeMode
+			if pvcSpec.Resources.Requests == nil {
+				pvcSpec.Resources.Requests = corev1.ResourceList{}
+			}
+			pvcSpec.Resources.Requests[corev1.ResourceStorage] = resource.MustParse(storage)
+		}
+		cdiConfig := cc.MakeEmptyCDIConfigSpec(common.ConfigName)
+		cdiConfig.Status.FilesystemOverhead = &cdiv1.FilesystemOverhead{
+			Global: common.DefaultGlobalOverhead,
+		}
+
+		p := creatHostClonePhase(&ResourceModifier{
+			modifySourcePvc: func(pvcSpec *corev1.PersistentVolumeClaimSpec) {
+				setPvcAttributes(pvcSpec, corev1.PersistentVolumeBlock, "8Gi")
+			},
+			modifyDesiredPvc: func(pvcSpec *corev1.PersistentVolumeClaimSpec) {
+				setPvcAttributes(pvcSpec, corev1.PersistentVolumeFilesystem, "8Gi")
+			},
+		}, cdiConfig)
+
+		result, err := p.Reconcile(context.Background())
+		Expect(err).ToNot(HaveOccurred())
+		Expect(result).ToNot(BeNil())
+		Expect(result.Requeue).To(BeFalse())
+		Expect(result.RequeueAfter).ToNot(BeZero())
+
+		pvc := getDesiredClaim(p)
+
+		Expect(*pvc.Spec.VolumeMode).To(Equal(corev1.PersistentVolumeFilesystem))
+		actualSize := pvc.Spec.Resources.Requests[corev1.ResourceStorage]
+		originalRequested := resource.MustParse("8Gi")
+
+		Expect(actualSize.Cmp(originalRequested)).To(BeNumerically(">", 0), "The actual should be greater than the requested", "actual", actualSize, "requested", originalRequested)
+	})
+
+	It("should use the same requested size for filesystem to filesystem volume mode clone", func() {
+		setPvcAttributes := func(pvcSpec *corev1.PersistentVolumeClaimSpec, volumeMode corev1.PersistentVolumeMode, storage string) {
+			pvcSpec.VolumeMode = &volumeMode
+			if pvcSpec.Resources.Requests == nil {
+				pvcSpec.Resources.Requests = corev1.ResourceList{}
+			}
+			pvcSpec.Resources.Requests[corev1.ResourceStorage] = resource.MustParse(storage)
+		}
+		cdiConfig := cc.MakeEmptyCDIConfigSpec(common.ConfigName)
+		cdiConfig.Status.FilesystemOverhead = &cdiv1.FilesystemOverhead{
+			Global: common.DefaultGlobalOverhead,
+		}
+
+		p := creatHostClonePhase(&ResourceModifier{
+			modifySourcePvc: func(pvcSpec *corev1.PersistentVolumeClaimSpec) {
+				setPvcAttributes(pvcSpec, corev1.PersistentVolumeFilesystem, "8Gi")
+			},
+			modifyDesiredPvc: func(pvcSpec *corev1.PersistentVolumeClaimSpec) {
+				setPvcAttributes(pvcSpec, corev1.PersistentVolumeFilesystem, "8Gi")
+			},
+		}, cdiConfig)
+
+		result, err := p.Reconcile(context.Background())
+		Expect(err).ToNot(HaveOccurred())
+		Expect(result).ToNot(BeNil())
+		Expect(result.Requeue).To(BeFalse())
+		Expect(result.RequeueAfter).ToNot(BeZero())
+
+		pvc := getDesiredClaim(p)
+
+		Expect(*pvc.Spec.VolumeMode).To(Equal(corev1.PersistentVolumeFilesystem))
+		actualSize := pvc.Spec.Resources.Requests[corev1.ResourceStorage]
+		originalRequested := resource.MustParse("8Gi")
+
+		Expect(actualSize.Cmp(originalRequested)).To(BeNumerically("==", 0), "The actual should be equal to the requested", "actual", actualSize, "requested", originalRequested)
+	})
+
 	Context("with desired claim created", func() {
 		getCliam := func() *corev1.PersistentVolumeClaim {
 			return &corev1.PersistentVolumeClaim{
@@ -141,7 +239,7 @@ var _ = Describe("HostClonePhase test", func() {
 		It("should wait for clone to succeed", func() {
 			desired := getCliam()
 			desired.Annotations[cc.AnnPodPhase] = "Running"
-			p := creatHostClonePhase(desired)
+			p := creatHostClonePhase(nil, desired)
 
 			result, err := p.Reconcile(context.Background())
 			Expect(err).ToNot(HaveOccurred())
@@ -153,7 +251,7 @@ var _ = Describe("HostClonePhase test", func() {
 		It("should wait for clone to succeed with preallocation", func() {
 			desired := getCliam()
 			desired.Annotations[cc.AnnPodPhase] = "Succeeded"
-			p := creatHostClonePhase(desired)
+			p := creatHostClonePhase(nil, desired)
 			p.Preallocation = true
 
 			result, err := p.Reconcile(context.Background())
@@ -166,7 +264,7 @@ var _ = Describe("HostClonePhase test", func() {
 		It("should succeed", func() {
 			desired := getCliam()
 			desired.Annotations[cc.AnnPodPhase] = "Succeeded"
-			p := creatHostClonePhase(desired)
+			p := creatHostClonePhase(nil, desired)
 
 			result, err := p.Reconcile(context.Background())
 			Expect(err).ToNot(HaveOccurred())
@@ -177,7 +275,7 @@ var _ = Describe("HostClonePhase test", func() {
 			desired := getCliam()
 			desired.Annotations[cc.AnnPodPhase] = "Succeeded"
 			desired.Annotations[cc.AnnPreallocationApplied] = "true"
-			p := creatHostClonePhase(desired)
+			p := creatHostClonePhase(nil, desired)
 			p.Preallocation = true
 
 			result, err := p.Reconcile(context.Background())

pkg/controller/common/util.go

@@ -2224,3 +2224,57 @@ func UpdatePVCBoundContionFromEvents(pvc *corev1.PersistentVolumeClaim, c client
 
 	return nil
 }
+
+func GetUsableSpace(ctx context.Context, c client.Client, pvc *corev1.PersistentVolumeClaim) (resource.Quantity, error) {
+	sizeRequest := pvc.Spec.Resources.Requests[corev1.ResourceStorage]
+	volumeMode := util.ResolveVolumeMode(pvc.Spec.VolumeMode)
+
+	if volumeMode == corev1.PersistentVolumeFilesystem {
+		fsOverhead, err := GetFilesystemOverheadForStorageClass(ctx, c, pvc.Spec.StorageClassName)
+		if err != nil {
+			return resource.Quantity{}, err
+		}
+		fsOverheadFloat, _ := strconv.ParseFloat(string(fsOverhead), 64)
+		usableSpaceRaw := util.GetUsableSpace(fsOverheadFloat, sizeRequest.Value())
+
+		return *resource.NewScaledQuantity(usableSpaceRaw, 0), nil
+	}
+
+	return sizeRequest, nil
+}
+
+func GetDVFromPVC(ctx context.Context, c client.Client, pvc *corev1.PersistentVolumeClaim) (*cdiv1.DataVolume, error) {
+	dv := &cdiv1.DataVolume{}
+	if err := c.Get(ctx, types.NamespacedName{Namespace: pvc.Namespace, Name: pvc.Name}, dv); err != nil {
+		if !k8serrors.IsNotFound(err) {
+			return nil, err
+		}
+	}
+	return dv, nil
+}
+
+func GetDVCloneSize(ctx context.Context, c client.Client, dv *cdiv1.DataVolume) (*resource.Quantity, error) {
+	if dv.Spec.Source != nil {
+		snapshot := &snapshotv1.VolumeSnapshot{}
+		if snapshotKey := dv.Spec.Source.Snapshot; snapshotKey != nil {
+			if err := c.Get(ctx, types.NamespacedName{Namespace: snapshotKey.Namespace, Name: snapshotKey.Name}, snapshot); err != nil {
+				return nil, err
+			}
+			if snapshot.Status != nil && snapshot.Status.ReadyToUse != nil && *snapshot.Status.ReadyToUse {
+				if snapshot.Status.RestoreSize != nil {
+					return snapshot.Status.RestoreSize, nil
+				}
+				return nil, fmt.Errorf("snapshot %s doesn't have a restore size specified in its status", snapshot.Name)
+			}
+			return nil, fmt.Errorf("snapshot %s is not ready", snapshot.Name)
+		}
+		if pvcKey := dv.Spec.Source.PVC; pvcKey != nil {
+			pvc := &corev1.PersistentVolumeClaim{}
+			if err := c.Get(ctx, types.NamespacedName{Namespace: pvcKey.Namespace, Name: pvcKey.Name}, pvc); err != nil {
+				return nil, err
+			}
+			return pvc.Status.Capacity.Storage(), nil
+		}
+	}
+	return nil, fmt.Errorf("dataVolume %s does not have a clone source", dv.Name)
+}