From 8d4ae2086899821b3a0f76786074566390ce70b0 Mon Sep 17 00:00:00 2001 From: Yury Kulazhenkov Date: Thu, 21 Nov 2024 15:59:12 +0200 Subject: [PATCH] Add waitForDevicesInitialization to systemd service This function ensures that the network devices specified in the configuration are registered and handled by UDEV. Sometimes, the initialization of network devices might take a significant amount of time, and the sriov-config systemd service may start before the devices are fully processed, leading to failure. --- cmd/sriov-network-config-daemon/service.go | 57 +++++++++++++++++++ .../service_test.go | 20 +++++++ pkg/helper/mock/mock_helper.go | 14 +++++ pkg/host/internal/udev/udev.go | 13 +++++ pkg/host/internal/udev/udev_test.go | 10 ++++ pkg/host/mock/mock_host.go | 14 +++++ pkg/host/types/interfaces.go | 3 + 7 files changed, 131 insertions(+) diff --git a/cmd/sriov-network-config-daemon/service.go b/cmd/sriov-network-config-daemon/service.go index 382ad976b..0209583cc 100644 --- a/cmd/sriov-network-config-daemon/service.go +++ b/cmd/sriov-network-config-daemon/service.go @@ -19,6 +19,7 @@ import ( "errors" "fmt" "os" + "time" "github.com/go-logr/logr" "github.com/spf13/cobra" @@ -40,6 +41,12 @@ import ( const ( PhasePre = "pre" PhasePost = "post" + + // InitializationDeviceDiscoveryTimeoutSec constant defines the number of + // seconds to wait for devices to be registered in the system with the expected name. + InitializationDeviceDiscoveryTimeoutSec = 60 + // InitializationDeviceUdevProcessingTimeoutSec constant defines the number of seconds to wait for udev rules to process + InitializationDeviceUdevProcessingTimeoutSec = 60 ) var ( @@ -104,6 +111,8 @@ func runServiceCmd(cmd *cobra.Command, args []string) error { return updateSriovResultErr(setupLog, phaseArg, fmt.Errorf("failed to create hostHelpers: %v", err)) } + waitForDevicesInitialization(setupLog, sriovConf, hostHelpers) + if phaseArg == PhasePre { err = phasePre(setupLog, sriovConf, hostHelpers) } else { @@ -303,3 +312,51 @@ func updateResult(setupLog logr.Logger, result, msg string) error { setupLog.V(0).Info("result file updated", "SyncStatus", sriovResult.SyncStatus, "LastSyncError", msg) return nil } + +// waitForDevicesInitialization should be executed in both the pre and post-networking stages. +// This function ensures that the network devices specified in the configuration are registered +// and handled by UDEV. Sometimes, the initialization of network devices might take a significant +// amount of time, and the sriov-config systemd service may start before the devices are fully +// processed, leading to failure. +// +// To address this, we not only check if the devices are registered with the correct name but also +// wait for the udev event queue to empty. This increases the likelihood that the service will start +// only when the devices are fully initialized. It is required to call this function in the +// "post-networking" phase as well because the OS network manager might change device configurations, +// and we need to ensure these changes are fully processed before starting the post-networking part. +// +// The timeouts used in this function are intentionally kept low to avoid blocking the OS loading +// process for too long in case of any issues. +// +// Note: Currently, this function handles only Baremetal clusters. We do not have evidence that +// this logic is required for virtual clusters. +func waitForDevicesInitialization(setupLog logr.Logger, conf *systemd.SriovConfig, hostHelpers helper.HostHelpersInterface) { + if conf.PlatformType != consts.Baremetal { + // skip waiting on virtual cluster + return + } + // wait for devices from the spec to be registered in the system with expected names + devicesToWait := make(map[string]string, len(conf.Spec.Interfaces)) + for _, d := range conf.Spec.Interfaces { + devicesToWait[d.PciAddress] = d.Name + } + deadline := time.Now().Add(time.Second * time.Duration(InitializationDeviceDiscoveryTimeoutSec)) + for time.Now().Before(deadline) { + for pciAddr, name := range devicesToWait { + if hostHelpers.TryGetInterfaceName(pciAddr) == name { + delete(devicesToWait, pciAddr) + } + } + if len(devicesToWait) == 0 { + break + } + time.Sleep(time.Second) + } + if len(devicesToWait) != 0 { + setupLog.Info("WARNING: some devices were not initialized", "devices", devicesToWait, "timeout", InitializationDeviceDiscoveryTimeoutSec) + } + if err := hostHelpers.WaitUdevEventsProcessed(InitializationDeviceUdevProcessingTimeoutSec); err != nil { + setupLog.Info("WARNING: failed to wait for udev events processing", "reason", err.Error(), + "timeout", InitializationDeviceUdevProcessingTimeoutSec) + } +} diff --git a/cmd/sriov-network-config-daemon/service_test.go b/cmd/sriov-network-config-daemon/service_test.go index 771cc3b1c..8ce4e2c5e 100644 --- a/cmd/sriov-network-config-daemon/service_test.go +++ b/cmd/sriov-network-config-daemon/service_test.go @@ -3,6 +3,7 @@ package main import ( "fmt" + "github.com/go-logr/logr" "github.com/golang/mock/gomock" "github.com/spf13/cobra" "gopkg.in/yaml.v3" @@ -158,6 +159,8 @@ var _ = Describe("Service", func() { "/etc/sriov-operator/sriov-interface-result.yaml": []byte("something"), }, }) + hostHelpers.EXPECT().TryGetInterfaceName("0000:d8:00.0").Return("enp216s0f0np0") + hostHelpers.EXPECT().WaitUdevEventsProcessed(60).Return(nil) hostHelpers.EXPECT().CheckRDMAEnabled().Return(true, nil) hostHelpers.EXPECT().TryEnableTun().Return() hostHelpers.EXPECT().TryEnableVhostNet().Return() @@ -211,6 +214,8 @@ var _ = Describe("Service", func() { "/etc/sriov-operator/sriov-interface-result.yaml": []byte("something"), }, }) + hostHelpers.EXPECT().TryGetInterfaceName("0000:d8:00.0").Return("enp216s0f0np0") + hostHelpers.EXPECT().WaitUdevEventsProcessed(60).Return(nil) hostHelpers.EXPECT().CheckRDMAEnabled().Return(true, nil) hostHelpers.EXPECT().TryEnableTun().Return() hostHelpers.EXPECT().TryEnableVhostNet().Return() @@ -236,6 +241,8 @@ var _ = Describe("Service", func() { "/etc/sriov-operator/sriov-interface-result.yaml": getTestResultFileContent("InProgress", ""), }, }) + hostHelpers.EXPECT().TryGetInterfaceName("0000:d8:00.0").Return("enp216s0f0np0") + hostHelpers.EXPECT().WaitUdevEventsProcessed(60).Return(nil) hostHelpers.EXPECT().DiscoverSriovDevices(hostHelpers).Return([]sriovnetworkv1.InterfaceExt{{ Name: "enp216s0f0np0", }}, nil) @@ -276,4 +283,17 @@ var _ = Describe("Service", func() { testHelpers.GinkgoAssertFileContentsEquals("/etc/sriov-operator/sriov-interface-result.yaml", string(getTestResultFileContent("Failed", "post: unexpected result of the pre phase: Failed, syncError: pretest"))) }) + It("waitForDevicesInitialization", func() { + cfg := &systemd.SriovConfig{Spec: sriovnetworkv1.SriovNetworkNodeStateSpec{ + Interfaces: []sriovnetworkv1.Interface{ + {Name: "name1", PciAddress: "0000:d8:00.0"}, + {Name: "name2", PciAddress: "0000:d8:00.1"}}}} + hostHelpers.EXPECT().TryGetInterfaceName("0000:d8:00.0").Return("other") + hostHelpers.EXPECT().TryGetInterfaceName("0000:d8:00.1").Return("") + hostHelpers.EXPECT().TryGetInterfaceName("0000:d8:00.0").Return("name1") + hostHelpers.EXPECT().TryGetInterfaceName("0000:d8:00.1").Return("") + hostHelpers.EXPECT().TryGetInterfaceName("0000:d8:00.1").Return("name2") + hostHelpers.EXPECT().WaitUdevEventsProcessed(60).Return(nil) + waitForDevicesInitialization(logr.Discard(), cfg, hostHelpers) + }) }) diff --git a/pkg/helper/mock/mock_helper.go b/pkg/helper/mock/mock_helper.go index b413ecdee..8498d5c4d 100644 --- a/pkg/helper/mock/mock_helper.go +++ b/pkg/helper/mock/mock_helper.go @@ -1224,6 +1224,20 @@ func (mr *MockHostHelpersInterfaceMockRecorder) VFIsReady(pciAddr interface{}) * return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "VFIsReady", reflect.TypeOf((*MockHostHelpersInterface)(nil).VFIsReady), pciAddr) } +// WaitUdevEventsProcessed mocks base method. +func (m *MockHostHelpersInterface) WaitUdevEventsProcessed(timeout int) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "WaitUdevEventsProcessed", timeout) + ret0, _ := ret[0].(error) + return ret0 +} + +// WaitUdevEventsProcessed indicates an expected call of WaitUdevEventsProcessed. +func (mr *MockHostHelpersInterfaceMockRecorder) WaitUdevEventsProcessed(timeout interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "WaitUdevEventsProcessed", reflect.TypeOf((*MockHostHelpersInterface)(nil).WaitUdevEventsProcessed), timeout) +} + // WriteCheckpointFile mocks base method. func (m *MockHostHelpersInterface) WriteCheckpointFile(arg0 *v1.SriovNetworkNodeState) error { m.ctrl.T.Helper() diff --git a/pkg/host/internal/udev/udev.go b/pkg/host/internal/udev/udev.go index 3f828bb70..841bc71d7 100644 --- a/pkg/host/internal/udev/udev.go +++ b/pkg/host/internal/udev/udev.go @@ -5,6 +5,7 @@ import ( "os" "path" "path/filepath" + "strconv" "strings" "sigs.k8s.io/controller-runtime/pkg/log" @@ -126,6 +127,18 @@ func (u *udev) LoadUdevRules() error { return nil } +// WaitUdevEventsProcessed calls `udevadm settle“ with provided timeout +// The command watches the udev event queue, and exits if all current events are handled. +func (u *udev) WaitUdevEventsProcessed(timeout int) error { + log.Log.V(2).Info("WaitUdevEventsProcessed()") + _, stderr, err := u.utilsHelper.RunCommand("udevadm", "settle", "-t", strconv.Itoa(timeout)) + if err != nil { + log.Log.Error(err, "WaitUdevEventsProcessed(): failed to wait for udev rules to process", "error", stderr, "timeout", timeout) + return err + } + return nil +} + func (u *udev) addUdevRule(pfPciAddress, ruleName, ruleContent string) error { log.Log.V(2).Info("addUdevRule()", "device", pfPciAddress, "rule", ruleName) rulePath := u.getRuleFolderPath() diff --git a/pkg/host/internal/udev/udev_test.go b/pkg/host/internal/udev/udev_test.go index 4a2e17e7e..fd1107af3 100644 --- a/pkg/host/internal/udev/udev_test.go +++ b/pkg/host/internal/udev/udev_test.go @@ -210,4 +210,14 @@ var _ = Describe("UDEV", func() { Expect(s.LoadUdevRules()).To(MatchError(testError)) }) }) + Context("WaitUdevEventsProcessed", func() { + It("Succeed", func() { + utilsMock.EXPECT().RunCommand("udevadm", "settle", "-t", "10").Return("", "", nil) + Expect(s.WaitUdevEventsProcessed(10)).NotTo(HaveOccurred()) + }) + It("Command Failed", func() { + utilsMock.EXPECT().RunCommand("udevadm", "settle", "-t", "20").Return("", "", testError) + Expect(s.WaitUdevEventsProcessed(20)).To(MatchError(testError)) + }) + }) }) diff --git a/pkg/host/mock/mock_host.go b/pkg/host/mock/mock_host.go index 095d270a9..b7f9271c8 100644 --- a/pkg/host/mock/mock_host.go +++ b/pkg/host/mock/mock_host.go @@ -1038,3 +1038,17 @@ func (mr *MockHostManagerInterfaceMockRecorder) VFIsReady(pciAddr interface{}) * mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "VFIsReady", reflect.TypeOf((*MockHostManagerInterface)(nil).VFIsReady), pciAddr) } + +// WaitUdevEventsProcessed mocks base method. +func (m *MockHostManagerInterface) WaitUdevEventsProcessed(timeout int) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "WaitUdevEventsProcessed", timeout) + ret0, _ := ret[0].(error) + return ret0 +} + +// WaitUdevEventsProcessed indicates an expected call of WaitUdevEventsProcessed. +func (mr *MockHostManagerInterfaceMockRecorder) WaitUdevEventsProcessed(timeout interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "WaitUdevEventsProcessed", reflect.TypeOf((*MockHostManagerInterface)(nil).WaitUdevEventsProcessed), timeout) +} diff --git a/pkg/host/types/interfaces.go b/pkg/host/types/interfaces.go index 6844ee5ae..2ffcb3268 100644 --- a/pkg/host/types/interfaces.go +++ b/pkg/host/types/interfaces.go @@ -164,6 +164,9 @@ type UdevInterface interface { RemoveVfRepresentorUdevRule(pfPciAddress string) error // LoadUdevRules triggers udev rules for network subsystem LoadUdevRules() error + // WaitUdevEventsProcessed calls `udevadm settle“ with provided timeout + // The command watches the udev event queue, and exits if all current events are handled. + WaitUdevEventsProcessed(timeout int) error } type VdpaInterface interface {