Skip to content

Commit c2a3372

Browse files
akalenyuakalenyu
committed
Correct wrong check of resulting image sparsiness
The first issue is that we look at content size since 2168, which is not beneficial in the sparseness check as opposed to disk usage. The second issue is that the check we have in tests for sparsiness is not honest because of the "equal to" part. The virtual size has to be strictly greater than the content (as the content is displayed by tools that understand sparseness). The third issue is that we almost always resize which results in significantly larger virtual size. What we really care about is comparing against the original virtual size of the image. Signed-off-by: Alex Kalenyuk <[email protected]>
1 parent aab2017 commit c2a3372

File tree

5 files changed

+15
-45
lines changed

5 files changed

+15
-45
lines changed

‎tests/cloner_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -1897,7 +1897,7 @@ var _ = Describe("all clone tests", func() {
18971897
Expect(err).ToNot(HaveOccurred())
18981898

18991899
By("Verifying the image is sparse")
1900-
Expect(f.VerifySparse(f.Namespace, utils.PersistentVolumeClaimFromDataVolume(targetDv), utils.DefaultImagePath)).To(BeTrue())
1900+
Expect(f.VerifySparse(f.Namespace, utils.PersistentVolumeClaimFromDataVolume(targetDv), utils.DefaultImagePath, utils.UploadFileSize)).To(BeTrue())
19011901
By("Deleting verifier pod")
19021902
err = utils.DeleteVerifierPod(f.K8sClient, f.Namespace.Name)
19031903
Expect(err).ToNot(HaveOccurred())

‎tests/framework/pvc.go

+4-34
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import (
44
"context"
55
"encoding/json"
66
"fmt"
7-
"strconv"
87
"strings"
98
"time"
109

@@ -282,24 +281,15 @@ func (f *Framework) VerifyBlankDisk(namespace *k8sv1.Namespace, pvc *k8sv1.Persi
282281
}
283282

284283
// VerifySparse checks a disk image being sparse after creation/resize.
285-
func (f *Framework) VerifySparse(namespace *k8sv1.Namespace, pvc *k8sv1.PersistentVolumeClaim, imagePath string) (bool, error) {
284+
func (f *Framework) VerifySparse(namespace *k8sv1.Namespace, pvc *k8sv1.PersistentVolumeClaim, imagePath string, originalVirtualSize int64) (bool, error) {
286285
var info image.ImgInfo
287-
var imageContentSize int64
288286
err := f.GetImageInfo(namespace, pvc, imagePath, &info)
289287
if err != nil {
290288
return false, err
291289
}
292-
// qemu-img info gives us ActualSize but that is size on disk
293-
// which isn't important to us in this comparison; we compare content size
294-
err = f.GetImageContentSize(namespace, pvc, imagePath, &imageContentSize)
295-
if err != nil {
296-
return false, err
297-
}
298-
if info.ActualSize-imageContentSize >= units.MiB {
299-
return false, fmt.Errorf("Diff between content size %d and size on disk %d is significant, something's not right", imageContentSize, info.ActualSize)
300-
}
301-
fmt.Fprintf(ginkgo.GinkgoWriter, "INFO: VerifySparse comparison: Virtual: %d vs Content: %d\n", info.VirtualSize, imageContentSize)
302-
return info.VirtualSize >= imageContentSize, nil
290+
fmt.Fprintf(ginkgo.GinkgoWriter, "INFO: VerifySparse comparison: OriginalVirtual: %d vs SizeOnDisk: %d\n", originalVirtualSize, info.ActualSize)
291+
// The size on disk of a sparse image is significantly lower than the image's virtual size
292+
return originalVirtualSize-info.ActualSize >= units.MiB, nil
303293
}
304294

305295
// VerifyFSOverhead checks whether virtual size is smaller than actual size. That means FS Overhead has been accounted for.
@@ -546,26 +536,6 @@ func (f *Framework) GetImageInfo(namespace *k8sv1.Namespace, pvc *k8sv1.Persiste
546536
return err
547537
}
548538

549-
// GetImageContentSize returns the content size (as opposed to size on disk) of an image
550-
func (f *Framework) GetImageContentSize(namespace *k8sv1.Namespace, pvc *k8sv1.PersistentVolumeClaim, imagePath string, imageSize *int64) error {
551-
cmd := fmt.Sprintf("du -s --apparent-size -B 1 %s | cut -f 1", imagePath)
552-
553-
_, err := f.verifyInPod(namespace, pvc, cmd, func(output, stderr string) (bool, error) {
554-
fmt.Fprintf(ginkgo.GinkgoWriter, "CMD (%s) output %s\n", cmd, output)
555-
556-
size, err := strconv.ParseInt(output, 10, 64)
557-
if err != nil {
558-
klog.Errorf("Invalid image content size:\n%s\n", output)
559-
return false, err
560-
}
561-
*imageSize = size
562-
563-
return true, nil
564-
})
565-
566-
return err
567-
}
568-
569539
func (f *Framework) startVerifierPod(namespace *k8sv1.Namespace, pvc *k8sv1.PersistentVolumeClaim) (*k8sv1.Pod, error) {
570540
var executorPod *k8sv1.Pod
571541
var err error

‎tests/import_test.go

+4-4
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ var _ = Describe("[rfe_id:1115][crit:high][vendor:[email protected]][level:compo
125125
By("Verify the image contents")
126126
Expect(f.VerifyBlankDisk(f.Namespace, pvc)).To(BeTrue())
127127
By("Verifying the image is sparse")
128-
Expect(f.VerifySparse(f.Namespace, pvc, utils.DefaultImagePath)).To(BeTrue())
128+
Expect(f.VerifySparse(f.Namespace, pvc, utils.DefaultImagePath, utils.UploadFileSize)).To(BeTrue())
129129
By("Verifying permissions are 660")
130130
Expect(f.VerifyPermissions(f.Namespace, pvc)).To(BeTrue(), "Permissions on disk image are not 660")
131131
if utils.DefaultStorageCSIRespectsFsGroup {
@@ -1659,7 +1659,7 @@ var _ = Describe("Import populator", func() {
16591659
Expect(ok).To(BeTrue())
16601660
} else {
16611661
By("Verifying the image is sparse")
1662-
Expect(f.VerifySparse(f.Namespace, pvc, utils.DefaultImagePath)).To(BeTrue())
1662+
Expect(f.VerifySparse(f.Namespace, pvc, utils.DefaultImagePath, utils.UploadFileSize)).To(BeTrue())
16631663
}
16641664

16651665
if utils.DefaultStorageCSIRespectsFsGroup {
@@ -1722,7 +1722,7 @@ var _ = Describe("Import populator", func() {
17221722
Expect(err).ToNot(HaveOccurred())
17231723
Expect(md5).To(Equal(expectedMD5))
17241724
By("Verifying the image is sparse")
1725-
Expect(f.VerifySparse(f.Namespace, pvc, utils.DefaultPvcMountPath)).To(BeTrue())
1725+
Expect(f.VerifySparse(f.Namespace, pvc, utils.DefaultPvcMountPath, utils.UploadFileSize)).To(BeTrue())
17261726

17271727
By("Verify 100.0% annotation")
17281728
progress, ok, err := utils.WaitForPVCAnnotation(f.K8sClient, f.Namespace.Name, pvc, controller.AnnPopulatorProgress)
@@ -1799,7 +1799,7 @@ var _ = Describe("Import populator", func() {
17991799
Expect(err).ToNot(HaveOccurred())
18001800
Expect(md5).To(Equal(utils.TinyCoreMD5))
18011801
By("Verifying the image is sparse")
1802-
Expect(f.VerifySparse(f.Namespace, pvc, utils.DefaultImagePath)).To(BeTrue())
1802+
Expect(f.VerifySparse(f.Namespace, pvc, utils.DefaultImagePath, utils.UploadFileSize)).To(BeTrue())
18031803
sourceMD5 := md5
18041804

18051805
By("Retaining PV")

‎tests/transport_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ var _ = Describe("Transport Tests", func() {
123123
}
124124
}
125125
By("Verifying the image is sparse")
126-
Expect(f.VerifySparse(f.Namespace, pvc, utils.DefaultImagePath)).To(BeTrue())
126+
Expect(f.VerifySparse(f.Namespace, pvc, utils.DefaultImagePath, utils.UploadFileSize)).To(BeTrue())
127127
} else {
128128
By("Verify PVC status annotation says failed")
129129
found, err := utils.WaitPVCPodStatusRunning(f.K8sClient, pvc)

‎tests/upload_test.go

+5-5
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,7 @@ var _ = Describe("[rfe_id:138][crit:high][vendor:[email protected]][level:compon
195195
Expect(err).ToNot(HaveOccurred())
196196
Expect(same).To(BeTrue())
197197
By("Verifying the image is sparse")
198-
Expect(f.VerifySparse(f.Namespace, pvc, utils.DefaultImagePath)).To(BeTrue())
198+
Expect(f.VerifySparse(f.Namespace, pvc, utils.DefaultImagePath, utils.UploadFileSize)).To(BeTrue())
199199
if utils.DefaultStorageCSIRespectsFsGroup {
200200
// CSI storage class, it should respect fsGroup
201201
By("Checking that disk image group is qemu")
@@ -411,7 +411,7 @@ var _ = Describe("[rfe_id:138][crit:high][vendor:[email protected]][level:compon
411411
Expect(err).ToNot(HaveOccurred())
412412
Expect(same).To(BeTrue())
413413
By("Verifying the image is sparse")
414-
Expect(f.VerifySparse(f.Namespace, archivePVC, pathInPvc)).To(BeTrue())
414+
Expect(f.VerifySparse(f.Namespace, archivePVC, pathInPvc, utils.UploadFileSize)).To(BeTrue())
415415
}
416416
} else {
417417
checkFailureNoValidToken(archivePVC)
@@ -525,7 +525,7 @@ var _ = Describe("[rfe_id:138][crit:high][vendor:[email protected]][level:compon
525525
Expect(err).ToNot(HaveOccurred())
526526
Expect(same).To(BeTrue())
527527
By("Verifying the image is sparse")
528-
Expect(f.VerifySparse(f.Namespace, pvc, utils.DefaultImagePath)).To(BeTrue())
528+
Expect(f.VerifySparse(f.Namespace, pvc, utils.DefaultImagePath, utils.UploadFileSize)).To(BeTrue())
529529
if utils.DefaultStorageCSIRespectsFsGroup {
530530
// CSI storage class, it should respect fsGroup
531531
By("Checking that disk image group is qemu")
@@ -599,7 +599,7 @@ var _ = Describe("[rfe_id:138][crit:high][vendor:[email protected]][level:compon
599599
Expect(err).ToNot(HaveOccurred())
600600
Expect(same).To(BeTrue())
601601
By("Verifying the image is sparse")
602-
Expect(f.VerifySparse(f.Namespace, pvc, utils.DefaultImagePath)).To(BeTrue())
602+
Expect(f.VerifySparse(f.Namespace, pvc, utils.DefaultImagePath, utils.UploadFileSize)).To(BeTrue())
603603
if utils.DefaultStorageCSIRespectsFsGroup {
604604
// CSI storage class, it should respect fsGroup
605605
By("Checking that disk image group is qemu")
@@ -730,7 +730,7 @@ var _ = Describe("[rfe_id:138][crit:high][vendor:[email protected]][level:compon
730730
Expect(err).ToNot(HaveOccurred())
731731
Expect(same).To(BeTrue())
732732
By("Verifying the image is sparse")
733-
Expect(f.VerifySparse(f.Namespace, pvc, pathInPvc)).To(BeTrue())
733+
Expect(f.VerifySparse(f.Namespace, pvc, pathInPvc, utils.UploadFileSize)).To(BeTrue())
734734
}
735735
} else {
736736
checkFailureNoValidToken(pvcPrime)

0 commit comments

Comments
 (0)