From 8aaab528b5cf8400ce8ef7250fc54f4bbb5b0322 Mon Sep 17 00:00:00 2001 From: Alex Kalenyuk Date: Wed, 1 May 2024 11:10:41 +0300 Subject: [PATCH] Use scratch space for non-archive uploads (with host assisted from block exception) Basically a follow up to 3219 for upload sources, which suffer from the same issue of losing sparseness. Signed-off-by: Alex Kalenyuk --- cmd/cdi-cloner/clone-source.go | 9 ++++--- pkg/importer/upload-datasource.go | 12 +++++---- pkg/importer/upload-datasource_test.go | 34 +++++++++++++------------- pkg/uploadserver/uploadserver.go | 3 ++- tests/upload_test.go | 4 --- 5 files changed, 31 insertions(+), 31 deletions(-) diff --git a/cmd/cdi-cloner/clone-source.go b/cmd/cdi-cloner/clone-source.go index b1570ca688..fded780519 100644 --- a/cmd/cdi-cloner/clone-source.go +++ b/cmd/cdi-cloner/clone-source.go @@ -6,6 +6,7 @@ import ( "crypto/x509" "errors" "flag" + "fmt" "io" "net/http" "os" @@ -55,7 +56,7 @@ func (er *execReader) Close() error { } func init() { - flag.StringVar(&contentType, "content-type", "", "filesystem-clone|blockdevice-clone") + flag.StringVar(&contentType, "content-type", "", fmt.Sprintf("%s|%s", common.FilesystemCloneContentType, common.BlockdeviceClone)) flag.StringVar(&mountPoint, "mount", "", "pvc mount point") flag.Uint64Var(&uploadBytes, "upload-bytes", 0, "approx number of bytes in input") klog.InitFlags(nil) @@ -133,7 +134,7 @@ func pipeToSnappy(reader io.ReadCloser) io.ReadCloser { func validateContentType() { switch contentType { - case "filesystem-clone", "blockdevice-clone": + case common.FilesystemCloneContentType, common.BlockdeviceClone: default: klog.Fatalf("Invalid content-type %q", contentType) } @@ -198,13 +199,13 @@ func newTarReader(preallocation bool) (io.ReadCloser, error) { func getInputStream(preallocation bool) io.ReadCloser { switch contentType { - case "filesystem-clone": + case common.FilesystemCloneContentType: rc, err := newTarReader(preallocation) if err != nil { klog.Fatalf("Error creating tar reader for %q: %+v", mountPoint, err) } return rc - case "blockdevice-clone": + case common.BlockdeviceClone: rc, err := os.Open(mountPoint) if err != nil { klog.Fatalf("Error opening block device %q: %+v", mountPoint, err) diff --git a/pkg/importer/upload-datasource.go b/pkg/importer/upload-datasource.go index 643aa38b3f..1d6d4e4fc1 100644 --- a/pkg/importer/upload-datasource.go +++ b/pkg/importer/upload-datasource.go @@ -29,13 +29,16 @@ type UploadDataSource struct { url *url.URL // contentType expected from the upload content contentType cdiv1.DataVolumeContentType + // directWriteException indicates an exception to write directly without scratch space + directWriteException bool } // NewUploadDataSource creates a new instance of an UploadDataSource -func NewUploadDataSource(stream io.ReadCloser, contentType cdiv1.DataVolumeContentType) *UploadDataSource { +func NewUploadDataSource(stream io.ReadCloser, contentType cdiv1.DataVolumeContentType, directWriteException bool) *UploadDataSource { return &UploadDataSource{ - stream: stream, - contentType: contentType, + stream: stream, + contentType: contentType, + directWriteException: directWriteException, } } @@ -51,8 +54,7 @@ func (ud *UploadDataSource) Info() (ProcessingPhase, error) { if ud.contentType == cdiv1.DataVolumeArchive { return ProcessingPhaseTransferDataDir, nil } - if !ud.readers.Convert { - // Uploading a raw file, we can write that directly to the target. + if ud.directWriteException { return ProcessingPhaseTransferDataFile, nil } return ProcessingPhaseTransferScratch, nil diff --git a/pkg/importer/upload-datasource_test.go b/pkg/importer/upload-datasource_test.go index bca95801e9..b86af6edf8 100644 --- a/pkg/importer/upload-datasource_test.go +++ b/pkg/importer/upload-datasource_test.go @@ -43,7 +43,7 @@ var _ = Describe("Upload data source", func() { Expect(err).NotTo(HaveOccurred()) err = file.Close() Expect(err).NotTo(HaveOccurred()) - ud = NewUploadDataSource(file, dvKubevirt) + ud = NewUploadDataSource(file, dvKubevirt, false) result, err := ud.Info() Expect(err).To(HaveOccurred()) Expect(ProcessingPhaseError).To(Equal(result)) @@ -53,7 +53,7 @@ var _ = Describe("Upload data source", func() { // Don't need to defer close, since ud.Close will close the reader file, err := os.Open(cirrosFilePath) Expect(err).NotTo(HaveOccurred()) - ud = NewUploadDataSource(file, dvKubevirt) + ud = NewUploadDataSource(file, dvKubevirt, false) result, err := ud.Info() Expect(err).NotTo(HaveOccurred()) @@ -64,21 +64,21 @@ var _ = Describe("Upload data source", func() { // Don't need to defer close, since ud.Close will close the reader file, err := os.Open(tinyCoreTarFilePath) Expect(err).NotTo(HaveOccurred()) - ud = NewUploadDataSource(file, dvArchive) + ud = NewUploadDataSource(file, dvArchive, false) result, err := ud.Info() Expect(err).NotTo(HaveOccurred()) Expect(ProcessingPhaseTransferDataDir).To(Equal(result)) }) - It("Info should return TransferData, when passed in a valid raw image", func() { + It("Info should return TransferScratch, when passed in a valid raw image", func() { // Don't need to defer close, since ud.Close will close the reader file, err := os.Open(tinyCoreFilePath) Expect(err).NotTo(HaveOccurred()) - ud = NewUploadDataSource(file, dvKubevirt) + ud = NewUploadDataSource(file, dvKubevirt, false) result, err := ud.Info() Expect(err).NotTo(HaveOccurred()) - Expect(ProcessingPhaseTransferDataFile).To(Equal(result)) + Expect(ProcessingPhaseTransferScratch).To(Equal(result)) }) DescribeTable("calling transfer should", func(fileName string, dvContentType cdiv1.DataVolumeContentType, expectedPhase ProcessingPhase, scratchPath string, want []byte, wantErr bool) { @@ -88,7 +88,7 @@ var _ = Describe("Upload data source", func() { sourceFile, err := os.Open(fileName) Expect(err).NotTo(HaveOccurred()) - ud = NewUploadDataSource(sourceFile, dvContentType) + ud = NewUploadDataSource(sourceFile, dvContentType, false) _, err = ud.Info() Expect(err).NotTo(HaveOccurred()) nextPhase, err := ud.Transfer(scratchPath) @@ -118,7 +118,7 @@ var _ = Describe("Upload data source", func() { sourceFile, err := os.Open(cirrosFilePath) Expect(err).NotTo(HaveOccurred()) - ud = NewUploadDataSource(sourceFile, dvKubevirt) + ud = NewUploadDataSource(sourceFile, dvKubevirt, false) nextPhase, err := ud.Info() Expect(err).NotTo(HaveOccurred()) Expect(ProcessingPhaseTransferScratch).To(Equal(nextPhase)) @@ -133,10 +133,10 @@ var _ = Describe("Upload data source", func() { // Don't need to defer close, since ud.Close will close the reader sourceFile, err := os.Open(tinyCoreFilePath) Expect(err).NotTo(HaveOccurred()) - ud = NewUploadDataSource(sourceFile, dvKubevirt) + ud = NewUploadDataSource(sourceFile, dvKubevirt, false) result, err := ud.Info() Expect(err).NotTo(HaveOccurred()) - Expect(ProcessingPhaseTransferDataFile).To(Equal(result)) + Expect(ProcessingPhaseTransferScratch).To(Equal(result)) result, err = ud.TransferFile(filepath.Join(tmpDir, "file")) Expect(err).ToNot(HaveOccurred()) Expect(ProcessingPhaseResize).To(Equal(result)) @@ -146,17 +146,17 @@ var _ = Describe("Upload data source", func() { // Don't need to defer close, since ud.Close will close the reader sourceFile, err := os.Open(tinyCoreFilePath) Expect(err).NotTo(HaveOccurred()) - ud = NewUploadDataSource(sourceFile, dvKubevirt) + ud = NewUploadDataSource(sourceFile, dvKubevirt, false) result, err := ud.Info() Expect(err).NotTo(HaveOccurred()) - Expect(ProcessingPhaseTransferDataFile).To(Equal(result)) + Expect(ProcessingPhaseTransferScratch).To(Equal(result)) result, err = ud.TransferFile("/invalidpath/invalidfile") Expect(err).To(HaveOccurred()) Expect(ProcessingPhaseError).To(Equal(result)) }) It("Close with nil stream should not fail", func() { - ud = NewUploadDataSource(nil, dvKubevirt) + ud = NewUploadDataSource(nil, dvKubevirt, false) err := ud.Close() Expect(err).NotTo(HaveOccurred()) }) @@ -204,14 +204,14 @@ var _ = Describe("Async Upload data source", func() { Expect(ProcessingPhaseTransferScratch).To(Equal(result)) }) - It("Info should return TransferData, when passed in a valid raw image", func() { + It("Info should return TransferScratch, when passed in a valid raw image", func() { // Don't need to defer close, since ud.Close will close the reader file, err := os.Open(tinyCoreFilePath) Expect(err).NotTo(HaveOccurred()) aud = NewAsyncUploadDataSource(file) result, err := aud.Info() Expect(err).NotTo(HaveOccurred()) - Expect(ProcessingPhaseTransferDataFile).To(Equal(result)) + Expect(ProcessingPhaseTransferScratch).To(Equal(result)) }) DescribeTable("calling transfer should", func(fileName, scratchPath string, want []byte, wantErr bool) { @@ -260,7 +260,7 @@ var _ = Describe("Async Upload data source", func() { aud = NewAsyncUploadDataSource(sourceFile) result, err := aud.Info() Expect(err).NotTo(HaveOccurred()) - Expect(ProcessingPhaseTransferDataFile).To(Equal(result)) + Expect(ProcessingPhaseTransferScratch).To(Equal(result)) result, err = aud.TransferFile(filepath.Join(tmpDir, "file")) Expect(err).ToNot(HaveOccurred()) Expect(ProcessingPhaseValidatePause).To(Equal(result)) @@ -274,7 +274,7 @@ var _ = Describe("Async Upload data source", func() { aud = NewAsyncUploadDataSource(sourceFile) result, err := aud.Info() Expect(err).NotTo(HaveOccurred()) - Expect(ProcessingPhaseTransferDataFile).To(Equal(result)) + Expect(ProcessingPhaseTransferScratch).To(Equal(result)) result, err = aud.TransferFile("/invalidpath/invalidfile") Expect(err).To(HaveOccurred()) Expect(ProcessingPhaseError).To(Equal(result)) diff --git a/pkg/uploadserver/uploadserver.go b/pkg/uploadserver/uploadserver.go index 71057a0c54..de1da97787 100644 --- a/pkg/uploadserver/uploadserver.go +++ b/pkg/uploadserver/uploadserver.go @@ -494,7 +494,8 @@ func newUploadStreamProcessor(stream io.ReadCloser, dest, imageSize string, file } // Clone block device to block device or file system - uds := importer.NewUploadDataSource(newContentReader(stream, sourceContentType), dvContentType) + directWriteException := sourceContentType == common.BlockdeviceClone + uds := importer.NewUploadDataSource(newContentReader(stream, sourceContentType), dvContentType, directWriteException) processor := importer.NewDataProcessor(uds, dest, common.ImporterVolumePath, common.ScratchDataDir, imageSize, filesystemOverhead, preallocation, "") err := processor.ProcessData() return processor.PreallocationApplied(), err diff --git a/tests/upload_test.go b/tests/upload_test.go index 2bd33238df..10dd60a5d3 100644 --- a/tests/upload_test.go +++ b/tests/upload_test.go @@ -413,8 +413,6 @@ var _ = Describe("[rfe_id:138][crit:high][vendor:cnv-qe@redhat.com][level:compon same, err := f.VerifyTargetPVCContentMD5(f.Namespace, archivePVC, pathInPvc, expectedMd5) Expect(err).ToNot(HaveOccurred()) Expect(same).To(BeTrue()) - By("Verifying the image is sparse") - Expect(f.VerifySparse(f.Namespace, archivePVC, pathInPvc, utils.UploadFileSize)).To(BeTrue()) } } else { checkFailureNoValidToken(archivePVC) @@ -733,8 +731,6 @@ var _ = Describe("[rfe_id:138][crit:high][vendor:cnv-qe@redhat.com][level:compon same, err := f.VerifyTargetPVCContentMD5(f.Namespace, pvc, pathInPvc, expectedMd5) Expect(err).ToNot(HaveOccurred()) Expect(same).To(BeTrue()) - By("Verifying the image is sparse") - Expect(f.VerifySparse(f.Namespace, pvc, pathInPvc, utils.UploadFileSize)).To(BeTrue()) } } else { checkFailureNoValidToken(pvcPrime)