From 2dc71531af9be769e6bca50a212ba17fe8ec51b3 Mon Sep 17 00:00:00 2001 From: Ivan Kolodiazhnyi Date: Mon, 30 Dec 2024 21:53:59 +0200 Subject: [PATCH] Make ovs-vswitchd service 'other_config' option configurable Signed-off-by: Ivan Kolodiazhnyi --- api/v1/sriovnetworknodestate_types.go | 3 +++ api/v1/sriovnetworkpoolconfig_types.go | 3 +++ .../ovs-units/ovs-vswitchd.service.yaml | 2 +- ...rk.openshift.io_sriovnetworknodestates.yaml | 10 ++++++++++ ...k.openshift.io_sriovnetworkpoolconfigs.yaml | 5 +++++ .../sriovnetworknodepolicy_controller.go | 1 + ...rk.openshift.io_sriovnetworknodestates.yaml | 10 ++++++++++ ...k.openshift.io_sriovnetworkpoolconfigs.yaml | 5 +++++ pkg/daemon/plugin.go | 7 +------ pkg/daemon/plugin_test.go | 4 ++-- pkg/helper/mock/mock_helper.go | 8 ++++---- pkg/host/internal/service/service.go | 13 +++++++++++-- pkg/host/mock/mock_host.go | 8 ++++---- pkg/host/types/interfaces.go | 2 +- pkg/plugins/k8s/k8s_plugin.go | 18 ++++++++++++------ pkg/plugins/k8s/k8s_plugin_test.go | 6 ++---- pkg/render/render.go | 3 +-- 17 files changed, 76 insertions(+), 32 deletions(-) diff --git a/api/v1/sriovnetworknodestate_types.go b/api/v1/sriovnetworknodestate_types.go index e5f59d71c..904387eee 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 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 011ffc7d9..54a735ae4 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 string `json:"rdmaMode,omitempty"` } // SriovNetworkPoolConfigStatus defines the observed state of SriovNetworkPoolConfig 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 66d7efed1..d3a0883df 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 --no-wait set Open_vSwitch . other_config:{{ .OvsConfig }} diff --git a/config/crd/bases/sriovnetwork.openshift.io_sriovnetworknodestates.yaml b/config/crd/bases/sriovnetwork.openshift.io_sriovnetworknodestates.yaml index 31ddf3bf1..556d12695 100644 --- a/config/crd/bases/sriovnetwork.openshift.io_sriovnetworknodestates.yaml +++ b/config/crd/bases/sriovnetwork.openshift.io_sriovnetworknodestates.yaml @@ -176,6 +176,11 @@ spec: type: array system: properties: + ovsConfig: + default: hw-offload=true + description: OVS config. It will be provided for ovs-vswitchd + service as other_config option + type: string rdmaMode: description: RDMA subsystem. Allowed value "shared", "exclusive". enum: @@ -346,6 +351,11 @@ spec: type: string system: properties: + ovsConfig: + default: hw-offload=true + description: OVS config. It will be provided for ovs-vswitchd + service as other_config option + type: string 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 3d8a6a105..abf40a753 100644 --- a/config/crd/bases/sriovnetwork.openshift.io_sriovnetworkpoolconfigs.yaml +++ b/config/crd/bases/sriovnetwork.openshift.io_sriovnetworkpoolconfigs.yaml @@ -110,6 +110,11 @@ spec: On OpenShift: Name is the name of MachineConfigPool to be enabled with OVS hardware offload type: string + rdmaMode: + default: hw-offload=true + description: OVS config. It will be provided for ovs-vswitchd + service as other_config option + type: string type: object rdmaMode: description: RDMA subsystem. Allowed value "shared", "exclusive". diff --git a/controllers/sriovnetworknodepolicy_controller.go b/controllers/sriovnetworknodepolicy_controller.go index 29438b176..ccf485586 100644 --- a/controllers/sriovnetworknodepolicy_controller.go +++ b/controllers/sriovnetworknodepolicy_controller.go @@ -305,6 +305,7 @@ func (r *SriovNetworkNodePolicyReconciler) syncAllSriovNetworkNodeStates(ctx con } if netPoolConfig != nil { ns.Spec.System.RdmaMode = netPoolConfig.Spec.RdmaMode + ns.Spec.System.OvsConfig = netPoolConfig.Spec.OvsHardwareOffloadConfig.OvsConfig } j, _ := json.Marshal(ns) logger.V(2).Info("SriovNetworkNodeState CR", "content", j) 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 31ddf3bf1..556d12695 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 @@ -176,6 +176,11 @@ spec: type: array system: properties: + ovsConfig: + default: hw-offload=true + description: OVS config. It will be provided for ovs-vswitchd + service as other_config option + type: string rdmaMode: description: RDMA subsystem. Allowed value "shared", "exclusive". enum: @@ -346,6 +351,11 @@ spec: type: string system: properties: + ovsConfig: + default: hw-offload=true + description: OVS config. It will be provided for ovs-vswitchd + service as other_config option + type: string 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 3d8a6a105..abf40a753 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 @@ -110,6 +110,11 @@ spec: On OpenShift: Name is the name of MachineConfigPool to be enabled with OVS hardware offload type: string + rdmaMode: + default: hw-offload=true + description: OVS config. It will be provided for ovs-vswitchd + service as other_config option + type: string type: object rdmaMode: description: RDMA subsystem. Allowed value "shared", "exclusive". diff --git a/pkg/daemon/plugin.go b/pkg/daemon/plugin.go index d0df619b6..e0179782f 100644 --- a/pkg/daemon/plugin.go +++ b/pkg/daemon/plugin.go @@ -52,12 +52,7 @@ func loadPlugins(ns *sriovnetworkv1.SriovNetworkNodeState, helpers helper.HostHe loadedPlugins = loadedVendorPlugins if vars.ClusterType != consts.ClusterTypeOpenshift { - k8sPlugin, err := K8sPlugin(helpers) - if err != nil { - log.Log.Error(err, "loadPlugins(): failed to load the k8s plugin") - return nil, err - } - + k8sPlugin := K8sPlugin(helpers) pluginName := k8sPlugin.Name() if !isPluginDisabled(pluginName, disabledPlugins) { loadedPlugins[pluginName] = k8sPlugin diff --git a/pkg/daemon/plugin_test.go b/pkg/daemon/plugin_test.go index 7b14a4504..73763a059 100644 --- a/pkg/daemon/plugin_test.go +++ b/pkg/daemon/plugin_test.go @@ -51,8 +51,8 @@ var _ = Describe("config daemon plugin loading tests", func() { // k8s plugin is ATM the only plugin which require mocking/faking, as its New method performs additional logic // other than simple plugin struct initialization - K8sPlugin = func(_ helper.HostHelpersInterface) (plugin.VendorPlugin, error) { - return &fakePlugin.FakePlugin{PluginName: "k8s"}, nil + K8sPlugin = func(_ helper.HostHelpersInterface) plugin.VendorPlugin { + return &fakePlugin.FakePlugin{PluginName: "k8s"} } }) diff --git a/pkg/helper/mock/mock_helper.go b/pkg/helper/mock/mock_helper.go index 8498d5c4d..633570bce 100644 --- a/pkg/helper/mock/mock_helper.go +++ b/pkg/helper/mock/mock_helper.go @@ -869,18 +869,18 @@ func (mr *MockHostHelpersInterfaceMockRecorder) ReadService(servicePath interfac } // ReadServiceInjectionManifestFile mocks base method. -func (m *MockHostHelpersInterface) ReadServiceInjectionManifestFile(path string) (*types.Service, error) { +func (m *MockHostHelpersInterface) ReadServiceInjectionManifestFile(path, ovsConfig 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 interface{}) *gomock.Call { +func (mr *MockHostHelpersInterfaceMockRecorder) ReadServiceInjectionManifestFile(path, ovsConfig interface{}) *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. diff --git a/pkg/host/internal/service/service.go b/pkg/host/internal/service/service.go index c8705068a..217060bde 100644 --- a/pkg/host/internal/service/service.go +++ b/pkg/host/internal/service/service.go @@ -2,6 +2,7 @@ package service import ( "fmt" + "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/render" "io" "os" "path" @@ -131,7 +132,7 @@ OUTER: } // ReadServiceInjectionManifestFile reads service injection file -func (s *service) ReadServiceInjectionManifestFile(path string) (*types.Service, error) { +func (s *service) ReadServiceInjectionManifestFile(path, ovsConfig string) (*types.Service, error) { data, err := os.ReadFile(path) if err != nil { return nil, err @@ -142,10 +143,18 @@ func (s *service) ReadServiceInjectionManifestFile(path string) (*types.Service, return nil, err } + d := render.MakeRenderData() + d.Data["OvsConfig"] = ovsConfig + + 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/mock/mock_host.go b/pkg/host/mock/mock_host.go index b7f9271c8..2ad042c75 100644 --- a/pkg/host/mock/mock_host.go +++ b/pkg/host/mock/mock_host.go @@ -733,18 +733,18 @@ func (mr *MockHostManagerInterfaceMockRecorder) ReadService(servicePath interfac } // ReadServiceInjectionManifestFile mocks base method. -func (m *MockHostManagerInterface) ReadServiceInjectionManifestFile(path string) (*types.Service, error) { +func (m *MockHostManagerInterface) ReadServiceInjectionManifestFile(path, ovsConfig 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 interface{}) *gomock.Call { +func (mr *MockHostManagerInterfaceMockRecorder) ReadServiceInjectionManifestFile(path, ovsConfig interface{}) *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. diff --git a/pkg/host/types/interfaces.go b/pkg/host/types/interfaces.go index 2ffcb3268..b3cff2995 100644 --- a/pkg/host/types/interfaces.go +++ b/pkg/host/types/interfaces.go @@ -108,7 +108,7 @@ type ServiceInterface interface { // 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, ovsConfig 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/plugins/k8s/k8s_plugin.go b/pkg/plugins/k8s/k8s_plugin.go index 1a0336049..c54cccafc 100644 --- a/pkg/plugins/k8s/k8s_plugin.go +++ b/pkg/plugins/k8s/k8s_plugin.go @@ -93,7 +93,7 @@ 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, SpecVersion: "1.0", @@ -101,7 +101,7 @@ func NewK8sPlugin(helper helper.HostHelpersInterface) (plugins.VendorPlugin, err updateTarget: &k8sUpdateTarget{}, } - return k8sPluging, k8sPluging.readManifestFiles() + return k8sPluging } // Name returns the name of the plugin @@ -117,6 +117,12 @@ func (p *K8sPlugin) Spec() 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 @@ -171,8 +177,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 string) error { + openVSwitchService, err := p.hostHelper.ReadServiceInjectionManifestFile(ovsUnitFile, ovsConfig) if err != nil { return err } @@ -198,8 +204,8 @@ func (p *K8sPlugin) readSriovPostNetworkServiceManifest() error { return nil } -func (p *K8sPlugin) readManifestFiles() error { - if err := p.readOpenVSwitchdManifest(); err != nil { +func (p *K8sPlugin) readManifestFiles(ovsConfig string) error { + if err := p.readOpenVSwitchdManifest(ovsConfig); err != nil { return err } if err := p.readSriovServiceManifest(); err != nil { diff --git a/pkg/plugins/k8s/k8s_plugin_test.go b/pkg/plugins/k8s/k8s_plugin_test.go index b4344b95f..4fc5e4017 100644 --- a/pkg/plugins/k8s/k8s_plugin_test.go +++ b/pkg/plugins/k8s/k8s_plugin_test.go @@ -70,7 +70,6 @@ func (snm *serviceNameMatcher) String() string { var _ = Describe("K8s plugin", func() { var ( k8sPlugin plugin.VendorPlugin - err error testCtrl *gomock.Controller hostHelper *mock_helper.MockHostHelpersInterface ) @@ -91,10 +90,9 @@ var _ = Describe("K8s plugin", func() { for _, s := range []string{ "bindata/manifests/switchdev-config/ovs-units/ovs-vswitchd.service.yaml", } { - registerCall(hostHelper.EXPECT().ReadServiceInjectionManifestFile(s), realHostMgr.ReadServiceInjectionManifestFile) + registerCall(hostHelper.EXPECT().ReadServiceInjectionManifestFile(s, ""), realHostMgr.ReadServiceInjectionManifestFile) } - k8sPlugin, err = NewK8sPlugin(hostHelper) - Expect(err).ToNot(HaveOccurred()) + k8sPlugin = NewK8sPlugin(hostHelper) }) AfterEach(func() { diff --git a/pkg/render/render.go b/pkg/render/render.go index f65bfd76f..86c827879 100644 --- a/pkg/render/render.go +++ b/pkg/render/render.go @@ -77,6 +77,7 @@ func RenderDir(manifestDir string, d *RenderData) ([]*unstructured.Unstructured, return out, nil } +// RenderTemplate renders provided template to string func RenderTemplate(template string, d *RenderData) (*bytes.Buffer, error) { return renderTemplate(template, d) } @@ -117,7 +118,6 @@ func RenderFileTemplate(path string, d *RenderData) ([]*unstructured.Unstructure } func renderTemplate(rawTemplate string, d *RenderData) (*bytes.Buffer, error) { - tmpl := template.New("template").Option("missingkey=error") if d.Funcs != nil { tmpl.Funcs(d.Funcs) @@ -140,7 +140,6 @@ func renderTemplate(rawTemplate string, d *RenderData) (*bytes.Buffer, error) { } 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)