Skip to content

Commit

Permalink
Revert to using nbdkit for converting
Browse files Browse the repository at this point in the history
Due to slow performance we stopped using nbdkit for most kind of imports.

This new behavior introduced minor differences such as stop converting raw images, which caused inconsistencies in some tests.

Since nbdkit performance issues have been addressed in v1.35.8, this commit reverts back to the old behavior.

Signed-off-by: Alvaro Romero <alromero@redhat.com>
  • Loading branch information
alromeros committed Apr 19, 2024
1 parent efa362f commit fc4d6a4
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 21 deletions.
24 changes: 12 additions & 12 deletions pkg/importer/http-datasource.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,20 +133,20 @@ func (hs *HTTPDataSource) Info() (ProcessingPhase, error) {
if hs.contentType == cdiv1.DataVolumeArchive {
return ProcessingPhaseTransferDataDir, nil
}
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
if hs.readers.Convert {
if hs.brokenForQemuImg || hs.readers.Archived || hs.customCA != "" {
return ProcessingPhaseTransferScratch, nil
}
return ProcessingPhaseConvert, 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
}
// 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
return ProcessingPhaseConvert, nil
}

// Transfer is called to transfer the data from the source to a scratch location.
Expand Down
7 changes: 6 additions & 1 deletion pkg/importer/http-datasource_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,11 +94,16 @@ 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())
}
},
Entry("return TransferScratch phase ", cirrosFileName, cdiv1.DataVolumeKubeVirt, ProcessingPhaseTransferScratch, cirrosData, false),
Entry("return Convert phase ", cirrosFileName, cdiv1.DataVolumeKubeVirt, ProcessingPhaseConvert, cirrosData, false),
Entry("return TransferTarget with archive content type but not archive endpoint ", cirrosFileName, cdiv1.DataVolumeArchive, ProcessingPhaseTransferDataDir, cirrosData, false),
Entry("return TransferTarget with archive content type and archive endpoint ", diskimageTarFileName, cdiv1.DataVolumeArchive, ProcessingPhaseTransferDataDir, diskimageArchiveData, false),
)
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 @@ -204,7 +204,7 @@ var _ = Describe("Import Proxy tests", func() {
imgName: tinyCoreQcow2,
isHTTPS: false,
withBasicAuth: false,
userAgent: golangUserAgent,
userAgent: nbdKitUserAgent,
expected: BeTrue}),
Entry("succeed creating iso import dv with a proxied server (http)", importProxyTestArguments{
name: "dv-import-http-proxy",
Expand All @@ -213,7 +213,7 @@ var _ = Describe("Import Proxy tests", func() {
imgName: tinyCoreIso,
isHTTPS: false,
withBasicAuth: false,
userAgent: golangUserAgent,
userAgent: nbdKitUserAgent,
expected: BeTrue}),
Entry("succeed creating iso.gz import dv with a proxied server (http)", importProxyTestArguments{
name: "dv-import-http-proxy",
Expand All @@ -222,7 +222,7 @@ var _ = Describe("Import Proxy tests", func() {
imgName: tinyCoreIsoGz,
isHTTPS: false,
withBasicAuth: false,
userAgent: golangUserAgent,
userAgent: nbdKitUserAgent,
expected: BeTrue}),
Entry("succeed creating import dv with a proxied server (http) with basic autentication", importProxyTestArguments{
name: "dv-import-http-proxy-auth",
Expand All @@ -231,7 +231,7 @@ var _ = Describe("Import Proxy tests", func() {
imgName: tinyCoreQcow2,
isHTTPS: false,
withBasicAuth: true,
userAgent: golangUserAgent,
userAgent: nbdKitUserAgent,
expected: BeTrue}),
Entry("succeed creating iso import dv with a proxied server (http) with basic autentication", importProxyTestArguments{
name: "dv-import-http-proxy-auth",
Expand Down
5 changes: 1 addition & 4 deletions tests/import_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -466,10 +466,7 @@ var _ = Describe("[rfe_id:1118][crit:high][vendor:cnv-qe@redhat.com][level:compo
afterCMD(portForwardCmd)
})

// 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() {
It("[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 fc4d6a4

Please sign in to comment.