Make ovs-vswitchd service 'other_config' option configurable

e0ne · e0ne · commit a26b54a56a3a · 2025-01-13T21:02:52.000+02:00
Signed-off-by: Ivan Kolodiazhnyi &lt;ikolodiazhny@nvidia.com&gt;
diff --git 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
@@ -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
diff --git 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 . {{ .OtherOvsConfig }}
diff --git a/config/crd/bases/sriovnetwork.openshift.io_sriovnetworknodestates.yaml b/config/crd/bases/sriovnetwork.openshift.io_sriovnetworknodestates.yaml
@@ -176,6 +176,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:
@@ -346,6 +354,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
@@ -110,6 +110,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
@@ -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/controllers/sriovnetworkpoolconfig_controller.go b/controllers/sriovnetworkpoolconfig_controller.go
@@ -159,6 +159,7 @@ func (r *SriovNetworkPoolConfigReconciler) syncOvsHardwareOffloadMachineConfigs(
 	}
 
 	data := render.MakeRenderData()
+	data.Data["OvsConfig"] = "hw-offload=true"
 	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
@@ -176,6 +176,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:
@@ -346,6 +354,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
@@ -110,6 +110,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/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
@@ -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
diff --git a/pkg/host/internal/service/service.go b/pkg/host/internal/service/service.go
@@ -1,6 +1,7 @@
 package service
 
 import (
+	"bytes"
 	"fmt"
 	"io"
 	"os"
@@ -14,6 +15,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"
 )
 
@@ -130,8 +132,16 @@ OUTER:
 	return false, nil
 }
 
+func (s *service) renderOtherOvsConfigOption(ovsConfig map[string]string) string {
+	b := new(bytes.Buffer)
+	for key, value := range ovsConfig {
+		fmt.Fprintf(b, "other_config:%s=\"%s\" ", key, value)
+	}
+	return b.String()
+}
+
 // 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 +152,18 @@ func (s *service) ReadServiceInjectionManifestFile(path string) (*types.Service,
 		return nil, err
 	}
 
+	d := render.MakeRenderData()
+	d.Data["OtherOvsConfig"] = s.renderOtherOvsConfigOption(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
diff --git 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 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/plugins/k8s/k8s_plugin.go b/pkg/plugins/k8s/k8s_plugin.go
@@ -93,15 +93,15 @@ 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",
 		hostHelper:   helper,
 		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 map[string]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 map[string]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
@@ -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,12 @@ 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)
+			// ovsConfig := sriovnetworkv1.OvsOtherConfig{}
+			// ovsConfig := map[string]string{}
+			ovsConfig := gomock.Any()
+			registerCall(hostHelper.EXPECT().ReadServiceInjectionManifestFile(s, ovsConfig), 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

Original file line number	Diff line number	Diff line change
`@@ -119,6 +119,9 @@ type System struct {`
`119`	`119`	`// +kubebuilder:validation:Enum=shared;exclusive`
`120`	`120`	`//RDMA subsystem. Allowed value "shared", "exclusive".`
`121`	`121`	RdmaMode string `json:"rdmaMode,omitempty"`
	`122`	`+ // OVS config. It will be provided for ovs-vswitchd service as other_config option`
	`123`	`+ // +kubebuilder:default:={hw-offload=: "true"}`
	`124`	+ OvsConfig map[string]string `json:"ovsConfig,omitempty"`
`122`	`125`	`}`
`123`	`126`
`124`	`127`	`// SriovNetworkNodeStateStatus defines the observed state of SriovNetworkNodeState`
Original file line number	Diff line number	Diff line change
`@@ -34,6 +34,9 @@ type OvsHardwareOffloadConfig struct {`
`34`	`34`	`// On OpenShift:`
`35`	`35`	`// Name is the name of MachineConfigPool to be enabled with OVS hardware offload`
`36`	`36`	Name string `json:"name,omitempty"`
	`37`	`+ // OVS config. It will be provided for ovs-vswitchd service as other_config option`
	`38`	`+ // +kubebuilder:default:={hw-offload: "true"}`
	`39`	+ OvsConfig map[string]string `json:"otherConfig,omitempty"`
`37`	`40`	`}`
`38`	`41`
`39`	`42`	`// SriovNetworkPoolConfigStatus defines the observed state of SriovNetworkPoolConfig`
Original file line number	Diff line number	Diff line change
`@@ -305,6 +305,7 @@ func (r *SriovNetworkNodePolicyReconciler) syncAllSriovNetworkNodeStates(ctx con`
`305`	`305`	`}`
`306`	`306`	`if netPoolConfig != nil {`
`307`	`307`	`ns.Spec.System.RdmaMode = netPoolConfig.Spec.RdmaMode`
	`308`	`+ ns.Spec.System.OvsConfig = netPoolConfig.Spec.OvsHardwareOffloadConfig.OvsConfig`
`308`	`309`	`}`
`309`	`310`	`j, _ := json.Marshal(ns)`
`310`	`311`	`logger.V(2).Info("SriovNetworkNodeState CR", "content", j)`
Original file line number	Diff line number	Diff line change
`@@ -159,6 +159,7 @@ func (r *SriovNetworkPoolConfigReconciler) syncOvsHardwareOffloadMachineConfigs(`
`159`	`159`	`}`
`160`	`160`
`161`	`161`	`data := render.MakeRenderData()`
	`162`	`+ data.Data["OvsConfig"] = "hw-offload=true"`
`162`	`163`	`mc, err := render.GenerateMachineConfig("bindata/manifests/switchdev-config", mcName, mcpName, true, &data)`
`163`	`164`	`if err != nil {`
`164`	`165`	`return err`