From 09c94a11b0274b2f5e7c631c7a7414754e3cee77 Mon Sep 17 00:00:00 2001 From: Andrea Panattoni Date: Thu, 14 Sep 2023 18:36:51 +0200 Subject: [PATCH] conformance-test: Filter NIC by env var In order to verify a specific NIC works well with the operator, the user who runs the test suite should be able to select involved devices. This commit introduces the environment variable `SRIOV_NODE_AND_DEVICE_NAME_FILTER` to implement the selection behavior. Signed-off-by: Andrea Panattoni --- test/util/cluster/cluster.go | 73 +++++++++---- test/util/cluster/cluster_test.go | 169 ++++++++++++++++++++++++++++++ 2 files changed, 222 insertions(+), 20 deletions(-) create mode 100644 test/util/cluster/cluster_test.go diff --git a/test/util/cluster/cluster.go b/test/util/cluster/cluster.go index 004f61e46..fdf06bdf7 100644 --- a/test/util/cluster/cluster.go +++ b/test/util/cluster/cluster.go @@ -5,6 +5,7 @@ import ( "fmt" "io" "os" + "regexp" "strings" "time" @@ -35,6 +36,24 @@ var ( intelVendorID = "8086" ) +// Name of environment variable to filter which decives can be discovered by FindSriovDevices and FindOneSriovDevice. +// The filter is a regexp matched against node names and device name in the form : +// +// For example, given the following devices in the cluster: +// +// worker-0:eno1 +// worker-0:eno2 +// worker-1:eno1 +// worker-1:eno2 +// worker-1:ens1f0 +// worker-1:ens1f1 +// +// Values: +// - `.*:eno1` matches `worker-0:eno1,worker-1:eno1` +// - `worker-0:eno.*` matches `worker-0:eno1,worker-0:eno2` +// - `worker-0:eno1|worker-1:eno2` matches `worker-0:eno1,worker-1:eno2` +const NodeAndDeviceNameFilterEnvVar string = "SRIOV_NODE_AND_DEVICE_NAME_FILTER" + // DiscoverSriov retrieves Sriov related information of a given cluster. func DiscoverSriov(clients *testclient.ClientSet, operatorNamespace string) (*EnabledNodes, error) { nodeStates, err := clients.SriovNetworkNodeStates(operatorNamespace).List(context.Background(), metav1.ListOptions{}) @@ -91,35 +110,49 @@ func DiscoverSriov(clients *testclient.ClientSet, operatorNamespace string) (*En return res, nil } -// FindOneSriovDevice retrieves a valid sriov device for the given node. +// FindOneSriovDevice retrieves a valid sriov device for the given node, filtered by `SRIOV_NODE_AND_DEVICE_NAME_FILTER` environment variable. func (n *EnabledNodes) FindOneSriovDevice(node string) (*sriovv1.InterfaceExt, error) { - s, ok := n.States[node] + ret, err := n.FindSriovDevices(node) + if err != nil { + return nil, err + } + + if len(ret) == 0 { + return nil, fmt.Errorf("unable to find sriov devices in node %s", node) + } + + return ret[0], nil +} + +// FindSriovDevices retrieves all valid sriov devices for the given node, filtered by `SRIOV_NODE_AND_DEVICE_NAME_FILTER` environment variable. +func (n *EnabledNodes) FindSriovDevices(node string) ([]*sriovv1.InterfaceExt, error) { + devices, err := n.FindSriovDevicesIgnoreFilters(node) + if err != nil { + return nil, err + } + + sriovDeviceNameFilter, ok := os.LookupEnv(NodeAndDeviceNameFilterEnvVar) if !ok { - return nil, fmt.Errorf("node %s not found", node) + return devices, nil } - for _, itf := range s.Status.Interfaces { - if IsPFDriverSupported(itf.Driver) && sriovv1.IsSupportedDevice(itf.DeviceID) { - // Skip mlx interfaces if secure boot is enabled - // TODO: remove this when mlx support secure boot/lockdown mode - if itf.Vendor == mlxVendorID && n.IsSecureBootEnabled[node] { - continue - } - // if the sriov is not enable in the kernel for intel nic the totalVF will be 0 so we skip the device - // That is not the case for Mellanox devices that will report 0 until we configure the sriov interfaces - // with the mstconfig package - if itf.Vendor == intelVendorID && itf.TotalVfs == 0 { - continue - } + filteredDevices := []*sriovv1.InterfaceExt{} + for _, device := range devices { + match, err := regexp.MatchString(sriovDeviceNameFilter, node+":"+device.Name) + if err != nil { + return nil, err + } - return &itf, nil + if match { + filteredDevices = append(filteredDevices, device) } } - return nil, fmt.Errorf("unable to find sriov devices in node %s", node) + + return filteredDevices, nil } -// FindSriovDevices retrieves all valid sriov devices for the given node. -func (n *EnabledNodes) FindSriovDevices(node string) ([]*sriovv1.InterfaceExt, error) { +// FindSriovDevicesIgnoreFilters retrieves all valid sriov devices for the given node. +func (n *EnabledNodes) FindSriovDevicesIgnoreFilters(node string) ([]*sriovv1.InterfaceExt, error) { devices := []*sriovv1.InterfaceExt{} s, ok := n.States[node] if !ok { diff --git a/test/util/cluster/cluster_test.go b/test/util/cluster/cluster_test.go new file mode 100644 index 000000000..e502a1238 --- /dev/null +++ b/test/util/cluster/cluster_test.go @@ -0,0 +1,169 @@ +package cluster + +import ( + "os" + "testing" + + "github.com/stretchr/testify/assert" + + sriovv1 "github.com/k8snetworkplumbingwg/sriov-network-operator/api/v1" +) + +func init() { + sriovv1.InitNicIDMapFromList([]string{ + "8086 158b 154c", + "15b3 1015 1016", + }) +} + +func TestFindSriovDevices(t *testing.T) { + nodes := &EnabledNodes{ + Nodes: []string{"worker-0", "worker-1"}, + States: map[string]sriovv1.SriovNetworkNodeState{ + "worker-0": { + Status: sriovv1.SriovNetworkNodeStateStatus{ + Interfaces: sriovv1.InterfaceExts{ + makeConnectX4LX("eno1np0", "0000:19:00.0"), + makeConnectX4LX("eno1np1", "0000:19:00.1"), + makeSfp28("ens3f0", "0000:d8:00.0"), + makeSfp28("ens3f1", "0000:d8:00.0"), + }, + }, + }, + "worker-1": { + Status: sriovv1.SriovNetworkNodeStateStatus{ + Interfaces: sriovv1.InterfaceExts{ + makeConnectX4LX("eno1np0", "0000:19:00.0"), + makeConnectX4LX("eno1np1", "0000:19:00.1"), + makeSfp28("ens3f0", "0000:d8:00.0"), + makeSfp28("ens3f1", "0000:d8:00.0"), + }, + }, + }, + }, + IsSecureBootEnabled: map[string]bool{ + "worker-0": false, + "worker-1": true, + }, + } + + w0, err := nodes.FindSriovDevices("worker-0") + assert.NoError(t, err) + + w0Names := extractNames(w0) + assert.Contains(t, w0Names, "eno1np0") + assert.Contains(t, w0Names, "eno1np1") + assert.Contains(t, w0Names, "ens3f0") + assert.Contains(t, w0Names, "ens3f1") + + w1, err := nodes.FindSriovDevices("worker-1") + assert.NoError(t, err) + + w1Names := extractNames(w1) + // worker-1 has SecureBoot enabled + assert.NotContains(t, w1Names, "eno1np0") + assert.NotContains(t, w1Names, "eno1np1") + assert.Contains(t, w1Names, "ens3f0") + assert.Contains(t, w1Names, "ens3f1") +} + +func TestFindSriovDevicesFilteredByEnv(t *testing.T) { + nodes := &EnabledNodes{ + Nodes: []string{"worker-0"}, + States: map[string]sriovv1.SriovNetworkNodeState{ + "worker-0": { + Status: sriovv1.SriovNetworkNodeStateStatus{ + Interfaces: sriovv1.InterfaceExts{ + makeConnectX4LX("eno1np0", "0000:19:00.0"), + makeConnectX4LX("eno1np1", "0000:19:00.1"), + }, + }, + }, + }, + IsSecureBootEnabled: map[string]bool{ + "worker-0": false, + }, + } + + originalValue := os.Getenv("SRIOV_NODE_AND_DEVICE_NAME_FILTER") + defer os.Setenv("SRIOV_NODE_AND_DEVICE_NAME_FILTER", originalValue) + + os.Setenv("SRIOV_NODE_AND_DEVICE_NAME_FILTER", ".*:eno1np1") + w0, err := nodes.FindSriovDevices("worker-0") + assert.NoError(t, err) + w0Names := extractNames(w0) + assert.NotContains(t, w0Names, "eno1np0") + assert.Contains(t, w0Names, "eno1np1") + + os.Setenv("SRIOV_NODE_AND_DEVICE_NAME_FILTER", ".*:eno1.*") + w0, err = nodes.FindSriovDevices("worker-0") + assert.NoError(t, err) + w0Names = extractNames(w0) + assert.Contains(t, w0Names, "eno1np0") + assert.Contains(t, w0Names, "eno1np1") +} + +func TestFindSriovDevicesFilteredByEnvOnDifferentNode(t *testing.T) { + nodes := &EnabledNodes{ + Nodes: []string{"worker-0"}, + States: map[string]sriovv1.SriovNetworkNodeState{ + "worker-0": { + Status: sriovv1.SriovNetworkNodeStateStatus{ + Interfaces: sriovv1.InterfaceExts{ + makeConnectX4LX("eno1", "0000:19:00.0"), + makeConnectX4LX("eno2", "0000:19:00.1"), + }, + }, + }, + "worker-1": { + Status: sriovv1.SriovNetworkNodeStateStatus{ + Interfaces: sriovv1.InterfaceExts{ + makeConnectX4LX("eno1", "0000:19:00.0"), + makeConnectX4LX("eno2", "0000:19:00.1"), + }, + }, + }, + }, + IsSecureBootEnabled: map[string]bool{ + "worker-0": false, + }, + } + + originalValue := os.Getenv("SRIOV_NODE_AND_DEVICE_NAME_FILTER") + defer os.Setenv("SRIOV_NODE_AND_DEVICE_NAME_FILTER", originalValue) + os.Setenv("SRIOV_NODE_AND_DEVICE_NAME_FILTER", "worker-0:eno1|worker-1:eno2") + + w0, err := nodes.FindSriovDevices("worker-0") + assert.NoError(t, err) + w0Names := extractNames(w0) + assert.Contains(t, w0Names, "eno1") + assert.NotContains(t, w0Names, "eno2") + + w1, err := nodes.FindSriovDevices("worker-1") + assert.NoError(t, err) + w1Names := extractNames(w1) + assert.NotContains(t, w1Names, "eno1") + assert.Contains(t, w1Names, "eno2") +} + +func makeConnectX4LX(name, pci string) sriovv1.InterfaceExt { + return sriovv1.InterfaceExt{ + Vendor: "15b3", DeviceID: "1015", Driver: "mlx5_core", + Name: name, TotalVfs: 8, PciAddress: pci, + } +} + +func makeSfp28(name, pci string) sriovv1.InterfaceExt { + return sriovv1.InterfaceExt{ + Vendor: "8086", DeviceID: "158b", Driver: "i40e", + Name: name, TotalVfs: 64, PciAddress: pci, + } +} + +func extractNames(in []*sriovv1.InterfaceExt) []string { + ret := make([]string, 0, len(in)) + for _, intf := range in { + ret = append(ret, intf.Name) + } + return ret +}