Preallocate even if the size is too small (#1637) (#1651)

This PR removes "skipped" condition for preallocation. Importer/uploader
will preallocate to the available size. Filesystem overhead needs to be
taken into account.

Signed-off-by: Tomasz Barański <tomob@users.noreply.github.com>

cmd/cdi-importer/importer.go

@@ -67,7 +67,7 @@ func main() {
 	previousCheckpoint, _ := util.ParseEnvVar(common.ImporterPreviousCheckpoint, false)
 	finalCheckpoint, _ := util.ParseEnvVar(common.ImporterFinalCheckpoint, false)
 	preallocation, err := strconv.ParseBool(os.Getenv(common.Preallocation))
-	var preallocationApplied string
+	var preallocationApplied bool
 
 	//Registry import currently support kubevirt content type only
 	if contentType != string(cdiv1.DataVolumeKubeVirt) && (source == controller.SourceRegistry || source == controller.SourceImageio) {
@@ -98,23 +98,18 @@ func main() {
 	if source == controller.SourceNone && contentType == string(cdiv1.DataVolumeKubeVirt) {
 		requestImageSizeQuantity := resource.MustParse(imageSize)
 		minSizeQuantity := util.MinQuantity(resource.NewScaledQuantity(availableDestSpace, 0), &requestImageSizeQuantity)
-		preallocationApplied = strconv.FormatBool(preallocation)
+		preallocationApplied = preallocation
 		if minSizeQuantity.Cmp(requestImageSizeQuantity) != 0 {
 			// Available dest space is smaller than the size we want to create
 			klog.Warningf("Available space less than requested size, creating blank image sized to available space: %s.\n", minSizeQuantity.String())
-			if preallocation {
-				preallocationApplied = "skipped"
-				preallocation = false
-			}
 		}
 
-		err := image.CreateBlankImage(common.ImporterWritePath, minSizeQuantity, preallocation)
+		quantityWithFSOverhead := importer.GetUsableSpace(filesystemOverhead, minSizeQuantity.Value())
+		klog.Infof("Space adjusted for filesystem overhead: %d.\n", quantityWithFSOverhead)
+		err := image.CreateBlankImage(common.ImporterWritePath, *resource.NewScaledQuantity(quantityWithFSOverhead, 0), preallocation)
 		if err != nil {
 			klog.Errorf("%+v", err)
 			message := fmt.Sprintf("Unable to create blank image: %+v", err)
-			if preallocationApplied == "skipped" {
-				message += ", " + controller.PreallocationSkipped
-			}
 			err = util.WriteTerminationMessage(message)
 			if err != nil {
 				klog.Errorf("%+v", err)
@@ -199,11 +194,8 @@ func main() {
 		preallocationApplied = processor.PreallocationApplied()
 	}
 	message := "Import Complete"
-	switch preallocationApplied {
-	case "true":
+	if preallocationApplied {
 		message += ", " + controller.PreallocationApplied
-	case "skipped":
-		message += ", " + controller.PreallocationSkipped
 	}
 	err = util.WriteTerminationMessage(message)
 	if err != nil {

cmd/cdi-uploadserver/uploadserver.go

@@ -90,11 +90,8 @@ func main() {
 	} else {
 		message = "Upload Complete"
 	}
-	switch server.PreallocationApplied() {
-	case "true":
+	if server.PreallocationApplied() {
 		message += ", " + controller.PreallocationApplied
-	case "skipped":
-		message += ", " + controller.PreallocationSkipped
 	}
 	err = util.WriteTerminationMessage(message)
 	if err != nil {

pkg/controller/import-controller.go

@@ -95,9 +95,6 @@ const (
 
 	// PreallocationApplied is a string inserted into importer's/uploader's exit message
 	PreallocationApplied = "Preallocation applied"
-
-	// PreallocationSkipped is a string inserted into importer's/uploader's exit message
-	PreallocationSkipped = "Preallocation skipped"
 )
 
 // ImportReconciler members
@@ -373,9 +370,6 @@ func (r *ImportReconciler) updatePvcFromPod(pvc *corev1.PersistentVolumeClaim, p
 		if strings.Contains(pod.Status.ContainerStatuses[0].State.Terminated.Message, PreallocationApplied) {
 			anno[AnnPreallocationApplied] = "true"
 		}
-		if strings.Contains(pod.Status.ContainerStatuses[0].State.Terminated.Message, PreallocationSkipped) {
-			anno[AnnPreallocationApplied] = "skipped"
-		}
 	}
 
 	if anno[AnnCurrentCheckpoint] != "" {
@@ -500,11 +494,6 @@ func (r *ImportReconciler) createImportEnvVar(pvc *corev1.PersistentVolumeClaim)
 		if err != nil {
 			return nil, err
 		}
-		fsOverhead, err := GetFilesystemOverhead(r.client, pvc)
-		if err != nil {
-			return nil, err
-		}
-		podEnvVar.filesystemOverhead = string(fsOverhead)
 		podEnvVar.insecureTLS, err = r.isInsecureTLS(pvc)
 		if err != nil {
 			return nil, err
@@ -518,6 +507,12 @@ func (r *ImportReconciler) createImportEnvVar(pvc *corev1.PersistentVolumeClaim)
 		podEnvVar.finalCheckpoint = getValueFromAnnotation(pvc, AnnFinalCheckpoint)
 	}
 
+	fsOverhead, err := GetFilesystemOverhead(r.client, pvc)
+	if err != nil {
+		return nil, err
+	}
+	podEnvVar.filesystemOverhead = string(fsOverhead)
+
 	if preallocation, err := strconv.ParseBool(getValueFromAnnotation(pvc, AnnPreallocationRequested)); err == nil {
 		podEnvVar.preallocation = preallocation
 	} // else use the default "false"

pkg/controller/upload-controller.go

@@ -247,9 +247,6 @@ func (r *UploadReconciler) reconcilePVC(log logr.Logger, pvc *corev1.PersistentV
 			if strings.Contains(pod.Status.ContainerStatuses[0].State.Terminated.Message, PreallocationApplied) {
 				anno[AnnPreallocationApplied] = "true"
 			}
-			if strings.Contains(pod.Status.ContainerStatuses[0].State.Terminated.Message, PreallocationSkipped) {
-				anno[AnnPreallocationApplied] = "skipped"
-			}
 		}
 	}
 	setConditionFromPodWithPrefix(anno, AnnRunningCondition, pod)

pkg/importer/data-processor.go

@@ -20,7 +20,6 @@ import (
 	"fmt"
 	"net/url"
 	"os"
-	"strconv"
 
 	"github.com/pkg/errors"
 
@@ -121,10 +120,8 @@ type DataProcessor struct {
 	needsDataCleanup bool
 	// preallocation is the flag controlling preallocation setting of qemu-img
 	preallocation bool
-	// preallocationApplied is used to pass information whether preallocation has been performed, skipped or not attempted
-	// "skipped" is used to indicate that preallocation would have been perfomed but there was not enough space, so the
-	// preallocation whould have failed.
-	preallocationApplied string
+	// preallocationApplied is used to pass information whether preallocation has been performed, or not
+	preallocationApplied bool
 }
 
 // NewDataProcessor create a new instance of a data processor using the passed in data provider.
@@ -264,7 +261,7 @@ func (dp *DataProcessor) convert(url *url.URL) (ProcessingPhase, error) {
 	if err != nil {
 		return ProcessingPhaseError, errors.Wrap(err, "Conversion to Raw failed")
 	}
-	dp.preallocationApplied = strconv.FormatBool(dp.preallocation)
+	dp.preallocationApplied = dp.preallocation
 
 	return ProcessingPhaseResize, nil
 }
@@ -293,7 +290,7 @@ func (dp *DataProcessor) resize() (ProcessingPhase, error) {
 		if shouldPreallocate {
 			return ProcessingPhasePreallocate, nil
 		}
-		dp.preallocationApplied = "skipped" // qemu did not preallicate space for a resized file
+		dp.preallocationApplied = false // qemu did not preallocate space for a resized file
 	}
 	return ProcessingPhaseComplete, nil
 }
@@ -311,7 +308,7 @@ func (dp *DataProcessor) preallocate() (ProcessingPhase, error) {
 	if err != nil {
 		return ProcessingPhaseError, errors.Wrap(err, "Preallocation or resized image failed")
 	}
-	dp.preallocationApplied = "true"
+	dp.preallocationApplied = true
 
 	return ProcessingPhaseComplete, nil
 }
@@ -326,22 +323,20 @@ func ResizeImage(dataFile, imageSize string, totalTargetSpace int64) (bool, erro
 		return false, err
 	}
 	if imageSize != "" {
-		shouldPreallocate := true
 		currentImageSizeQuantity := resource.NewScaledQuantity(info.VirtualSize, 0)
 		newImageSizeQuantity := resource.MustParse(imageSize)
 		minSizeQuantity := util.MinQuantity(resource.NewScaledQuantity(totalTargetSpace, 0), &newImageSizeQuantity)
 		if minSizeQuantity.Cmp(newImageSizeQuantity) != 0 {
 			// Available destination space is smaller than the size we want to resize to
 			klog.Warningf("Available space less than requested size, resizing image to available space %s.\n", minSizeQuantity.String())
-			shouldPreallocate = false
 		}
 		if currentImageSizeQuantity.Cmp(minSizeQuantity) == 0 {
 			klog.V(1).Infof("No need to resize image. Requested size: %s, Image size: %d.\n", imageSize, info.VirtualSize)
 			return false, nil
 		}
 		klog.V(1).Infof("Expanding image size to: %s\n", minSizeQuantity.String())
 		err := qemuOperations.Resize(dataFile, minSizeQuantity)
-		return err == nil && shouldPreallocate, err
+		return err == nil, err
 	}
 	return false, errors.New("Image resize called with blank resize")
 }
@@ -378,6 +373,20 @@ func (dp *DataProcessor) calculateTargetSize() int64 {
 }
 
 // PreallocationApplied returns true if data processing path included preallocation step
-func (dp *DataProcessor) PreallocationApplied() string {
+func (dp *DataProcessor) PreallocationApplied() bool {
 	return dp.preallocationApplied
 }
+
+func (dp *DataProcessor) getUsableSpace() int64 {
+	return GetUsableSpace(dp.filesystemOverhead, dp.availableSpace)
+}
+
+// GetUsableSpace calculates space to use taking file system overhead into account
+func GetUsableSpace(filesystemOverhead float64, availableSpace int64) int64 {
+	blockSize := int64(512)
+	spaceWithOverhead := int64((1 - filesystemOverhead) * float64(availableSpace))
+	// qemu-img will round up, making us use more than the usable space.
+	// This later conflicts with image size validation.
+	qemuImgCorrection := util.RoundDown(spaceWithOverhead, blockSize)
+	return qemuImgCorrection
+}

pkg/importer/data-processor_test.go

@@ -404,7 +404,7 @@ var _ = Describe("ResizeImage", func() {
 		})
 	},
 		table.Entry("successfully resize to imageSize when imageSize > info.VirtualSize and < totalSize", NewFakeQEMUOperations(nil, nil, fakeInfoRet, nil, nil, resource.NewScaledQuantity(int64(1500), 0)), "1500", int64(2048), false, true),
-		table.Entry("successfully resize to totalSize when imageSize > info.VirtualSize and > totalSize", NewFakeQEMUOperations(nil, nil, fakeInfoRet, nil, nil, resource.NewScaledQuantity(int64(2048), 0)), "2500", int64(2048), false, false),
+		table.Entry("successfully resize to totalSize when imageSize > info.VirtualSize and > totalSize", NewFakeQEMUOperations(nil, nil, fakeInfoRet, nil, nil, resource.NewScaledQuantity(int64(2048), 0)), "2500", int64(2048), false, true),
 		table.Entry("successfully do nothing when imageSize = info.VirtualSize and > totalSize", NewFakeQEMUOperations(nil, nil, fakeInfoRet, nil, nil, resource.NewScaledQuantity(int64(1024), 0)), "1024", int64(1024), false, false),
 		table.Entry("fail to resize to with blank imageSize", NewFakeQEMUOperations(nil, nil, fakeInfoRet, nil, nil, resource.NewScaledQuantity(int64(2048), 0)), "", int64(2048), true, false),
 		table.Entry("fail to resize to with blank imageSize", NewQEMUAllErrors(), "", int64(2048), true, false),

pkg/uploadserver/uploadserver.go

@@ -49,7 +49,7 @@ const (
 // UploadServer is the interface to uploadServerApp
 type UploadServer interface {
 	Run() error
-	PreallocationApplied() string
+	PreallocationApplied() bool
 }
 
 type uploadServerApp struct {
@@ -69,7 +69,7 @@ type uploadServerApp struct {
 	uploading            bool
 	processing           bool
 	done                 bool
-	preallocationApplied string
+	preallocationApplied bool
 	doneChan             chan struct{}
 	errChan              chan error
 	mutex                sync.Mutex
@@ -393,7 +393,7 @@ func (app *uploadServerApp) uploadHandler(irc imageReadCloser) http.HandlerFunc
 	}
 }
 
-func (app *uploadServerApp) PreallocationApplied() string {
+func (app *uploadServerApp) PreallocationApplied() bool {
 	return app.preallocationApplied
 }
 
@@ -407,9 +407,9 @@ func newAsyncUploadStreamProcessor(stream io.ReadCloser, dest, imageSize string,
 	return processor, processor.ProcessDataWithPause()
 }
 
-func newUploadStreamProcessor(stream io.ReadCloser, dest, imageSize string, filesystemOverhead float64, preallocation bool, contentType string) (string, error) {
+func newUploadStreamProcessor(stream io.ReadCloser, dest, imageSize string, filesystemOverhead float64, preallocation bool, contentType string) (bool, error) {
 	if contentType == common.FilesystemCloneContentType {
-		return "false", filesystemCloneProcessor(stream, common.ImporterVolumePath)
+		return false, filesystemCloneProcessor(stream, common.ImporterVolumePath)
 	}
 
 	uds := importer.NewUploadDataSource(newContentReader(stream, contentType))

pkg/uploadserver/uploadserver_test.go

@@ -88,12 +88,12 @@ func newHTTPClient(clientKeyPair *triple.KeyPair, serverCACert *x509.Certificate
 	return client
 }
 
-func saveProcessorSuccess(stream io.ReadCloser, dest, imageSize string, filesystemOverhead float64, preallocation bool, contentType string) (string, error) {
-	return "false", nil
+func saveProcessorSuccess(stream io.ReadCloser, dest, imageSize string, filesystemOverhead float64, preallocation bool, contentType string) (bool, error) {
+	return false, nil
 }
 
-func saveProcessorFailure(stream io.ReadCloser, dest, imageSize string, filesystemOverhead float64, preallocation bool, contentType string) (string, error) {
-	return "false", fmt.Errorf("Error using datastream")
+func saveProcessorFailure(stream io.ReadCloser, dest, imageSize string, filesystemOverhead float64, preallocation bool, contentType string) (bool, error) {
+	return false, fmt.Errorf("Error using datastream")
 }
 
 func withProcessorSuccess(f func()) {
@@ -104,7 +104,7 @@ func withProcessorFailure(f func()) {
 	replaceProcessorFunc(saveProcessorFailure, f)
 }
 
-func replaceProcessorFunc(replacement func(io.ReadCloser, string, string, float64, bool, string) (string, error), f func()) {
+func replaceProcessorFunc(replacement func(io.ReadCloser, string, string, float64, bool, string) (bool, error), f func()) {
 	origProcessorFunc := uploadProcessorFunc
 	uploadProcessorFunc = replacement
 	defer func() {

pkg/util/util.go

@@ -273,3 +273,8 @@ func CopyDir(source string, dest string) (err error) {
 	}
 	return
 }
+
+// RoundDown returns the number rounded down to the nearest multiple.
+func RoundDown(number, multiple int64) int64 {
+	return number / multiple * multiple
+}