Don't use scratch space for registry node pull imports (#2846)

When importing via node container runtime cache, we always have the image handy locally. This manifests itself in the form of a bug where we loop over ```bash E0813 13:32:38.443088 1 data-processor.go:251] scratch space required and none found E0813 13:32:38.443102 1 importer.go:181] scratch space required and none found ``` On registry node pull imports where images are not raw Signed-off-by: Alex Kalenyuk <akalenyu@redhat.com> Co-authored-by: Alex Kalenyuk <akalenyu@redhat.com>
kubevirt · Aug 14, 2023 · 217c0c9 · 217c0c9
1 parent ac42b10
commit 217c0c9
Show file tree

Hide file tree

Showing 9 changed files with 61 additions and 15 deletions.
diff --git a/BUILD.bazel b/BUILD.bazel
@@ -65,6 +65,7 @@ container_bundle(
         "$(container_prefix)/vddk-test:$(container_tag)": "//tools/vddk-test:vddk-test-image",
         "$(container_prefix)/cdi-func-test-tinycore:$(container_tag)": "//tests:cdi-func-test-tinycore",
         "$(container_prefix)/cdi-func-test-imageio:$(container_tag)": "//tools/image-io:cdi-func-test-imageio-image",
+        "$(container_prefix)/cdi-func-test-cirros-qcow2:$(container_tag)": "//tests:cdi-func-test-cirros-qcow2",
     },
 )
 
@@ -83,6 +84,7 @@ container_bundle(
         "$(container_prefix)/vcenter-simulator:$(container_tag)": "//tools/vddk-test:vcenter-simulator",
         "$(container_prefix)/cdi-func-test-tinycore:$(container_tag)": "//tests:cdi-func-test-tinycore",
         "$(container_prefix)/cdi-func-test-imageio:$(container_tag)": "//tools/image-io:cdi-func-test-imageio-image",
+        "$(container_prefix)/cdi-func-test-cirros-qcow2:$(container_tag)": "//tests:cdi-func-test-cirros-qcow2",
     },
 )
 

diff --git a/pkg/common/common.go b/pkg/common/common.go
@@ -107,6 +107,8 @@ const (
 	ImporterDiskID = "IMPORTER_DISK_ID"
 	// ImporterUUID provides a constant to capture our env variable "IMPORTER_UUID"
 	ImporterUUID = "IMPORTER_UUID"
+	// ImporterPullMethod provides a constant to capture our env variable "IMPORTER_PULL_METHOD"
+	ImporterPullMethod = "IMPORTER_PULL_METHOD"
 	// ImporterReadyFile provides a constant to capture our env variable "IMPORTER_READY_FILE"
 	ImporterReadyFile = "IMPORTER_READY_FILE"
 	// ImporterDoneFile provides a constant to capture our env variable "IMPORTER_DONE_FILE"

diff --git a/pkg/controller/import-controller.go b/pkg/controller/import-controller.go
@@ -82,6 +82,7 @@ type importPodEnvVar struct {
 	certConfigMap      string
 	diskID             string
 	uuid               string
+	pullMethod         string
 	readyFile          string
 	doneFile           string
 	backingFile        string
@@ -982,6 +983,7 @@ func makeNodeImporterPodSpec(args *importerPodArgs) *corev1.Pod {
 
 	args.podEnvVar.source = cc.SourceHTTP
 	args.podEnvVar.ep = "http://localhost:8100/disk.img"
+	args.podEnvVar.pullMethod = string(cdiv1.RegistryPullNode)
 	args.podEnvVar.readyFile = "/shared/ready"
 	args.podEnvVar.doneFile = "/shared/done"
 	setImporterPodCommons(pod, args.podEnvVar, args.pvc, args.podResourceRequirements, args.imagePullSecrets)
@@ -1262,6 +1264,10 @@ func makeImportEnv(podEnvVar *importPodEnvVar, uid types.UID) []corev1.EnvVar {
 			Name:  common.ImporterUUID,
 			Value: podEnvVar.uuid,
 		},
+		{
+			Name:  common.ImporterPullMethod,
+			Value: podEnvVar.pullMethod,
+		},
 		{
 			Name:  common.ImporterReadyFile,
 			Value: podEnvVar.readyFile,

diff --git a/pkg/controller/import-controller_test.go b/pkg/controller/import-controller_test.go
@@ -1048,6 +1048,10 @@ func createImportTestEnv(podEnvVar *importPodEnvVar, uid string) []corev1.EnvVar
 			Name:  common.ImporterUUID,
 			Value: podEnvVar.uuid,
 		},
+		{
+			Name:  common.ImporterPullMethod,
+			Value: podEnvVar.pullMethod,
+		},
 		{
 			Name:  common.ImporterReadyFile,
 			Value: podEnvVar.readyFile,

diff --git a/pkg/image/qemu.go b/pkg/image/qemu.go
@@ -123,7 +123,7 @@ func convertToRaw(src, dest string, preallocate bool) error {
 			return qemuExecFunction(nil, reportProgress, "qemu-img", args...)
 		})
 	} else {
-		klog.V(3).Infof("Running qemu-img convert with args: %v", args)
+		klog.V(1).Infof("Running qemu-img with args: %v", args)
 		_, err = qemuExecFunction(nil, reportProgress, "qemu-img", args...)
 	}
 	if err != nil {
@@ -333,7 +333,7 @@ func addPreallocation(args []string, preallocationMethods [][]string, qemuFn fun
 		// For some subcommands (e.g. resize), preallocation optinos must come before other options
 		argsToTry := append([]string{args[0]}, preallocationMethod...)
 		argsToTry = append(argsToTry, args[1:]...)
-		klog.V(3).Infof("Attempting preallocation method, qemu-img convert args: %v", argsToTry)
+		klog.V(1).Infof("Attempting preallocation method, qemu-img args: %v", argsToTry)
 
 		output, err = qemuFn(argsToTry)
 		if err != nil && strings.Contains(string(output), "Unsupported preallocation mode") {

diff --git a/pkg/importer/http-datasource.go b/pkg/importer/http-datasource.go
@@ -133,6 +133,13 @@ func (hs *HTTPDataSource) Info() (ProcessingPhase, error) {
 	if !hs.readers.Convert {
 		return ProcessingPhaseTransferDataFile, nil
 	}
+	if pullMethod, _ := util.ParseEnvVar(common.ImporterPullMethod, false); pullMethod == string(cdiv1.RegistryPullNode) {
+		hs.url, _ = url.Parse(fmt.Sprintf("nbd+unix:///?socket=%s", nbdkitSocket))
+		if err = hs.n.StartNbdkit(hs.endpoint.String()); err != nil {
+			return ProcessingPhaseError, err
+		}
+		return ProcessingPhaseConvert, nil
+	}
 	// removing check for hs.brokenForQemuImg, and always assuming it is true
 	// revert once we are able to get nbdkit 1.35.8, which contains a fix for the
 	// slow download speed.

diff --git a/tests/BUILD.bazel b/tests/BUILD.bazel
@@ -142,3 +142,17 @@ container_image(
     tars = [":tinycore-tar"],
     visibility = ["//visibility:public"],
 )
+
+pkg_tar(
+    name = "cirros-qcow2-tar",
+    srcs = [":images/cirros-qcow2.img"],
+    owner = "107.107",
+    package_dir = "/disk",
+    strip_prefix = "/tests/images",
+)
+
+container_image(
+    name = "cdi-func-test-cirros-qcow2",
+    tars = [":cirros-qcow2-tar"],
+    visibility = ["//visibility:public"],
+)
diff --git a/tests/import_test.go b/tests/import_test.go
@@ -1094,18 +1094,19 @@ var _ = Describe("Preallocation", func() {
 	dvName := "import-dv"
 
 	var (
-		dataVolume          *cdiv1.DataVolume
-		err                 error
-		tinyCoreIsoURL      = func() string { return fmt.Sprintf(utils.TinyCoreIsoURL, f.CdiInstallNs) }
-		tinyCoreQcow2URL    = func() string { return fmt.Sprintf(utils.TinyCoreQcow2URL, f.CdiInstallNs) }
-		tinyCoreTarURL      = func() string { return fmt.Sprintf(utils.TarArchiveURL, f.CdiInstallNs) }
-		tinyCoreRegistryURL = func() string { return fmt.Sprintf(utils.TinyCoreIsoRegistryURL, f.CdiInstallNs) }
-		imageioURL          = func() string { return fmt.Sprintf(utils.ImageioURL, f.CdiInstallNs) }
-		vcenterURL          = func() string { return fmt.Sprintf(utils.VcenterURL, f.CdiInstallNs) }
-		config              *cdiv1.CDIConfig
-		origSpec            *cdiv1.CDIConfigSpec
-		trustedRegistryURL  = func() string { return fmt.Sprintf(utils.TrustedRegistryURL, f.DockerPrefix) }
-		trustedRegistryIS   = func() string { return fmt.Sprintf(utils.TrustedRegistryIS, f.DockerPrefix) }
+		dataVolume              *cdiv1.DataVolume
+		err                     error
+		tinyCoreIsoURL          = func() string { return fmt.Sprintf(utils.TinyCoreIsoURL, f.CdiInstallNs) }
+		tinyCoreQcow2URL        = func() string { return fmt.Sprintf(utils.TinyCoreQcow2URL, f.CdiInstallNs) }
+		tinyCoreTarURL          = func() string { return fmt.Sprintf(utils.TarArchiveURL, f.CdiInstallNs) }
+		tinyCoreRegistryURL     = func() string { return fmt.Sprintf(utils.TinyCoreIsoRegistryURL, f.CdiInstallNs) }
+		imageioURL              = func() string { return fmt.Sprintf(utils.ImageioURL, f.CdiInstallNs) }
+		vcenterURL              = func() string { return fmt.Sprintf(utils.VcenterURL, f.CdiInstallNs) }
+		config                  *cdiv1.CDIConfig
+		origSpec                *cdiv1.CDIConfigSpec
+		trustedRegistryURL      = func() string { return fmt.Sprintf(utils.TrustedRegistryURL, f.DockerPrefix) }
+		trustedRegistryURLQcow2 = func() string { return fmt.Sprintf(utils.TrustedRegistryURLQcow2, f.DockerPrefix) }
+		trustedRegistryIS       = func() string { return fmt.Sprintf(utils.TrustedRegistryIS, f.DockerPrefix) }
 	)
 
 	BeforeEach(func() {
@@ -1311,12 +1312,18 @@ var _ = Describe("Preallocation", func() {
 			dataVolume.Spec.Source.Registry.CertConfigMap = &cm
 			return dataVolume
 		}),
-		Entry("Registry node pull import", true, utils.TinyCoreMD5, utils.DefaultImagePath, func() *cdiv1.DataVolume {
+		Entry("Registry node pull import raw", true, utils.TinyCoreMD5, utils.DefaultImagePath, func() *cdiv1.DataVolume {
 			pullMethod := cdiv1.RegistryPullNode
 			dataVolume = utils.NewDataVolumeWithRegistryImport("import-dv", "100Mi", trustedRegistryURL())
 			dataVolume.Spec.Source.Registry.PullMethod = &pullMethod
 			return dataVolume
 		}),
+		Entry("Registry node pull import qcow2", true, utils.CirrosMD5, utils.DefaultImagePath, func() *cdiv1.DataVolume {
+			pullMethod := cdiv1.RegistryPullNode
+			dataVolume = utils.NewDataVolumeWithRegistryImport("import-dv", "100Mi", trustedRegistryURLQcow2())
+			dataVolume.Spec.Source.Registry.PullMethod = &pullMethod
+			return dataVolume
+		}),
 		Entry("Registry ImageStream-wannabe node pull import", true, utils.TinyCoreMD5, utils.DefaultImagePath, func() *cdiv1.DataVolume {
 			pullMethod := cdiv1.RegistryPullNode
 			imageStreamWannabe := trustedRegistryIS()

diff --git a/tests/utils/datavolume.go b/tests/utils/datavolume.go
@@ -89,13 +89,17 @@ const (
 	VcenterURL = "https://vcenter.%s:8989/sdk"
 	// TrustedRegistryURL provides the base path to trusted registry test url for the tinycore.iso image wrapped in docker container
 	TrustedRegistryURL = "docker://%s/cdi-func-test-tinycore"
+	// TrustedRegistryURLQcow2 provides the base path to trusted registry test url for the cirros qcow2 image wrapped in docker container
+	TrustedRegistryURLQcow2 = "docker://%s/cdi-func-test-cirros-qcow2"
 	// TrustedRegistryIS provides the base path to trusted registry test fake imagestream for the tinycore.iso image wrapped in docker container
 	TrustedRegistryIS = "%s/cdi-func-test-tinycore"
 
 	// MD5PrefixSize is the number of bytes used by the MD5 constants below
 	MD5PrefixSize = int64(100000)
 	// TinyCoreMD5 is the MD5 hash of first 100k bytes of tinyCore image
 	TinyCoreMD5 = "3710416a680523c7d07538cb1026c60c"
+	// CirrosMD5 is the MD5 hash of first 100k bytes of cirros image
+	CirrosMD5 = "91150be031835ccfac458744da57d4f6"
 	// TinyCoreTarMD5 is the MD5 hash of first 100k bytes of tinyCore tar image
 	TinyCoreTarMD5 = "aec1a39d753b4b7cc81ee02bc625a342"
 	// ImageioMD5 is the MD5 hash of first 100k bytes of imageio image