Fix a target pvc validation bug

Corrects the validation logic for target volume.

Below description of the original problem:
When validating whether an image will fit into a PV we compare the
image's virtual size to the filesystem's reported available space to
guage whether it will fit.  The current calculation reduces the apparent
available space by the configured filesystem overhead value but the
overhead is already (mostly) factored into the result of Statfs.  This
causes the check to fail for PVCs that are just large enough to
accommodate an image plus overhead (ie. when using the DataVolume
Storage API with filesystem PVs with capacity constrained by the PVC
storage request size).

This was not caught in testing because HPP does not have capacity
constrained PVs and we are typically testing block volumes in the ceph
lanes.  It can be triggered in our CI by allocating a Filesystem PV on
ceph-rbd storage because these volumes are capacity constrained and
subject to filesystem overhead.

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

pkg/image/qemu.go

@@ -62,7 +62,7 @@ type QEMUOperations interface {
 	ConvertToRawStream(*url.URL, string, bool) error
 	Resize(string, resource.Quantity, bool) error
 	Info(url *url.URL) (*ImgInfo, error)
-	Validate(*url.URL, int64, float64) error
+	Validate(*url.URL, int64) error
 	CreateBlankImage(string, resource.Quantity, bool) error
 	Rebase(backingFile string, delta string) error
 	Commit(image string) error
@@ -215,7 +215,7 @@ func isSupportedFormat(value string) bool {
 	}
 }
 
-func checkIfURLIsValid(info *ImgInfo, availableSize int64, filesystemOverhead float64, image string) error {
+func checkIfURLIsValid(info *ImgInfo, availableSize int64, image string) error {
 	if !isSupportedFormat(info.Format) {
 		return errors.Errorf("Invalid format %s for image %s", info.Format, image)
 	}
@@ -226,18 +226,18 @@ func checkIfURLIsValid(info *ImgInfo, availableSize int64, filesystemOverhead fl
 		}
 	}
 
-	if int64(float64(availableSize)*(1-filesystemOverhead)) < info.VirtualSize {
-		return errors.Errorf("Virtual image size %d is larger than available size %d (PVC size %d, reserved overhead %f%%). A larger PVC is required.", info.VirtualSize, int64((1-filesystemOverhead)*float64(availableSize)), info.VirtualSize, filesystemOverhead)
+	if availableSize < info.VirtualSize {
+		return errors.Errorf("Virtual image size %d is larger than the reported available storage %d (PVC size %d). A larger PVC is required.", info.VirtualSize, availableSize, info.VirtualSize)
 	}
 	return nil
 }
 
-func (o *qemuOperations) Validate(url *url.URL, availableSize int64, filesystemOverhead float64) error {
+func (o *qemuOperations) Validate(url *url.URL, availableSize int64) error {
 	info, err := o.Info(url)
 	if err != nil {
 		return err
 	}
-	return checkIfURLIsValid(info, availableSize, filesystemOverhead, url.String())
+	return checkIfURLIsValid(info, availableSize, url.String())
 }
 
 // ConvertToRawStream converts an http accessible image to raw format without locally caching the image
@@ -246,8 +246,8 @@ func ConvertToRawStream(url *url.URL, dest string, preallocate bool) error {
 }
 
 // Validate does basic validation of a qemu image
-func Validate(url *url.URL, availableSize int64, filesystemOverhead float64) error {
-	return qemuIterface.Validate(url, availableSize, filesystemOverhead)
+func Validate(url *url.URL, availableSize int64) error {
+	return qemuIterface.Validate(url, availableSize)
 }
 
 func reportProgress(line string) {

pkg/image/qemu_test.go

@@ -217,9 +217,9 @@ var _ = Describe("Resize", func() {
 var _ = Describe("Validate", func() {
 	imageName, _ := url.Parse("myimage.qcow2")
 
-	table.DescribeTable("Validate should", func(execfunc execFunctionType, errString string, image *url.URL, overhead float64) {
+	table.DescribeTable("Validate should", func(execfunc execFunctionType, errString string, image *url.URL) {
 		replaceExecFunction(execfunc, func() {
-			err := Validate(image, 42949672960, overhead)
+			err := Validate(image, 42949672960)
 
 			if errString == "" {
 				Expect(err).NotTo(HaveOccurred())
@@ -232,13 +232,12 @@ var _ = Describe("Validate", func() {
 			}
 		})
 	},
-		table.Entry("should return success", mockExecFunction(goodValidateJSON, "", expectedLimits, "info", "--output=json", imageName.String()), "", imageName, 0.0),
-		table.Entry("should return error", mockExecFunction("explosion", "exit 1", expectedLimits), "explosion, exit 1", imageName, 0.0),
-		table.Entry("should return error on bad json", mockExecFunction(badValidateJSON, "", expectedLimits), "unexpected end of JSON input", imageName, 0.0),
-		table.Entry("should return error on bad format", mockExecFunction(badFormatValidateJSON, "", expectedLimits), fmt.Sprintf("Invalid format raw2 for image %s", imageName), imageName, 0.0),
-		table.Entry("should return error on invalid backing file", mockExecFunction(backingFileValidateJSON, "", expectedLimits), fmt.Sprintf("Image %s is invalid because it has invalid backing file backing-file.qcow2", imageName), imageName, 0.0),
-		table.Entry("should return error when PVC is too small", mockExecFunction(hugeValidateJSON, "", expectedLimits), fmt.Sprintf("Virtual image size %d is larger than available size %d (PVC size %d, reserved overhead %f%%). A larger PVC is required.", 52949672960, 42949672960, 52949672960, 0.0), imageName, 0.0),
-		table.Entry("should return error when PVC is too small with overhead", mockExecFunction(hugeValidateJSON, "", expectedLimits), fmt.Sprintf("Virtual image size %d is larger than available size %d (PVC size %d, reserved overhead %f%%). A larger PVC is required.", 52949672960, 34359738368, 52949672960, 0.2), imageName, 0.2),
+		table.Entry("should return success", mockExecFunction(goodValidateJSON, "", expectedLimits, "info", "--output=json", imageName.String()), "", imageName),
+		table.Entry("should return error", mockExecFunction("explosion", "exit 1", expectedLimits), "explosion, exit 1", imageName),
+		table.Entry("should return error on bad json", mockExecFunction(badValidateJSON, "", expectedLimits), "unexpected end of JSON input", imageName),
+		table.Entry("should return error on bad format", mockExecFunction(badFormatValidateJSON, "", expectedLimits), fmt.Sprintf("Invalid format raw2 for image %s", imageName), imageName),
+		table.Entry("should return error on invalid backing file", mockExecFunction(backingFileValidateJSON, "", expectedLimits), fmt.Sprintf("Image %s is invalid because it has invalid backing file backing-file.qcow2", imageName), imageName),
+		table.Entry("should return error when PVC is too small", mockExecFunction(hugeValidateJSON, "", expectedLimits), fmt.Sprintf("Virtual image size %d is larger than the reported available storage %d (PVC size %d). A larger PVC is required.", 52949672960, 42949672960, 52949672960), imageName),
 	)
 
 })

pkg/importer/data-processor.go

@@ -286,7 +286,7 @@ func (dp *DataProcessor) ProcessDataWithPause() error {
 
 func (dp *DataProcessor) validate(url *url.URL) error {
 	klog.V(1).Infoln("Validating image")
-	err := qemuOperations.Validate(url, dp.availableSpace, dp.filesystemOverhead)
+	err := qemuOperations.Validate(url, dp.availableSpace)
 	if err != nil {
 		return ValidationSizeError{err: err}
 	}

pkg/importer/data-processor_test.go

@@ -579,7 +579,7 @@ func (o *fakeQEMUOperations) ConvertToRawStream(*url.URL, string, bool) error {
 	return o.e2
 }
 
-func (o *fakeQEMUOperations) Validate(*url.URL, int64, float64) error {
+func (o *fakeQEMUOperations) Validate(*url.URL, int64) error {
 	return o.e5
 }