GetAvailableSpace(block) now returns error (#1244)

Modified function that gets the size of a block device/available to return error as well as -1, so we
can distinguish the path not existing from the binary not existing in case the container doesn't have
the required binaries.

Last lane also passed, but due to slow CI timed out before reporting results.

Signed-off-by: Alexander Wels <awels@redhat.com>

cmd/cdi-importer/importer.go

@@ -80,7 +80,11 @@ func main() {
 	}
 
 	dataDir := common.ImporterDataDir
-	availableDestSpace := util.GetAvailableSpaceByVolumeMode(volumeMode)
+	availableDestSpace, err := util.GetAvailableSpaceByVolumeMode(volumeMode)
+	if err != nil {
+		klog.Errorf("%+v", err)
+		os.Exit(1)
+	}
 	if source == controller.SourceNone && contentType == string(cdiv1.DataVolumeKubeVirt) {
 		requestImageSizeQuantity := resource.MustParse(imageSize)
 		minSizeQuantity := util.MinQuantity(resource.NewScaledQuantity(availableDestSpace, 0), &requestImageSizeQuantity)

go.sum

@@ -669,7 +669,7 @@ k8s.io/kubernetes v1.14.0/go.mod h1:ocZa8+6APFNC2tX1DZASIbocyYT5jHzqFVsY5aoB7Jk=
 k8s.io/utils v0.0.0-20190712204705-3dccf664f023/go.mod h1:sZAwmy6armz5eXlNoLmJcl4F1QuKu7sr+mFQ0byX7Ew=
 k8s.io/utils v0.0.0-20190801114015-581e00157fb1 h1:+ySTxfHnfzZb9ys375PXNlLhkJPLKgHajBU0N62BDvE=
 k8s.io/utils v0.0.0-20190801114015-581e00157fb1/go.mod h1:sZAwmy6armz5eXlNoLmJcl4F1QuKu7sr+mFQ0byX7Ew=
-kubevirt.io/qe-tools v0.1.3 h1:TUQdOK40a5/wLwYQwiy56WBaS+OYcGxAEHe9rhEPZZM=
+kubevirt.io/qe-tools v0.1.3 h1:ZDDolkD2IsHuPW8PNyty5fWO6wpI2BXcQpC65en/9FU=
 kubevirt.io/qe-tools v0.1.3/go.mod h1:PJyH/YXC4W0AmxfheDmXWMbLNsMSboVGXKpMAwfKzVE=
 modernc.org/cc v1.0.0/go.mod h1:1Sk4//wdnYJiUIxnW8ddKpaOJCF37yAdqYnkxUpaYxw=
 modernc.org/golex v1.0.0/go.mod h1:b/QX9oBD/LhixY6NDh+IdGv17hgB+51fET1i2kPSmvk=

pkg/importer/data-processor.go

@@ -125,15 +125,16 @@ func NewDataProcessor(dataSource DataSourceInterface, dataFile, dataDir, scratch
 
 // ProcessData is the main synchronous processing loop
 func (dp *DataProcessor) ProcessData() error {
-	if util.GetAvailableSpace(dp.scratchDataDir) > int64(0) {
+	if size, _ := util.GetAvailableSpace(dp.scratchDataDir); size > int64(0) {
 		// Clean up before trying to write, in case a previous attempt left a mess. Note the deferred cleanup is intentional.
 		if err := CleanDir(dp.scratchDataDir); err != nil {
 			return errors.Wrap(err, "Failure cleaning up temporary scratch space")
 		}
 		// Attempt to be a good citizen and clean up my mess at the end.
 		defer CleanDir(dp.scratchDataDir)
 	}
-	if util.GetAvailableSpace(dp.dataDir) > int64(0) {
+
+	if size, _ := util.GetAvailableSpace(dp.dataDir); size > int64(0) {
 		// Clean up data dir before trying to write in case a previous attempt failed and left some stuff behind.
 		if err := CleanDir(dp.dataDir); err != nil {
 			return errors.Wrap(err, "Failure cleaning up target space")
@@ -234,8 +235,9 @@ func (dp *DataProcessor) convert(url *url.URL) (ProcessingPhase, error) {
 
 func (dp *DataProcessor) resize() (ProcessingPhase, error) {
 	// Resize only if we have a resize request, and if the image is on a file system pvc.
-	klog.V(3).Infof("Available space in dataFile: %d", getAvailableSpaceBlockFunc(dp.dataFile))
-	if dp.requestImageSize != "" && getAvailableSpaceBlockFunc(dp.dataFile) < int64(0) {
+	size, _ := getAvailableSpaceBlockFunc(dp.dataFile)
+	klog.V(3).Infof("Available space in dataFile: %d", size)
+	if dp.requestImageSize != "" && size < int64(0) {
 		klog.V(3).Infoln("Resizing image")
 		err := ResizeImage(dp.dataFile, dp.requestImageSize, dp.availableSpace)
 		if err != nil {
@@ -282,14 +284,22 @@ func ResizeImage(dataFile, imageSize string, totalTargetSpace int64) error {
 func (dp *DataProcessor) calculateTargetSize() int64 {
 	klog.V(1).Infof("Calculating available size\n")
 	var targetQuantity *resource.Quantity
-	if getAvailableSpaceBlockFunc(dp.dataFile) >= int64(0) {
+	size, err := getAvailableSpaceBlockFunc(dp.dataFile)
+	if err != nil {
+		klog.Error(err)
+	}
+	if size >= int64(0) {
 		// Block volume.
 		klog.V(1).Infof("Checking out block volume size.\n")
-		targetQuantity = resource.NewScaledQuantity(getAvailableSpaceBlockFunc(dp.dataFile), 0)
+		targetQuantity = resource.NewScaledQuantity(size, 0)
 	} else {
 		// File system volume.
 		klog.V(1).Infof("Checking out file system volume size.\n")
-		targetQuantity = resource.NewScaledQuantity(getAvailableSpaceFunc(dp.dataDir), 0)
+		size, err := getAvailableSpaceFunc(dp.dataDir)
+		if err != nil {
+			klog.Error(err)
+		}
+		targetQuantity = resource.NewScaledQuantity(size, 0)
 	}
 	if dp.requestImageSize != "" {
 		klog.V(1).Infof("Request image size not empty.\n")

pkg/importer/data-processor_test.go

@@ -338,9 +338,9 @@ var _ = Describe("Resize", func() {
 	})
 
 	It("Should not resize and return complete, when requestedSize is valid, but datadir doesn't exist (block device)", func() {
-		replaceAvailableSpaceBlockFunc(func(dataDir string) int64 {
+		replaceAvailableSpaceBlockFunc(func(dataDir string) (int64, error) {
 			Expect("dest").To(Equal(dataDir))
-			return int64(100000)
+			return int64(100000), nil
 		}, func() {
 			url, err := url.Parse("http://fakeurl-notreal.fake")
 			Expect(err).ToNot(HaveOccurred())
@@ -389,14 +389,26 @@ var _ = Describe("Resize", func() {
 	})
 
 	It("Should return same value as replaced function", func() {
-		replaceAvailableSpaceBlockFunc(func(dataDir string) int64 {
-			return int64(100000)
+		replaceAvailableSpaceBlockFunc(func(dataDir string) (int64, error) {
+			return int64(100000), nil
 		}, func() {
 			mdp := &MockDataProvider{}
 			dp := NewDataProcessor(mdp, "dest", "dataDir", "scratchDataDir", "")
 			Expect(int64(100000)).To(Equal(dp.calculateTargetSize()))
 		})
 	})
+
+	It("Should fail if calculate size returns failure", func() {
+		replaceAvailableSpaceBlockFunc(func(dataDir string) (int64, error) {
+			return int64(-1), errors.New("error")
+		}, func() {
+			mdp := &MockDataProvider{}
+			dp := NewDataProcessor(mdp, "dest", "dataDir", "scratchDataDir", "")
+			// We just log the error if one happens.
+			Expect(int64(-1)).To(Equal(dp.calculateTargetSize()))
+
+		})
+	})
 })
 
 var _ = Describe("ResizeImage", func() {
@@ -478,7 +490,7 @@ func NewQEMUAllErrors() image.QEMUOperations {
 	return NewFakeQEMUOperations(err, err, fakeInfoOpRetVal{nil, err}, err, err, nil)
 }
 
-func replaceAvailableSpaceBlockFunc(replacement func(string) int64, f func()) {
+func replaceAvailableSpaceBlockFunc(replacement func(string) (int64, error), f func()) {
 	origFunc := getAvailableSpaceBlockFunc
 	getAvailableSpaceBlockFunc = replacement
 	defer func() {
@@ -487,7 +499,7 @@ func replaceAvailableSpaceBlockFunc(replacement func(string) int64, f func()) {
 	f()
 }
 
-func replaceAvailableSpaceFunc(replacement func(string) int64, f func()) {
+func replaceAvailableSpaceFunc(replacement func(string) (int64, error), f func()) {
 	origFunc := getAvailableSpaceFunc
 	getAvailableSpaceFunc = replacement
 	defer func() {

pkg/importer/http-datasource.go

@@ -131,12 +131,13 @@ func (hs *HTTPDataSource) Info() (ProcessingPhase, error) {
 // Transfer is called to transfer the data from the source to a scratch location.
 func (hs *HTTPDataSource) Transfer(path string) (ProcessingPhase, error) {
 	if hs.contentType == cdiv1.DataVolumeKubeVirt {
-		if util.GetAvailableSpace(path) <= int64(0) {
+		size, err := util.GetAvailableSpace(path)
+		if size <= int64(0) {
 			//Path provided is invalid.
 			return ProcessingPhaseError, ErrInvalidPath
 		}
 		file := filepath.Join(path, tempFile)
-		err := util.StreamDataToFile(hs.readers.TopReader(), file)
+		err = util.StreamDataToFile(hs.readers.TopReader(), file)
 		if err != nil {
 			return ProcessingPhaseError, err
 		}

pkg/importer/imageio-datasource.go

@@ -93,7 +93,8 @@ func (is *ImageioDataSource) Info() (ProcessingPhase, error) {
 // Transfer is called to transfer the data from the source to a scratch location.
 func (is *ImageioDataSource) Transfer(path string) (ProcessingPhase, error) {
 	// we know that there won't be archives
-	if util.GetAvailableSpace(path) <= int64(0) {
+	size, _ := util.GetAvailableSpace(path)
+	if size <= int64(0) {
 		//Path provided is invalid.
 		return ProcessingPhaseError, ErrInvalidPath
 	}

pkg/importer/registry-datasource.go

@@ -71,14 +71,18 @@ func (rd *RegistryDataSource) Info() (ProcessingPhase, error) {
 
 // Transfer is called to transfer the data from the source registry to a temporary location.
 func (rd *RegistryDataSource) Transfer(path string) (ProcessingPhase, error) {
-	if util.GetAvailableSpace(path) <= int64(0) {
-		// Path provided is invalid.
+	size, err := util.GetAvailableSpace(path)
+	if err != nil {
+		return ProcessingPhaseError, err
+	}
+	if size <= int64(0) {
+		//Path provided is invalid.
 		return ProcessingPhaseError, ErrInvalidPath
 	}
 	rd.imageDir = filepath.Join(path, containerDiskImageDir)
 
 	klog.V(1).Infof("Copying registry image to scratch space.")
-	err := image.CopyRegistryImage(rd.endpoint, path, containerDiskImageDir, rd.accessKey, rd.secKey, rd.certDir, rd.insecureTLS)
+	err = image.CopyRegistryImage(rd.endpoint, path, containerDiskImageDir, rd.accessKey, rd.secKey, rd.certDir, rd.insecureTLS)
 	if err != nil {
 		return ProcessingPhaseError, errors.Wrapf(err, "Failed to read registry image")
 	}

pkg/importer/s3-datasource.go

@@ -80,12 +80,16 @@ func (sd *S3DataSource) Info() (ProcessingPhase, error) {
 
 // Transfer is called to transfer the data from the source to a temporary location.
 func (sd *S3DataSource) Transfer(path string) (ProcessingPhase, error) {
-	if util.GetAvailableSpace(path) <= int64(0) {
+	size, err := util.GetAvailableSpace(path)
+	if err != nil {
+		return ProcessingPhaseError, err
+	}
+	if size <= int64(0) {
 		//Path provided is invalid.
 		return ProcessingPhaseError, ErrInvalidPath
 	}
 	file := filepath.Join(path, tempFile)
-	err := util.StreamDataToFile(sd.readers.TopReader(), file)
+	err = util.StreamDataToFile(sd.readers.TopReader(), file)
 	if err != nil {
 		return ProcessingPhaseError, err
 	}

pkg/importer/upload-datasource.go

@@ -50,12 +50,16 @@ func (ud *UploadDataSource) Info() (ProcessingPhase, error) {
 
 // Transfer is called to transfer the data from the source to the passed in path.
 func (ud *UploadDataSource) Transfer(path string) (ProcessingPhase, error) {
-	if util.GetAvailableSpace(path) <= int64(0) {
+	size, err := util.GetAvailableSpace(path)
+	if err != nil {
+		return ProcessingPhaseError, err
+	}
+	if size <= int64(0) {
 		//Path provided is invalid.
 		return ProcessingPhaseError, ErrInvalidPath
 	}
 	file := filepath.Join(path, tempFile)
-	err := util.StreamDataToFile(ud.readers.TopReader(), file)
+	err = util.StreamDataToFile(ud.readers.TopReader(), file)
 	if err != nil {
 		return ProcessingPhaseError, err
 	}
@@ -116,12 +120,16 @@ func (aud *AsyncUploadDataSource) Info() (ProcessingPhase, error) {
 
 // Transfer is called to transfer the data from the source to the passed in path.
 func (aud *AsyncUploadDataSource) Transfer(path string) (ProcessingPhase, error) {
-	if util.GetAvailableSpace(path) <= int64(0) {
+	size, err := util.GetAvailableSpace(path)
+	if err != nil {
+		return ProcessingPhaseError, err
+	}
+	if size <= int64(0) {
 		//Path provided is invalid.
 		return ProcessingPhaseError, ErrInvalidPath
 	}
 	file := filepath.Join(path, tempFile)
-	err := util.StreamDataToFile(aud.uploadDataSource.readers.TopReader(), file)
+	err = util.StreamDataToFile(aud.uploadDataSource.readers.TopReader(), file)
 	if err != nil {
 		return ProcessingPhaseError, err
 	}

pkg/util/util.go

@@ -24,6 +24,10 @@ import (
 	"kubevirt.io/containerized-data-importer/pkg/common"
 )
 
+const (
+	blockdevFileName = "/usr/sbin/blockdev"
+)
+
 // CountingReader is a reader that keeps track of how much has been read
 type CountingReader struct {
 	Reader  io.ReadCloser
@@ -84,39 +88,48 @@ func (r *CountingReader) Close() error {
 
 // GetAvailableSpaceByVolumeMode calls another method based on the volumeMode parameter to get the amount of
 // available space at the path specified.
-func GetAvailableSpaceByVolumeMode(volumeMode v1.PersistentVolumeMode) int64 {
+func GetAvailableSpaceByVolumeMode(volumeMode v1.PersistentVolumeMode) (int64, error) {
 	if volumeMode == v1.PersistentVolumeBlock {
 		return GetAvailableSpaceBlock(common.WriteBlockPath)
 	}
 	return GetAvailableSpace(common.ImporterVolumePath)
 }
 
 // GetAvailableSpace gets the amount of available space at the path specified.
-func GetAvailableSpace(path string) int64 {
+func GetAvailableSpace(path string) (int64, error) {
 	var stat syscall.Statfs_t
 	err := syscall.Statfs(path, &stat)
 	if err != nil {
-		return int64(-1)
+		return int64(-1), err
 	}
-	return int64(stat.Bavail) * int64(stat.Bsize)
+	return int64(stat.Bavail) * int64(stat.Bsize), nil
 }
 
 // GetAvailableSpaceBlock gets the amount of available space at the block device path specified.
-func GetAvailableSpaceBlock(deviceName string) int64 {
-	cmd := exec.Command("/usr/sbin/blockdev", "--getsize64", deviceName)
+func GetAvailableSpaceBlock(deviceName string) (int64, error) {
+	// Check if device exists.
+	info, err := os.Stat(deviceName)
+	if os.IsNotExist(err) {
+		return int64(-1), nil
+	}
+	if info.IsDir() {
+		return int64(-1), nil
+	}
+	// Device exists and is not a directory attempt to get size
+	cmd := exec.Command(blockdevFileName, "--getsize64", deviceName)
 	var out bytes.Buffer
-	var stderr bytes.Buffer
+	var errBuf bytes.Buffer
 	cmd.Stdout = &out
-	cmd.Stderr = &stderr
-	err := cmd.Run()
+	cmd.Stderr = &errBuf
+	err = cmd.Run()
 	if err != nil {
-		return int64(-1)
+		return int64(-1), errors.Errorf("%v, %s", err, errBuf.String())
 	}
 	i, err := strconv.ParseInt(strings.TrimSpace(out.String()), 10, 64)
 	if err != nil {
-		return int64(-1)
+		return int64(-1), err
 	}
-	return i
+	return i, nil
 }
 
 // MinQuantity calculates the minimum of two quantities.
@@ -130,12 +143,16 @@ func MinQuantity(availableSpace, imageSize *resource.Quantity) resource.Quantity
 // StreamDataToFile provides a function to stream the specified io.Reader to the specified local file
 func StreamDataToFile(r io.Reader, fileName string) error {
 	var outFile *os.File
-	var err error
-	if GetAvailableSpaceBlock(fileName) < 0 {
+	blockSize, err := GetAvailableSpaceBlock(fileName)
+	if err != nil {
+		return errors.Wrapf(err, "error determining if block device exists")
+	}
+	if blockSize >= 0 {
+		// Block device found and size determined.
+		outFile, err = os.OpenFile(fileName, os.O_EXCL|os.O_WRONLY, os.ModePerm)
+	} else {
 		// Attempt to create the file with name filePath.  If it exists, fail.
 		outFile, err = os.OpenFile(fileName, os.O_CREATE|os.O_EXCL|os.O_WRONLY, os.ModePerm)
-	} else {
-		outFile, err = os.OpenFile(fileName, os.O_EXCL|os.O_WRONLY, os.ModePerm)
 	}
 	if err != nil {
 		return errors.Wrapf(err, "could not open file %q", fileName)