Externally manage e2e tests

Makefile

@@ -75,7 +75,7 @@ image: ; $(info Building image...)
 
 # Run tests
 test: generate vet manifests envtest
-	KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) --bin-dir=/tmp -p path)" HOME="$(shell pwd)" go test ./... -coverprofile cover.out -v
+	KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) --bin-dir=/tmp -p path)" HOME="$(shell pwd)" go test `go list ./... | grep -v test/` -coverprofile cover.out -v
 
 # Build manager binary
 manager: generate vet _build-manager

api/v1/helper.go

@@ -271,12 +271,13 @@ func (p *SriovNetworkNodePolicy) Apply(state *SriovNetworkNodeState, equalPriori
 		if s.Selected(&iface) {
 			log.Info("Update interface", "name:", iface.Name)
 			result := Interface{
-				PciAddress:  iface.PciAddress,
-				Mtu:         p.Spec.Mtu,
-				Name:        iface.Name,
-				LinkType:    p.Spec.LinkType,
-				EswitchMode: p.Spec.EswitchMode,
-				NumVfs:      p.Spec.NumVfs,
+				PciAddress:        iface.PciAddress,
+				Mtu:               p.Spec.Mtu,
+				Name:              iface.Name,
+				LinkType:          p.Spec.LinkType,
+				EswitchMode:       p.Spec.EswitchMode,
+				NumVfs:            p.Spec.NumVfs,
+				ExternallyCreated: p.Spec.ExternallyCreated,
 			}
 			if p.Spec.NumVfs > 0 {
 				group, err := p.generateVfGroup(&iface)

api/v1/sriovnetworknodepolicy_types.go

@@ -59,6 +59,8 @@ type SriovNetworkNodePolicySpec struct {
 	VdpaType string `json:"vdpaType,omitempty"`
 	// Exclude device's NUMA node when advertising this resource by SRIOV network device plugin. Default to false.
 	ExcludeTopology bool `json:"excludeTopology,omitempty"`
+	// don't create the virtual function only allocated them to the device plugin. Defaults to false.
+	ExternallyCreated bool `json:"externallyCreated,omitempty"`
 }
 
 type SriovNetworkNicSelector struct {

api/v1/sriovnetworknodestate_types.go

@@ -29,14 +29,17 @@ type SriovNetworkNodeStateSpec struct {
 	Interfaces      Interfaces `json:"interfaces,omitempty"`
 }
 
+type Interfaces []Interface
+
 type Interface struct {
-	PciAddress  string    `json:"pciAddress"`
-	NumVfs      int       `json:"numVfs,omitempty"`
-	Mtu         int       `json:"mtu,omitempty"`
-	Name        string    `json:"name,omitempty"`
-	LinkType    string    `json:"linkType,omitempty"`
-	EswitchMode string    `json:"eSwitchMode,omitempty"`
-	VfGroups    []VfGroup `json:"vfGroups,omitempty"`
+	PciAddress        string    `json:"pciAddress"`
+	NumVfs            int       `json:"numVfs,omitempty"`
+	Mtu               int       `json:"mtu,omitempty"`
+	Name              string    `json:"name,omitempty"`
+	LinkType          string    `json:"linkType,omitempty"`
+	EswitchMode       string    `json:"eSwitchMode,omitempty"`
+	VfGroups          []VfGroup `json:"vfGroups,omitempty"`
+	ExternallyCreated bool      `json:"externallyCreated,omitempty"`
 }
 
 type VfGroup struct {
@@ -49,23 +52,22 @@ type VfGroup struct {
 	VdpaType     string `json:"vdpaType,omitempty"`
 }
 
-type Interfaces []Interface
-
 type InterfaceExt struct {
-	Name        string            `json:"name,omitempty"`
-	Mac         string            `json:"mac,omitempty"`
-	Driver      string            `json:"driver,omitempty"`
-	PciAddress  string            `json:"pciAddress"`
-	Vendor      string            `json:"vendor,omitempty"`
-	DeviceID    string            `json:"deviceID,omitempty"`
-	NetFilter   string            `json:"netFilter,omitempty"`
-	Mtu         int               `json:"mtu,omitempty"`
-	NumVfs      int               `json:"numVfs,omitempty"`
-	LinkSpeed   string            `json:"linkSpeed,omitempty"`
-	LinkType    string            `json:"linkType,omitempty"`
-	EswitchMode string            `json:"eSwitchMode,omitempty"`
-	TotalVfs    int               `json:"totalvfs,omitempty"`
-	VFs         []VirtualFunction `json:"Vfs,omitempty"`
+	Name              string            `json:"name,omitempty"`
+	Mac               string            `json:"mac,omitempty"`
+	Driver            string            `json:"driver,omitempty"`
+	PciAddress        string            `json:"pciAddress"`
+	Vendor            string            `json:"vendor,omitempty"`
+	DeviceID          string            `json:"deviceID,omitempty"`
+	NetFilter         string            `json:"netFilter,omitempty"`
+	Mtu               int               `json:"mtu,omitempty"`
+	NumVfs            int               `json:"numVfs,omitempty"`
+	LinkSpeed         string            `json:"linkSpeed,omitempty"`
+	LinkType          string            `json:"linkType,omitempty"`
+	EswitchMode       string            `json:"eSwitchMode,omitempty"`
+	ExternallyCreated bool              `json:"externallyCreated,omitempty"`
+	TotalVfs          int               `json:"totalvfs,omitempty"`
+	VFs               []VirtualFunction `json:"Vfs,omitempty"`
 }
 type InterfaceExts []InterfaceExt
 

config/crd/bases/sriovnetwork.openshift.io_sriovnetworknodepolicies.yaml

@@ -53,6 +53,10 @@ spec:
                 description: Exclude device's NUMA node when advertising this resource
                   by SRIOV network device plugin. Default to false.
                 type: boolean
+              externallyCreated:
+                description: don't create the virtual function only allocated them
+                  to the device plugin. Defaults to false.
+                type: boolean
               isRdma:
                 description: RDMA mode. Defaults to false.
                 type: boolean

config/crd/bases/sriovnetwork.openshift.io_sriovnetworknodestates.yaml

@@ -50,6 +50,8 @@ spec:
                   properties:
                     eSwitchMode:
                       type: string
+                    externallyCreated:
+                      type: boolean
                     linkType:
                       type: string
                     mtu:
@@ -125,6 +127,8 @@ spec:
                       type: string
                     eSwitchMode:
                       type: string
+                    externallyCreated:
+                      type: boolean
                     linkSpeed:
                       type: string
                     linkType:

deployment/sriov-network-operator/crds/sriovnetwork.openshift.io_sriovnetworknodepolicies.yaml

@@ -53,6 +53,10 @@ spec:
                 description: Exclude device's NUMA node when advertising this resource
                   by SRIOV network device plugin. Default to false.
                 type: boolean
+              externallyCreated:
+                description: don't create the virtual function only allocated them
+                  to the device plugin. Defaults to false.
+                type: boolean
               isRdma:
                 description: RDMA mode. Defaults to false.
                 type: boolean

deployment/sriov-network-operator/crds/sriovnetwork.openshift.io_sriovnetworknodestates.yaml

@@ -50,6 +50,8 @@ spec:
                   properties:
                     eSwitchMode:
                       type: string
+                    externallyCreated:
+                      type: boolean
                     linkType:
                       type: string
                     mtu:
@@ -125,6 +127,8 @@ spec:
                       type: string
                     eSwitchMode:
                       type: string
+                    externallyCreated:
+                      type: boolean
                     linkSpeed:
                       type: string
                     linkType:

pkg/daemon/daemon.go

@@ -436,6 +436,12 @@ func (dn *Daemon) nodeStateSyncHandler() error {
 	}
 
 	if latestState.GetGeneration() == 1 && len(latestState.Spec.Interfaces) == 0 {
+		err = utils.ClearPCIAddressFolder()
+		if err != nil {
+			glog.Errorf("failed to clear the PCI address configuration: %v", err)
+			return err
+		}
+
 		glog.V(0).Infof("nodeStateSyncHandler(): Name: %s, Interface policy spec not yet set by controller", latestState.Name)
 		if latestState.Status.SyncStatus != "Succeeded" {
 			dn.refreshCh <- Message{
@@ -452,6 +458,15 @@ func (dn *Daemon) nodeStateSyncHandler() error {
 		syncStatus:    syncStatusInProgress,
 		lastSyncError: "",
 	}
+	// wait for writer to refresh status then pull again the latest node state
+	<-dn.syncCh
+	updatedState, err := dn.client.SriovnetworkV1().SriovNetworkNodeStates(namespace).Get(context.Background(), dn.name, metav1.GetOptions{})
+	if err != nil {
+		glog.Warningf("nodeStateSyncHandler(): Failed to fetch node state %s: %v", dn.name, err)
+		return err
+	}
+	// we only update the status to avoid any race conditions where a new policy can be applied
+	latestState.Status = updatedState.Status
 
 	// load plugins if has not loaded
 	if len(dn.enabledPlugins) == 0 {

pkg/daemon/writer.go

@@ -100,10 +100,7 @@ func (w *NodeStateStatusWriter) Run(stop <-chan struct{}, refresh <-chan Message
 			if err != nil {
 				glog.Errorf("Run() refresh: writing to node status failed: %v", err)
 			}
-
-			if msg.syncStatus == syncStatusSucceeded || msg.syncStatus == syncStatusFailed {
-				syncCh <- struct{}{}
-			}
+			syncCh <- struct{}{}
 		case <-time.After(30 * time.Second):
 			glog.V(2).Info("Run(): period refresh")
 			if err := w.pollNicStatus(platformType); err != nil {

pkg/plugins/generic/generic_plugin.go

@@ -198,6 +198,27 @@ func needDrainNode(desired sriovnetworkv1.Interfaces, current sriovnetworkv1.Int
 			}
 		}
 		if !configured && ifaceStatus.NumVfs > 0 {
+			// load the PF info
+			pfStatus, exist, err := utils.LoadPfsStatus(ifaceStatus.PciAddress, true)
+			if err != nil {
+				glog.Errorf("generic-plugin needDrainNode(): failed to load info about PF status for pci address %s: %v", ifaceStatus.PciAddress, err)
+				continue
+			}
+
+			if !exist {
+				glog.Infof("generic-plugin needDrainNode(): PF name %s with pci address %s has VFs configured but they weren't created by the sriov operator. Skipping drain",
+					ifaceStatus.Name,
+					ifaceStatus.PciAddress)
+				continue
+			}
+
+			if pfStatus.ExternallyCreated {
+				glog.Infof("generic-plugin needDrainNode()(): PF name %s with pci address %s was externally created. Skipping drain",
+					ifaceStatus.Name,
+					ifaceStatus.PciAddress)
+				continue
+			}
+
 			glog.V(2).Infof("generic-plugin needDrainNode(): need drain, %v needs to be reset", ifaceStatus)
 			needDrain = true
 			return

pkg/plugins/mellanox/mellanox_plugin.go

@@ -107,6 +107,11 @@ func (p *MellanoxPlugin) OnNodeStateChange(new *sriovnetworkv1.SriovNetworkNodeS
 	}
 
 	for _, ifaceSpec := range mellanoxNicsSpec {
+		// skip configuration for the MLX firmware if externallyCreated is true
+		if ifaceSpec.ExternallyCreated {
+			continue
+		}
+
 		pciPrefix := getPciAddressPrefix(ifaceSpec.PciAddress)
 		// skip processed nics, help not running the same logic 2 times for dual port NICs
 		if _, ok := processedNics[pciPrefix]; ok {

pkg/utils/host.go

@@ -0,0 +1,135 @@
+package utils
+
+import (
+	"encoding/json"
+	"fmt"
+	"os"
+	"path/filepath"
+
+	"github.com/golang/glog"
+
+	sriovnetworkv1 "github.com/k8snetworkplumbingwg/sriov-network-operator/api/v1"
+)
+
+const (
+	SriovConfBasePath     = "/etc/sriov-operator"
+	PfAppliedConfig       = SriovConfBasePath + "/pci"
+	HostSriovConfBasePath = "/host" + SriovConfBasePath
+	HostPfAppliedConfig   = HostSriovConfBasePath + "/pci"
+)
+
+type PfStatus struct {
+	NumVfs            int    `json:"numVfs"`
+	Mtu               int    `json:"mtu"`
+	LinkType          string `json:"linkType"`
+	EswitchMode       string `json:"eSwitchMode"`
+	ExternallyCreated bool   `json:"externallyCreated"`
+}
+
+// create the operator base folder on the host together with the pci folder to save the PF status objects as json files
+func CreateOperatorConfigFolderIfNeeded() error {
+	_, err := os.Stat(SriovConfBasePath)
+	if err != nil {
+		if os.IsNotExist(err) {
+			err = os.Mkdir(SriovConfBasePath, os.ModeDir)
+			if err != nil {
+				return fmt.Errorf("failed to create the sriov config folder on host in path %s: %v", SriovConfBasePath, err)
+			}
+		} else {
+			return fmt.Errorf("failed to check if the sriov config folder on host in path %s exist: %v", SriovConfBasePath, err)
+		}
+	}
+
+	_, err = os.Stat(PfAppliedConfig)
+	if err != nil {
+		if os.IsNotExist(err) {
+			err = os.Mkdir(PfAppliedConfig, os.ModeDir)
+			if err != nil {
+				return fmt.Errorf("failed to create the pci folder on host in path %s: %v", PfAppliedConfig, err)
+			}
+		} else {
+			return fmt.Errorf("failed to check if the pci folder on host in path %s exist: %v", PfAppliedConfig, err)
+		}
+	}
+
+	return nil
+}
+
+func ClearPCIAddressFolder() error {
+	_, err := os.Stat(HostPfAppliedConfig)
+	if err != nil {
+		if os.IsNotExist(err) {
+			return nil
+		}
+		return fmt.Errorf("failed to check the pci address folder path %s", HostPfAppliedConfig)
+	}
+
+	err = os.RemoveAll(HostPfAppliedConfig)
+	if err != nil {
+		return fmt.Errorf("failed to remove the PCI address folder on path %s: %v", HostPfAppliedConfig, err)
+	}
+
+	err = os.Mkdir(HostPfAppliedConfig, os.ModeDir)
+	if err != nil {
+		return fmt.Errorf("failed to create the pci folder on host in path %s: %v", HostPfAppliedConfig, err)
+	}
+
+	return nil
+}
+
+func CreatePfAppliedStatusFromSpec(p *sriovnetworkv1.Interface) *PfStatus {
+	return &PfStatus{
+		ExternallyCreated: p.ExternallyCreated,
+		NumVfs:            p.NumVfs,
+		EswitchMode:       p.EswitchMode,
+		Mtu:               p.Mtu,
+		LinkType:          p.LinkType,
+	}
+}
+
+// SaveLastPfAppliedStatus will save the PF object as a json into the /etc/sriov-operator/pci/<pci-address>
+// this function must be called after running the chroot function
+func SaveLastPfAppliedStatus(pciAddress string, pfStatus *PfStatus) error {
+	if err := CreateOperatorConfigFolderIfNeeded(); err != nil {
+		return err
+	}
+
+	data, err := json.Marshal(pfStatus)
+	if err != nil {
+		glog.Errorf("failed to marshal PF status %+v: %v", *pfStatus, err)
+		return err
+	}
+
+	path := filepath.Join(PfAppliedConfig, pciAddress)
+	err = os.WriteFile(path, data, 0644)
+	return err
+}
+
+// LoadPfsStatus convert the /etc/sriov-operator/pci/<pci-address> json to pfstatus
+// returns false if the file doesn't exist.
+func LoadPfsStatus(pciAddress string, chroot bool) (*PfStatus, bool, error) {
+	if chroot {
+		exit, err := Chroot("/host")
+		if err != nil {
+			return nil, false, err
+		}
+		defer exit()
+	}
+
+	pfStatus := &PfStatus{}
+	path := filepath.Join(PfAppliedConfig, pciAddress)
+	data, err := os.ReadFile(path)
+	if err != nil {
+		if os.IsNotExist(err) {
+			return nil, false, nil
+		}
+		glog.Errorf("failed to read PF status from path %s: %v", path, err)
+	}
+
+	err = json.Unmarshal(data, pfStatus)
+	if err != nil {
+		glog.Errorf("failed to unmarshal PF status %s: %v", data, err)
+	}
+
+	return pfStatus, true, nil
+}