Revert "Add StandardDeviceAttributes and StandardPCIDeviceAttributes helper functions"

This reverts commit 2869b6f42c.
This commit is contained in:
Shingo Omura
2025-06-18 21:04:16 +09:00
parent 2869b6f42c
commit f7461a12dc
5 changed files with 62 additions and 277 deletions

View File

@@ -1,52 +0,0 @@
/*
Copyright 2025 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 resourceapi "k8s.io/dynamic-resource-allocation/api"
type StandardDeviceAttributesOption func(args *StandardDeviceAttributesOptions)
// WithStandardPCIDeviceAttributesOpts sets the standard PCI device attributes options
func WithStandardPCIDeviceAttributesOpts(opts ...StandardPCIDeviceAttributesOption) StandardDeviceAttributesOption {
return func(args *StandardDeviceAttributesOptions) {
args.pciDeviceAttributeOpts = append(args.pciDeviceAttributeOpts, opts...)
}
}
type StandardDeviceAttributesOptions struct {
pciDeviceAttributeOpts []StandardPCIDeviceAttributesOption
}
// StandardDeviceAttributes generates standard device attributes based on the provided options.
// It currently supports standard PCI device attributes if options are provided.
func StandardDeviceAttributes(
opts ...StandardDeviceAttributesOption,
) (map[resourceapi.QualifiedName]resourceapi.DeviceAttribute, error) {
options := &StandardDeviceAttributesOptions{}
for _, opt := range opts {
opt(options)
}
attrs := map[resourceapi.QualifiedName]resourceapi.DeviceAttribute{}
if len(options.pciDeviceAttributeOpts) > 0 {
// Standard PCI device attributes
pciAttrs, err := StandardPCIDeviceAttributes(options.pciDeviceAttributeOpts...)
if err != nil {
return nil, err
}
for k, v := range pciAttrs {
attrs[k] = v
}
}
return attrs, nil
}

View File

@@ -1,112 +0,0 @@
/*
Copyright 2025 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 (
"path/filepath"
"reflect"
"strings"
"testing"
"k8s.io/utils/ptr"
resourceapi "k8s.io/dynamic-resource-allocation/api"
)
func TestAddStandardDeviceAttributes(t *testing.T) {
testPCIBusID := "0123:45:1e.7"
testACIIntermediateBusID := "0123:46:7.1"
testPCIRoot := "pci0123:56"
tests := map[string]struct {
mockSysfsSetup func(t *testing.T, mockSysfs sysfs)
opts []StandardDeviceAttributesOption
expectedAttrs map[resourceapi.QualifiedName]resourceapi.DeviceAttribute
expectsError bool
expectedErrMsg string
}{
"valid nil attrs": {
opts: nil,
expectedAttrs: map[resourceapi.QualifiedName]resourceapi.DeviceAttribute{},
},
"valid empty attrs": {
opts: []StandardDeviceAttributesOption{},
expectedAttrs: map[resourceapi.QualifiedName]resourceapi.DeviceAttribute{},
},
"valid empty pci attrs": {
opts: []StandardDeviceAttributesOption{WithStandardPCIDeviceAttributesOpts()},
expectedAttrs: map[resourceapi.QualifiedName]resourceapi.DeviceAttribute{},
},
"valid pci attrs": {
mockSysfsSetup: func(t *testing.T, mockSysfs sysfs) {
// /sys/bus/pci/devices/0123:45:1e.7 --> /sys/devices/pci0123:56/0123:46:7.1/10123:45:1e.7
testPCIDevicePath := mockSysfs.Devices(filepath.Join(testPCIRoot, testACIIntermediateBusID, testPCIBusID))
TouchFile(t, testPCIDevicePath)
testPCIBusPath := mockSysfs.Bus(filepath.Join("pci", "devices", testPCIBusID))
CreateSymlink(t, testPCIDevicePath, testPCIBusPath)
},
opts: []StandardDeviceAttributesOption{
WithStandardPCIDeviceAttributesOpts(
WithPCIDeviceAddress(testPCIBusID),
),
},
expectedAttrs: map[resourceapi.QualifiedName]resourceapi.DeviceAttribute{
qualifiedNamePCIeRoot: {
StringValue: ptr.To(testPCIRoot),
},
},
},
"invalid empty pciBusID": {
opts: []StandardDeviceAttributesOption{
WithStandardPCIDeviceAttributesOpts(
WithPCIDeviceAddress(""),
),
},
expectsError: true,
expectedErrMsg: "PCI Bus ID cannot be empty",
},
}
for name, test := range tests {
t.Run(name, func(t *testing.T) {
mockSysfsPath := t.TempDir()
mockSysfs := sysfs(mockSysfsPath)
if test.mockSysfsSetup != nil {
test.mockSysfsSetup(t, mockSysfs)
test.opts = append(test.opts, WithStandardPCIDeviceAttributesOpts(withSysfs(mockSysfs)))
}
attrs, err := StandardDeviceAttributes(test.opts...)
if test.expectsError {
if err == nil {
t.Errorf("expected error but got nil")
return
}
if !strings.Contains(err.Error(), test.expectedErrMsg) {
t.Errorf("expected error message to contain %q, got %q", test.expectedErrMsg, err.Error())
return
}
return
}
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if !reflect.DeepEqual(attrs, test.expectedAttrs) {
t.Errorf("expected attributes %v, got %v", test.expectedAttrs, attrs)
return
}
})
}
}

View File

@@ -28,66 +28,16 @@ import (
var (
bdfRegexp = regexp.MustCompile(`^([0-9a-f]{4}):([0-9a-f]{2}):([0-9a-f]{2})\.([0-9a-f]{1})$`)
qualifiedNamePCIeRoot = resourceapi.QualifiedName(resourceapi.StandardDeviceAttributePCIeRoot)
)
type StandardPCIDeviceAttributesOption func(args *StandardPCIDeviceAttributeArgs)
// WithPCIDeviceAddress sets the PCI address for the PCI Bus ID
// GetPCIeRootAttributeByPCIBusID retrieves the PCIe Root Complex for a given PCI Bus ID.
// in BDF (Bus-Device-Function) format, e.g., "0123:45:1e.7".
//
// It returns a DeviceAttribute with the PCIe Root Complex information("pci<domain>:<bus>")
// 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 WithPCIDeviceAddress(pciBusID string) StandardPCIDeviceAttributesOption {
return func(args *StandardPCIDeviceAttributeArgs) {
args.pciAddress = pciBusID
}
}
// StandardPCIDeviceAttributeArgs holds the arguments for generating standard PCI device attributes.
type StandardPCIDeviceAttributeArgs struct {
pciAddress string
sysfs sysfs
}
// StandardPCIDeviceAttributes returns standard device attributes for a PCI device.
func StandardPCIDeviceAttributes(opts ...StandardPCIDeviceAttributesOption) (map[resourceapi.QualifiedName]resourceapi.DeviceAttribute, error) {
args := &StandardPCIDeviceAttributeArgs{
sysfs: sysfs(defaultSysfsRoot),
}
for _, opt := range opts {
opt(args)
}
attrs := make(map[resourceapi.QualifiedName]resourceapi.DeviceAttribute)
pciRootAttribute, err := getPCIeRootAttributeByPCIBusID(args.pciAddress, args.sysfs)
if err != nil {
return nil, err
}
attrs[qualifiedNamePCIeRoot] = pciRootAttribute
return attrs, nil
}
// This is only for testing purposes.
func withSysfs(sysfs sysfs) StandardPCIDeviceAttributesOption {
return func(args *StandardPCIDeviceAttributeArgs) {
args.sysfs = sysfs
}
}
// GetPCIeRootAttributeByPCI Bus ID retrieves the PCIe Root Complex for a given PCI Bus ID.
// in BDF (Bus-Device-Function) format, e.g., "0123:45:1e.7".
//
// It returns a DeviceAttribute with the PCIe Root Complex information("pci<domain>:<bus>")
// 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, sysfs sysfs) (resourceapi.DeviceAttribute, error) {
func GetPCIeRootAttributeByPCIBusID(pciBusID string) (resourceapi.DeviceAttribute, error) {
if pciBusID == "" {
return resourceapi.DeviceAttribute{}, fmt.Errorf("PCI Bus ID cannot be empty")
}
@@ -97,7 +47,7 @@ func getPCIeRootAttributeByPCIBusID(pciBusID string, sysfs sysfs) (resourceapi.D
}
// e.g. /sys/devices/pci0000:01/...<intermediate PCI devices>.../0000:00:1f.0,
sysDevicesPath, err := resolveSysDevicesPath(pciBusID, sysfs)
sysDevicesPath, err := resolveSysDevicesPath(pciBusID)
if err != nil {
return resourceapi.DeviceAttribute{}, fmt.Errorf("failed to resolve sysfs path for PCI Bus ID %s: %w", pciBusID, err)
}
@@ -123,7 +73,7 @@ func getPCIeRootAttributeByPCIBusID(pciBusID string, sysfs sysfs) (resourceapi.D
// For example, if the PCIAddress is "0000:00:1f.0",
// /sys/bus/pci/devices/0000:00:1f.0 points to
// /sys/devices/pci0000:01/...<intermediate PCI devices>.../0000:00:1f.0,
func resolveSysDevicesPath(pciBusID string, sysfs sysfs) (string, error) {
func resolveSysDevicesPath(pciBusID string) (string, error) {
// e.g. /sys/bus/pci/devices/0000:00:1f.0
sysBusPath := sysfs.Bus(filepath.Join("pci", "devices", pciBusID))

View File

@@ -28,83 +28,78 @@ import (
resourceapi "k8s.io/dynamic-resource-allocation/api"
)
func TestStandardPCIDeviceAttributes(t *testing.T) {
func TestGetPCIeRootBAttributeyPCIBusID(t *testing.T) {
pciBusID := "0000:01:02.3"
pciIntermediateBusID := "0000:00:13.1"
pcieRoot := "pci0000:01"
expectedAttribute := &resourceapi.DeviceAttribute{
StringValue: ptr.To(pcieRoot),
}
tests := map[string]struct {
smockSysfsSetup func(t *testing.T, mockSysfs sysfs)
opts []StandardPCIDeviceAttributesOption
expected map[resourceapi.QualifiedName]resourceapi.DeviceAttribute
expectsError bool
expectedErrMsg string
smockSysfsSetup func(t *testing.T, mockSysfs sysfsPath)
address string
expectedAttribute *resourceapi.DeviceAttribute
expectsError bool
expectedErrMsg string
}{
"valid": {
smockSysfsSetup: func(t *testing.T, mockSysfs sysfs) {
devicePath := mockSysfs.Devices(filepath.Join(pcieRoot, pciIntermediateBusID, pciBusID))
smockSysfsSetup: func(t *testing.T, mockSysfs sysfsPath) {
devicePath := mockSysfs.Devices(filepath.Join(pcieRoot, "0000:00:13.1", pciBusID))
TouchFile(t, devicePath)
busPath := mockSysfs.Bus(filepath.Join("pci", "devices", pciBusID))
CreateSymlink(t, devicePath, busPath)
},
opts: []StandardPCIDeviceAttributesOption{
WithPCIDeviceAddress(pciBusID),
},
expected: map[resourceapi.QualifiedName]resourceapi.DeviceAttribute{
qualifiedNamePCIeRoot: {
StringValue: ptr.To(pcieRoot),
},
},
address: pciBusID,
expectedAttribute: expectedAttribute,
expectsError: false,
},
"invalid empty PCI Bus ID": {
opts: []StandardPCIDeviceAttributesOption{
WithPCIDeviceAddress(""),
},
expected: nil,
expectsError: true,
expectedErrMsg: "PCI Bus ID cannot be empty",
address: "",
expectedAttribute: nil,
expectsError: true,
expectedErrMsg: "PCI Bus ID cannot be empty",
},
"invalid PCI Bus ID format": {
opts: []StandardPCIDeviceAttributesOption{
WithPCIDeviceAddress("invalid-pci-id"),
},
expected: nil,
expectsError: true,
expectedErrMsg: "invalid PCI Bus ID format: invalid-pci-id",
address: "invalid-pci-id",
expectedAttribute: nil,
expectsError: true,
expectedErrMsg: "invalid PCI Bus ID format: invalid-pci-id",
},
"invalid no exist PCI Bus ID": {
opts: []StandardPCIDeviceAttributesOption{
WithPCIDeviceAddress(pciBusID),
},
expected: nil,
expectsError: true,
expectedErrMsg: "no such file or directory",
address: pciBusID,
expectedAttribute: nil,
expectsError: true,
expectedErrMsg: "no such file or directory",
},
"invalid symlink": {
smockSysfsSetup: func(t *testing.T, mockSysfs sysfs) {
smockSysfsSetup: func(t *testing.T, mockSysfs sysfsPath) {
devicePath := mockSysfs.Devices(filepath.Join("invalid-pci-root", "0000:00:13.1", pciBusID))
TouchFile(t, devicePath)
busPath := mockSysfs.Bus(filepath.Join("pci", "devices", pciBusID))
CreateSymlink(t, devicePath, busPath)
},
opts: []StandardPCIDeviceAttributesOption{
WithPCIDeviceAddress(pciBusID),
},
expected: nil,
expectsError: true,
expectedErrMsg: "invalid symlink target for PCI Bus ID",
address: pciBusID,
expectedAttribute: nil,
expectsError: true,
expectedErrMsg: "invalid symlink target for PCI Bus ID",
},
}
for name, test := range tests {
t.Run(name, func(t *testing.T) {
mockSysfsPath := t.TempDir()
mockSysfs := sysfs(mockSysfsPath)
mockSysfs := sysfsPath(mockSysfsPath)
if test.smockSysfsSetup != nil {
test.smockSysfsSetup(t, mockSysfs)
}
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 := StandardPCIDeviceAttributes(append(test.opts, withSysfs(mockSysfs))...)
got, err := GetPCIeRootAttributeByPCIBusID(test.address)
if test.expectsError {
if err == nil {
t.Errorf("Expected error but got none")
@@ -119,8 +114,8 @@ func TestStandardPCIDeviceAttributes(t *testing.T) {
if err != nil {
t.Fatalf("Unexpected error: %v", err)
}
if !reflect.DeepEqual(got, test.expected) {
t.Errorf("Expected attributes %v, got %v", test.expected, got)
if !reflect.DeepEqual(got, *test.expectedAttribute) {
t.Errorf("Expected attribute %v, got %v", test.expectedAttribute, got)
}
})
}

View File

@@ -19,41 +19,45 @@ package deviceattribute
import "path/filepath"
const (
defaultSysfsRoot = "/sys"
sysfsRoot = "/sys"
)
// sysfs provides methods to construct sysfs paths for various subsystems.
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 sysfs string
type sysfsPath string
func (s sysfs) Devices(path string) string {
func (s sysfsPath) Devices(path string) string {
return filepath.Join(string(s), "devices", path)
}
func (s sysfs) Bus(path string) string {
func (s sysfsPath) Bus(path string) string {
return filepath.Join(string(s), "bus", path)
}
func (s sysfs) Block(path string) string {
func (s sysfsPath) Block(path string) string {
return filepath.Join(string(s), "block", path)
}
func (s sysfs) Class(path string) string {
func (s sysfsPath) Class(path string) string {
return filepath.Join(string(s), "class", path)
}
func (s sysfs) Dev(path string) string {
func (s sysfsPath) Dev(path string) string {
return filepath.Join(string(s), "dev", path)
}
func (s sysfs) Firmware(path string) string {
func (s sysfsPath) Firmware(path string) string {
return filepath.Join(string(s), "firmware", path)
}
func (s sysfs) Kernel(path string) string {
func (s sysfsPath) Kernel(path string) string {
return filepath.Join(string(s), "kernel", path)
}
func (s sysfs) Module(path string) string {
func (s sysfsPath) Module(path string) string {
return filepath.Join(string(s), "module", path)
}