diff --git a/staging/src/k8s.io/dynamic-resource-allocation/deviceattribute/machine.go b/staging/src/k8s.io/dynamic-resource-allocation/deviceattribute/machine.go new file mode 100644 index 00000000000..342325a2e10 --- /dev/null +++ b/staging/src/k8s.io/dynamic-resource-allocation/deviceattribute/machine.go @@ -0,0 +1,47 @@ +/* +Copyright The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package deviceattribute + +import ( + "io/fs" + "os" +) + +// machine is the bare minimum machine/host abstraction we need. +// we use a wrapping struct to enable the functional option pattern +type machine struct { + sysfs fs.ReadLinkFS +} + +// MachineModifier is the type of functional options +type MachineModifier func(*machine) + +// WithFS replaces the host sysfs with a custom FS +func WithFS(sysfs fs.ReadLinkFS) MachineModifier { + return func(mc *machine) { + mc.sysfs = sysfs + } +} + +// WithFSFromRoot replaces the host sysfs with a FS whose root +// is the provided path. Like chroot, this is for convenience +// and testability and for usage within containers, not for security. +func WithFSFromRoot(root string) MachineModifier { + return func(mc *machine) { + mc.sysfs = os.DirFS(root).(fs.ReadLinkFS) + } +} diff --git a/staging/src/k8s.io/dynamic-resource-allocation/deviceattribute/sysfs_linux.go b/staging/src/k8s.io/dynamic-resource-allocation/deviceattribute/machine_linux.go similarity index 55% rename from staging/src/k8s.io/dynamic-resource-allocation/deviceattribute/sysfs_linux.go rename to staging/src/k8s.io/dynamic-resource-allocation/deviceattribute/machine_linux.go index 8baa3154496..3054b2796c4 100644 --- a/staging/src/k8s.io/dynamic-resource-allocation/deviceattribute/sysfs_linux.go +++ b/staging/src/k8s.io/dynamic-resource-allocation/deviceattribute/machine_linux.go @@ -16,25 +16,15 @@ limitations under the License. package deviceattribute -import "path/filepath" +import ( + "io/fs" + "os" +) const ( - sysfsRoot = "/sys" + SysfsRoot = "/sys" ) -var ( - sysfs sysfsPath = sysfsPath(sysfsRoot) -) - -// sysfsPath provides methods to construct sysfs paths for various subsystems. -// It is used to abstract the sysfs path construction -// and can be replaced with a mock in tests. -type sysfsPath string - -func (s sysfsPath) devices(path string) string { - return filepath.Join(string(s), "devices", path) -} - -func (s sysfsPath) bus(path string) string { - return filepath.Join(string(s), "bus", path) +func initDefaultMachine(mc *machine) { + mc.sysfs = os.DirFS(SysfsRoot).(fs.ReadLinkFS) } diff --git a/staging/src/k8s.io/dynamic-resource-allocation/deviceattribute/machine_test.go b/staging/src/k8s.io/dynamic-resource-allocation/deviceattribute/machine_test.go new file mode 100644 index 00000000000..289037d9988 --- /dev/null +++ b/staging/src/k8s.io/dynamic-resource-allocation/deviceattribute/machine_test.go @@ -0,0 +1,68 @@ +/* +Copyright The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package deviceattribute + +import ( + "io/fs" + "path/filepath" + "testing" + "testing/fstest" +) + +func TestWithFS(t *testing.T) { + mc := machine{} + path := "dra.test" + content := "DRA deviceattribute" + mode := fs.FileMode(0o640) // any random mode pattern which is not the default + fakefs := fstest.MapFS{ + path: &fstest.MapFile{ + Data: []byte(content), + Mode: mode, + }, + } + + mod := WithFS(fakefs) + mod(&mc) + + got, err := mc.sysfs.Lstat(path) + if err != nil { + t.Fatalf("cannot Lstat %q: %v", path, err) + } + if got.Size() != int64(len(content)) { + t.Fatalf("did not found content: got %v expected %v", got.Size(), content) + } + if got.Mode() != mode { + t.Fatalf("found mismatched file mode: got %v expected %v", got.Mode(), mode) + } +} + +func TestWithFSFromRoot(t *testing.T) { + mc := machine{} + // we are gonna using real files from sysfs pseudofs, but we are using files which + // are standard since pretty much the inception of sysfs + mod := WithFSFromRoot("/sys") + mod(&mc) + + srcPath := filepath.Join("devices", "system", "cpu", "online") + got, err := fs.ReadFile(mc.sysfs, srcPath) + if err != nil { + t.Fatalf("cannot read %q: %v", srcPath, err) + } + if len(got) == 0 { + t.Fatalf("empty content in %q", srcPath) + } +} diff --git a/staging/src/k8s.io/dynamic-resource-allocation/deviceattribute/pci_linux.go b/staging/src/k8s.io/dynamic-resource-allocation/deviceattribute/pci_linux.go index 5576f7d43dd..b43a4fee0b2 100644 --- a/staging/src/k8s.io/dynamic-resource-allocation/deviceattribute/pci_linux.go +++ b/staging/src/k8s.io/dynamic-resource-allocation/deviceattribute/pci_linux.go @@ -18,7 +18,7 @@ package deviceattribute import ( "fmt" - "os" + "io/fs" "path/filepath" "regexp" "strings" @@ -33,12 +33,18 @@ import ( // as a string value or an error if the PCI Bus ID is invalid or the root complex cannot be determined. // // ref: https://wiki.xenproject.org/wiki/Bus:Device.Function_(BDF)_Notation -func GetPCIeRootAttributeByPCIBusID(pciBusID string) (DeviceAttribute, error) { +func GetPCIeRootAttributeByPCIBusID(pciBusID string, mods ...MachineModifier) (DeviceAttribute, error) { + var mc machine + initDefaultMachine(&mc) + for _, mod := range mods { + mod(&mc) + } + if err := verifyPCIBDFFormat(pciBusID); err != nil { return DeviceAttribute{}, err } - pcieRoot, err := resolvePCIeRoot(pciBusID) + pcieRoot, err := resolvePCIeRoot(mc, pciBusID) if err != nil { return DeviceAttribute{}, fmt.Errorf("failed to resolve PCIe Root Complex for PCI Bus ID %s: %w", pciBusID, err) } @@ -67,22 +73,22 @@ func GetPCIeRootAttributeByPCIBusID(pciBusID string) (DeviceAttribute, error) { // /sys/bus/pci/devices/0000:04:1f.0 points to // /sys/devices/pci0000:00/....../0000:04:1f.0, // where "pci0000:00" is the PCIe Root. -func resolvePCIeRoot(pciBusID string) (string, error) { +func resolvePCIeRoot(mc machine, pciBusID string) (string, error) { // e.g. /sys/bus/pci/devices/0000:04:1f.0 - sysBusPath := sysfs.bus(filepath.Join("pci", "devices", pciBusID)) + sysBusPath := filepath.Join("bus", "pci", "devices", pciBusID) - target, err := os.Readlink(sysBusPath) + target, err := fs.ReadLink(mc.sysfs, sysBusPath) if err != nil { return "", fmt.Errorf("failed to read symlink for PCI Bus ID %s: %w", sysBusPath, err) } // If the target is a relative path, we need to resolve it relative to the symlink's directory. if !filepath.IsAbs(target) { - target = filepath.Join(filepath.Dir(sysBusPath), target) + target = filepath.Clean(filepath.Join(filepath.Dir(sysBusPath), target)) } - // targetAbs must be /sys/devices/pci0000:00/....../0000:04:1f.0 - devicePathPrefix := sysfs.devices("pci") + // Once the symlink was resolved, we must have a path like /sys/devices/pci0000:00/....../0000:04:1f.0 + devicePathPrefix := filepath.Join("devices", "pci") if !strings.HasPrefix(target, devicePathPrefix) { return "", fmt.Errorf("symlink target for PCI Bus ID %s is invalid: it must start with %s: %s", pciBusID, devicePathPrefix, target) } @@ -91,9 +97,9 @@ func resolvePCIeRoot(pciBusID string) (string, error) { } // We need to extract the PCIe Root part, which is the first part of the path after /sys/devices/. - pcieRootPart := strings.Split(strings.TrimPrefix(target, sysfs.devices("")+"/"), "/")[0] - - return pcieRootPart, nil + target = strings.TrimPrefix(target, "devices"+string(filepath.Separator)) + pcieRootParts := strings.Split(target, string(filepath.Separator)) + return pcieRootParts[0], nil } // GetPCIBusIDAttribute returns a DeviceAttribute with the PCI Bus Address("::.") diff --git a/staging/src/k8s.io/dynamic-resource-allocation/deviceattribute/pci_linux_test.go b/staging/src/k8s.io/dynamic-resource-allocation/deviceattribute/pci_linux_test.go index 0af01530787..947a280a6e2 100644 --- a/staging/src/k8s.io/dynamic-resource-allocation/deviceattribute/pci_linux_test.go +++ b/staging/src/k8s.io/dynamic-resource-allocation/deviceattribute/pci_linux_test.go @@ -39,18 +39,22 @@ func TestGetPCIeRootAttributePCIBusID(t *testing.T) { } tests := map[string]struct { - mockSysfsSetup func(t *testing.T, mockSysfs sysfsPath) + testMachineSetup func(t *testing.T, testRootPath string) address string expectedAttribute *DeviceAttribute expectsError bool expectedErrMsg string }{ "valid": { - mockSysfsSetup: func(t *testing.T, mockSysfs sysfsPath) { - devicePath := mockSysfs.devices(filepath.Join(pcieRoot, intermediateBusID, pciBusID)) - touchFile(t, devicePath) - busPath := mockSysfs.bus(filepath.Join("pci", "devices", pciBusID)) - createSymlink(t, devicePath, busPath) + testMachineSetup: func(t *testing.T, testRootPath string) { + relDevicePath := filepath.Join("devices", pcieRoot, intermediateBusID, pciBusID) + touchFile(t, filepath.Join(testRootPath, relDevicePath)) + busDir := filepath.Join(testRootPath, "bus", "pci", "devices") + mkDirAll(t, busDir) + createSymlink(t, + filepath.Join("..", "..", "..", relDevicePath), + filepath.Join(busDir, pciBusID), + ) }, address: pciBusID, expectedAttribute: &expectedAttribute, @@ -75,11 +79,15 @@ func TestGetPCIeRootAttributePCIBusID(t *testing.T) { expectedErrMsg: "no such file or directory", }, "invalid symlink (invalid prefix)": { - mockSysfsSetup: func(t *testing.T, mockSysfs sysfsPath) { - devicePath := mockSysfs.devices(filepath.Join("invalid-pci-root", intermediateBusID, pciBusID)) - touchFile(t, devicePath) - busPath := mockSysfs.bus(filepath.Join("pci", "devices", pciBusID)) - createSymlink(t, devicePath, busPath) + testMachineSetup: func(t *testing.T, testRootPath string) { + relDevicePath := filepath.Join("devices", "invalid-pci-root", intermediateBusID, pciBusID) + touchFile(t, filepath.Join(testRootPath, relDevicePath)) + busDir := filepath.Join(testRootPath, "bus", "pci", "devices") + mkDirAll(t, busDir) + createSymlink(t, + filepath.Join("..", "..", "..", relDevicePath), + filepath.Join(busDir, pciBusID), + ) }, address: pciBusID, expectedAttribute: nil, @@ -87,11 +95,15 @@ func TestGetPCIeRootAttributePCIBusID(t *testing.T) { expectedErrMsg: fmt.Sprintf("symlink target for PCI Bus ID %s is invalid: it must start with", pciBusID), }, "invalid symlink (invalid suffix)": { - mockSysfsSetup: func(t *testing.T, mockSysfs sysfsPath) { - devicePath := mockSysfs.devices(filepath.Join(pcieRoot, intermediateBusID, "0000:00:13.1")) // different PCI Bus ID - touchFile(t, devicePath) - busPath := mockSysfs.bus(filepath.Join("pci", "devices", pciBusID)) - createSymlink(t, devicePath, busPath) + testMachineSetup: func(t *testing.T, testRootPath string) { + relDevicePath := filepath.Join("devices", pcieRoot, intermediateBusID, "0000:00:13.1") // different PCI Bus ID + touchFile(t, filepath.Join(testRootPath, relDevicePath)) + busDir := filepath.Join(testRootPath, "bus", "pci", "devices") + mkDirAll(t, busDir) + createSymlink(t, + filepath.Join("..", "..", "..", relDevicePath), + filepath.Join(busDir, pciBusID), + ) }, address: pciBusID, expectedAttribute: nil, @@ -101,20 +113,12 @@ func TestGetPCIeRootAttributePCIBusID(t *testing.T) { } for name, test := range tests { t.Run(name, func(t *testing.T) { - mockSysfsPath := t.TempDir() - mockSysfs := sysfsPath(mockSysfsPath) - if test.mockSysfsSetup != nil { - test.mockSysfsSetup(t, mockSysfs) + // per docs, the testing package ensures cleanup, so no need to do that ourselves + testMachinePath := t.TempDir() + if test.testMachineSetup != nil { + test.testMachineSetup(t, testMachinePath) } - sysfs = mockSysfs - t.Cleanup(func() { - sysfs = sysfsPath(sysfsRoot) - if err := os.RemoveAll(mockSysfsPath); err != nil { - t.Errorf("Failed to clean up mock sysfs path %s: %v", mockSysfsPath, err) - } - }) - - got, err := GetPCIeRootAttributeByPCIBusID(test.address) + got, err := GetPCIeRootAttributeByPCIBusID(test.address, WithFSFromRoot(testMachinePath)) if test.expectsError { if err == nil { t.Errorf("Expected error but got none") @@ -191,11 +195,16 @@ func TestGetPCIBusIDAttribute(t *testing.T) { } } +func mkDirAll(t *testing.T, path string) { + t.Helper() + if err := os.MkdirAll(path, 0755); err != nil { + t.Fatalf("Failed to create directory %s: %v", path, err) + } +} + func touchFile(t *testing.T, path string) { t.Helper() - if err := os.MkdirAll(filepath.Dir(path), 0755); err != nil { - t.Fatalf("Failed to create directory %s: %v", filepath.Dir(path), err) - } + mkDirAll(t, filepath.Dir(path)) if _, err := os.Create(path); err != nil { t.Fatalf("Failed to create file %s: %v", path, err) } @@ -203,9 +212,6 @@ func touchFile(t *testing.T, path string) { func createSymlink(t *testing.T, target, link string) { t.Helper() - if err := os.MkdirAll(filepath.Dir(link), 0755); err != nil { - t.Fatalf("Failed to create directory for symlink %s: %v", filepath.Dir(link), err) - } if err := os.Symlink(target, link); err != nil { t.Fatalf("Failed to create symlink from %s to %s: %v", target, link, err) }