diff --git a/pkg/controller/clone-controller.go b/pkg/controller/clone-controller.go index d2ff9208d7..8d6f2f7d6a 100644 --- a/pkg/controller/clone-controller.go +++ b/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 -} diff --git a/pkg/controller/clone/BUILD.bazel b/pkg/controller/clone/BUILD.bazel index 82f1232344..3b83e680f1 100644 --- a/pkg/controller/clone/BUILD.bazel +++ b/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", diff --git a/pkg/controller/clone/host-clone.go b/pkg/controller/clone/host-clone.go index 15e27b2b81..c103998566 100644 --- a/pkg/controller/clone/host-clone.go +++ b/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 diff --git a/pkg/controller/clone/host-clone_test.go b/pkg/controller/clone/host-clone_test.go index ce6b5fa8ff..17daa136df 100644 --- a/pkg/controller/clone/host-clone_test.go +++ b/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()) diff --git a/pkg/controller/common/util.go b/pkg/controller/common/util.go index 93773da07d..fa824b087c 100644 --- a/pkg/controller/common/util.go +++ b/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) +}