From 6cdb4827aaf23ab6193aaa2ff1d9bc9ed8bcf069 Mon Sep 17 00:00:00 2001 From: Sebastian Sch Date: Mon, 29 Aug 2022 11:26:39 +0300 Subject: [PATCH] improve drain check We need to check in the drain function also if VFs exist If they exist we need to check if any other configuration is required (link driver and MTU) If a change to the VF is requested we should drain the node because we can have pods using the VF Signed-off-by: Sebastian Sch --- pkg/plugins/generic/generic_plugin.go | 9 +- pkg/plugins/generic/generic_plugin_test.go | 131 +++++++++++++++++++++ pkg/utils/switchdev.go | 1 + pkg/utils/utils.go | 16 +-- 4 files changed, 142 insertions(+), 15 deletions(-) create mode 100644 pkg/plugins/generic/generic_plugin_test.go diff --git a/pkg/plugins/generic/generic_plugin.go b/pkg/plugins/generic/generic_plugin.go index 3443ff6dd..5a951b88c 100644 --- a/pkg/plugins/generic/generic_plugin.go +++ b/pkg/plugins/generic/generic_plugin.go @@ -158,13 +158,8 @@ func needDrainNode(desired sriovnetworkv1.Interfaces, current sriovnetworkv1.Int // TODO: no need to perform further checks if ifaceStatus.NumVfs equals to 0 // once https://github.com/kubernetes/kubernetes/issues/109595 will be fixed configured = true - if iface.NumVfs != ifaceStatus.NumVfs { - glog.V(2).Infof("generic-plugin needDrainNode(): need drain, expect NumVfs %v, current NumVfs %v", iface.NumVfs, ifaceStatus.NumVfs) - needDrain = true - return - } - if iface.Mtu != 0 && iface.Mtu != ifaceStatus.Mtu { - glog.V(2).Infof("generic-plugin needDrainNode(): need drain, expect MTU %v, current MTU %v", iface.Mtu, ifaceStatus.Mtu) + if utils.NeedUpdate(&iface, &ifaceStatus) { + glog.V(2).Infof("generic-plugin needDrainNode(): need drain, PF %s request update", iface.PciAddress) needDrain = true return } diff --git a/pkg/plugins/generic/generic_plugin_test.go b/pkg/plugins/generic/generic_plugin_test.go new file mode 100644 index 000000000..e5722ece7 --- /dev/null +++ b/pkg/plugins/generic/generic_plugin_test.go @@ -0,0 +1,131 @@ +package generic_test + +import ( + "testing" + + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + + sriovnetworkv1 "github.com/k8snetworkplumbingwg/sriov-network-operator/api/v1" + plugin "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/plugins" + "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/plugins/generic" +) + +func TestGenericPlugin(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Test Generic Plugin") +} + +var _ = Describe("Generic plugin", func() { + var genericPlugin plugin.VendorPlugin + var err error + BeforeEach(func() { + genericPlugin, err = generic.NewGenericPlugin() + Expect(err).ToNot(HaveOccurred()) + }) + + Context("OnNodeStateChange", func() { + It("should not drain", func() { + networkNodeState := &sriovnetworkv1.SriovNetworkNodeState{ + Spec: sriovnetworkv1.SriovNetworkNodeStateSpec{ + Interfaces: sriovnetworkv1.Interfaces{{ + PciAddress: "0000:00:00.0", + NumVfs: 1, + VfGroups: []sriovnetworkv1.VfGroup{{ + DeviceType: "netdevice", + PolicyName: "policy-1", + ResourceName: "resource-1", + VfRange: "0-0", + }}}}, + }, + Status: sriovnetworkv1.SriovNetworkNodeStateStatus{ + Interfaces: sriovnetworkv1.InterfaceExts{{ + PciAddress: "0000:00:00.0", + NumVfs: 1, + TotalVfs: 1, + DeviceID: "1015", + Vendor: "15b3", + Name: "sriovif1", + Mtu: 1500, + Mac: "0c:42:a1:55:ee:46", + Driver: "mlx5_core", + EswitchMode: "legacy", + LinkSpeed: "25000 Mb/s", + LinkType: "ETH", + VFs: []sriovnetworkv1.VirtualFunction{{ + PciAddress: "0000:00:00.1", + DeviceID: "1016", + Vendor: "15b3", + VfID: 0, + Name: "sriovif1v0", + Mtu: 1500, + Mac: "8e:d6:2c:62:87:1b", + Driver: "mlx5_core", + }}, + }}, + }, + } + + needDrain, needReboot, err := genericPlugin.OnNodeStateChange(networkNodeState) + Expect(err).ToNot(HaveOccurred()) + Expect(needReboot).To(BeFalse()) + Expect(needDrain).To(BeFalse()) + }) + + It("should drain", func() { + networkNodeState := &sriovnetworkv1.SriovNetworkNodeState{ + Spec: sriovnetworkv1.SriovNetworkNodeStateSpec{ + Interfaces: sriovnetworkv1.Interfaces{{ + PciAddress: "0000:00:00.0", + NumVfs: 2, + Mtu: 1500, + VfGroups: []sriovnetworkv1.VfGroup{{ + DeviceType: "netdevice", + PolicyName: "policy-1", + ResourceName: "resource-1", + VfRange: "0-1", + Mtu: 1500, + }}}}, + }, + Status: sriovnetworkv1.SriovNetworkNodeStateStatus{ + Interfaces: sriovnetworkv1.InterfaceExts{{ + PciAddress: "0000:00:00.0", + NumVfs: 2, + TotalVfs: 2, + DeviceID: "1015", + Vendor: "15b3", + Name: "sriovif1", + Mtu: 1500, + Mac: "0c:42:a1:55:ee:46", + Driver: "mlx5_core", + EswitchMode: "legacy", + LinkSpeed: "25000 Mb/s", + LinkType: "ETH", + VFs: []sriovnetworkv1.VirtualFunction{{ + PciAddress: "0000:00:00.1", + DeviceID: "1016", + Vendor: "15b3", + VfID: 0, + Driver: "mlx5_core", + Name: "sriovif1v0", + Mtu: 999, // Bad MTU value, changed by the user application + Mac: "8e:d6:2c:62:87:1b", + }, { + PciAddress: "0000:00:00.2", + DeviceID: "1016", + Vendor: "15b3", + VfID: 0, + Driver: "mlx5_core", + }}, + }}, + }, + } + + needDrain, needReboot, err := genericPlugin.OnNodeStateChange(networkNodeState) + Expect(err).ToNot(HaveOccurred()) + Expect(needReboot).To(BeFalse()) + Expect(needDrain).To(BeTrue()) + }) + }) + +}) diff --git a/pkg/utils/switchdev.go b/pkg/utils/switchdev.go index b93dbeb10..4e1bbc390 100644 --- a/pkg/utils/switchdev.go +++ b/pkg/utils/switchdev.go @@ -58,6 +58,7 @@ func WriteSwitchdevConfFile(newState *sriovnetworkv1.SriovNetworkNodeState) (upd if err != nil { if os.IsNotExist(err) { if len(cfg.Interfaces) == 0 { + err = nil return } glog.V(2).Infof("WriteSwitchdevConfFile(): file not existed, create it") diff --git a/pkg/utils/utils.go b/pkg/utils/utils.go index e83dbc6de..d5d3fec2a 100644 --- a/pkg/utils/utils.go +++ b/pkg/utils/utils.go @@ -161,7 +161,7 @@ func SyncNodeState(newState *sriovnetworkv1.SriovNetworkNodeState) error { glog.V(2).Infof("syncNodeState(): skip config VF in config daemon for %s, it shall be done by switchdev-configuration.service", iface.PciAddress) break } - if !needUpdate(&iface, &ifaceStatus) { + if !NeedUpdate(&iface, &ifaceStatus) { glog.V(2).Infof("syncNodeState(): no need update interface %s", iface.PciAddress) break } @@ -198,17 +198,17 @@ func SkipConfigVf(ifSpec sriovnetworkv1.Interface, ifStatus sriovnetworkv1.Inter return false } -func needUpdate(iface *sriovnetworkv1.Interface, ifaceStatus *sriovnetworkv1.InterfaceExt) bool { +func NeedUpdate(iface *sriovnetworkv1.Interface, ifaceStatus *sriovnetworkv1.InterfaceExt) bool { if iface.Mtu > 0 { mtu := iface.Mtu if mtu != ifaceStatus.Mtu { - glog.V(2).Infof("needUpdate(): MTU needs update, desired=%d, current=%d", mtu, ifaceStatus.Mtu) + glog.V(2).Infof("NeedUpdate(): MTU needs update, desired=%d, current=%d", mtu, ifaceStatus.Mtu) return true } } if iface.NumVfs != ifaceStatus.NumVfs { - glog.V(2).Infof("needUpdate(): NumVfs needs update desired=%d, current=%d", iface.NumVfs, ifaceStatus.NumVfs) + glog.V(2).Infof("NeedUpdate(): NumVfs needs update desired=%d, current=%d", iface.NumVfs, ifaceStatus.NumVfs) return true } if iface.NumVfs > 0 { @@ -219,16 +219,16 @@ func needUpdate(iface *sriovnetworkv1.Interface, ifaceStatus *sriovnetworkv1.Int ingroup = true if group.DeviceType != "netdevice" { if group.DeviceType != vf.Driver { - glog.V(2).Infof("needUpdate(): Driver needs update, desired=%s, current=%s", group.DeviceType, vf.Driver) + glog.V(2).Infof("NeedUpdate(): Driver needs update, desired=%s, current=%s", group.DeviceType, vf.Driver) return true } } else { if sriovnetworkv1.StringInArray(vf.Driver, DpdkDrivers) { - glog.V(2).Infof("needUpdate(): Driver needs update, desired=%s, current=%s", group.DeviceType, vf.Driver) + glog.V(2).Infof("NeedUpdate(): Driver needs update, desired=%s, current=%s", group.DeviceType, vf.Driver) return true } - if vf.Mtu != 0 && vf.Mtu != group.Mtu { - glog.V(2).Infof("needUpdate(): VF %d MTU needs update, desired=%d", vf.VfID, group.Mtu) + if vf.Mtu != 0 && group.Mtu != 0 && vf.Mtu != group.Mtu { + glog.V(2).Infof("NeedUpdate(): VF %d MTU needs update, desired=%d, current=%d", vf.VfID, group.Mtu, vf.Mtu) return true } }