Merge pull request #640 from awels/bugfix_cdiimporter_registry-b

BugFix: cdi importer fails to import from registry when run in unpriv…

cmd/cdi-importer/importer.go

@@ -50,6 +50,12 @@ func main() {
 	contentType, _ := util.ParseEnvVar(common.ImporterContentType, false)
 	imageSize, _ := util.ParseEnvVar(common.ImporterImageSize, false)
 
+	//Registry import currently support only kubevirt content type
+	if contentType != string(cdiv1.DataVolumeKubeVirt) && source == controller.SourceRegistry {
+		glog.Errorf("Unsupported content type %s when importing from registry", contentType)
+		os.Exit(1)
+	}
+
 	dest := common.ImporterWritePath
 	if contentType == string(cdiv1.DataVolumeArchive) || source == controller.SourceRegistry {
 		dest = common.ImporterVolumePath

hack/build/docker/cdi-func-test-registry-populate/populate-registry.sh

@@ -32,6 +32,7 @@ function health {
 #Convert all images to docker build consumable format
 DIR="-dir"
 DOCKERFILE="Dockerfile"
+VMIMAGEFILENAME="disk.img"
 
 function prepareImages {
    images_in=$1
@@ -45,12 +46,11 @@ function prepareImages {
 
    for FILENAME in $(ls); do
         mkdir -p $FILENAME$DIR
-        cp  $FILENAME $FILENAME$DIR
-
+        cp  $FILENAME $FILENAME$DIR"/"$VMIMAGEFILENAME
         FILE=$FILENAME$DIR"/"$DOCKERFILE
         /bin/cat  >$FILE <<-EOF
                 FROM scratch
-                ADD / $FILENAME
+                COPY $VMIMAGEFILENAME /
 EOF
 
         rm $FILENAME

pkg/image/skopeo.go

@@ -77,25 +77,32 @@ func (o *skopeoOperations) CopyImage(url, dest, accessKey, secKey string) error
 }
 
 // CopyRegistryImage download image from registry with skopeo
-func CopyRegistryImage(url, dest, accessKey, secKey string) error {
+func CopyRegistryImage(url, dest, destFile, accessKey, secKey string) error {
 	skopeoDest := "dir:" + dest + dataTmpDir
 	err := SkopeoInterface.CopyImage(url, skopeoDest, accessKey, secKey)
 	if err != nil {
 		os.RemoveAll(dest + dataTmpDir)
 		return errors.Wrap(err, "Failed to download from registry")
 	}
-	err = extractImageLayers(dest)
+	err = extractImageLayers(dest, destFile)
 	if err != nil {
 		return errors.Wrap(err, "Failed to extract image layers")
 	}
 
+	//If a specifc file was requested verify it exists, if not - fail
+	if len(destFile) > 0 {
+		if _, err = os.Stat(filepath.Join(dest, destFile)); err != nil {
+			glog.Errorf("Failed to find VM disk image file in the container image")
+			err = errors.New("Failed to find VM disk image file in the container image")
+		}
+	}
 	// Clean temp folder
 	os.RemoveAll(dest + dataTmpDir)
 
 	return err
 }
 
-var extractImageLayers = func(dest string) error {
+var extractImageLayers = func(dest string, arg ...string) error {
 	glog.V(1).Infof("extracting image layers to %q\n", dest)
 	// Parse manifest file
 	manifest, err := getImageManifest(dest + dataTmpDir)
@@ -120,9 +127,16 @@ var extractImageLayers = func(dest string) error {
 		layer := strings.TrimPrefix(layerID, "sha256:")
 		filePath := filepath.Join(dest, dataTmpDir, layer)
 
-		if err := util.UnArchiveLocalTar(filePath, dest, "z"); err != nil {
-			return errors.Wrap(err, "could not extract layer tar")
+		//prepend z option to the beggining of untar arguments
+		args := append([]string{"z"}, arg...)
+
+		if err := util.UnArchiveLocalTar(filePath, dest, args...); err != nil {
+			//ignore errors if specific file extract was requested - we validate whether the file was extracted at the end of the sequence
+			if len(arg) == 0 {
+				return errors.Wrap(err, "could not extract layer tar")
+			}
 		}
+
 		err = cleanWhiteoutFiles(dest)
 	}
 	return err

pkg/image/skopeo_test.go

@@ -52,8 +52,8 @@ var _ = Describe("Registry Importer", func() {
 			}
 		})
 	},
-		table.Entry("copy success", mockExecFunction("", ""), "", func() error { return CopyRegistryImage(source, dest, "", "") }),
-		table.Entry("copy failure", mockExecFunction("", "exit status 1"), "exit status 1", func() error { return CopyRegistryImage(source, dest, "", "") }),
+		table.Entry("copy success", mockExecFunction("", ""), "", func() error { return CopyRegistryImage(source, dest, "", "", "") }),
+		table.Entry("copy failure", mockExecFunction("", "Failed to find VM disk image file in the container image"), "Failed to find VM disk image file in the container image", func() error { return CopyRegistryImage(source, dest, "", "", "") }),
 	)
 
 })
@@ -168,7 +168,7 @@ func replaceSkopeoFunctions(mockSkopeoExecFunction execFunctionType, f func()) {
 	f()
 }
 
-func mockExtractImageLayers(dest string) error {
+func mockExtractImageLayers(dest string, arg ...string) error {
 	return nil
 }
 

pkg/importer/dataStream.go

@@ -294,7 +294,7 @@ func CopyData(dso *DataStreamOptions) error {
 	switch dso.Source {
 	case controller.SourceRegistry:
 		glog.V(1).Infof("using skopeo to copy from registry")
-		return image.CopyRegistryImage(dso.Endpoint, dso.Dest, dso.AccessKey, dso.SecKey)
+		return image.CopyRegistryImage(dso.Endpoint, dso.Dest, common.DiskImageName, dso.AccessKey, dso.SecKey)
 	default:
 		ds, err := NewDataStream(dso)
 		if err != nil {

pkg/importer/registry_test.go

@@ -16,7 +16,7 @@ import (
 )
 
 var (
-	imageFile = filepath.Join(imageDir, "docker-image.tar")
+	imageFile = filepath.Join(imageDir, "diskimage.tar")
 	imageData = filepath.Join(imageDir, "data")
 )
 

pkg/util/util.go

@@ -95,8 +95,15 @@ func MinQuantity(availableSpace, imageSize *resource.Quantity) resource.Quantity
 // using the specified io.Reader to the specified destination.
 func UnArchiveTar(reader io.Reader, destDir string, arg ...string) error {
 	glog.V(1).Infof("begin untar...\n")
-	args := fmt.Sprintf("-%s%s", strings.Join(arg, ""), "xvC")
-	untar := exec.Command("/usr/bin/tar", args, destDir)
+
+	var tarOptions string
+	var args = arg
+	if len(arg) > 0 {
+		tarOptions = arg[0]
+		args = arg[1:]
+	}
+	options := fmt.Sprintf("-%s%s", tarOptions, "xvC")
+	untar := exec.Command("/usr/bin/tar", options, destDir, strings.Join(args, ""))
 	untar.Stdin = reader
 	var errBuf bytes.Buffer
 	untar.Stderr = &errBuf

tests/transport_test.go

@@ -24,6 +24,7 @@ var _ = Describe("Transport Tests", func() {
 	const (
 		secretPrefix   = "transport-e2e-sec"
 		targetFile     = "tinyCore.iso"
+		targetImage    = "disk.img"
 		targetQCOWFile = "tinyCore.qcow2"
 		sizeCheckPod   = "size-checker"
 	)
@@ -108,12 +109,6 @@ var _ = Describe("Transport Tests", func() {
 				exitCode, _ := f.ExecShellInPod(pod.Name, ns, command)
 				// A 0 exitCode should indicate that $expSize == $haveSize
 				Expect(strconv.Atoi(exitCode)).To(BeZero())
-			case controller.SourceRegistry:
-				binFile := "/pvc/bin/" + file
-				command := fmt.Sprintf("[ -e %s ]; echo $?", binFile)
-				exitCode, _ := f.ExecShellInPod(pod.Name, ns, command)
-				// A 0 exitCode should indicate that the bin file exists
-				Expect(strconv.Atoi(exitCode)).To(BeZero())
 			}
 		} else {
 			By("Verifying PVC is empty")
@@ -123,14 +118,14 @@ var _ = Describe("Transport Tests", func() {
 
 	httpNoAuthEp := fmt.Sprintf("http://%s:%d", utils.FileHostName+"."+utils.FileHostNs, utils.HTTPNoAuthPort)
 	httpAuthEp := fmt.Sprintf("http://%s:%d", utils.FileHostName+"."+utils.FileHostNs, utils.HTTPAuthPort)
-	registryNoAuthEp := fmt.Sprintf("docker://%s", "docker.io")
+	registryNoAuthEp := fmt.Sprintf("docker://%s", utils.RegistryHostName+"."+utils.RegistryHostNs)
 	DescribeTable("Transport Test Table", it,
 		Entry("should connect to http endpoint without credentials", httpNoAuthEp, targetFile, "", "", controller.SourceHTTP, true),
 		Entry("should connect to http endpoint with credentials", httpAuthEp, targetFile, utils.AccessKeyValue, utils.SecretKeyValue, controller.SourceHTTP, true),
 		Entry("should not connect to http endpoint with invalid credentials", httpAuthEp, targetFile, "gopats", "bradyisthegoat", controller.SourceHTTP, false),
 		Entry("should connect to QCOW http endpoint without credentials", httpNoAuthEp, targetQCOWFile, "", "", controller.SourceHTTP, true),
 		Entry("should connect to QCOW http endpoint with credentials", httpAuthEp, targetQCOWFile, utils.AccessKeyValue, utils.SecretKeyValue, controller.SourceHTTP, true),
-		Entry("should connect to registry endpoint without credentials", registryNoAuthEp, "registry", "", "", controller.SourceRegistry, true),
+		Entry("should connect to registry endpoint without credentials", registryNoAuthEp, targetImage, "", "", controller.SourceRegistry, true),
 		//		Entry("should not connect to registry endpoint with invalid credentials", registryNoAuthEp, "registry", "gopats", "bradyisthegoat", controller.SourceRegistry, false),
 	)
 })

tests/utils/common.go

@@ -2,6 +2,10 @@ package utils
 
 // cdi-file-host pod/service relative values
 const (
+	//RegistryHostName provides a deploymnet and service name for registry
+	RegistryHostName = "cdi-docker-registry-host"
+	// RegistryHostNs provides a deployment ans service namespace for tests
+	RegistryHostNs = "cdi"
 	// FileHostName provides a deployment and service name for tests
 	FileHostName = "cdi-file-host"
 	// FileHostNs provides a deployment ans service namespace for tests

tests/utils/datavolume.go

@@ -25,7 +25,7 @@ const (
 	// TinyCoreIsoURL provides a test url for the tineyCore iso image
 	TinyCoreIsoURL = "http://cdi-file-host.cdi/tinyCore.iso"
 	// TinyCoreIsoRegistryURL provides a test url for the tineyCore iso image stored in docker registry
-	TinyCoreIsoRegistryURL = "docker://cdi-docker-registry-host.cdi/tinycore.qcow2"
+	TinyCoreIsoRegistryURL = "docker://cdi-docker-registry-host.cdi/disk.img"
 )
 
 // CreateDataVolumeFromDefinition is used by tests to create a testable Data Volume