diff --git a/bindata/manifests/sriov-config-service/kubernetes/sriov-config-post-network-service.yaml b/bindata/manifests/sriov-config-service/kubernetes/sriov-config-post-network-service.yaml new file mode 100644 index 0000000000..35d00c7e7e --- /dev/null +++ b/bindata/manifests/sriov-config-service/kubernetes/sriov-config-post-network-service.yaml @@ -0,0 +1,16 @@ +contents: | + [Unit] + Description=Configures SRIOV NIC - post network configuration + After=systemd-networkd-wait-online.service NetworkManager-wait-online.service + Before=network-online.target + + [Service] + Type=oneshot + # TODO replace with service call + ExecStart=/bin/true + StandardOutput=journal+console + + [Install] + WantedBy=network-online.target +enabled: true +name: sriov-config-post-network.service diff --git a/bindata/manifests/sriov-config-service/kubernetes/sriov-config-service.yaml b/bindata/manifests/sriov-config-service/kubernetes/sriov-config-service.yaml index 47b10c7ac3..e33b0cada4 100644 --- a/bindata/manifests/sriov-config-service/kubernetes/sriov-config-service.yaml +++ b/bindata/manifests/sriov-config-service/kubernetes/sriov-config-service.yaml @@ -1,14 +1,15 @@ contents: | [Unit] - Description=Configures SRIOV NIC - After=network-pre.target - Before=NetworkManager.service kubelet.service - + Description=Configures SRIOV NIC - pre network configuration + DefaultDependencies=no + After=network-pre.target systemd-udev-settle.service systemd-sysusers.service systemd-sysctl.service + Before=network.target NetworkManager.service systemd-networkd.service ovs-vswitchd.service ovsdb-server.service + [Service] Type=oneshot ExecStart=/var/lib/sriov/sriov-network-config-daemon -v 2 service StandardOutput=journal+console - + [Install] WantedBy=multi-user.target enabled: true diff --git a/bindata/manifests/sriov-config-service/openshift/sriov-config-service.yaml b/bindata/manifests/sriov-config-service/openshift/sriov-config-service.yaml index bbfed7c6c6..d5a0097758 100644 --- a/bindata/manifests/sriov-config-service/openshift/sriov-config-service.yaml +++ b/bindata/manifests/sriov-config-service/openshift/sriov-config-service.yaml @@ -12,19 +12,37 @@ spec: units: - contents: | [Unit] - Description=Configures SRIOV NIC # Removal of this file signals firstboot completion ConditionPathExists=!/etc/ignition-machine-config-encapsulated.json - # This service is used to configure the SR-IOV VFs on NICs - After=network-pre.target - Before=NetworkManager.service kubelet.service - + Description=Configures SRIOV NIC - pre network configuration + DefaultDependencies=no + After=network-pre.target systemd-udev-settle.service systemd-sysusers.service systemd-sysctl.service + Before=network.target NetworkManager.service systemd-networkd.service ovs-vswitchd.service ovsdb-server.service + [Service] Type=oneshot ExecStart=/var/lib/sriov/sriov-network-config-daemon service -v {{ .LogLevel }} StandardOutput=journal+console - + [Install] WantedBy=multi-user.target enabled: true name: "sriov-config.service" + - contents: | + [Unit] + # Removal of this file signals firstboot completion + ConditionPathExists=!/etc/ignition-machine-config-encapsulated.json + Description=Configures SRIOV NIC - post network configuration + After=systemd-networkd-wait-online.service NetworkManager-wait-online.service + Before=network-online.target + + [Service] + Type=oneshot + # TODO replace with service call + ExecStart=/bin/true + StandardOutput=journal+console + + [Install] + WantedBy=network-online.target + enabled: true + name: "sriov-config-post-network.service" diff --git a/pkg/daemon/daemon.go b/pkg/daemon/daemon.go index 9d4a43e94d..5665d9f284 100644 --- a/pkg/daemon/daemon.go +++ b/pkg/daemon/daemon.go @@ -431,13 +431,19 @@ func (dn *Daemon) nodeStateSyncHandler() error { log.Log.Error(err, "nodeStateSyncHandler(): failed to check if sriov-config service exist on host") return err } + postNetworkServiceEnabled, err := dn.HostHelpers.IsServiceEnabled(systemd.SriovPostNetworkServicePath) + if err != nil { + log.Log.Error(err, "nodeStateSyncHandler(): failed to check if sriov-config-post-network service exist on host") + return err + } // if the service doesn't exist we should continue to let the k8s plugin to create the service files // this is only for k8s base environments, for openshift the sriov-operator creates a machine config to will apply // the system service and reboot the node the config-daemon doesn't need to do anything. - if !serviceEnabled { + if !(serviceEnabled && postNetworkServiceEnabled) { sriovResult = &systemd.SriovResult{SyncStatus: consts.SyncStatusFailed, - LastSyncError: "sriov-config systemd service is not available on node"} + LastSyncError: fmt.Sprintf("some sriov systemd services are not available on node: "+ + "sriov-config available:%t, sriov-config-post-network available:%t", serviceEnabled, postNetworkServiceEnabled)} } else { sriovResult, err = systemd.ReadSriovResult() if err != nil { diff --git a/pkg/plugins/k8s/k8s_plugin.go b/pkg/plugins/k8s/k8s_plugin.go index 02b4e63366..98057425a1 100644 --- a/pkg/plugins/k8s/k8s_plugin.go +++ b/pkg/plugins/k8s/k8s_plugin.go @@ -29,6 +29,7 @@ type K8sPlugin struct { openVSwitchService *hostTypes.Service networkManagerService *hostTypes.Service sriovService *hostTypes.Service + sriovPostNetworkService *hostTypes.Service updateTarget *k8sUpdateTarget hostHelper helper.HostHelpersInterface } @@ -40,15 +41,29 @@ type k8sUpdateTarget struct { switchdevAfterNMRunScript bool switchdevUdevScript bool sriovScript bool + sriovPostNetworkScript bool systemServices []*hostTypes.Service } func (u *k8sUpdateTarget) needUpdate() bool { - return u.switchdevBeforeNMService || u.switchdevAfterNMService || u.switchdevBeforeNMRunScript || u.switchdevAfterNMRunScript || u.switchdevUdevScript || u.sriovScript || len(u.systemServices) > 0 + return u.switchdevBeforeNMService || + u.switchdevAfterNMService || + u.switchdevBeforeNMRunScript || + u.switchdevAfterNMRunScript || + u.switchdevUdevScript || + u.sriovScript || + u.sriovPostNetworkScript || + len(u.systemServices) > 0 } func (u *k8sUpdateTarget) needReboot() bool { - return u.switchdevBeforeNMService || u.switchdevAfterNMService || u.switchdevBeforeNMRunScript || u.switchdevAfterNMRunScript || u.switchdevUdevScript || u.sriovScript + return u.switchdevBeforeNMService || + u.switchdevAfterNMService || + u.switchdevBeforeNMRunScript || + u.switchdevAfterNMRunScript || + u.switchdevUdevScript || + u.sriovScript || + u.sriovPostNetworkScript } func (u *k8sUpdateTarget) reset() { @@ -58,6 +73,7 @@ func (u *k8sUpdateTarget) reset() { u.switchdevAfterNMRunScript = false u.switchdevUdevScript = false u.sriovScript = false + u.sriovPostNetworkScript = false u.systemServices = []*hostTypes.Service{} } @@ -85,6 +101,7 @@ const ( switchdevUnits = switchdevManifestPath + "switchdev-units/" sriovUnits = bindataManifestPath + "sriov-config-service/kubernetes/" sriovUnitFile = sriovUnits + "sriov-config-service.yaml" + sriovPostNetworkUnitFile = sriovUnits + "sriov-config-post-network-service.yaml" switchdevBeforeNMUnitFile = switchdevUnits + "switchdev-configuration-before-nm.yaml" switchdevAfterNMUnitFile = switchdevUnits + "switchdev-configuration-after-nm.yaml" networkManagerUnitFile = switchdevUnits + "NetworkManager.service.yaml" @@ -140,7 +157,7 @@ func (p *K8sPlugin) OnNodeStateChange(new *sriovnetworkv1.SriovNetworkNodeState) if vars.UsingSystemdMode { // Check sriov service - err = p.sriovServiceStateUpdate() + err = p.sriovServicesStateUpdate() if err != nil { log.Log.Error(err, "k8s-plugin OnNodeStateChange(): failed") return @@ -168,7 +185,7 @@ func (p *K8sPlugin) Apply() error { } if vars.UsingSystemdMode { - if err := p.updateSriovService(); err != nil { + if err := p.updateSriovServices(); err != nil { return err } } @@ -256,6 +273,16 @@ func (p *K8sPlugin) readSriovServiceManifest() error { return nil } +func (p *K8sPlugin) readSriovPostNetworkServiceManifest() error { + sriovService, err := p.hostHelper.ReadServiceManifestFile(sriovPostNetworkUnitFile) + if err != nil { + return err + } + + p.sriovPostNetworkService = sriovService + return nil +} + func (p *K8sPlugin) readManifestFiles() error { if err := p.readSwitchdevManifest(); err != nil { return err @@ -273,6 +300,10 @@ func (p *K8sPlugin) readManifestFiles() error { return err } + if err := p.readSriovPostNetworkServiceManifest(); err != nil { + return err + } + return nil } @@ -311,23 +342,31 @@ func (p *K8sPlugin) switchdevServiceStateUpdate() error { return nil } -func (p *K8sPlugin) sriovServiceStateUpdate() error { - log.Log.Info("sriovServiceStateUpdate()") - isServiceEnabled, err := p.hostHelper.IsServiceEnabled(p.sriovService.Path) - if err != nil { - return err - } - - // create and enable the service if it doesn't exist or is not enabled - if !isServiceEnabled { - p.updateTarget.sriovScript = true - } else { - p.updateTarget.sriovScript = p.isSystemServiceNeedUpdate(p.sriovService) +func (p *K8sPlugin) sriovServicesStateUpdate() error { + log.Log.Info("sriovServicesStateUpdate()") + + for _, s := range []struct { + srv *hostTypes.Service + update *bool + }{ + {srv: p.sriovService, update: &p.updateTarget.sriovScript}, + {srv: p.sriovPostNetworkService, update: &p.updateTarget.sriovPostNetworkScript}, + } { + isServiceEnabled, err := p.hostHelper.IsServiceEnabled(s.srv.Path) + if err != nil { + return err + } + // create and enable the service if it doesn't exist or is not enabled + if !isServiceEnabled { + *s.update = true + } else { + *s.update = p.isSystemServiceNeedUpdate(s.srv) + } + if *s.update { + p.updateTarget.systemServices = append(p.updateTarget.systemServices, s.srv) + } } - if p.updateTarget.sriovScript { - p.updateTarget.systemServices = append(p.updateTarget.systemServices, p.sriovService) - } return nil } @@ -420,11 +459,19 @@ func (p *K8sPlugin) switchDevServicesStateUpdate() error { return nil } -func (p *K8sPlugin) updateSriovService() error { - if p.updateTarget.sriovScript { - err := p.hostHelper.EnableService(p.sriovService) - if err != nil { - return err +func (p *K8sPlugin) updateSriovServices() error { + for _, s := range []struct { + srv *hostTypes.Service + update bool + }{ + {srv: p.sriovService, update: p.updateTarget.sriovScript}, + {srv: p.sriovPostNetworkService, update: p.updateTarget.sriovPostNetworkScript}, + } { + if s.update { + err := p.hostHelper.EnableService(s.srv) + if err != nil { + return err + } } } diff --git a/pkg/plugins/k8s/k8s_plugin_test.go b/pkg/plugins/k8s/k8s_plugin_test.go new file mode 100644 index 0000000000..c3a64725b5 --- /dev/null +++ b/pkg/plugins/k8s/k8s_plugin_test.go @@ -0,0 +1,194 @@ +package k8s + +import ( + "os" + "testing" + + "github.com/golang/mock/gomock" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "go.uber.org/zap/zapcore" + "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/controller-runtime/pkg/log/zap" + + sriovnetworkv1 "github.com/k8snetworkplumbingwg/sriov-network-operator/api/v1" + mock_helper "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/helper/mock" + "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/host" + hostTypes "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/host/types" + plugin "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/plugins" + "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/vars" +) + +func TestK8sPlugin(t *testing.T) { + log.SetLogger(zap.New( + zap.WriteTo(GinkgoWriter), + zap.Level(zapcore.Level(-2)), + zap.UseDevMode(true))) + RegisterFailHandler(Fail) + RunSpecs(t, "Test K8s Plugin") +} + +// changes current working dir before calling the real function +func registerCall(m *gomock.Call, realF interface{}) *gomock.Call { + cur, _ := os.Getwd() + return m.Do(func(_ ...interface{}) { + os.Chdir("../../..") + }).DoAndReturn(realF).Do(func(_ ...interface{}) { + os.Chdir(cur) + }).AnyTimes() +} + +func setIsSystemdMode(val bool) { + origUsingSystemdMode := vars.UsingSystemdMode + DeferCleanup(func() { + vars.UsingSystemdMode = origUsingSystemdMode + }) + vars.UsingSystemdMode = val +} + +func newServiceNameMatcher(name string) gomock.Matcher { + return &serviceNameMatcher{name: name} +} + +type serviceNameMatcher struct { + name string +} + +func (snm *serviceNameMatcher) Matches(x interface{}) bool { + s, ok := x.(*hostTypes.Service) + if !ok { + return false + } + return snm.name == s.Name +} + +func (snm *serviceNameMatcher) String() string { + return "service name match: " + snm.name +} + +var _ = Describe("K8s plugin", func() { + var ( + k8sPlugin plugin.VendorPlugin + err error + testCtrl *gomock.Controller + hostHelper *mock_helper.MockHostHelpersInterface + ) + + BeforeEach(func() { + testCtrl = gomock.NewController(GinkgoT()) + + hostHelper = mock_helper.NewMockHostHelpersInterface(testCtrl) + realHostMgr := host.NewHostManager(hostHelper) + + // proxy some functions to real host manager to simplify testing and to additionally validate manifests + for _, f := range []string{ + "bindata/manifests/switchdev-config/files/switchdev-configuration-before-nm.sh.yaml", + "bindata/manifests/switchdev-config/files/switchdev-configuration-after-nm.sh.yaml", + "bindata/manifests/switchdev-config/files/switchdev-vf-link-name.sh.yaml", + } { + registerCall(hostHelper.EXPECT().ReadScriptManifestFile(f), realHostMgr.ReadScriptManifestFile) + } + for _, f := range []string{ + "bindata/manifests/switchdev-config/switchdev-units/switchdev-configuration-before-nm.yaml", + "bindata/manifests/switchdev-config/switchdev-units/switchdev-configuration-after-nm.yaml", + "bindata/manifests/sriov-config-service/kubernetes/sriov-config-service.yaml", + "bindata/manifests/sriov-config-service/kubernetes/sriov-config-post-network-service.yaml", + } { + registerCall(hostHelper.EXPECT().ReadServiceManifestFile(f), realHostMgr.ReadServiceManifestFile) + } + for _, s := range []string{ + "switchdev-configuration-before-nm.service", + "switchdev-configuration-after-nm.service", + } { + registerCall(hostHelper.EXPECT().RemoveFromService(newServiceNameMatcher(s), gomock.Any()), realHostMgr.RemoveFromService) + } + for _, s := range []string{ + "bindata/manifests/switchdev-config/switchdev-units/NetworkManager.service.yaml", + "bindata/manifests/switchdev-config/ovs-units/ovs-vswitchd.service.yaml", + } { + registerCall(hostHelper.EXPECT().ReadServiceInjectionManifestFile(s), realHostMgr.ReadServiceInjectionManifestFile) + } + k8sPlugin, err = NewK8sPlugin(hostHelper) + Expect(err).ToNot(HaveOccurred()) + }) + + AfterEach(func() { + testCtrl.Finish() + }) + + It("no switchdev, no systemd", func() { + setIsSystemdMode(false) + needDrain, needReboot, err := k8sPlugin.OnNodeStateChange(&sriovnetworkv1.SriovNetworkNodeState{}) + Expect(err).ToNot(HaveOccurred()) + Expect(needReboot).To(BeFalse()) + Expect(needDrain).To(BeFalse()) + Expect(k8sPlugin.Apply()).NotTo(HaveOccurred()) + }) + + It("systemd, created", func() { + setIsSystemdMode(true) + + hostHelper.EXPECT().IsServiceEnabled("/etc/systemd/system/sriov-config.service").Return(false, nil) + hostHelper.EXPECT().IsServiceEnabled("/etc/systemd/system/sriov-config-post-network.service").Return(false, nil) + hostHelper.EXPECT().EnableService(newServiceNameMatcher("sriov-config.service")).Return(nil) + hostHelper.EXPECT().EnableService(newServiceNameMatcher("sriov-config-post-network.service")).Return(nil) + hostHelper.EXPECT().UpdateSystemService(newServiceNameMatcher("sriov-config.service")).Return(nil) + hostHelper.EXPECT().UpdateSystemService(newServiceNameMatcher("sriov-config-post-network.service")).Return(nil) + + needDrain, needReboot, err := k8sPlugin.OnNodeStateChange(&sriovnetworkv1.SriovNetworkNodeState{}) + Expect(err).ToNot(HaveOccurred()) + Expect(needReboot).To(BeTrue()) + Expect(needDrain).To(BeTrue()) + Expect(k8sPlugin.Apply()).NotTo(HaveOccurred()) + }) + It("systemd, already configured", func() { + setIsSystemdMode(true) + + hostHelper.EXPECT().IsServiceEnabled("/etc/systemd/system/sriov-config.service").Return(true, nil) + hostHelper.EXPECT().ReadService("/etc/systemd/system/sriov-config.service").Return( + &hostTypes.Service{Name: "sriov-config.service"}, nil) + hostHelper.EXPECT().CompareServices( + &hostTypes.Service{Name: "sriov-config.service"}, + newServiceNameMatcher("sriov-config.service"), + ).Return(false, nil) + + hostHelper.EXPECT().IsServiceEnabled("/etc/systemd/system/sriov-config-post-network.service").Return(true, nil) + hostHelper.EXPECT().ReadService("/etc/systemd/system/sriov-config-post-network.service").Return( + &hostTypes.Service{Name: "sriov-config-post-network.service"}, nil) + hostHelper.EXPECT().CompareServices(&hostTypes.Service{Name: "sriov-config-post-network.service"}, + newServiceNameMatcher("sriov-config-post-network.service"), + ).Return(false, nil) + + needDrain, needReboot, err := k8sPlugin.OnNodeStateChange(&sriovnetworkv1.SriovNetworkNodeState{}) + Expect(err).ToNot(HaveOccurred()) + Expect(needReboot).To(BeFalse()) + Expect(needDrain).To(BeFalse()) + Expect(k8sPlugin.Apply()).NotTo(HaveOccurred()) + }) + It("systemd, update required", func() { + setIsSystemdMode(true) + + hostHelper.EXPECT().IsServiceEnabled("/etc/systemd/system/sriov-config.service").Return(true, nil) + hostHelper.EXPECT().ReadService("/etc/systemd/system/sriov-config.service").Return( + &hostTypes.Service{Name: "sriov-config.service"}, nil) + hostHelper.EXPECT().CompareServices( + &hostTypes.Service{Name: "sriov-config.service"}, + newServiceNameMatcher("sriov-config.service"), + ).Return(true, nil) + hostHelper.EXPECT().EnableService(newServiceNameMatcher("sriov-config.service")).Return(nil) + hostHelper.EXPECT().UpdateSystemService(newServiceNameMatcher("sriov-config.service")).Return(nil) + + hostHelper.EXPECT().IsServiceEnabled("/etc/systemd/system/sriov-config-post-network.service").Return(true, nil) + hostHelper.EXPECT().ReadService("/etc/systemd/system/sriov-config-post-network.service").Return( + &hostTypes.Service{Name: "sriov-config-post-network.service"}, nil) + hostHelper.EXPECT().CompareServices(&hostTypes.Service{Name: "sriov-config-post-network.service"}, + newServiceNameMatcher("sriov-config-post-network.service"), + ).Return(false, nil) + + needDrain, needReboot, err := k8sPlugin.OnNodeStateChange(&sriovnetworkv1.SriovNetworkNodeState{}) + Expect(err).ToNot(HaveOccurred()) + Expect(needReboot).To(BeTrue()) + Expect(needDrain).To(BeTrue()) + Expect(k8sPlugin.Apply()).NotTo(HaveOccurred()) + }) +}) diff --git a/pkg/systemd/systemd.go b/pkg/systemd/systemd.go index f682d85f5f..844cf99944 100644 --- a/pkg/systemd/systemd.go +++ b/pkg/systemd/systemd.go @@ -40,8 +40,10 @@ const ( sriovHostSystemdSupportedNicPath = consts.Host + sriovSystemdSupportedNicPath sriovHostSystemdServiceBinaryPath = consts.Host + sriovSystemdServiceBinaryPath - SriovServicePath = "/etc/systemd/system/sriov-config.service" - SriovHostServicePath = consts.Host + SriovServicePath + SriovServicePath = "/etc/systemd/system/sriov-config.service" + SriovPostNetworkServicePath = "/etc/systemd/system/sriov-config-post-network.service" + SriovHostServicePath = consts.Host + SriovServicePath + SriovHostPostNetworkServicePath = consts.Host + SriovPostNetworkServicePath HostSriovConfBasePath = consts.Host + consts.SriovConfBasePath ) @@ -307,6 +309,10 @@ func CleanSriovFilesFromHost(isOpenShift bool) error { if err != nil && !os.IsNotExist(err) { return err } + err = os.Remove(SriovHostPostNetworkServicePath) + if err != nil && !os.IsNotExist(err) { + return err + } } return nil