diff --git a/api/v1/sriovnetworknodestate_types.go b/api/v1/sriovnetworknodestate_types.go index e5f59d71cf..a6d23cb40d 100644 --- a/api/v1/sriovnetworknodestate_types.go +++ b/api/v1/sriovnetworknodestate_types.go @@ -119,6 +119,9 @@ type System struct { // +kubebuilder:validation:Enum=shared;exclusive //RDMA subsystem. Allowed value "shared", "exclusive". RdmaMode string `json:"rdmaMode,omitempty"` + // OVS config. It will be provided for ovs-vswitchd service as other_config option + // +kubebuilder:default:={hw-offload: "true"} + OvsConfig map[string]string `json:"ovsConfig,omitempty"` } // SriovNetworkNodeStateStatus defines the observed state of SriovNetworkNodeState diff --git a/api/v1/sriovnetworkpoolconfig_types.go b/api/v1/sriovnetworkpoolconfig_types.go index 011ffc7d91..8a4800fd3b 100644 --- a/api/v1/sriovnetworkpoolconfig_types.go +++ b/api/v1/sriovnetworkpoolconfig_types.go @@ -34,6 +34,9 @@ type OvsHardwareOffloadConfig struct { // On OpenShift: // Name is the name of MachineConfigPool to be enabled with OVS hardware offload Name string `json:"name,omitempty"` + // OVS config. It will be provided for ovs-vswitchd service as other_config option + // +kubebuilder:default:={hw-offload: "true"} + OvsConfig map[string]string `json:"otherConfig,omitempty"` } // SriovNetworkPoolConfigStatus defines the observed state of SriovNetworkPoolConfig diff --git a/api/v1/zz_generated.deepcopy.go b/api/v1/zz_generated.deepcopy.go index 307d07de69..67d611ec6d 100644 --- a/api/v1/zz_generated.deepcopy.go +++ b/api/v1/zz_generated.deepcopy.go @@ -416,6 +416,13 @@ func (in *OVSUplinkConfigExt) DeepCopy() *OVSUplinkConfigExt { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *OvsHardwareOffloadConfig) DeepCopyInto(out *OvsHardwareOffloadConfig) { *out = *in + if in.OvsConfig != nil { + in, out := &in.OvsConfig, &out.OvsConfig + *out = make(map[string]string, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new OvsHardwareOffloadConfig. @@ -788,7 +795,7 @@ func (in *SriovNetworkNodeStateSpec) DeepCopyInto(out *SriovNetworkNodeStateSpec } } in.Bridges.DeepCopyInto(&out.Bridges) - out.System = in.System + in.System.DeepCopyInto(&out.System) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new SriovNetworkNodeStateSpec. @@ -812,7 +819,7 @@ func (in *SriovNetworkNodeStateStatus) DeepCopyInto(out *SriovNetworkNodeStateSt } } in.Bridges.DeepCopyInto(&out.Bridges) - out.System = in.System + in.System.DeepCopyInto(&out.System) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new SriovNetworkNodeStateStatus. @@ -887,7 +894,7 @@ func (in *SriovNetworkPoolConfigList) DeepCopyObject() runtime.Object { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *SriovNetworkPoolConfigSpec) DeepCopyInto(out *SriovNetworkPoolConfigSpec) { *out = *in - out.OvsHardwareOffloadConfig = in.OvsHardwareOffloadConfig + in.OvsHardwareOffloadConfig.DeepCopyInto(&out.OvsHardwareOffloadConfig) if in.NodeSelector != nil { in, out := &in.NodeSelector, &out.NodeSelector *out = new(metav1.LabelSelector) @@ -1083,6 +1090,13 @@ func (in *SriovOperatorConfigStatus) DeepCopy() *SriovOperatorConfigStatus { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *System) DeepCopyInto(out *System) { *out = *in + if in.OvsConfig != nil { + in, out := &in.OvsConfig, &out.OvsConfig + *out = make(map[string]string, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new System. diff --git a/bindata/manifests/switchdev-config/ovs-units/ovs-vswitchd.service.yaml b/bindata/manifests/switchdev-config/ovs-units/ovs-vswitchd.service.yaml index 66d7efed13..0e09b28bc7 100644 --- a/bindata/manifests/switchdev-config/ovs-units/ovs-vswitchd.service.yaml +++ b/bindata/manifests/switchdev-config/ovs-units/ovs-vswitchd.service.yaml @@ -3,4 +3,4 @@ dropins: - name: 10-hw-offload.conf contents: | [Service] - ExecStartPre=/bin/ovs-vsctl --no-wait set Open_vSwitch . other_config:hw-offload=true + ExecStartPre=/bin/ovs-vsctl -t 5 set Open_vSwitch . external_ids:sriov-operator-owned-keys='{{ .ExternalIds }}' && /bin/ovs-vsctl/ovs-vsctl -t 5 get Open_vSwitch . external_ids:sriov-operator-owned-keys | xargs /bin/ovs-vsctl --no-wait remove Open_vSwitch . other_config && /bin/ovs-vsctl --no-wait set Open_vSwitch . {{ .OtherOvsConfig }} diff --git a/config/crd/bases/sriovnetwork.openshift.io_sriovnetworknodestates.yaml b/config/crd/bases/sriovnetwork.openshift.io_sriovnetworknodestates.yaml index e54e273907..0fbefecdd9 100644 --- a/config/crd/bases/sriovnetwork.openshift.io_sriovnetworknodestates.yaml +++ b/config/crd/bases/sriovnetwork.openshift.io_sriovnetworknodestates.yaml @@ -187,6 +187,14 @@ spec: type: array system: properties: + ovsConfig: + additionalProperties: + type: string + default: + hw-offload: "true" + description: OVS config. It will be provided for ovs-vswitchd + service as other_config option + type: object rdmaMode: description: RDMA subsystem. Allowed value "shared", "exclusive". enum: @@ -368,6 +376,14 @@ spec: type: string system: properties: + ovsConfig: + additionalProperties: + type: string + default: + hw-offload: "true" + description: OVS config. It will be provided for ovs-vswitchd + service as other_config option + type: object rdmaMode: description: RDMA subsystem. Allowed value "shared", "exclusive". enum: diff --git a/config/crd/bases/sriovnetwork.openshift.io_sriovnetworkpoolconfigs.yaml b/config/crd/bases/sriovnetwork.openshift.io_sriovnetworkpoolconfigs.yaml index bb047310c8..bce2dec736 100644 --- a/config/crd/bases/sriovnetwork.openshift.io_sriovnetworkpoolconfigs.yaml +++ b/config/crd/bases/sriovnetwork.openshift.io_sriovnetworkpoolconfigs.yaml @@ -111,6 +111,14 @@ spec: On OpenShift: Name is the name of MachineConfigPool to be enabled with OVS hardware offload type: string + otherConfig: + additionalProperties: + type: string + default: + hw-offload: "true" + description: OVS config. It will be provided for ovs-vswitchd + service as other_config option + type: object type: object rdmaMode: description: RDMA subsystem. Allowed value "shared", "exclusive". diff --git a/controllers/sriovnetworknodepolicy_controller.go b/controllers/sriovnetworknodepolicy_controller.go index cedf0ef8d9..5ec921079a 100644 --- a/controllers/sriovnetworknodepolicy_controller.go +++ b/controllers/sriovnetworknodepolicy_controller.go @@ -308,6 +308,12 @@ func (r *SriovNetworkNodePolicyReconciler) syncAllSriovNetworkNodeStates(ctx con } if netPoolConfig != nil { ns.Spec.System.RdmaMode = netPoolConfig.Spec.RdmaMode + if netPoolConfig.Spec.OvsHardwareOffloadConfig.OvsConfig != nil { + ns.Spec.System.OvsConfig = make(map[string]string, len(netPoolConfig.Spec.OvsHardwareOffloadConfig.OvsConfig)) + for k, v := range netPoolConfig.Spec.OvsHardwareOffloadConfig.OvsConfig { + ns.Spec.System.OvsConfig[k] = v + } + } } j, _ := json.Marshal(ns) logger.V(2).Info("SriovNetworkNodeState CR", "content", j) diff --git a/controllers/sriovnetworkpoolconfig_controller.go b/controllers/sriovnetworkpoolconfig_controller.go index a455522cd5..7a82e6432f 100644 --- a/controllers/sriovnetworkpoolconfig_controller.go +++ b/controllers/sriovnetworkpoolconfig_controller.go @@ -20,6 +20,7 @@ import ( constants "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/consts" "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/orchestrator" "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/render" + "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/utils" "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/vars" ) @@ -160,6 +161,9 @@ func (r *SriovNetworkPoolConfigReconciler) syncOvsHardwareOffloadMachineConfigs( } data := render.MakeRenderData() + externalIds, otherOvsConfig := utils.RenderOtherOvsConfigOption(nc.Spec.OvsHardwareOffloadConfig.OvsConfig) + data.Data["OtherOvsConfig"] = otherOvsConfig + data.Data["ExternalIds"] = externalIds mc, err := render.GenerateMachineConfig("bindata/manifests/switchdev-config", mcName, mcpName, true, &data) if err != nil { return err diff --git a/deployment/sriov-network-operator-chart/crds/sriovnetwork.openshift.io_sriovnetworknodestates.yaml b/deployment/sriov-network-operator-chart/crds/sriovnetwork.openshift.io_sriovnetworknodestates.yaml index e54e273907..0fbefecdd9 100644 --- a/deployment/sriov-network-operator-chart/crds/sriovnetwork.openshift.io_sriovnetworknodestates.yaml +++ b/deployment/sriov-network-operator-chart/crds/sriovnetwork.openshift.io_sriovnetworknodestates.yaml @@ -187,6 +187,14 @@ spec: type: array system: properties: + ovsConfig: + additionalProperties: + type: string + default: + hw-offload: "true" + description: OVS config. It will be provided for ovs-vswitchd + service as other_config option + type: object rdmaMode: description: RDMA subsystem. Allowed value "shared", "exclusive". enum: @@ -368,6 +376,14 @@ spec: type: string system: properties: + ovsConfig: + additionalProperties: + type: string + default: + hw-offload: "true" + description: OVS config. It will be provided for ovs-vswitchd + service as other_config option + type: object rdmaMode: description: RDMA subsystem. Allowed value "shared", "exclusive". enum: diff --git a/deployment/sriov-network-operator-chart/crds/sriovnetwork.openshift.io_sriovnetworkpoolconfigs.yaml b/deployment/sriov-network-operator-chart/crds/sriovnetwork.openshift.io_sriovnetworkpoolconfigs.yaml index bb047310c8..bce2dec736 100644 --- a/deployment/sriov-network-operator-chart/crds/sriovnetwork.openshift.io_sriovnetworkpoolconfigs.yaml +++ b/deployment/sriov-network-operator-chart/crds/sriovnetwork.openshift.io_sriovnetworkpoolconfigs.yaml @@ -111,6 +111,14 @@ spec: On OpenShift: Name is the name of MachineConfigPool to be enabled with OVS hardware offload type: string + otherConfig: + additionalProperties: + type: string + default: + hw-offload: "true" + description: OVS config. It will be provided for ovs-vswitchd + service as other_config option + type: object type: object rdmaMode: description: RDMA subsystem. Allowed value "shared", "exclusive". diff --git a/pkg/daemon/daemon_test.go b/pkg/daemon/daemon_test.go index e13b14bd32..18ad59ddd5 100644 --- a/pkg/daemon/daemon_test.go +++ b/pkg/daemon/daemon_test.go @@ -212,7 +212,7 @@ var _ = Describe("Daemon Controller", Ordered, func() { // k8s plugin for k8s cluster type if vars.ClusterType == constants.ClusterTypeKubernetes { hostHelper.EXPECT().ReadServiceManifestFile(gomock.Any()).Return(&hostTypes.Service{Name: "test"}, nil).AnyTimes() - hostHelper.EXPECT().ReadServiceInjectionManifestFile(gomock.Any()).Return(&hostTypes.Service{Name: "test"}, nil).AnyTimes() + hostHelper.EXPECT().ReadServiceInjectionManifestFile(gomock.Any(), gomock.Any()).Return(&hostTypes.Service{Name: "test"}, nil).AnyTimes() } platformMock.EXPECT().Init().Return(nil) @@ -253,6 +253,16 @@ var _ = Describe("Daemon Controller", Ordered, func() { }, waitTime, retryTime).Should(Succeed()) }) + Context("Config Daemon generic flow", func() { + BeforeEach(func() { + // k8s plugin for k8s cluster type + if vars.ClusterType == constants.ClusterTypeKubernetes { + hostHelper.EXPECT().ReadServiceManifestFile(gomock.Any()).Return(&hostTypes.Service{Name: "test"}, nil).AnyTimes() + hostHelper.EXPECT().ReadServiceInjectionManifestFile(gomock.Any(), gomock.Any()).Return(&hostTypes.Service{Name: "test"}, nil).AnyTimes() + } + }) + }) + Context("Config Daemon generic flow", func() { It("Should expose nodeState Status section", func(ctx context.Context) { originalInterface := sriovnetworkv1.InterfaceExt{ diff --git a/pkg/helper/mock/mock_helper.go b/pkg/helper/mock/mock_helper.go index cc6655cbad..5dbec6cba2 100644 --- a/pkg/helper/mock/mock_helper.go +++ b/pkg/helper/mock/mock_helper.go @@ -934,18 +934,18 @@ func (mr *MockHostHelpersInterfaceMockRecorder) ReadService(servicePath any) *go } // ReadServiceInjectionManifestFile mocks base method. -func (m *MockHostHelpersInterface) ReadServiceInjectionManifestFile(path string) (*types.Service, error) { +func (m *MockHostHelpersInterface) ReadServiceInjectionManifestFile(path string, ovsConfig map[string]string) (*types.Service, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "ReadServiceInjectionManifestFile", path) + ret := m.ctrl.Call(m, "ReadServiceInjectionManifestFile", path, ovsConfig) ret0, _ := ret[0].(*types.Service) ret1, _ := ret[1].(error) return ret0, ret1 } // ReadServiceInjectionManifestFile indicates an expected call of ReadServiceInjectionManifestFile. -func (mr *MockHostHelpersInterfaceMockRecorder) ReadServiceInjectionManifestFile(path any) *gomock.Call { +func (mr *MockHostHelpersInterfaceMockRecorder) ReadServiceInjectionManifestFile(path, ovsConfig any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ReadServiceInjectionManifestFile", reflect.TypeOf((*MockHostHelpersInterface)(nil).ReadServiceInjectionManifestFile), path) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ReadServiceInjectionManifestFile", reflect.TypeOf((*MockHostHelpersInterface)(nil).ReadServiceInjectionManifestFile), path, ovsConfig) } // ReadServiceManifestFile mocks base method. @@ -1007,6 +1007,20 @@ func (mr *MockHostHelpersInterfaceMockRecorder) RebindVfToDefaultDriver(pciAddr return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RebindVfToDefaultDriver", reflect.TypeOf((*MockHostHelpersInterface)(nil).RebindVfToDefaultDriver), pciAddr) } +// ReloadService mocks base method. +func (m *MockHostHelpersInterface) ReloadService() error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ReloadService") + ret0, _ := ret[0].(error) + return ret0 +} + +// ReloadService indicates an expected call of ReloadService. +func (mr *MockHostHelpersInterfaceMockRecorder) ReloadService() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ReloadService", reflect.TypeOf((*MockHostHelpersInterface)(nil).ReloadService)) +} + // RemoveDisableNMUdevRule mocks base method. func (m *MockHostHelpersInterface) RemoveDisableNMUdevRule(pfPciAddress string) error { m.ctrl.T.Helper() @@ -1091,6 +1105,20 @@ func (mr *MockHostHelpersInterfaceMockRecorder) ResetSriovDevice(ifaceStatus any return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ResetSriovDevice", reflect.TypeOf((*MockHostHelpersInterface)(nil).ResetSriovDevice), ifaceStatus) } +// RestartService mocks base method. +func (m *MockHostHelpersInterface) RestartService(service *types.Service) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "RestartService", service) + ret0, _ := ret[0].(error) + return ret0 +} + +// RestartService indicates an expected call of RestartService. +func (mr *MockHostHelpersInterfaceMockRecorder) RestartService(service any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RestartService", reflect.TypeOf((*MockHostHelpersInterface)(nil).RestartService), service) +} + // RunCommand mocks base method. func (m *MockHostHelpersInterface) RunCommand(arg0 string, arg1 ...string) (string, string, error) { m.ctrl.T.Helper() diff --git a/pkg/host/internal/service/service.go b/pkg/host/internal/service/service.go index 3b082791df..a02b44257e 100644 --- a/pkg/host/internal/service/service.go +++ b/pkg/host/internal/service/service.go @@ -14,6 +14,7 @@ import ( "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/consts" "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/host/types" + "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/render" "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/utils" ) @@ -105,6 +106,34 @@ func (s *service) EnableService(service *types.Service) error { return err } +// ReloadService reloads a systemd unit files on the host +func (s *service) ReloadService() error { + // Change root dir + exit, err := s.utilsHelper.Chroot(consts.Chroot) + if err != nil { + return err + } + defer exit() + + // Restart the service + _, _, err = s.utilsHelper.RunCommand("systemctl", "daemon-reload") + return err +} + +// RestartService restarts systemd service with systemctl restart +func (s *service) RestartService(service *types.Service) error { + // Change root dir + exit, err := s.utilsHelper.Chroot(consts.Chroot) + if err != nil { + return err + } + defer exit() + + // Restart the service + _, _, err = s.utilsHelper.RunCommand("systemctl", "restart", service.Name) + return err +} + // CompareServices returns true if serviceA needs update(doesn't contain all fields from service B) func (s *service) CompareServices(serviceA, serviceB *types.Service) (bool, error) { optsA, err := unit.DeserializeOptions(strings.NewReader(serviceA.Content)) @@ -130,8 +159,12 @@ OUTER: return false, nil } +func (s *service) renderOtherOvsConfigOption(ovsConfig map[string]string) (string, string) { + return utils.RenderOtherOvsConfigOption(ovsConfig) +} + // ReadServiceInjectionManifestFile reads service injection file -func (s *service) ReadServiceInjectionManifestFile(path string) (*types.Service, error) { +func (s *service) ReadServiceInjectionManifestFile(path string, ovsConfig map[string]string) (*types.Service, error) { data, err := os.ReadFile(path) if err != nil { return nil, err @@ -142,10 +175,20 @@ func (s *service) ReadServiceInjectionManifestFile(path string) (*types.Service, return nil, err } + d := render.MakeRenderData() + externalIds, otherOvsConfig := s.renderOtherOvsConfigOption(ovsConfig) + d.Data["ExternalIds"] = externalIds + d.Data["OtherOvsConfig"] = otherOvsConfig + + srv, err := render.RenderTemplate(serviceContent.Dropins[0].Contents, &d) + if err != nil { + return nil, err + } + return &types.Service{ Name: serviceContent.Name, Path: systemdDir + serviceContent.Name, - Content: serviceContent.Dropins[0].Contents, + Content: srv.String(), }, nil } diff --git a/pkg/host/internal/service/service_suite_test.go b/pkg/host/internal/service/service_suite_test.go new file mode 100644 index 0000000000..22e7c2fb43 --- /dev/null +++ b/pkg/host/internal/service/service_suite_test.go @@ -0,0 +1,21 @@ +package service + +import ( + "testing" + + . "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" +) + +func TestSystemd(t *testing.T) { + log.SetLogger(zap.New( + zap.WriteTo(GinkgoWriter), + zap.Level(zapcore.Level(-2)), + zap.UseDevMode(true))) + RegisterFailHandler(Fail) + RunSpecs(t, "Package Service Suite") +} diff --git a/pkg/host/internal/service/service_test.go b/pkg/host/internal/service/service_test.go new file mode 100644 index 0000000000..f0c95527a8 --- /dev/null +++ b/pkg/host/internal/service/service_test.go @@ -0,0 +1,48 @@ +package service + +import ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "go.uber.org/mock/gomock" + + "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/host/types" + mock_utils "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/utils/mock" +) + +var _ = Describe("Systemd", func() { + var ( + utilsMock = &mock_utils.MockCmdInterface{} + s types.ServiceInterface + srv = types.Service{ + Name: "test-service", + Path: "", + Content: "", + } + t FullGinkgoTInterface + mockCtrl *gomock.Controller + ) + + BeforeEach(func() { + t = GinkgoT() + mockCtrl = gomock.NewController(t) + utilsMock = mock_utils.NewMockCmdInterface(mockCtrl) + s = New(utilsMock) + }) + + Context("Service manage", func() { + It("should restart service", func() { + utilsMock.EXPECT().Chroot(gomock.Any()).Return(func() error { return nil }, nil) + utilsMock.EXPECT().RunCommand("systemctl", "restart", srv.Name).Return("", "", nil) + err := s.RestartService(&srv) + Expect(err).ToNot(HaveOccurred()) + }) + + It("should reload service", func() { + utilsMock.EXPECT().Chroot(gomock.Any()).Return(func() error { return nil }, nil) + utilsMock.EXPECT().RunCommand("systemctl", "daemon-reload").Return("", "", nil) + err := s.ReloadService() + Expect(err).ToNot(HaveOccurred()) + }) + }) +}) diff --git a/pkg/host/mock/mock_host.go b/pkg/host/mock/mock_host.go index c2009a3a83..e736d44e5b 100644 --- a/pkg/host/mock/mock_host.go +++ b/pkg/host/mock/mock_host.go @@ -783,18 +783,18 @@ func (mr *MockHostManagerInterfaceMockRecorder) ReadService(servicePath any) *go } // ReadServiceInjectionManifestFile mocks base method. -func (m *MockHostManagerInterface) ReadServiceInjectionManifestFile(path string) (*types.Service, error) { +func (m *MockHostManagerInterface) ReadServiceInjectionManifestFile(path string, ovsConfig map[string]string) (*types.Service, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "ReadServiceInjectionManifestFile", path) + ret := m.ctrl.Call(m, "ReadServiceInjectionManifestFile", path, ovsConfig) ret0, _ := ret[0].(*types.Service) ret1, _ := ret[1].(error) return ret0, ret1 } // ReadServiceInjectionManifestFile indicates an expected call of ReadServiceInjectionManifestFile. -func (mr *MockHostManagerInterfaceMockRecorder) ReadServiceInjectionManifestFile(path any) *gomock.Call { +func (mr *MockHostManagerInterfaceMockRecorder) ReadServiceInjectionManifestFile(path, ovsConfig any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ReadServiceInjectionManifestFile", reflect.TypeOf((*MockHostManagerInterface)(nil).ReadServiceInjectionManifestFile), path) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ReadServiceInjectionManifestFile", reflect.TypeOf((*MockHostManagerInterface)(nil).ReadServiceInjectionManifestFile), path, ovsConfig) } // ReadServiceManifestFile mocks base method. @@ -856,6 +856,20 @@ func (mr *MockHostManagerInterfaceMockRecorder) RebindVfToDefaultDriver(pciAddr return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RebindVfToDefaultDriver", reflect.TypeOf((*MockHostManagerInterface)(nil).RebindVfToDefaultDriver), pciAddr) } +// ReloadService mocks base method. +func (m *MockHostManagerInterface) ReloadService() error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ReloadService") + ret0, _ := ret[0].(error) + return ret0 +} + +// ReloadService indicates an expected call of ReloadService. +func (mr *MockHostManagerInterfaceMockRecorder) ReloadService() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ReloadService", reflect.TypeOf((*MockHostManagerInterface)(nil).ReloadService)) +} + // RemoveDisableNMUdevRule mocks base method. func (m *MockHostManagerInterface) RemoveDisableNMUdevRule(pfPciAddress string) error { m.ctrl.T.Helper() @@ -926,6 +940,20 @@ func (mr *MockHostManagerInterfaceMockRecorder) ResetSriovDevice(ifaceStatus any return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ResetSriovDevice", reflect.TypeOf((*MockHostManagerInterface)(nil).ResetSriovDevice), ifaceStatus) } +// RestartService mocks base method. +func (m *MockHostManagerInterface) RestartService(service *types.Service) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "RestartService", service) + ret0, _ := ret[0].(error) + return ret0 +} + +// RestartService indicates an expected call of RestartService. +func (mr *MockHostManagerInterfaceMockRecorder) RestartService(service any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RestartService", reflect.TypeOf((*MockHostManagerInterface)(nil).RestartService), service) +} + // SetDevlinkDeviceParam mocks base method. func (m *MockHostManagerInterface) SetDevlinkDeviceParam(pciAddr, paramName, value string) error { m.ctrl.T.Helper() diff --git a/pkg/host/types/interfaces.go b/pkg/host/types/interfaces.go index dfecb10e5c..b8ec3d31c1 100644 --- a/pkg/host/types/interfaces.go +++ b/pkg/host/types/interfaces.go @@ -103,12 +103,16 @@ type ServiceInterface interface { IsServiceEnabled(servicePath string) (bool, error) // ReadService reads a systemd servers and return it as a struct ReadService(servicePath string) (*Service, error) - // EnableService enables a systemd server on the host + // EnableService enables a systemd service on the host EnableService(service *Service) error + // ReloadService reloads a systemd unit files on the host + ReloadService() error + // RestartService restarts a systemd service on the host + RestartService(service *Service) error // ReadServiceManifestFile reads the systemd manifest for a specific service ReadServiceManifestFile(path string) (*Service, error) // ReadServiceInjectionManifestFile reads the injection manifest file for the systemd service - ReadServiceInjectionManifestFile(path string) (*Service, error) + ReadServiceInjectionManifestFile(path string, ovsConfig map[string]string) (*Service, error) // CompareServices returns true if serviceA needs update(doesn't contain all fields from service B) CompareServices(serviceA, serviceB *Service) (bool, error) // UpdateSystemService updates a system service on the host diff --git a/pkg/platform/baremetal/baremetal.go b/pkg/platform/baremetal/baremetal.go index 9e08377fe5..219e71def6 100644 --- a/pkg/platform/baremetal/baremetal.go +++ b/pkg/platform/baremetal/baremetal.go @@ -77,10 +77,7 @@ func (bm *Baremetal) GetVendorPlugins(ns *sriovnetworkv1.SriovNetworkNodeState) } if clusterOrchestrator.ClusterType() == consts.ClusterTypeKubernetes { - k8sPlugin, err := k8splugin.NewK8sPlugin(bm.hostHelpers) - if err != nil { - return nil, nil, err - } + k8sPlugin := k8splugin.NewK8sPlugin(bm.hostHelpers) additionalPlugins = append(additionalPlugins, k8sPlugin) } diff --git a/pkg/platform/baremetal/baremetal_test.go b/pkg/platform/baremetal/baremetal_test.go index 748d6bdf20..915a16a887 100644 --- a/pkg/platform/baremetal/baremetal_test.go +++ b/pkg/platform/baremetal/baremetal_test.go @@ -94,7 +94,7 @@ var _ = Describe("Baremetal", func() { }) hostHelper.EXPECT().ReadServiceManifestFile(gomock.Any()).Return(&hosttypes.Service{Name: "test"}, nil).AnyTimes() - hostHelper.EXPECT().ReadServiceInjectionManifestFile(gomock.Any()).Return(&hosttypes.Service{Name: "test"}, nil).AnyTimes() + hostHelper.EXPECT().ReadServiceInjectionManifestFile(gomock.Any(), gomock.Any()).Return(&hosttypes.Service{Name: "test"}, nil).AnyTimes() // Assuming default cluster type is Kubernetes for these tests // In a real-world scenario, you might need to mock orchestrator.New() diff --git a/pkg/plugins/k8s/k8s_plugin.go b/pkg/plugins/k8s/k8s_plugin.go index bd6023dd5f..621cd7b2d3 100644 --- a/pkg/plugins/k8s/k8s_plugin.go +++ b/pkg/plugins/k8s/k8s_plugin.go @@ -93,14 +93,14 @@ const ( ) // Initialize our plugin and set up initial values -func NewK8sPlugin(helper helper.HostHelpersInterface) (plugins.VendorPlugin, error) { +func NewK8sPlugin(helper helper.HostHelpersInterface) plugins.VendorPlugin { k8sPluging := &K8sPlugin{ PluginName: PluginName, hostHelper: helper, updateTarget: &k8sUpdateTarget{}, } - return k8sPluging, k8sPluging.readManifestFiles() + return k8sPluging } // Name returns the name of the plugin @@ -111,6 +111,12 @@ func (p *K8sPlugin) Name() string { // OnNodeStateChange Invoked when SriovNetworkNodeState CR is created or updated, return if need dain and/or reboot node func (p *K8sPlugin) OnNodeStateChange(new *sriovnetworkv1.SriovNetworkNodeState) (needDrain bool, needReboot bool, err error) { log.Log.Info("k8s plugin OnNodeStateChange()") + err = p.readManifestFiles(new.Spec.System.OvsConfig) + if err != nil { + log.Log.Error(err, "k8s plugin OnNodeStateChange(): failed to read manifests") + return + } + needDrain = false needReboot = false @@ -165,8 +171,8 @@ func (p *K8sPlugin) Apply() error { return p.updateOVSService() } -func (p *K8sPlugin) readOpenVSwitchdManifest() error { - openVSwitchService, err := p.hostHelper.ReadServiceInjectionManifestFile(ovsUnitFile) +func (p *K8sPlugin) readOpenVSwitchdManifest(ovsConfig map[string]string) error { + openVSwitchService, err := p.hostHelper.ReadServiceInjectionManifestFile(ovsUnitFile, ovsConfig) if err != nil { return err } @@ -192,8 +198,8 @@ func (p *K8sPlugin) readSriovPostNetworkServiceManifest() error { return nil } -func (p *K8sPlugin) readManifestFiles() error { - if err := p.readOpenVSwitchdManifest(); err != nil { +func (p *K8sPlugin) readManifestFiles(ovsConfig map[string]string) error { + if err := p.readOpenVSwitchdManifest(ovsConfig); err != nil { return err } if err := p.readSriovServiceManifest(); err != nil { @@ -258,20 +264,34 @@ func (p *K8sPlugin) ovsServiceStateUpdate() error { return nil } if !p.isSystemDServiceNeedUpdate(p.openVSwitchService) { - // service is up to date + // service is up-to-date return nil + } else { + p.updateTarget.openVSwitch.SetNeedUpdate() } if p.isOVSHwOffloadingEnabled() { p.updateTarget.openVSwitch.SetNeedUpdate() - } else { - p.updateTarget.openVSwitch.SetNeedReboot() } return nil } func (p *K8sPlugin) updateOVSService() error { if p.updateTarget.openVSwitch.NeedUpdate() { - return p.hostHelper.UpdateSystemService(p.openVSwitchService) + err := p.hostHelper.UpdateSystemService(p.openVSwitchService) + if err != nil { + log.Log.Error(err, "k8s plugin updateOVSService(): failed to update systemd service") + return err + } + err = p.hostHelper.ReloadService() + if err != nil { + log.Log.Error(err, "k8s plugin updateOVSService(): failed to reload systemd unit files") + return err + } + err = p.hostHelper.RestartService(p.openVSwitchService) + if err != nil { + log.Log.Error(err, "k8s plugin updateOVSService(): failed to restart OVS service") + return err + } } return nil } diff --git a/pkg/plugins/k8s/k8s_plugin_test.go b/pkg/plugins/k8s/k8s_plugin_test.go index 23631169a2..8b6a5e019c 100644 --- a/pkg/plugins/k8s/k8s_plugin_test.go +++ b/pkg/plugins/k8s/k8s_plugin_test.go @@ -29,16 +29,6 @@ func TestK8sPlugin(t *testing.T) { 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) - }) -} - func setIsSystemdMode(val bool) { origUsingSystemdMode := vars.UsingSystemdMode DeferCleanup(func() { @@ -70,7 +60,6 @@ func (snm *serviceNameMatcher) String() string { var _ = Describe("K8s plugin", func() { var ( k8sPlugin plugin.VendorPlugin - err error testCtrl *gomock.Controller hostHelper *mock_helper.MockHostHelpersInterface ) @@ -81,20 +70,29 @@ var _ = Describe("K8s plugin", func() { hostHelper = mock_helper.NewMockHostHelpersInterface(testCtrl) realHostMgr, _ := host.NewHostManager(hostHelper) + curDir, _ := os.Getwd() // proxy some functions to real host manager to simplify testing and to additionally validate manifests for _, f := range []string{ "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) + hostHelper.EXPECT().ReadServiceManifestFile(f).Do(func(_ any) { + os.Chdir("../../..") + }).DoAndReturn(realHostMgr.ReadServiceManifestFile).Do(func(_ any) { + os.Chdir(curDir) + }) } for _, s := range []string{ "bindata/manifests/switchdev-config/ovs-units/ovs-vswitchd.service.yaml", } { - registerCall(hostHelper.EXPECT().ReadServiceInjectionManifestFile(s), realHostMgr.ReadServiceInjectionManifestFile) + var ovsConfig map[string]string + hostHelper.EXPECT().ReadServiceInjectionManifestFile(s, ovsConfig).Do(func(_, _ any) { + os.Chdir("../../..") + }).DoAndReturn(realHostMgr.ReadServiceInjectionManifestFile).Do(func(_, _ any) { + os.Chdir(curDir) + }) } - k8sPlugin, err = NewK8sPlugin(hostHelper) - Expect(err).ToNot(HaveOccurred()) + k8sPlugin = NewK8sPlugin(hostHelper) }) AfterEach(func() { @@ -194,11 +192,13 @@ var _ = Describe("K8s plugin", func() { ).Return(true, nil) hostHelper.EXPECT().Chroot("/host").Return(nil, fmt.Errorf("test")) hostHelper.EXPECT().UpdateSystemService(newServiceNameMatcher("ovs-vswitchd.service")).Return(nil) + hostHelper.EXPECT().ReloadService().Return(nil) + hostHelper.EXPECT().RestartService(newServiceNameMatcher("ovs-vswitchd.service")).Return(nil) needDrain, needReboot, err := k8sPlugin.OnNodeStateChange(&sriovnetworkv1.SriovNetworkNodeState{ Spec: sriovnetworkv1.SriovNetworkNodeStateSpec{Interfaces: []sriovnetworkv1.Interface{{EswitchMode: "switchdev"}}}}) Expect(err).ToNot(HaveOccurred()) - Expect(needReboot).To(BeTrue()) - Expect(needDrain).To(BeTrue()) + Expect(needReboot).To(BeFalse()) + Expect(needDrain).To(BeFalse()) Expect(k8sPlugin.Apply()).NotTo(HaveOccurred()) }) It("ovs service updated - hw offloading already enabled", func() { @@ -213,6 +213,8 @@ var _ = Describe("K8s plugin", func() { hostHelper.EXPECT().Chroot("/host").Return(func() error { return nil }, nil) hostHelper.EXPECT().RunCommand("ovs-vsctl", "get", "Open_vSwitch", ".", "other_config:hw-offload").Return("\"true\"\n", "", nil) hostHelper.EXPECT().UpdateSystemService(newServiceNameMatcher("ovs-vswitchd.service")).Return(nil) + hostHelper.EXPECT().ReloadService().Return(nil) + hostHelper.EXPECT().RestartService(newServiceNameMatcher("ovs-vswitchd.service")).Return(nil) needDrain, needReboot, err := k8sPlugin.OnNodeStateChange(&sriovnetworkv1.SriovNetworkNodeState{ Spec: sriovnetworkv1.SriovNetworkNodeStateSpec{Interfaces: []sriovnetworkv1.Interface{{EswitchMode: "switchdev"}}}}) Expect(err).ToNot(HaveOccurred()) diff --git a/pkg/render/render.go b/pkg/render/render.go index c26754ace7..e21e3dc27d 100644 --- a/pkg/render/render.go +++ b/pkg/render/render.go @@ -63,7 +63,7 @@ func RenderDir(manifestDir string, d *RenderData) ([]*unstructured.Unstructured, return nil } - objs, err := RenderTemplate(path, d) + objs, err := RenderFileTemplate(path, d) if err != nil { return err } @@ -76,10 +76,15 @@ func RenderDir(manifestDir string, d *RenderData) ([]*unstructured.Unstructured, return out, nil } -// RenderTemplate reads, renders, and attempts to parse a yaml or +// RenderTemplate renders provided template to string +func RenderTemplate(template string, d *RenderData) (*bytes.Buffer, error) { + return renderTemplate(template, d) +} + +// RenderFileTemplate reads, renders, and attempts to parse a yaml or // json file representing one or more k8s api objects -func RenderTemplate(path string, d *RenderData) ([]*unstructured.Unstructured, error) { - rendered, err := renderTemplate(path, d) +func RenderFileTemplate(path string, d *RenderData) ([]*unstructured.Unstructured, error) { + rendered, err := renderFileTemplate(path, d) if err != nil { return nil, err } @@ -111,8 +116,8 @@ func RenderTemplate(path string, d *RenderData) ([]*unstructured.Unstructured, e return out, nil } -func renderTemplate(path string, d *RenderData) (*bytes.Buffer, error) { - tmpl := template.New(path).Option("missingkey=error") +func renderTemplate(rawTemplate string, d *RenderData) (*bytes.Buffer, error) { + tmpl := template.New("template").Option("missingkey=error") if d.Funcs != nil { tmpl.Funcs(d.Funcs) } @@ -121,23 +126,27 @@ func renderTemplate(path string, d *RenderData) (*bytes.Buffer, error) { tmpl.Funcs(template.FuncMap{"getOr": getOr, "isSet": isSet}) tmpl.Funcs(sprig.TxtFuncMap()) - source, err := os.ReadFile(path) - if err != nil { - return nil, errors.Wrapf(err, "failed to read manifest %s", path) - } - - if _, err := tmpl.Parse(string(source)); err != nil { - return nil, errors.Wrapf(err, "failed to parse manifest %s as template", path) + if _, err := tmpl.Parse(rawTemplate); err != nil { + return nil, errors.Wrapf(err, "failed to parse manifest %s as template", rawTemplate) } rendered := bytes.Buffer{} if err := tmpl.Execute(&rendered, d.Data); err != nil { - return nil, errors.Wrapf(err, "failed to render manifest %s", path) + return nil, errors.Wrapf(err, "failed to render manifest %s", rawTemplate) } return &rendered, nil } +func renderFileTemplate(path string, d *RenderData) (*bytes.Buffer, error) { + source, err := os.ReadFile(path) + if err != nil { + return nil, errors.Wrapf(err, "failed to read manifest %s", path) + } + + return renderTemplate(string(source[:]), d) +} + func formateDeviceList(devs []DeviceInfo) string { out := "" for _, dev := range devs { @@ -231,7 +240,7 @@ func filterTemplates(toFilter map[string]string, path string, d *RenderData) err } // Render the template file - renderedData, err := renderTemplate(path, d) + renderedData, err := renderFileTemplate(path, d) if err != nil { return err } diff --git a/pkg/render/render_test.go b/pkg/render/render_test.go index be4d0f0bdd..8be41eee48 100644 --- a/pkg/render/render_test.go +++ b/pkg/render/render_test.go @@ -12,7 +12,7 @@ func TestRenderSimple(t *testing.T) { d := MakeRenderData() - o1, err := RenderTemplate("testdata/manifests/simple.yaml", &d) + o1, err := RenderFileTemplate("testdata/manifests/simple.yaml", &d) g.Expect(err).NotTo(HaveOccurred()) g.Expect(o1).To(HaveLen(1)) @@ -36,7 +36,7 @@ func TestRenderSimple(t *testing.T) { g.Expect(o1[0].MarshalJSON()).To(MatchJSON(expected)) // test that json parses the same - o2, err := RenderTemplate("testdata/manifests/simple.json", &d) + o2, err := RenderFileTemplate("testdata/manifests/simple.json", &d) g.Expect(err).NotTo(HaveOccurred()) g.Expect(o2).To(Equal(o1)) } @@ -47,7 +47,7 @@ func TestRenderMultiple(t *testing.T) { p := "testdata/manifests/multiple.yaml" d := MakeRenderData() - o, err := RenderTemplate(p, &d) + o, err := RenderFileTemplate(p, &d) g.Expect(err).NotTo(HaveOccurred()) g.Expect(o).To(HaveLen(3)) @@ -64,19 +64,19 @@ func TestTemplate(t *testing.T) { // Test that missing variables are detected d := MakeRenderData() - _, err := RenderTemplate(p, &d) + _, err := RenderFileTemplate(p, &d) g.Expect(err).To(HaveOccurred()) g.Expect(err.Error()).To(HaveSuffix(`function "fname" not defined`)) // Set expected function (but not variable) d.Funcs["fname"] = func(s string) string { return "test-" + s } - _, err = RenderTemplate(p, &d) + _, err = RenderFileTemplate(p, &d) g.Expect(err).To(HaveOccurred()) g.Expect(err.Error()).To(HaveSuffix(`has no entry for key "Namespace"`)) // now we can render d.Data["Namespace"] = "myns" - o, err := RenderTemplate(p, &d) + o, err := RenderFileTemplate(p, &d) g.Expect(err).NotTo(HaveOccurred()) g.Expect(o[0].GetName()).To(Equal("test-podname")) @@ -95,7 +95,7 @@ func TestTemplateWithEmptyObject(t *testing.T) { d := MakeRenderData() d.Data["Enable"] = true - o, err := RenderTemplate(p, &d) + o, err := RenderFileTemplate(p, &d) g.Expect(err).NotTo(HaveOccurred()) g.Expect(len(o)).To(Equal(2)) diff --git a/pkg/utils/utils.go b/pkg/utils/utils.go index d2c2b41365..c45eb52cea 100644 --- a/pkg/utils/utils.go +++ b/pkg/utils/utils.go @@ -8,6 +8,7 @@ import ( "os" "os/exec" "path/filepath" + "strings" "syscall" "time" @@ -128,3 +129,13 @@ func GetChrootExtension() string { } return fmt.Sprintf("chroot %s%s", vars.FilesystemRoot, consts.Host) } + +func RenderOtherOvsConfigOption(ovsConfig map[string]string) (string, string) { + otherConfig := new(bytes.Buffer) + externalIds := make([]string, 0, len(ovsConfig)) + for key, value := range ovsConfig { + fmt.Fprintf(otherConfig, "other_config:%s=\"%s\" ", key, value) + externalIds = append(externalIds, key) + } + return strings.Join(externalIds, " "), otherConfig.String() +}