Skip to content

Commit

Permalink
Always use scratchspace when importing and converting (#2832)
Browse files Browse the repository at this point in the history
* Remove older nbdkit

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

* When converting always use scratch space importing instead
of ndbkit. Once we are able to get nbdkit 1.35.8 or newer
we can revert this change since that will include improvements
to the downloading speed.

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

* Disable metrics test for import because straight import doesn't
return total, and this means the metrics are disabled.

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

* Fix broken functional tests

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

* Address review comments

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

* Additional review comments.
Fixed functional test that was not doing the right
thing while running the test.

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

* Always set preallocation on block devices when directly
writing to the device

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

---------

Signed-off-by: Alexander Wels <awels@redhat.com>
  • Loading branch information
awels committed Aug 11, 2023
1 parent b7800d6 commit d7c5a68
Show file tree
Hide file tree
Showing 9 changed files with 33 additions and 217 deletions.
12 changes: 0 additions & 12 deletions WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -2498,18 +2498,6 @@ rpm(
],
)

rpm(
name = "nbdkit-server-0__1.30.1-2.el9.aarch64",
sha256 = "66a465ea8d131491fb581082ec7c5044c3918e6d2016db361796cd76cb4286ee",
urls = ["https://storage.googleapis.com/builddeps/66a465ea8d131491fb581082ec7c5044c3918e6d2016db361796cd76cb4286ee"],
)

rpm(
name = "nbdkit-server-0__1.30.1-2.el9.x86_64",
sha256 = "3e677a5613e9fea1bc27503e282222b7131db4f177d9222948313c912c57b610",
urls = ["https://storage.googleapis.com/builddeps/3e677a5613e9fea1bc27503e282222b7131db4f177d9222948313c912c57b610"],
)

rpm(
name = "nbdkit-server-0__1.34.1-1.el9.aarch64",
sha256 = "92c9b1ea142bfae32266ea15522f82e12ed68bbd03dd6eca09f8e38b36c16332",
Expand Down
8 changes: 4 additions & 4 deletions pkg/controller/import-controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -601,19 +601,19 @@ func (r *ImportReconciler) createImportEnvVar(pvc *corev1.PersistentVolumeClaim)

var field string
if field, err = GetImportProxyConfig(cdiConfig, common.ImportProxyHTTP); err != nil {
r.log.V(3).Info("no proxy http url will be supplied:", err.Error())
r.log.V(3).Info("no proxy http url will be supplied:", "error", err.Error())
}
podEnvVar.httpProxy = field
if field, err = GetImportProxyConfig(cdiConfig, common.ImportProxyHTTPS); err != nil {
r.log.V(3).Info("no proxy https url will be supplied:", err.Error())
r.log.V(3).Info("no proxy https url will be supplied:", "error", err.Error())
}
podEnvVar.httpsProxy = field
if field, err = GetImportProxyConfig(cdiConfig, common.ImportProxyNoProxy); err != nil {
r.log.V(3).Info("the noProxy field will not be supplied:", err.Error())
r.log.V(3).Info("the noProxy field will not be supplied:", "error", err.Error())
}
podEnvVar.noProxy = field
if field, err = GetImportProxyConfig(cdiConfig, common.ImportProxyConfigMapName); err != nil {
r.log.V(3).Info("no proxy CA certiticate will be supplied:", err.Error())
r.log.V(3).Info("no proxy CA certiticate will be supplied:", "error", err.Error())
}
podEnvVar.certConfigMapProxy = field
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/importer/data-processor.go
Original file line number Diff line number Diff line change
Expand Up @@ -307,8 +307,8 @@ func (dp *DataProcessor) resize() (ProcessingPhase, error) {
if err != nil {
return ProcessingPhaseError, err
}
dp.preallocationApplied = dp.preallocation
}
dp.preallocationApplied = dp.preallocation
if dp.dataFile != "" && !isBlockDev {
// Change permissions to 0660
err := os.Chmod(dp.dataFile, 0660)
Expand Down
19 changes: 6 additions & 13 deletions pkg/importer/http-datasource.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,20 +130,13 @@ func (hs *HTTPDataSource) Info() (ProcessingPhase, error) {
if hs.contentType == cdiv1.DataVolumeArchive {
return ProcessingPhaseTransferDataDir, nil
}
if hs.readers.Convert {
if hs.brokenForQemuImg || hs.readers.Archived || hs.customCA != "" {
return ProcessingPhaseTransferScratch, nil
}
} else {
if hs.readers.Archived || hs.customCA != "" {
return ProcessingPhaseTransferDataFile, nil
}
}
hs.url, _ = url.Parse(fmt.Sprintf("nbd+unix:///?socket=%s", nbdkitSocket))
if err = hs.n.StartNbdkit(hs.endpoint.String()); err != nil {
return ProcessingPhaseError, err
if !hs.readers.Convert {
return ProcessingPhaseTransferDataFile, nil
}
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.
return ProcessingPhaseTransferScratch, nil
}

// Transfer is called to transfer the data from the source to a scratch location.
Expand Down
7 changes: 1 addition & 6 deletions pkg/importer/http-datasource_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,16 +94,11 @@ var _ = Describe("Http data source", func() {
if !wantErr {
Expect(err).NotTo(HaveOccurred())
Expect(expectedPhase).To(Equal(newPhase))
if newPhase == ProcessingPhaseConvert {
expectURL, err := url.Parse("nbd+unix:///?socket=/tmp/nbdkit.sock")
Expect(err).NotTo(HaveOccurred())
Expect(expectURL).To(Equal(dp.GetURL()))
}
} else {
Expect(err).To(HaveOccurred())
}
},
table.Entry("return Convert phase ", cirrosFileName, cdiv1.DataVolumeKubeVirt, ProcessingPhaseConvert, cirrosData, false),
table.Entry("return TransferScratch phase ", cirrosFileName, cdiv1.DataVolumeKubeVirt, ProcessingPhaseTransferScratch, cirrosData, false),
table.Entry("return TransferTarget with archive content type but not archive endpoint ", cirrosFileName, cdiv1.DataVolumeArchive, ProcessingPhaseTransferDataDir, cirrosData, false),
table.Entry("return TransferTarget with archive content type and archive endpoint ", diskimageTarFileName, cdiv1.DataVolumeArchive, ProcessingPhaseTransferDataDir, diskimageArchiveData, false),
)
Expand Down
160 changes: 0 additions & 160 deletions rpm/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,85 +1,6 @@
load("@bazeldnf//:deps.bzl", "rpm", "rpmtree", "tar2files")
load("@bazeldnf//:def.bzl", "bazeldnf")

rpmtree(
name = "cdi_importer_aarch64",
rpms = [
"@acl-0__2.3.1-3.el9.aarch64//rpm",
"@alternatives-0__1.20-2.el9.aarch64//rpm",
"@basesystem-0__11-13.el9.aarch64//rpm",
"@bash-0__5.1.8-4.el9.aarch64//rpm",
"@ca-certificates-0__2020.2.50-94.el9.aarch64//rpm",
"@centos-gpg-keys-0__9.0-12.el9.aarch64//rpm",
"@centos-stream-release-0__9.0-12.el9.aarch64//rpm",
"@centos-stream-repos-0__9.0-12.el9.aarch64//rpm",
"@coreutils-single-0__8.32-31.el9.aarch64//rpm",
"@crypto-policies-0__20220223-1.git5203b41.el9.aarch64//rpm",
"@curl-0__7.76.1-14.el9.aarch64//rpm",
"@filesystem-0__3.16-2.el9.aarch64//rpm",
"@gawk-0__5.1.0-6.el9.aarch64//rpm",
"@glib2-0__2.68.4-5.el9.aarch64//rpm",
"@glibc-0__2.34-29.el9.aarch64//rpm",
"@glibc-common-0__2.34-29.el9.aarch64//rpm",
"@glibc-minimal-langpack-0__2.34-29.el9.aarch64//rpm",
"@gmp-1__6.2.0-10.el9.aarch64//rpm",
"@gnutls-0__3.7.3-9.el9.aarch64//rpm",
"@grep-0__3.6-5.el9.aarch64//rpm",
"@keyutils-libs-0__1.6.1-4.el9.aarch64//rpm",
"@krb5-libs-0__1.19.1-15.el9.aarch64//rpm",
"@libacl-0__2.3.1-3.el9.aarch64//rpm",
"@libaio-0__0.3.111-13.el9.aarch64//rpm",
"@libattr-0__2.5.1-3.el9.aarch64//rpm",
"@libblkid-0__2.37.2-1.el9.aarch64//rpm",
"@libcap-0__2.48-8.el9.aarch64//rpm",
"@libcom_err-0__1.46.5-2.el9.aarch64//rpm",
"@libcurl-minimal-0__7.76.1-14.el9.aarch64//rpm",
"@libffi-0__3.4.2-7.el9.aarch64//rpm",
"@libgcc-0__11.2.1-9.4.el9.aarch64//rpm",
"@libgcrypt-0__1.10.0-2.el9.aarch64//rpm",
"@libgpg-error-0__1.42-5.el9.aarch64//rpm",
"@libidn2-0__2.3.0-7.el9.aarch64//rpm",
"@libmount-0__2.37.2-1.el9.aarch64//rpm",
"@libnghttp2-0__1.43.0-5.el9.aarch64//rpm",
"@libselinux-0__3.3-2.el9.aarch64//rpm",
"@libsepol-0__3.3-2.el9.aarch64//rpm",
"@libsigsegv-0__2.13-4.el9.aarch64//rpm",
"@libsmartcols-0__2.37.2-1.el9.aarch64//rpm",
"@libtasn1-0__4.16.0-7.el9.aarch64//rpm",
"@libunistring-0__0.9.10-15.el9.aarch64//rpm",
"@libuuid-0__2.37.2-1.el9.aarch64//rpm",
"@libverto-0__0.3.2-3.el9.aarch64//rpm",
"@libxcrypt-0__4.4.18-3.el9.aarch64//rpm",
"@libzstd-0__1.5.1-2.el9.aarch64//rpm",
"@lz4-libs-0__1.9.3-5.el9.aarch64//rpm",
"@mpfr-0__4.1.0-7.el9.aarch64//rpm",
"@nbdkit-basic-filters-0__1.30.1-2.el9.aarch64//rpm",
"@nbdkit-curl-plugin-0__1.30.1-2.el9.aarch64//rpm",
"@nbdkit-gzip-filter-0__1.30.1-2.el9.aarch64//rpm",
"@nbdkit-server-0__1.30.1-2.el9.aarch64//rpm",
"@nbdkit-xz-filter-0__1.30.1-2.el9.aarch64//rpm",
"@ncurses-base-0__6.2-8.20210508.el9.aarch64//rpm",
"@ncurses-libs-0__6.2-8.20210508.el9.aarch64//rpm",
"@nettle-0__3.7.3-2.el9.aarch64//rpm",
"@openssl-libs-1__3.0.1-18.el9.aarch64//rpm",
"@p11-kit-0__0.24.1-2.el9.aarch64//rpm",
"@p11-kit-trust-0__0.24.1-2.el9.aarch64//rpm",
"@pcre-0__8.44-3.el9.3.aarch64//rpm",
"@pcre2-0__10.37-3.el9.1.aarch64//rpm",
"@pcre2-syntax-0__10.37-3.el9.1.aarch64//rpm",
"@qemu-img-17__6.2.0-12.el9.aarch64//rpm",
"@readline-0__8.1-4.el9.aarch64//rpm",
"@sed-0__4.8-9.el9.aarch64//rpm",
"@setup-0__2.13.7-6.el9.aarch64//rpm",
"@systemd-libs-0__250-4.el9.aarch64//rpm",
"@tzdata-0__2021e-1.el9.aarch64//rpm",
"@util-linux-core-0__2.37.2-1.el9.aarch64//rpm",
"@vim-minimal-2__8.2.2637-15.el9.aarch64//rpm",
"@xz-libs-0__5.2.5-7.el9.aarch64//rpm",
"@zlib-0__1.2.11-32.el9.aarch64//rpm",
],
visibility = ["//visibility:public"],
)

rpmtree(
name = "cdi_importer_base_aarch64",
rpms = [
Expand Down Expand Up @@ -253,87 +174,6 @@ rpmtree(
visibility = ["//visibility:public"],
)

rpmtree(
name = "cdi_importer_x86_64",
rpms = [
"@acl-0__2.3.1-3.el9.x86_64//rpm",
"@alternatives-0__1.20-2.el9.x86_64//rpm",
"@basesystem-0__11-13.el9.x86_64//rpm",
"@bash-0__5.1.8-4.el9.x86_64//rpm",
"@ca-certificates-0__2020.2.50-94.el9.x86_64//rpm",
"@centos-gpg-keys-0__9.0-12.el9.x86_64//rpm",
"@centos-stream-release-0__9.0-12.el9.x86_64//rpm",
"@centos-stream-repos-0__9.0-12.el9.x86_64//rpm",
"@coreutils-single-0__8.32-31.el9.x86_64//rpm",
"@crypto-policies-0__20220223-1.git5203b41.el9.x86_64//rpm",
"@curl-0__7.76.1-14.el9.x86_64//rpm",
"@filesystem-0__3.16-2.el9.x86_64//rpm",
"@gawk-0__5.1.0-6.el9.x86_64//rpm",
"@glib2-0__2.68.4-5.el9.x86_64//rpm",
"@glibc-0__2.34-29.el9.x86_64//rpm",
"@glibc-common-0__2.34-29.el9.x86_64//rpm",
"@glibc-minimal-langpack-0__2.34-29.el9.x86_64//rpm",
"@gmp-1__6.2.0-10.el9.x86_64//rpm",
"@gnutls-0__3.7.3-9.el9.x86_64//rpm",
"@grep-0__3.6-5.el9.x86_64//rpm",
"@keyutils-libs-0__1.6.1-4.el9.x86_64//rpm",
"@krb5-libs-0__1.19.1-15.el9.x86_64//rpm",
"@libacl-0__2.3.1-3.el9.x86_64//rpm",
"@libaio-0__0.3.111-13.el9.x86_64//rpm",
"@libattr-0__2.5.1-3.el9.x86_64//rpm",
"@libblkid-0__2.37.2-1.el9.x86_64//rpm",
"@libcap-0__2.48-8.el9.x86_64//rpm",
"@libcom_err-0__1.46.5-2.el9.x86_64//rpm",
"@libcurl-minimal-0__7.76.1-14.el9.x86_64//rpm",
"@libffi-0__3.4.2-7.el9.x86_64//rpm",
"@libgcc-0__11.2.1-9.4.el9.x86_64//rpm",
"@libgcrypt-0__1.10.0-2.el9.x86_64//rpm",
"@libgpg-error-0__1.42-5.el9.x86_64//rpm",
"@libidn2-0__2.3.0-7.el9.x86_64//rpm",
"@libmount-0__2.37.2-1.el9.x86_64//rpm",
"@libnghttp2-0__1.43.0-5.el9.x86_64//rpm",
"@libselinux-0__3.3-2.el9.x86_64//rpm",
"@libsepol-0__3.3-2.el9.x86_64//rpm",
"@libsigsegv-0__2.13-4.el9.x86_64//rpm",
"@libsmartcols-0__2.37.2-1.el9.x86_64//rpm",
"@libtasn1-0__4.16.0-7.el9.x86_64//rpm",
"@libunistring-0__0.9.10-15.el9.x86_64//rpm",
"@libuuid-0__2.37.2-1.el9.x86_64//rpm",
"@libverto-0__0.3.2-3.el9.x86_64//rpm",
"@libxcrypt-0__4.4.18-3.el9.x86_64//rpm",
"@libxcrypt-compat-0__4.4.18-3.el9.x86_64//rpm",
"@libzstd-0__1.5.1-2.el9.x86_64//rpm",
"@lz4-libs-0__1.9.3-5.el9.x86_64//rpm",
"@mpfr-0__4.1.0-7.el9.x86_64//rpm",
"@nbdkit-basic-filters-0__1.30.1-2.el9.x86_64//rpm",
"@nbdkit-curl-plugin-0__1.30.1-2.el9.x86_64//rpm",
"@nbdkit-gzip-filter-0__1.30.1-2.el9.x86_64//rpm",
"@nbdkit-server-0__1.30.1-2.el9.x86_64//rpm",
"@nbdkit-vddk-plugin-0__1.30.1-2.el9.x86_64//rpm",
"@nbdkit-xz-filter-0__1.30.1-2.el9.x86_64//rpm",
"@ncurses-base-0__6.2-8.20210508.el9.x86_64//rpm",
"@ncurses-libs-0__6.2-8.20210508.el9.x86_64//rpm",
"@nettle-0__3.7.3-2.el9.x86_64//rpm",
"@openssl-libs-1__3.0.1-18.el9.x86_64//rpm",
"@p11-kit-0__0.24.1-2.el9.x86_64//rpm",
"@p11-kit-trust-0__0.24.1-2.el9.x86_64//rpm",
"@pcre-0__8.44-3.el9.3.x86_64//rpm",
"@pcre2-0__10.37-3.el9.1.x86_64//rpm",
"@pcre2-syntax-0__10.37-3.el9.1.x86_64//rpm",
"@qemu-img-17__6.2.0-12.el9.x86_64//rpm",
"@readline-0__8.1-4.el9.x86_64//rpm",
"@sed-0__4.8-9.el9.x86_64//rpm",
"@setup-0__2.13.7-6.el9.x86_64//rpm",
"@systemd-libs-0__250-4.el9.x86_64//rpm",
"@tzdata-0__2021e-1.el9.x86_64//rpm",
"@util-linux-core-0__2.37.2-1.el9.x86_64//rpm",
"@vim-minimal-2__8.2.2637-15.el9.x86_64//rpm",
"@xz-libs-0__5.2.5-7.el9.x86_64//rpm",
"@zlib-0__1.2.11-32.el9.x86_64//rpm",
],
visibility = ["//visibility:public"],
)

rpmtree(
name = "cdi_uploadserver_aarch64",
rpms = [
Expand Down
29 changes: 13 additions & 16 deletions tests/datavolume_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1717,25 +1717,25 @@ var _ = Describe("[vendor:cnv-qe@redhat.com][level:component]DataVolume tests",

var sourcePod *v1.Pod
var uploadPod *v1.Pod
Eventually(func() bool {
Eventually(func() error {
uploadPod, err = utils.FindPodByPrefix(f.K8sClient, dataVolume.Namespace, common.UploadPodName, common.CDILabelSelector)
return err == nil
}, timeout, pollingInterval).Should(BeTrue())
return err
}, timeout, pollingInterval).Should(BeNil())
verifyAnnotations(uploadPod)
// Remove non existent network so upload pod succeeds and clone can continue (some envs like OpenShift check network validity)
Eventually(func() bool {
Eventually(func() error {
uploadPod, err = utils.FindPodByPrefix(f.K8sClient, dataVolume.Namespace, "cdi-upload", common.CDILabelSelector)
if err != nil {
return false
return err
}
delete(uploadPod.Annotations, controller.AnnPodNetwork)
_, err = f.K8sClient.CoreV1().Pods(dataVolume.Namespace).Update(context.TODO(), uploadPod, metav1.UpdateOptions{})
return err == nil
}, 60*time.Second, 2*time.Second).Should(BeTrue())
Eventually(func() bool {
return err
}, 60*time.Second, 2*time.Second).Should(BeNil())
Eventually(func() error {
sourcePod, err = utils.FindPodBySuffix(f.K8sClient, dataVolume.Namespace, "source-pod", common.CDILabelSelector)
return err == nil
}, timeout, pollingInterval).Should(BeTrue())
return err
}, timeout, pollingInterval).Should(BeNil())
verifyAnnotations(sourcePod)
})
})
Expand Down Expand Up @@ -2859,14 +2859,11 @@ var _ = Describe("[vendor:cnv-qe@redhat.com][level:component]DataVolume tests",
Expect(err).ToNot(HaveOccurred())

// here we want to have more than 0, to be sure it started
progressRegExp := regexp.MustCompile(`[1-9]\d{0,2}\.?\d{1,2}%`)
Eventually(func() bool {
Eventually(func() string {
dv, err := f.CdiClient.CdiV1beta1().DataVolumes(f.Namespace.Name).Get(context.TODO(), dataVolume.Name, metav1.GetOptions{})
Expect(err).ToNot(HaveOccurred())
progress := dv.Status.Progress
fmt.Fprintf(GinkgoWriter, "INFO: current progress:%v, matches:%v\n", progress, progressRegExp.MatchString(string(progress)))
return progressRegExp.MatchString(string(progress))
}, timeout, pollingInterval).Should(BeTrue())
return string(dv.Status.Progress)
}, timeout, pollingInterval).Should(Equal("N/A"))

By("Remove source image file & kill http container to force restart")
fileHostPod, err := utils.FindPodByPrefix(f.K8sClient, f.CdiInstallNs, utils.FileHostName, "name="+utils.FileHostName)
Expand Down
8 changes: 4 additions & 4 deletions tests/import_proxy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ var _ = Describe("Import Proxy tests", func() {
imgName: tinyCoreQcow2,
isHTTPS: false,
withBasicAuth: false,
userAgent: nbdKitUserAgent,
userAgent: golangUserAgent,
expected: BeTrue}),
Entry("succeed creating iso import dv with a proxied server (http)", importProxyTestArguments{
name: "dv-import-http-proxy",
Expand All @@ -205,7 +205,7 @@ var _ = Describe("Import Proxy tests", func() {
imgName: tinyCoreIso,
isHTTPS: false,
withBasicAuth: false,
userAgent: nbdKitUserAgent,
userAgent: golangUserAgent,
expected: BeTrue}),
Entry("succeed creating iso.gz import dv with a proxied server (http)", importProxyTestArguments{
name: "dv-import-http-proxy",
Expand All @@ -223,7 +223,7 @@ var _ = Describe("Import Proxy tests", func() {
imgName: tinyCoreQcow2,
isHTTPS: false,
withBasicAuth: true,
userAgent: nbdKitUserAgent,
userAgent: golangUserAgent,
expected: BeTrue}),
Entry("succeed creating iso import dv with a proxied server (http) with basic autentication", importProxyTestArguments{
name: "dv-import-http-proxy-auth",
Expand All @@ -232,7 +232,7 @@ var _ = Describe("Import Proxy tests", func() {
imgName: tinyCoreIso,
isHTTPS: false,
withBasicAuth: true,
userAgent: nbdKitUserAgent,
userAgent: golangUserAgent,
expected: BeTrue}),
Entry("succeed creating iso.gz import dv with a proxied server (http) with basic autentication", importProxyTestArguments{
name: "dv-import-http-proxy-auth",
Expand Down
5 changes: 4 additions & 1 deletion tests/import_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -455,7 +455,10 @@ var _ = Describe("[rfe_id:1118][crit:high][vendor:cnv-qe@redhat.com][level:compo
afterCMD(portForwardCmd)
})

It("[test_id:4970]Import pod should have prometheus stats available while importing", func() {
// Skipping this test until we can get progress information again. What happens is that the go
// http client cannot determine the total size, and thus the prometheus endpoint is not initialized
// This causes this test to now fail because the endpoint is not there, skipping for now.
PIt("[test_id:4970]Import pod should have prometheus stats available while importing", func() {
var endpoint *v1.Endpoints
c := f.K8sClient
ns := f.Namespace.Name
Expand Down

0 comments on commit d7c5a68

Please sign in to comment.