Merge pull request #875 from zeeke/414-filter-nic-e2e

OCPBUGS-25794: [release-4.14] conformance-test: Filter NIC by environment variable

test/util/cluster/cluster.go

@@ -4,6 +4,8 @@ import (
 	"context"
 	"fmt"
 	"io"
+	"os"
+	"regexp"
 	"strings"
 	"time"
 
@@ -34,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 <node_name>:<device_name>
+//
+// 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{})
@@ -90,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 {

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
+}