Skip to content

Commit 14f3839

Browse files
akalenyuclaude
andcommitted
Add unit testing for block volume hotplug allow list
Make necessary adaptations so unit tests are real, avoiding any direct os calls, which allows us to add unit tests covering VFIO, USB, hypervisor device, RNG, and VSOCK rule discovery. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Alex Kalenyuk <akalenyu@redhat.com>
1 parent 436dd88 commit 14f3839

4 files changed

Lines changed: 311 additions & 78 deletions

File tree

pkg/virt-handler/cgroup/BUILD.bazel

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,13 +39,14 @@ go_test(
3939
race = "on",
4040
deps = [
4141
"//pkg/safepath:go_default_library",
42-
"//pkg/virt-handler/isolation:go_default_library",
4342
"//staging/src/kubevirt.io/api/core/v1:go_default_library",
4443
"//staging/src/kubevirt.io/client-go/testutils:go_default_library",
4544
"//vendor/github.com/onsi/ginkgo/v2:go_default_library",
4645
"//vendor/github.com/onsi/gomega:go_default_library",
46+
"//vendor/github.com/onsi/gomega/gstruct:go_default_library",
4747
"//vendor/github.com/opencontainers/cgroups:go_default_library",
4848
"//vendor/github.com/opencontainers/cgroups/devices/config:go_default_library",
4949
"//vendor/go.uber.org/mock/gomock:go_default_library",
50+
"//vendor/golang.org/x/sys/unix:go_default_library",
5051
],
5152
)

pkg/virt-handler/cgroup/cgroup.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,12 @@ func NewManagerFromVM(vmi *v1.VirtualMachineInstance, host string, hypervisorDev
146146
return nil, err
147147
}
148148

149-
vmiDeviceRules, err := generateDeviceRulesForVMI(vmi, isolationRes, host, hypervisorDevice, allowEmulation)
149+
mountRoot, err := isolationRes.MountRoot()
150+
if err != nil {
151+
return nil, err
152+
}
153+
154+
vmiDeviceRules, err := generateDeviceRulesForVMI(vmi, mountRoot, host, hypervisorDevice, allowEmulation)
150155
if err != nil {
151156
return nil, err
152157
}

pkg/virt-handler/cgroup/cgroup_test.go

Lines changed: 257 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -20,21 +20,24 @@
2020
package cgroup
2121

2222
import (
23+
"io/fs"
2324
"os"
2425
"path"
25-
"path/filepath"
2626
"strings"
27+
"syscall"
28+
"time"
2729

2830
. "github.com/onsi/ginkgo/v2"
2931
. "github.com/onsi/gomega"
32+
. "github.com/onsi/gomega/gstruct"
3033
cgroups "github.com/opencontainers/cgroups"
3134
devices "github.com/opencontainers/cgroups/devices/config"
3235
"go.uber.org/mock/gomock"
36+
"golang.org/x/sys/unix"
3337

3438
v1 "kubevirt.io/api/core/v1"
3539

3640
"kubevirt.io/kubevirt/pkg/safepath"
37-
"kubevirt.io/kubevirt/pkg/virt-handler/isolation"
3841
)
3942

4043
var _ = Describe("cgroup manager", func() {
@@ -346,53 +349,285 @@ var _ = Describe("parseDevicesList", func() {
346349

347350
var _ = Describe("generateDeviceRulesForVMI", func() {
348351
var (
349-
ctrl *gomock.Controller
350-
tempDir string
352+
origStatDevice func(*safepath.Path, string) (os.FileInfo, error)
353+
origReadDevDir func(*safepath.Path, string) ([]os.DirEntry, error)
351354
)
352355

356+
noDevices := func(_ *safepath.Path, _ string) (os.FileInfo, error) {
357+
return nil, os.ErrNotExist
358+
}
359+
noDirs := func(_ *safepath.Path, _ string) ([]os.DirEntry, error) {
360+
return nil, os.ErrNotExist
361+
}
362+
353363
BeforeEach(func() {
354-
ctrl = gomock.NewController(GinkgoT())
355-
tempDir = GinkgoT().TempDir()
356-
Expect(os.MkdirAll(filepath.Join(tempDir, "dev"), 0755)).To(Succeed())
364+
origStatDevice = statDevice
365+
origReadDevDir = readDeviceDir
357366
})
358367

359-
newMockIsolationWithMountRoot := func() isolation.IsolationResult {
360-
mountRoot, err := safepath.NewPathNoFollow(tempDir)
361-
Expect(err).ToNot(HaveOccurred())
362-
363-
mockIso := isolation.NewMockIsolationResult(ctrl)
364-
mockIso.EXPECT().MountRoot().Return(mountRoot, nil)
365-
return mockIso
366-
}
368+
AfterEach(func() {
369+
statDevice = origStatDevice
370+
readDeviceDir = origReadDevDir
371+
})
367372

368373
It("should skip hypervisor device rule when emulation is allowed and device is missing", func() {
369-
rules, err := generateDeviceRulesForVMI(&v1.VirtualMachineInstance{}, newMockIsolationWithMountRoot(), "", "kvm", true)
374+
statDevice = noDevices
375+
readDeviceDir = noDirs
376+
377+
rules, err := generateDeviceRulesForVMI(&v1.VirtualMachineInstance{}, nil, "", "kvm", true)
370378
Expect(err).ToNot(HaveOccurred())
371379
Expect(rules).To(BeEmpty())
372380
})
373381

374382
It("should fail when hypervisor device is missing and emulation is not allowed", func() {
375-
_, err := generateDeviceRulesForVMI(&v1.VirtualMachineInstance{}, newMockIsolationWithMountRoot(), "", "kvm", false)
383+
statDevice = noDevices
384+
readDeviceDir = noDirs
385+
386+
_, err := generateDeviceRulesForVMI(&v1.VirtualMachineInstance{}, nil, "", "kvm", false)
376387
Expect(err).To(HaveOccurred())
377388
})
378389

390+
It("should create a rule for the hypervisor device", func() {
391+
statDevice = func(_ *safepath.Path, relPath string) (os.FileInfo, error) {
392+
if relPath == "/dev/kvm" {
393+
return charDeviceInfo(10, 232), nil
394+
}
395+
return nil, os.ErrNotExist
396+
}
397+
readDeviceDir = noDirs
398+
399+
rules, err := generateDeviceRulesForVMI(&v1.VirtualMachineInstance{}, nil, "", "kvm", false)
400+
Expect(err).ToNot(HaveOccurred())
401+
Expect(rules).To(ConsistOf(
402+
PointTo(MatchFields(IgnoreExtras, Fields{
403+
"Type": Equal(devices.CharDevice), "Major": Equal(int64(10)), "Minor": Equal(int64(232)),
404+
})),
405+
))
406+
})
407+
408+
It("should discover VFIO device nodes", func() {
409+
statDevice = func(_ *safepath.Path, relPath string) (os.FileInfo, error) {
410+
switch relPath {
411+
case "/dev/vfio/vfio":
412+
return charDeviceInfo(10, 196), nil
413+
case "/dev/vfio/42":
414+
return charDeviceInfo(243, 0), nil
415+
default:
416+
return nil, os.ErrNotExist
417+
}
418+
}
419+
readDeviceDir = func(_ *safepath.Path, relPath string) ([]os.DirEntry, error) {
420+
if relPath == "/dev/vfio" {
421+
return []os.DirEntry{
422+
&fakeDirEntry{name: "vfio"},
423+
&fakeDirEntry{name: "42"},
424+
}, nil
425+
}
426+
return nil, os.ErrNotExist
427+
}
428+
429+
rules, err := generateDeviceRulesForVMI(&v1.VirtualMachineInstance{}, nil, "", "kvm", true)
430+
Expect(err).ToNot(HaveOccurred())
431+
Expect(rules).To(ConsistOf(
432+
PointTo(MatchFields(IgnoreExtras, Fields{
433+
"Type": Equal(devices.CharDevice), "Major": Equal(int64(10)), "Minor": Equal(int64(196)),
434+
})),
435+
PointTo(MatchFields(IgnoreExtras, Fields{
436+
"Type": Equal(devices.CharDevice), "Major": Equal(int64(243)), "Minor": Equal(int64(0)),
437+
})),
438+
))
439+
})
440+
441+
It("should discover USB device nodes in nested directories", func() {
442+
statDevice = func(_ *safepath.Path, relPath string) (os.FileInfo, error) {
443+
switch relPath {
444+
case "/dev/bus/usb/001/001":
445+
return charDeviceInfo(189, 0), nil
446+
case "/dev/bus/usb/001/002":
447+
return charDeviceInfo(189, 1), nil
448+
default:
449+
return nil, os.ErrNotExist
450+
}
451+
}
452+
readDeviceDir = func(_ *safepath.Path, relPath string) ([]os.DirEntry, error) {
453+
switch relPath {
454+
case "/dev/bus/usb":
455+
return []os.DirEntry{&fakeDirEntry{name: "001", isDir: true}}, nil
456+
case "/dev/bus/usb/001":
457+
return []os.DirEntry{
458+
&fakeDirEntry{name: "001"},
459+
&fakeDirEntry{name: "002"},
460+
}, nil
461+
default:
462+
return nil, os.ErrNotExist
463+
}
464+
}
465+
466+
rules, err := generateDeviceRulesForVMI(&v1.VirtualMachineInstance{}, nil, "", "kvm", true)
467+
Expect(err).ToNot(HaveOccurred())
468+
Expect(rules).To(ConsistOf(
469+
PointTo(MatchFields(IgnoreExtras, Fields{
470+
"Type": Equal(devices.CharDevice), "Major": Equal(int64(189)), "Minor": Equal(int64(0)),
471+
})),
472+
PointTo(MatchFields(IgnoreExtras, Fields{
473+
"Type": Equal(devices.CharDevice), "Major": Equal(int64(189)), "Minor": Equal(int64(1)),
474+
})),
475+
))
476+
})
477+
478+
It("should discover devices from both VFIO and USB", func() {
479+
statDevice = func(_ *safepath.Path, relPath string) (os.FileInfo, error) {
480+
switch relPath {
481+
case "/dev/vfio/vfio":
482+
return charDeviceInfo(10, 196), nil
483+
case "/dev/vfio/0":
484+
return charDeviceInfo(243, 0), nil
485+
case "/dev/bus/usb/001/001":
486+
return charDeviceInfo(189, 0), nil
487+
case "/dev/bus/usb/002/001":
488+
return charDeviceInfo(189, 128), nil
489+
default:
490+
return nil, os.ErrNotExist
491+
}
492+
}
493+
readDeviceDir = func(_ *safepath.Path, relPath string) ([]os.DirEntry, error) {
494+
switch relPath {
495+
case "/dev/vfio":
496+
return []os.DirEntry{
497+
&fakeDirEntry{name: "vfio"},
498+
&fakeDirEntry{name: "0"},
499+
}, nil
500+
case "/dev/bus/usb":
501+
return []os.DirEntry{
502+
&fakeDirEntry{name: "001", isDir: true},
503+
&fakeDirEntry{name: "002", isDir: true},
504+
}, nil
505+
case "/dev/bus/usb/001":
506+
return []os.DirEntry{&fakeDirEntry{name: "001"}}, nil
507+
case "/dev/bus/usb/002":
508+
return []os.DirEntry{&fakeDirEntry{name: "001"}}, nil
509+
default:
510+
return nil, os.ErrNotExist
511+
}
512+
}
513+
514+
rules, err := generateDeviceRulesForVMI(&v1.VirtualMachineInstance{}, nil, "", "kvm", true)
515+
Expect(err).ToNot(HaveOccurred())
516+
Expect(rules).To(HaveLen(4))
517+
})
518+
519+
It("should create a rule for urandom when RNG is enabled", func() {
520+
statDevice = func(_ *safepath.Path, relPath string) (os.FileInfo, error) {
521+
if relPath == "/dev/urandom" {
522+
return charDeviceInfo(1, 9), nil
523+
}
524+
return nil, os.ErrNotExist
525+
}
526+
readDeviceDir = noDirs
527+
528+
vmi := &v1.VirtualMachineInstance{}
529+
vmi.Spec.Domain.Devices.Rng = &v1.Rng{}
530+
531+
rules, err := generateDeviceRulesForVMI(vmi, nil, "", "kvm", true)
532+
Expect(err).ToNot(HaveOccurred())
533+
Expect(rules).To(ConsistOf(
534+
PointTo(MatchFields(IgnoreExtras, Fields{
535+
"Type": Equal(devices.CharDevice), "Major": Equal(int64(1)), "Minor": Equal(int64(9)),
536+
})),
537+
))
538+
})
539+
540+
It("should create a rule for vhost-vsock when AutoattachVSOCK is enabled", func() {
541+
statDevice = func(_ *safepath.Path, relPath string) (os.FileInfo, error) {
542+
if relPath == "/dev/vhost-vsock" {
543+
return charDeviceInfo(10, 241), nil
544+
}
545+
return nil, os.ErrNotExist
546+
}
547+
readDeviceDir = noDirs
548+
549+
autoAttach := true
550+
vmi := &v1.VirtualMachineInstance{}
551+
vmi.Spec.Domain.Devices.AutoattachVSOCK = &autoAttach
552+
553+
rules, err := generateDeviceRulesForVMI(vmi, nil, "", "kvm", true)
554+
Expect(err).ToNot(HaveOccurred())
555+
Expect(rules).To(ConsistOf(
556+
PointTo(MatchFields(IgnoreExtras, Fields{
557+
"Type": Equal(devices.CharDevice), "Major": Equal(int64(10)), "Minor": Equal(int64(241)),
558+
})),
559+
))
560+
})
561+
379562
It("should not fail when /dev/vfio does not exist", func() {
380-
rules, err := generateDeviceRulesForVMI(&v1.VirtualMachineInstance{}, newMockIsolationWithMountRoot(), "", "kvm", true)
563+
statDevice = noDevices
564+
readDeviceDir = noDirs
565+
566+
rules, err := generateDeviceRulesForVMI(&v1.VirtualMachineInstance{}, nil, "", "kvm", true)
381567
Expect(err).ToNot(HaveOccurred())
382568
Expect(rules).To(BeEmpty())
383569
})
384570

385571
It("should not fail when /dev/vfio exists but is empty", func() {
386-
Expect(os.MkdirAll(filepath.Join(tempDir, "dev", "vfio"), 0755)).To(Succeed())
387-
rules, err := generateDeviceRulesForVMI(&v1.VirtualMachineInstance{}, newMockIsolationWithMountRoot(), "", "kvm", true)
572+
statDevice = noDevices
573+
readDeviceDir = func(_ *safepath.Path, relPath string) ([]os.DirEntry, error) {
574+
if relPath == "/dev/vfio" {
575+
return nil, nil
576+
}
577+
return nil, os.ErrNotExist
578+
}
579+
580+
rules, err := generateDeviceRulesForVMI(&v1.VirtualMachineInstance{}, nil, "", "kvm", true)
388581
Expect(err).ToNot(HaveOccurred())
389582
Expect(rules).To(BeEmpty())
390583
})
391584

392585
It("should not fail when /dev/bus/usb exists but is empty", func() {
393-
Expect(os.MkdirAll(filepath.Join(tempDir, "dev", "bus", "usb"), 0755)).To(Succeed())
394-
rules, err := generateDeviceRulesForVMI(&v1.VirtualMachineInstance{}, newMockIsolationWithMountRoot(), "", "kvm", true)
586+
statDevice = noDevices
587+
readDeviceDir = func(_ *safepath.Path, relPath string) ([]os.DirEntry, error) {
588+
if relPath == "/dev/bus/usb" {
589+
return nil, nil
590+
}
591+
return nil, os.ErrNotExist
592+
}
593+
594+
rules, err := generateDeviceRulesForVMI(&v1.VirtualMachineInstance{}, nil, "", "kvm", true)
395595
Expect(err).ToNot(HaveOccurred())
396596
Expect(rules).To(BeEmpty())
397597
})
398598
})
599+
600+
func charDeviceInfo(major, minor uint32) os.FileInfo {
601+
return &fakeFileInfo{
602+
mode: os.ModeDevice | os.ModeCharDevice,
603+
rdev: unix.Mkdev(major, minor),
604+
}
605+
}
606+
607+
type fakeFileInfo struct {
608+
name string
609+
mode os.FileMode
610+
rdev uint64
611+
}
612+
613+
func (f *fakeFileInfo) Name() string { return f.name }
614+
func (f *fakeFileInfo) Size() int64 { return 0 }
615+
func (f *fakeFileInfo) Mode() os.FileMode { return f.mode }
616+
func (f *fakeFileInfo) ModTime() time.Time { return time.Time{} }
617+
func (f *fakeFileInfo) IsDir() bool { return f.mode.IsDir() }
618+
func (f *fakeFileInfo) Sys() interface{} { return &syscall.Stat_t{Rdev: f.rdev} }
619+
620+
type fakeDirEntry struct {
621+
name string
622+
isDir bool
623+
}
624+
625+
func (e *fakeDirEntry) Name() string { return e.name }
626+
func (e *fakeDirEntry) IsDir() bool { return e.isDir }
627+
func (e *fakeDirEntry) Type() fs.FileMode {
628+
if e.isDir {
629+
return fs.ModeDir
630+
}
631+
return 0
632+
}
633+
func (e *fakeDirEntry) Info() (fs.FileInfo, error) { return nil, nil }

0 commit comments

Comments
 (0)