From daecbcb19a5384081ff6f9d856ce0a2dafb9ef25 Mon Sep 17 00:00:00 2001 From: Sebastian Sch Date: Mon, 8 May 2023 17:23:47 +0300 Subject: [PATCH 1/7] Support externally created virtual functions this commit add the new API allow the operator to use virtual functions that the user configure manually on the system this will allow new use-cases like using a VF as the SDN primary nic. the nic will be created before the operator start by the user, for example using nmstate or NetworkManager and the operator will allocate all the remaining vfs to pods. Signed-off-by: Sebastian Sch --- api/v1/helper.go | 13 ++--- api/v1/sriovnetworknodepolicy_types.go | 2 + api/v1/sriovnetworknodestate_types.go | 48 ++++++++++--------- ...openshift.io_sriovnetworknodepolicies.yaml | 4 ++ ...k.openshift.io_sriovnetworknodestates.yaml | 4 ++ ...openshift.io_sriovnetworknodepolicies.yaml | 4 ++ ...k.openshift.io_sriovnetworknodestates.yaml | 4 ++ 7 files changed, 50 insertions(+), 29 deletions(-) diff --git a/api/v1/helper.go b/api/v1/helper.go index 6479b4c0a..36aadfa00 100644 --- a/api/v1/helper.go +++ b/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) diff --git a/api/v1/sriovnetworknodepolicy_types.go b/api/v1/sriovnetworknodepolicy_types.go index 8955c300d..a400ddcee 100644 --- a/api/v1/sriovnetworknodepolicy_types.go +++ b/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 { diff --git a/api/v1/sriovnetworknodestate_types.go b/api/v1/sriovnetworknodestate_types.go index 1203abdf2..6c1eba7eb 100644 --- a/api/v1/sriovnetworknodestate_types.go +++ b/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 diff --git a/config/crd/bases/sriovnetwork.openshift.io_sriovnetworknodepolicies.yaml b/config/crd/bases/sriovnetwork.openshift.io_sriovnetworknodepolicies.yaml index 5bebf01e0..f9b28fd65 100644 --- a/config/crd/bases/sriovnetwork.openshift.io_sriovnetworknodepolicies.yaml +++ b/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 diff --git a/config/crd/bases/sriovnetwork.openshift.io_sriovnetworknodestates.yaml b/config/crd/bases/sriovnetwork.openshift.io_sriovnetworknodestates.yaml index 1e7ef32b0..e57247359 100644 --- a/config/crd/bases/sriovnetwork.openshift.io_sriovnetworknodestates.yaml +++ b/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: diff --git a/deployment/sriov-network-operator/crds/sriovnetwork.openshift.io_sriovnetworknodepolicies.yaml b/deployment/sriov-network-operator/crds/sriovnetwork.openshift.io_sriovnetworknodepolicies.yaml index 5bebf01e0..f9b28fd65 100644 --- a/deployment/sriov-network-operator/crds/sriovnetwork.openshift.io_sriovnetworknodepolicies.yaml +++ b/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 diff --git a/deployment/sriov-network-operator/crds/sriovnetwork.openshift.io_sriovnetworknodestates.yaml b/deployment/sriov-network-operator/crds/sriovnetwork.openshift.io_sriovnetworknodestates.yaml index 1e7ef32b0..e57247359 100644 --- a/deployment/sriov-network-operator/crds/sriovnetwork.openshift.io_sriovnetworknodestates.yaml +++ b/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: From 4d1bb1b9d3735f93c16b13d04d54f86a3a19a8d5 Mon Sep 17 00:00:00 2001 From: Sebastian Sch Date: Mon, 8 May 2023 17:24:26 +0300 Subject: [PATCH 2/7] Create the webhook validations for externallyCreated variable Signed-off-by: Sebastian Sch --- pkg/webhook/validate.go | 38 +++++ pkg/webhook/validate_test.go | 312 +++++++++++++++++++++++++++++++++++ 2 files changed, 350 insertions(+) diff --git a/pkg/webhook/validate.go b/pkg/webhook/validate.go index 01f753608..151080fa6 100644 --- a/pkg/webhook/validate.go +++ b/pkg/webhook/validate.go @@ -165,6 +165,13 @@ func staticValidateSriovNetworkNodePolicy(cr *sriovnetworkv1.SriovNetworkNodePol if cr.Spec.VdpaType == constants.VdpaTypeVirtio && cr.Spec.EswitchMode != sriovnetworkv1.ESwithModeSwitchDev { return false, fmt.Errorf("virtio/vdpa requires the device to be configured in switchdev mode") } + + // Externally created: we don't support ExternallyCreated + EswitchMode + //TODO: if needed we will need to add this in the future as today EswitchMode is for HWOFFLOAD + if cr.Spec.ExternallyCreated && cr.Spec.EswitchMode == sriovnetworkv1.ESwithModeSwitchDev { + return false, fmt.Errorf("ExternallyCreated doesn't support the device to be configured in switchdev mode") + } + return true, nil } @@ -240,6 +247,21 @@ func validatePolicyForNodeState(policy *sriovnetworkv1.SriovNetworkNodePolicy, s if policy.Spec.NumVfs > MlxMaxVFs && iface.Vendor == MellanoxID { return fmt.Errorf("numVfs(%d) in CR %s exceed the maximum allowed value(%d)", policy.Spec.NumVfs, policy.GetName(), MlxMaxVFs) } + + // Externally create validations + if policy.Spec.ExternallyCreated { + if policy.Spec.NumVfs != iface.NumVfs { + return fmt.Errorf("numVfs(%d) in CR %s is not equal to the number of virtual functions allocated for the PF externally value(%d)", policy.Spec.NumVfs, policy.GetName(), iface.NumVfs) + } + + if policy.Spec.Mtu != 0 && policy.Spec.Mtu != iface.Mtu { + return fmt.Errorf("MTU(%d) in CR %s is not equal to the MTU for the PF externally value(%d)", policy.Spec.Mtu, policy.GetName(), iface.Mtu) + } + + if policy.Spec.LinkType != "" && strings.ToLower(policy.Spec.LinkType) != strings.ToLower(iface.LinkType) { + return fmt.Errorf("LinkType(%d) in CR %s is not equal to the LinkType for the PF externally value(%d)", policy.Spec.Mtu, policy.GetName(), iface.Mtu) + } + } // vdpa: only mellanox cards are supported if policy.Spec.VdpaType == constants.VdpaTypeVirtio && iface.Vendor != MellanoxID { return fmt.Errorf("vendor(%s) in CR %s not supported for virtio-vdpa", iface.Vendor, policy.GetName()) @@ -281,6 +303,22 @@ func validatePfNames(current *sriovnetworkv1.SriovNetworkNodePolicy, previous *s // since it should already be evaluated in previous run. preName, preRngSt, preRngEnd, _ := sriovnetworkv1.ParsePFName(prePf) if curName == preName { + // reject policy with externallyManage if there is a policy on the same PF without it + if current.Spec.ExternallyCreated != previous.Spec.ExternallyCreated { + return fmt.Errorf("externallyManage is inconsistent with existing policy %s", previous.GetName()) + } + + // reject policy with externallyManage if there is a policy on the same PF with switch dev + if current.Spec.ExternallyCreated && previous.Spec.EswitchMode == sriovnetworkv1.ESwithModeSwitchDev { + return fmt.Errorf("externallyManage overlap with switchdev mode in existing policy %s", previous.GetName()) + } + + // reject policy with externallyManage if there is a policy on the same PF with switch dev + if previous.Spec.ExternallyCreated && current.Spec.EswitchMode == sriovnetworkv1.ESwithModeSwitchDev { + return fmt.Errorf("switchdev overlap with externallyManage mode in existing policy %s", previous.GetName()) + } + + // Check for overlapping ranges if curRngEnd < preRngSt || curRngSt > preRngEnd { return nil } else { diff --git a/pkg/webhook/validate_test.go b/pkg/webhook/validate_test.go index 028d62a5e..904f6377c 100644 --- a/pkg/webhook/validate_test.go +++ b/pkg/webhook/validate_test.go @@ -71,6 +71,7 @@ func newNodeState() *SriovNetworkNodeState { Vendor: "8086", NumVfs: 4, TotalVfs: 64, + LinkType: "ETH", }, { VFs: []VirtualFunction{ @@ -84,6 +85,7 @@ func newNodeState() *SriovNetworkNodeState { Vendor: "8086", NumVfs: 4, TotalVfs: 64, + LinkType: "ETH", }, { VFs: []VirtualFunction{ @@ -97,6 +99,7 @@ func newNodeState() *SriovNetworkNodeState { Vendor: "8086", NumVfs: 4, TotalVfs: 64, + LinkType: "ETH", }, }, }, @@ -243,6 +246,291 @@ func TestValidatePolicyForNodeStateWithInvalidNumVfsPolicy(t *testing.T) { g.Expect(err).To(MatchError(ContainSubstring(fmt.Sprintf("numVfs(%d) in CR %s exceed the maximum allowed value(%d)", policy.Spec.NumVfs, policy.GetName(), state.Status.Interfaces[0].TotalVfs)))) } +func TestValidatePolicyForNodeStateWithInvalidNumVfsExternallyCreated(t *testing.T) { + state := newNodeState() + policy := &SriovNetworkNodePolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "p1", + }, + Spec: SriovNetworkNodePolicySpec{ + DeviceType: "netdevice", + NicSelector: SriovNetworkNicSelector{ + PfNames: []string{"ens803f0"}, + RootDevices: []string{"0000:86:00.0"}, + Vendor: "8086", + }, + NodeSelector: map[string]string{ + "feature.node.kubernetes.io/network-sriov.capable": "true", + }, + NumVfs: 5, + Priority: 99, + ResourceName: "p0", + ExternallyCreated: true, + }, + } + g := NewGomegaWithT(t) + err := validatePolicyForNodeState(policy, state, NewNode()) + g.Expect(err).To(MatchError(ContainSubstring(fmt.Sprintf("numVfs(%d) in CR %s is not equal to the number of virtual functions allocated for the PF externally value(%d)", policy.Spec.NumVfs, policy.GetName(), state.Status.Interfaces[0].NumVfs)))) +} + +func TestValidatePolicyForNodeStateWithValidNumVfsExternallyCreated(t *testing.T) { + state := newNodeState() + policy := &SriovNetworkNodePolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "p1", + }, + Spec: SriovNetworkNodePolicySpec{ + DeviceType: "netdevice", + NicSelector: SriovNetworkNicSelector{ + PfNames: []string{"ens803f0"}, + RootDevices: []string{"0000:86:00.0"}, + Vendor: "8086", + }, + NodeSelector: map[string]string{ + "feature.node.kubernetes.io/network-sriov.capable": "true", + }, + NumVfs: 4, + Priority: 99, + ResourceName: "p0", + ExternallyCreated: true, + }, + } + g := NewGomegaWithT(t) + err := validatePolicyForNodeState(policy, state, NewNode()) + g.Expect(err).ToNot(HaveOccurred()) +} + +func TestValidatePolicyForNodePolicyWithOutExternallyManageConflict(t *testing.T) { + appliedPolicy := newNodePolicy() + appliedPolicy.Spec.ExternallyCreated = true + policy := &SriovNetworkNodePolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "p0", + }, + Spec: SriovNetworkNodePolicySpec{ + DeviceType: "netdevice", + NicSelector: SriovNetworkNicSelector{ + PfNames: []string{"ens803f1#3-4"}, + Vendor: "8086", + }, + NodeSelector: map[string]string{ + "feature.node.kubernetes.io/network-sriov.capable": "true", + }, + NumVfs: 63, + Priority: 99, + ResourceName: "p0", + ExternallyCreated: true, + }, + } + g := NewGomegaWithT(t) + err := validatePolicyForNodePolicy(policy, appliedPolicy) + g.Expect(err).ToNot(HaveOccurred()) +} + +func TestValidatePolicyForNodePolicyWithExternallyManageConflict(t *testing.T) { + appliedPolicy := newNodePolicy() + policy := &SriovNetworkNodePolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "p0", + }, + Spec: SriovNetworkNodePolicySpec{ + DeviceType: "netdevice", + NicSelector: SriovNetworkNicSelector{ + PfNames: []string{"ens803f1#3-4"}, + Vendor: "8086", + }, + NodeSelector: map[string]string{ + "feature.node.kubernetes.io/network-sriov.capable": "true", + }, + NumVfs: 63, + Priority: 99, + ResourceName: "p0", + ExternallyCreated: true, + }, + } + g := NewGomegaWithT(t) + err := validatePolicyForNodePolicy(policy, appliedPolicy) + g.Expect(err).To(MatchError(ContainSubstring(fmt.Sprintf("externallyManage is inconsistent with existing policy %s", appliedPolicy.ObjectMeta.Name)))) +} + +func TestValidatePolicyForNodePolicyWithExternallyManageConflictWithSwitchDev(t *testing.T) { + appliedPolicy := newNodePolicy() + appliedPolicy.Spec.EswitchMode = ESwithModeSwitchDev + + policy := &SriovNetworkNodePolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "p0", + }, + Spec: SriovNetworkNodePolicySpec{ + DeviceType: "netdevice", + NicSelector: SriovNetworkNicSelector{ + PfNames: []string{"ens803f1#3-4"}, + Vendor: "8086", + }, + NodeSelector: map[string]string{ + "feature.node.kubernetes.io/network-sriov.capable": "true", + }, + NumVfs: 63, + Priority: 99, + ResourceName: "p0", + ExternallyCreated: true, + }, + } + g := NewGomegaWithT(t) + err := validatePolicyForNodePolicy(policy, appliedPolicy) + g.Expect(err).To(HaveOccurred()) +} + +func TestValidatePolicyForNodePolicyWithSwitchDevConflictWithExternallyManage(t *testing.T) { + appliedPolicy := newNodePolicy() + appliedPolicy.Spec.ExternallyCreated = true + + policy := &SriovNetworkNodePolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "p0", + }, + Spec: SriovNetworkNodePolicySpec{ + DeviceType: "netdevice", + NicSelector: SriovNetworkNicSelector{ + PfNames: []string{"ens803f1#3-4"}, + Vendor: "8086", + }, + NodeSelector: map[string]string{ + "feature.node.kubernetes.io/network-sriov.capable": "true", + }, + NumVfs: 63, + Priority: 99, + ResourceName: "p0", + EswitchMode: ESwithModeSwitchDev, + }, + } + g := NewGomegaWithT(t) + err := validatePolicyForNodePolicy(policy, appliedPolicy) + g.Expect(err).To(HaveOccurred()) +} + +func TestValidatePolicyForNodeStateWithExternallyManageAndMTU(t *testing.T) { + state := newNodeState() + policy := &SriovNetworkNodePolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "p1", + }, + Spec: SriovNetworkNodePolicySpec{ + DeviceType: "netdevice", + NicSelector: SriovNetworkNicSelector{ + PfNames: []string{"ens803f0"}, + RootDevices: []string{"0000:86:00.0"}, + Vendor: "8086", + }, + NodeSelector: map[string]string{ + "feature.node.kubernetes.io/network-sriov.capable": "true", + }, + NumVfs: 4, + Priority: 99, + ResourceName: "p0", + ExternallyCreated: true, + Mtu: 1500, + }, + } + g := NewGomegaWithT(t) + err := validatePolicyForNodeState(policy, state, NewNode()) + g.Expect(err).ToNot(HaveOccurred()) +} + +func TestValidatePolicyForNodeStateWithExternallyManageAndDifferentMTU(t *testing.T) { + state := newNodeState() + policy := &SriovNetworkNodePolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "p1", + }, + Spec: SriovNetworkNodePolicySpec{ + DeviceType: "netdevice", + NicSelector: SriovNetworkNicSelector{ + PfNames: []string{"ens803f0"}, + RootDevices: []string{"0000:86:00.0"}, + Vendor: "8086", + }, + NodeSelector: map[string]string{ + "feature.node.kubernetes.io/network-sriov.capable": "true", + }, + NumVfs: 4, + Priority: 99, + ResourceName: "p0", + ExternallyCreated: true, + Mtu: 9000, + }, + } + g := NewGomegaWithT(t) + err := validatePolicyForNodeState(policy, state, NewNode()) + g.Expect(err).To(HaveOccurred()) +} + +func TestValidatePolicyForNodeStateWithExternallyManageAndLinkType(t *testing.T) { + state := newNodeState() + policy := &SriovNetworkNodePolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "p1", + }, + Spec: SriovNetworkNodePolicySpec{ + DeviceType: "netdevice", + NicSelector: SriovNetworkNicSelector{ + PfNames: []string{"ens803f0"}, + RootDevices: []string{"0000:86:00.0"}, + Vendor: "8086", + }, + NodeSelector: map[string]string{ + "feature.node.kubernetes.io/network-sriov.capable": "true", + }, + NumVfs: 4, + Priority: 99, + ResourceName: "p0", + ExternallyCreated: true, + LinkType: "ETH", + }, + } + g := NewGomegaWithT(t) + err := validatePolicyForNodeState(policy, state, NewNode()) + g.Expect(err).ToNot(HaveOccurred()) + + policy.Spec.LinkType = "eth" + err = validatePolicyForNodeState(policy, state, NewNode()) + g.Expect(err).ToNot(HaveOccurred()) + + policy.Spec.LinkType = "ETH" + state.Status.Interfaces[0].LinkType = "eth" + err = validatePolicyForNodeState(policy, state, NewNode()) + g.Expect(err).ToNot(HaveOccurred()) +} + +func TestValidatePolicyForNodeStateWithExternallyManageAndDifferentLinkType(t *testing.T) { + state := newNodeState() + policy := &SriovNetworkNodePolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "p1", + }, + Spec: SriovNetworkNodePolicySpec{ + DeviceType: "netdevice", + NicSelector: SriovNetworkNicSelector{ + PfNames: []string{"ens803f0"}, + RootDevices: []string{"0000:86:00.0"}, + Vendor: "8086", + }, + NodeSelector: map[string]string{ + "feature.node.kubernetes.io/network-sriov.capable": "true", + }, + NumVfs: 4, + Priority: 99, + ResourceName: "p0", + ExternallyCreated: true, + Mtu: 9000, + LinkType: "IB", + }, + } + g := NewGomegaWithT(t) + err := validatePolicyForNodeState(policy, state, NewNode()) + g.Expect(err).To(HaveOccurred()) +} + func TestValidatePolicyForNodePolicyWithOverlappedVfRange(t *testing.T) { appliedPolicy := newNodePolicy() policy := &SriovNetworkNodePolicy{ @@ -545,6 +833,30 @@ func TestStaticValidateSriovNetworkNodePolicyVdpaMustSpecifySwitchDev(t *testing g.Expect(ok).To(Equal(false)) } +func TestStaticValidateSriovNetworkNodePolicyWithExternallyCreatedAndSwitchDev(t *testing.T) { + policy := &SriovNetworkNodePolicy{ + Spec: SriovNetworkNodePolicySpec{ + DeviceType: "netdevice", + NicSelector: SriovNetworkNicSelector{ + Vendor: "8086", + DeviceID: "158b", + }, + NodeSelector: map[string]string{ + "feature.node.kubernetes.io/network-sriov.capable": "true", + }, + NumVfs: 63, + Priority: 99, + ResourceName: "p0", + EswitchMode: "switchdev", + ExternallyCreated: true, + }, + } + g := NewGomegaWithT(t) + ok, err := staticValidateSriovNetworkNodePolicy(policy) + g.Expect(err).To(HaveOccurred()) + g.Expect(ok).To(BeFalse()) +} + func TestValidatePolicyForNodeStateVdpaWithNotSupportedVendor(t *testing.T) { state := newNodeState() policy := &SriovNetworkNodePolicy{ From 19a98adcd359b507bd2be39d32da50389130484d Mon Sep 17 00:00:00 2001 From: Sebastian Sch Date: Mon, 8 May 2023 17:24:50 +0300 Subject: [PATCH 3/7] skip mlx firmware config if the PF is on externallyCreated Signed-off-by: Sebastian Sch --- pkg/plugins/mellanox/mellanox_plugin.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/pkg/plugins/mellanox/mellanox_plugin.go b/pkg/plugins/mellanox/mellanox_plugin.go index f6b8ab185..ddf168ad7 100644 --- a/pkg/plugins/mellanox/mellanox_plugin.go +++ b/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 { From bb6f9a1dd8f367c11a64364b17d23769a6fa7581 Mon Sep 17 00:00:00 2001 From: Sebastian Sch Date: Mon, 8 May 2023 17:27:11 +0300 Subject: [PATCH 4/7] Create the needed functions to save info on the host this commit adds the functionality needed to save the PF information into the host. we need this in case the user removes a policy that was externallyCreated. we must NOT delete reset the VFs in that case and the only way to know that the device was externallyCreated after the user removes the policy is to check this file on the host. Signed-off-by: Sebastian Sch --- pkg/daemon/daemon.go | 6 ++ pkg/plugins/generic/generic_plugin.go | 21 ++++ pkg/utils/host.go | 135 ++++++++++++++++++++++++++ pkg/utils/utils.go | 65 ++++++++++++- 4 files changed, 225 insertions(+), 2 deletions(-) create mode 100644 pkg/utils/host.go diff --git a/pkg/daemon/daemon.go b/pkg/daemon/daemon.go index 727ffe642..3803f3de8 100644 --- a/pkg/daemon/daemon.go +++ b/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{ diff --git a/pkg/plugins/generic/generic_plugin.go b/pkg/plugins/generic/generic_plugin.go index a2a7e1c00..1f3cdd397 100644 --- a/pkg/plugins/generic/generic_plugin.go +++ b/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 diff --git a/pkg/utils/host.go b/pkg/utils/host.go new file mode 100644 index 000000000..7032b818f --- /dev/null +++ b/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/ +// 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/ 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 +} diff --git a/pkg/utils/utils.go b/pkg/utils/utils.go index 37888e4b1..2b721b69a 100644 --- a/pkg/utils/utils.go +++ b/pkg/utils/utils.go @@ -122,6 +122,15 @@ func DiscoverSriovDevices(withUnsupported bool) ([]sriovnetworkv1.InterfaceExt, } iface.LinkType = getLinkType(iface) + pfStatus, exist, err := LoadPfsStatus(iface.PciAddress, true) + if err != nil { + glog.Warningf("DiscoverSriovDevices(): failed to load PF status from disk: %v", err) + } + + if exist { + iface.ExternallyCreated = pfStatus.ExternallyCreated + } + if dputils.IsSriovPF(device.Address) { iface.TotalVfs = dputils.GetSriovVFcapacity(device.Address) iface.NumVfs = dputils.GetVFconfigured(device.Address) @@ -165,15 +174,34 @@ func SyncNodeState(newState *sriovnetworkv1.SriovNetworkNodeState, pfsToConfig m if !NeedUpdate(&iface, &ifaceStatus) { glog.V(2).Infof("syncNodeState(): no need update interface %s", iface.PciAddress) + + // Save the PF status to the host + err = SaveLastPfAppliedStatus(iface.PciAddress, CreatePfAppliedStatusFromSpec(&iface)) + if err != nil { + glog.Errorf("SyncNodeState(): failed to save PF applied config to host: %v", err) + return err + } + break } if err = configSriovDevice(&iface, &ifaceStatus); err != nil { glog.Errorf("SyncNodeState(): fail to configure sriov interface %s: %v. resetting interface.", iface.PciAddress, err) - if resetErr := resetSriovDevice(ifaceStatus); resetErr != nil { - glog.Errorf("SyncNodeState(): fail to reset on error SR-IOV interface: %s", resetErr) + if iface.ExternallyCreated { + glog.Infof("SyncNodeState(): skipping device reset as the nic is marked as externally created") + } else { + if resetErr := resetSriovDevice(ifaceStatus); resetErr != nil { + glog.Errorf("SyncNodeState(): failed to reset on error SR-IOV interface: %s", resetErr) + } } return err } + + // Save the PF status to the host + err = SaveLastPfAppliedStatus(iface.PciAddress, CreatePfAppliedStatusFromSpec(&iface)) + if err != nil { + glog.Errorf("SyncNodeState(): failed to save PF applied config to host: %v", err) + return err + } break } } @@ -182,6 +210,27 @@ func SyncNodeState(newState *sriovnetworkv1.SriovNetworkNodeState, pfsToConfig m continue } + // load the PF info + pfStatus, exist, err := LoadPfsStatus(ifaceStatus.PciAddress, false) + if err != nil { + glog.Errorf("SyncNodeState(): failed to load info about PF status for pci address %s: %v", ifaceStatus.PciAddress, err) + return err + } + + if !exist { + glog.Infof("SyncNodeState(): PF name %s with pci address %s has VFs configured but they weren't created by the sriov operator. Skipping the device reset", + ifaceStatus.Name, + ifaceStatus.PciAddress) + continue + } + + if pfStatus.ExternallyCreated { + glog.Infof("SyncNodeState(): PF name %s with pci address %s was externally created skipping the device reset", + ifaceStatus.Name, + ifaceStatus.PciAddress) + continue + } + if err = resetSriovDevice(ifaceStatus); err != nil { return err } @@ -270,6 +319,12 @@ func NeedUpdate(iface *sriovnetworkv1.Interface, ifaceStatus *sriovnetworkv1.Int glog.V(2).Infof("NeedUpdate(): VF %d MTU needs update, desired=%d, current=%d", vf.VfID, group.Mtu, vf.Mtu) return true } + + // this is needed to be sure the admin mac address is configured as expected + if iface.ExternallyCreated { + glog.V(2).Infof("NeedUpdate(): need to update the device as it's externally manage for pci address %s", ifaceStatus.PciAddress) + return true + } } break } @@ -293,6 +348,12 @@ func configSriovDevice(iface *sriovnetworkv1.Interface, ifaceStatus *sriovnetwor } // set numVFs if iface.NumVfs != ifaceStatus.NumVfs { + if iface.ExternallyCreated { + errMsg := fmt.Sprintf("configSriovDevice(): number of request virtual functions %d is not equal to configured virtual functions %d but the policy is configured as ExternallyCreated for device %s", iface.NumVfs, ifaceStatus.NumVfs, iface.PciAddress) + glog.Error(errMsg) + return fmt.Errorf(errMsg) + } + err = setSriovNumVfs(iface.PciAddress, iface.NumVfs) if err != nil { glog.Errorf("configSriovDevice(): fail to set NumVfs for device %s", iface.PciAddress) From 4e5a5e07b2291b6f617f3e6f987346db3e08317b Mon Sep 17 00:00:00 2001 From: Sebastian Sch Date: Mon, 8 May 2023 17:27:37 +0300 Subject: [PATCH 5/7] Update the make test command to not try to run the functional tests Signed-off-by: Sebastian Sch --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 04499e416..8b94b2181 100644 --- a/Makefile +++ b/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 From 4b6cd350078ea168bf39d57cd89efb13c7ae8240 Mon Sep 17 00:00:00 2001 From: Sebastian Sch Date: Mon, 19 Jun 2023 12:44:00 +0300 Subject: [PATCH 6/7] Fix in the code for the externallyManage Signed-off-by: Sebastian Sch --- pkg/daemon/daemon.go | 9 +++++++++ pkg/daemon/writer.go | 5 +---- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/pkg/daemon/daemon.go b/pkg/daemon/daemon.go index 3803f3de8..2947e16ef 100644 --- a/pkg/daemon/daemon.go +++ b/pkg/daemon/daemon.go @@ -458,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 { diff --git a/pkg/daemon/writer.go b/pkg/daemon/writer.go index d70ea8843..e2abffe8c 100644 --- a/pkg/daemon/writer.go +++ b/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 { From 46b15e6d1cd21f2d29b7c90691fbae089bedbae5 Mon Sep 17 00:00:00 2001 From: Sebastian Sch Date: Mon, 19 Jun 2023 12:45:53 +0300 Subject: [PATCH 7/7] functional tests for externally manage feature Signed-off-by: Sebastian Sch --- test/conformance/tests/test_sriov_operator.go | 171 ++++++++++++++++-- 1 file changed, 158 insertions(+), 13 deletions(-) diff --git a/test/conformance/tests/test_sriov_operator.go b/test/conformance/tests/test_sriov_operator.go index d093ba326..55adcee18 100644 --- a/test/conformance/tests/test_sriov_operator.go +++ b/test/conformance/tests/test_sriov_operator.go @@ -21,6 +21,8 @@ import ( rbacv1 "k8s.io/api/rbac/v1" k8serrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/fields" + "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/types" "k8s.io/utils/pointer" runtimeclient "sigs.k8s.io/controller-runtime/pkg/client" @@ -907,12 +909,6 @@ var _ = Describe("[sriov] operator", func() { }) Describe("Custom SriovNetworkNodePolicy", func() { - var vfioNode string - var vfioNic sriovv1.InterfaceExt - execute.BeforeAll(func() { - vfioNode, vfioNic = sriovInfos.FindOneVfioSriovDevice() - }) - BeforeEach(func() { err := namespaces.Clean(operatorNamespace, namespaces.Test, clients, discovery.Enabled()) Expect(err).ToNot(HaveOccurred()) @@ -920,15 +916,17 @@ var _ = Describe("[sriov] operator", func() { }) Describe("Configuration", func() { - Context("Create vfio-pci node policy", func() { + var vfioNode string + var vfioNic sriovv1.InterfaceExt + BeforeEach(func() { if discovery.Enabled() { Skip("Test unsuitable to be run in discovery mode") } - if vfioNode == "" { - Skip("skip test as no vfio-pci capable PF was found") - } + + vfioNode, vfioNic = sriovInfos.FindOneVfioSriovDevice() + Expect(vfioNode).ToNot(Equal("")) }) It("Should be possible to create a vfio-pci resource", func() { @@ -964,14 +962,16 @@ var _ = Describe("[sriov] operator", func() { }) Context("PF Partitioning", func() { + var vfioNode string + var vfioNic sriovv1.InterfaceExt + // 27633 BeforeEach(func() { if discovery.Enabled() { Skip("Test unsuitable to be run in discovery mode") } - if vfioNode == "" { - Skip("skip test as no vfio-pci capable PF was found") - } + vfioNode, vfioNic = sriovInfos.FindOneVfioSriovDevice() + Expect(vfioNode).ToNot(Equal("")) }) It("Should be possible to partition the pf's vfs", func() { @@ -1681,6 +1681,137 @@ var _ = Describe("[sriov] operator", func() { Expect(err).ToNot(HaveOccurred()) }) }) + + Context("ExternallyCreated Validation", func() { + numVfs := 5 + var node string + var nic sriovv1.InterfaceExt + externallyManage := func(policy *sriovv1.SriovNetworkNodePolicy) { + policy.Spec.ExternallyCreated = true + } + + execute.BeforeAll(func() { + node, nic = sriovInfos.FindOneVfioSriovDevice() + Expect(node).ToNot(Equal("")) + }) + + It("Should not allow to create a policy if there are no vfs configured", func() { + resourceName := "test" + _, err := network.CreateSriovPolicy(clients, "test-policy-", operatorNamespace, nic.Name, node, numVfs, resourceName, "netdevice", externallyManage) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("is not equal to the number of virtual functions allocated for the PF externally")) + }) + + It("Should create a policy if the number of requested vfs is equal", func() { + resourceName := "testexternally" //nolint:goconst + By("allocating the 5 virtual functions to the selected device") + _, errOutput, err := runCommandOnConfigDaemon(node, "/bin/bash", "-c", fmt.Sprintf("echo 5 > /host/sys/class/net/%s/device/sriov_numvfs", nic.Name)) + Expect(err).ToNot(HaveOccurred()) + Expect(errOutput).To(Equal("")) + + By("creating the policy that will use the 5 virtual functions we create manually on the system") + Eventually(func() error { + _, err := network.CreateSriovPolicy(clients, "test-policy-", operatorNamespace, nic.Name, node, numVfs, resourceName, "netdevice", externallyManage) + return err + }, 1*time.Minute, time.Second).ShouldNot(HaveOccurred()) + + Eventually(func() int64 { + testedNode, err := clients.CoreV1Interface.Nodes().Get(context.Background(), node, metav1.GetOptions{}) + Expect(err).ToNot(HaveOccurred()) + resNum := testedNode.Status.Allocatable[corev1.ResourceName("openshift.io/"+resourceName)] + allocatable, _ := resNum.AsInt64() + return allocatable + }, 2*time.Minute, time.Second).Should(Equal(int64(numVfs))) + + By("cleaning the manual sriov created") + _, errOutput, err = runCommandOnConfigDaemon(node, "/bin/bash", "-c", fmt.Sprintf("echo 0 > /host/sys/class/net/%s/device/sriov_numvfs", nic.Name)) + Expect(err).ToNot(HaveOccurred()) + Expect(errOutput).To(Equal("")) + }) + + It("Should create a policy if the number of requested vfs is equal and not delete them when the policy is removed", func() { + resourceName := "testexternally" + var sriovPolicy *sriovv1.SriovNetworkNodePolicy + By("allocating the 5 virtual functions to the selected device") + _, errOutput, err := runCommandOnConfigDaemon(node, "/bin/bash", "-c", fmt.Sprintf("echo 5 > /host/sys/class/net/%s/device/sriov_numvfs", nic.Name)) + Expect(err).ToNot(HaveOccurred()) + Expect(errOutput).To(Equal("")) + + By("creating the policy that will use the 5 virtual functions we create manually on the system") + Eventually(func() error { + sriovPolicy, err = network.CreateSriovPolicy(clients, "test-policy-", operatorNamespace, nic.Name, node, numVfs, resourceName, "netdevice", externallyManage) + return err + }, 2*time.Minute, time.Second).ShouldNot(HaveOccurred()) + + Eventually(func() int64 { + testedNode, err := clients.CoreV1Interface.Nodes().Get(context.Background(), node, metav1.GetOptions{}) + Expect(err).ToNot(HaveOccurred()) + resNum := testedNode.Status.Allocatable[corev1.ResourceName("openshift.io/"+resourceName)] + allocatable, _ := resNum.AsInt64() + return allocatable + }, 3*time.Minute, time.Second).Should(Equal(int64(numVfs))) + + By("deleting the policy") + err = clients.Delete(context.Background(), sriovPolicy, &runtimeclient.DeleteOptions{}) + Expect(err).ToNot(HaveOccurred()) + WaitForSRIOVStable() + + Eventually(func() int64 { + testedNode, err := clients.CoreV1Interface.Nodes().Get(context.Background(), node, metav1.GetOptions{}) + Expect(err).ToNot(HaveOccurred()) + resNum := testedNode.Status.Allocatable[corev1.ResourceName("openshift.io/"+resourceName)] + allocatable, _ := resNum.AsInt64() + return allocatable + }, 2*time.Minute, time.Second).Should(Equal(int64(0))) + + By("checking the virtual functions are still on the host") + output, errOutput, err := runCommandOnConfigDaemon(node, "/bin/bash", "-c", fmt.Sprintf("cat /host/sys/class/net/%s/device/sriov_numvfs", nic.Name)) + Expect(err).ToNot(HaveOccurred()) + Expect(errOutput).To(Equal("")) + Expect(output).To(ContainSubstring("5")) + + By("cleaning the manual sriov created") + _, errOutput, err = runCommandOnConfigDaemon(node, "/bin/bash", "-c", fmt.Sprintf("echo 0 > /host/sys/class/net/%s/device/sriov_numvfs", nic.Name)) + Expect(err).ToNot(HaveOccurred()) + Expect(errOutput).To(Equal("")) + }) + + It("should reset the virtual functions if externallyCreated is false", func() { + resourceName := "testexternally" //nolint:goconst + + var sriovPolicy *sriovv1.SriovNetworkNodePolicy + By("creating the policy for 5 virtual functions") + sriovPolicy, err := network.CreateSriovPolicy(clients, "test-policy-", operatorNamespace, nic.Name, node, numVfs, resourceName, "netdevice") + Expect(err).ToNot(HaveOccurred()) + + Eventually(func() int64 { + testedNode, err := clients.CoreV1Interface.Nodes().Get(context.Background(), node, metav1.GetOptions{}) + Expect(err).ToNot(HaveOccurred()) + resNum := testedNode.Status.Allocatable[corev1.ResourceName("openshift.io/"+resourceName)] + allocatable, _ := resNum.AsInt64() + return allocatable + }, 3*time.Minute, time.Second).Should(Equal(int64(numVfs))) + + By("deleting the policy") + err = clients.Delete(context.Background(), sriovPolicy, &runtimeclient.DeleteOptions{}) + Expect(err).ToNot(HaveOccurred()) + WaitForSRIOVStable() + + Eventually(func() int64 { + testedNode, err := clients.CoreV1Interface.Nodes().Get(context.Background(), node, metav1.GetOptions{}) + Expect(err).ToNot(HaveOccurred()) + resNum := testedNode.Status.Allocatable[corev1.ResourceName("openshift.io/"+resourceName)] + allocatable, _ := resNum.AsInt64() + return allocatable + }, 3*time.Minute, time.Second).Should(Equal(int64(0))) + + By("checking the virtual functions don't exist anymore on the system") + output, errOutput, err := runCommandOnConfigDaemon(node, "/bin/bash", "-c", fmt.Sprintf("cat /host/sys/class/net/%s/device/sriov_numvfs", nic.Name)) + Expect(err).ToNot(HaveOccurred()) + Expect(errOutput).To(Equal("")) + Expect(output).To(ContainSubstring("0")) + }) + }) }) }) @@ -2080,6 +2211,20 @@ func createVanillaNetworkPolicy(node string, sriovInfos *cluster.EnabledNodes, n }))) } +func runCommandOnConfigDaemon(nodeName string, command ...string) (string, string, error) { + pods := &corev1.PodList{} + label, err := labels.Parse("app=sriov-network-config-daemon") + Expect(err).ToNot(HaveOccurred()) + field, err := fields.ParseSelector(fmt.Sprintf("spec.nodeName=%s", nodeName)) + Expect(err).ToNot(HaveOccurred()) + err = clients.List(context.Background(), pods, &runtimeclient.ListOptions{Namespace: operatorNamespace, LabelSelector: label, FieldSelector: field}) + Expect(err).ToNot(HaveOccurred()) + Expect(len(pods.Items)).To(Equal(1)) + + output, errOutput, err := pod.ExecCommand(clients, &pods.Items[0], command...) + return output, errOutput, err +} + func defaultFilterPolicy(policy sriovv1.SriovNetworkNodePolicy) bool { return policy.Spec.DeviceType == "netdevice" }