|
| 1 | +From fe271ce9c879d110d43d55ffb79c725bca8ed8f5 Mon Sep 17 00:00:00 2001 |
| 2 | +From: Jed Lejosne <jed@redhat.com> |
| 3 | +Date: Wed, 25 Jun 2025 09:19:41 -0400 |
| 4 | +Subject: [PATCH 1/2] host-path: only chown files we created |
| 5 | + |
| 6 | +Signed-off-by: Jed Lejosne <jed@redhat.com> |
| 7 | + |
| 8 | +From 977390fbff6c91ba53d494701bb937962acdfddd Mon Sep 17 00:00:00 2001 |
| 9 | +From: Jed Lejosne <jed@redhat.com> |
| 10 | +Date: Tue, 1 Jul 2025 09:09:14 -0400 |
| 11 | +Subject: [PATCH 2/2] tests: adjust host-path test according to previous fix |
| 12 | + |
| 13 | +Signed-off-by: Jed Lejosne <jed@redhat.com> |
| 14 | + |
| 15 | +Upstream Patch Reference: https://github.com/kubevirt/kubevirt/commit/00d03e43e3bf03e563136695a4732b65ed42d764.patch |
| 16 | +--- |
| 17 | + pkg/ephemeral-disk-utils/utils.go | 19 +++++++++++++++++-- |
| 18 | + pkg/host-disk/host-disk.go | 14 +++++++------- |
| 19 | + pkg/host-disk/host-disk_test.go | 13 ++++++++++--- |
| 20 | + tests/storage/storage.go | 20 ++++++++++++++++---- |
| 21 | + 4 files changed, 50 insertions(+), 16 deletions(-) |
| 22 | + |
| 23 | +diff --git a/pkg/ephemeral-disk-utils/utils.go b/pkg/ephemeral-disk-utils/utils.go |
| 24 | +index fc1a07b..863b267 100644 |
| 25 | +--- a/pkg/ephemeral-disk-utils/utils.go |
| 26 | ++++ b/pkg/ephemeral-disk-utils/utils.go |
| 27 | +@@ -44,14 +44,29 @@ func MockDefaultOwnershipManager() { |
| 28 | + type nonOpManager struct { |
| 29 | + } |
| 30 | + |
| 31 | +-func (no *nonOpManager) UnsafeSetFileOwnership(file string) error { |
| 32 | ++func (no *nonOpManager) UnsafeSetFileOwnership(_ string) error { |
| 33 | + return nil |
| 34 | + } |
| 35 | + |
| 36 | +-func (no *nonOpManager) SetFileOwnership(file *safepath.Path) error { |
| 37 | ++func (no *nonOpManager) SetFileOwnership(_ *safepath.Path) error { |
| 38 | + return nil |
| 39 | + } |
| 40 | + |
| 41 | ++func MockDefaultOwnershipManagerWithFailure() { |
| 42 | ++ DefaultOwnershipManager = &failureManager{} |
| 43 | ++} |
| 44 | ++ |
| 45 | ++type failureManager struct { |
| 46 | ++} |
| 47 | ++ |
| 48 | ++func (no *failureManager) UnsafeSetFileOwnership(_ string) error { |
| 49 | ++ panic("unexpected call to UnsafeSetFileOwnership") |
| 50 | ++} |
| 51 | ++ |
| 52 | ++func (no *failureManager) SetFileOwnership(_ *safepath.Path) error { |
| 53 | ++ panic("unexpected call to SetFileOwnership") |
| 54 | ++} |
| 55 | ++ |
| 56 | + type OwnershipManager struct { |
| 57 | + user string |
| 58 | + } |
| 59 | +diff --git a/pkg/host-disk/host-disk.go b/pkg/host-disk/host-disk.go |
| 60 | +index ca6893a..895006f 100644 |
| 61 | +--- a/pkg/host-disk/host-disk.go |
| 62 | ++++ b/pkg/host-disk/host-disk.go |
| 63 | +@@ -213,7 +213,7 @@ func (hdc *DiskImgCreator) setlessPVCSpaceToleration(toleration int) { |
| 64 | + hdc.lessPVCSpaceToleration = toleration |
| 65 | + } |
| 66 | + |
| 67 | +-func (hdc DiskImgCreator) Create(vmi *v1.VirtualMachineInstance) error { |
| 68 | ++func (hdc *DiskImgCreator) Create(vmi *v1.VirtualMachineInstance) error { |
| 69 | + for _, volume := range vmi.Spec.Volumes { |
| 70 | + if hostDisk := volume.VolumeSource.HostDisk; shouldMountHostDisk(hostDisk) { |
| 71 | + if err := hdc.mountHostDiskAndSetOwnership(vmi, volume.Name, hostDisk); err != nil { |
| 72 | +@@ -236,14 +236,14 @@ func (hdc *DiskImgCreator) mountHostDiskAndSetOwnership(vmi *v1.VirtualMachineIn |
| 73 | + return err |
| 74 | + } |
| 75 | + if !fileExists { |
| 76 | +- if err := hdc.handleRequestedSizeAndCreateSparseRaw(vmi, diskDir, diskPath, hostDisk); err != nil { |
| 77 | ++ if err = hdc.handleRequestedSizeAndCreateSparseRaw(vmi, diskDir, diskPath, hostDisk); err != nil { |
| 78 | + return err |
| 79 | + } |
| 80 | +- } |
| 81 | +- // Change file ownership to the qemu user. |
| 82 | +- if err := ephemeraldiskutils.DefaultOwnershipManager.UnsafeSetFileOwnership(diskPath); err != nil { |
| 83 | +- log.Log.Reason(err).Errorf("Couldn't set Ownership on %s: %v", diskPath, err) |
| 84 | +- return err |
| 85 | ++ // Change file ownership to the qemu user. |
| 86 | ++ if err = ephemeraldiskutils.DefaultOwnershipManager.UnsafeSetFileOwnership(diskPath); err != nil { |
| 87 | ++ log.Log.Reason(err).Errorf("Couldn't set Ownership on %s: %v", diskPath, err) |
| 88 | ++ return err |
| 89 | ++ } |
| 90 | + } |
| 91 | + return nil |
| 92 | + } |
| 93 | +diff --git a/pkg/host-disk/host-disk_test.go b/pkg/host-disk/host-disk_test.go |
| 94 | +index 184c0d8..57f3a26 100644 |
| 95 | +--- a/pkg/host-disk/host-disk_test.go |
| 96 | ++++ b/pkg/host-disk/host-disk_test.go |
| 97 | +@@ -35,13 +35,13 @@ import ( |
| 98 | + "k8s.io/client-go/kubernetes/fake" |
| 99 | + "k8s.io/client-go/tools/record" |
| 100 | + |
| 101 | +- "kubevirt.io/kubevirt/pkg/safepath" |
| 102 | +- |
| 103 | + "kubevirt.io/client-go/api" |
| 104 | + |
| 105 | + v1 "kubevirt.io/api/core/v1" |
| 106 | + "kubevirt.io/client-go/kubecli" |
| 107 | + |
| 108 | ++ ephemeraldiskutils "kubevirt.io/kubevirt/pkg/ephemeral-disk-utils" |
| 109 | ++ "kubevirt.io/kubevirt/pkg/safepath" |
| 110 | + "kubevirt.io/kubevirt/pkg/testutils" |
| 111 | + ) |
| 112 | + |
| 113 | +@@ -321,7 +321,14 @@ var _ = Describe("HostDisk", func() { |
| 114 | + }) |
| 115 | + }) |
| 116 | + Context("With existing disk.img", func() { |
| 117 | +- It("Should not re-create disk.img", func() { |
| 118 | ++ AfterEach(func() { |
| 119 | ++ By("Switching back to the regular mock ownership manager") |
| 120 | ++ ephemeraldiskutils.MockDefaultOwnershipManager() |
| 121 | ++ }) |
| 122 | ++ |
| 123 | ++ It("Should not re-create or chown disk.img", func() { |
| 124 | ++ By("Switching to an ownership manager that panics when called") |
| 125 | ++ ephemeraldiskutils.MockDefaultOwnershipManagerWithFailure() |
| 126 | + By("Creating a disk.img before adding a HostDisk volume") |
| 127 | + tmpDiskImg := createTempDiskImg("volume1") |
| 128 | + |
| 129 | +diff --git a/tests/storage/storage.go b/tests/storage/storage.go |
| 130 | +index 1ed1d86..5977e93 100644 |
| 131 | +--- a/tests/storage/storage.go |
| 132 | ++++ b/tests/storage/storage.go |
| 133 | +@@ -285,18 +285,30 @@ var _ = SIGDescribe("Storage", func() { |
| 134 | + |
| 135 | + if storageEngine == "nfs" { |
| 136 | + vmi = tests.RunVMIAndExpectLaunchIgnoreWarnings(vmi, 180) |
| 137 | +- } else { |
| 138 | +- vmi = tests.RunVMIAndExpectLaunch(vmi, 180) |
| 139 | + } |
| 140 | ++ if imageOwnedByQEMU { |
| 141 | ++ vmi = tests.RunVMIAndExpectLaunch(vmi, 180) |
| 142 | + |
| 143 | +- By(checkingVMInstanceConsoleOut) |
| 144 | +- Expect(console.LoginToAlpine(vmi)).To(Succeed()) |
| 145 | ++ By(checkingVMInstanceConsoleOut) |
| 146 | ++ Expect(console.LoginToAlpine(vmi)).To(Succeed()) |
| 147 | ++ } else { |
| 148 | ++ By("Starting a VirtualMachineInstance") |
| 149 | ++ createdVMI := tests.RunVMIAndExpectScheduling(vmi, 60) |
| 150 | ++ |
| 151 | ++ By(fmt.Sprintf("Checking that VirtualMachineInstance start failed: starting at %v", time.Now())) |
| 152 | ++ ctx, cancel := context.WithCancel(context.Background()) |
| 153 | ++ defer cancel() |
| 154 | ++ event := watcher.New(createdVMI).Timeout(60*time.Second).SinceWatchedObjectResourceVersion().WaitFor(ctx, watcher.WarningEvent, "SyncFailed") |
| 155 | ++ Expect(event.Message).To(ContainSubstring("Could not open '/var/run/kubevirt-private/vmi-disks/disk0/disk.img': Permission denied"), "VMI should not be started") |
| 156 | ++ } |
| 157 | + }, |
| 158 | ++ |
| 159 | + Entry("[test_id:3130]with Disk PVC", newRandomVMIWithPVC, "", nil, true), |
| 160 | + Entry("[test_id:3131]with CDRom PVC", newRandomVMIWithCDRom, "", nil, true), |
| 161 | + Entry("[test_id:4618]with NFS Disk PVC using ipv4 address of the NFS pod", newRandomVMIWithPVC, "nfs", k8sv1.IPv4Protocol, true), |
| 162 | + Entry("[Serial]with NFS Disk PVC using ipv6 address of the NFS pod", Serial, newRandomVMIWithPVC, "nfs", k8sv1.IPv6Protocol, true), |
| 163 | + Entry("[Serial]with NFS Disk PVC using ipv4 address of the NFS pod not owned by qemu", Serial, newRandomVMIWithPVC, "nfs", k8sv1.IPv4Protocol, false), |
| 164 | ++ Entry("unless hostpath disk image file not owned by qemu", newRandomVMIWithPVC, false), |
| 165 | + ) |
| 166 | + }) |
| 167 | + |
| 168 | +-- |
| 169 | +2.43.0 |
| 170 | + |
0 commit comments