chore: add spec.ConfigDaemonEnvVars to the SriovOperatorConfig

Signed-off-by: Tobias Giese <[email protected]>

api/v1/sriovoperatorconfig_types.go

@@ -17,6 +17,7 @@ limitations under the License.
 package v1
 
 import (
+	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
@@ -65,6 +66,8 @@ type SriovOperatorConfigSpec struct {
 	DisablePlugins PluginNameSlice `json:"disablePlugins,omitempty"`
 	// FeatureGates to enable experimental features
 	FeatureGates map[string]bool `json:"featureGates,omitempty"`
+	// ConfigDaemonEnvVars specifies a set of environment variables to be passed to the sriov-network-config-daemon.
+	ConfigDaemonEnvVars []corev1.EnvVar `json:"configDaemonEnvVars,omitempty"`
 }
 
 // SriovOperatorConfigStatus defines the observed state of SriovOperatorConfig

bindata/manifests/daemon/daemonset.yaml

@@ -140,6 +140,9 @@ spec:
             value: "{{.ClusterType}}"
           - name: DEV_MODE
             value: "{{.DevMode}}"
+          {{- if .ConfigDaemonEnvVars }}
+          {{ .ConfigDaemonEnvVars | nindent 10 }}
+          {{- end  }}
         resources:
           requests:
             cpu: 100m

cmd/sriov-network-config-daemon/start.go

@@ -109,6 +109,10 @@ func runStartCmd(cmd *cobra.Command, args []string) error {
 
 	vars.ParallelNicConfig = startOpts.parallelNicConfig
 
+	if strings.ToLower(os.Getenv("MLX_PLUGIN_FW_RESET")) == "true" {
+		vars.MlxPluginFwReset = true
+	}
+
 	if startOpts.nodeName == "" {
 		name, ok := os.LookupEnv("NODE_NAME")
 		if !ok || name == "" {

config/crd/bases/sriovnetwork.openshift.io_sriovoperatorconfigs.yaml

@@ -40,6 +40,119 @@ spec:
           spec:
             description: SriovOperatorConfigSpec defines the desired state of SriovOperatorConfig
             properties:
+              configDaemonEnvVars:
+                description: ConfigDaemonEnvVars specifies a set of environment variables
+                  to be passed to the sriov-network-config-daemon.
+                items:
+                  description: EnvVar represents an environment variable present in
+                    a Container.
+                  properties:
+                    name:
+                      description: Name of the environment variable. Must be a C_IDENTIFIER.
+                      type: string
+                    value:
+                      description: |-
+                        Variable references $(VAR_NAME) are expanded
+                        using the previously defined environment variables in the container and
+                        any service environment variables. If a variable cannot be resolved,
+                        the reference in the input string will be unchanged. Double $$ are reduced
+                        to a single $, which allows for escaping the $(VAR_NAME) syntax: i.e.
+                        "$$(VAR_NAME)" will produce the string literal "$(VAR_NAME)".
+                        Escaped references will never be expanded, regardless of whether the variable
+                        exists or not.
+                        Defaults to "".
+                      type: string
+                    valueFrom:
+                      description: Source for the environment variable's value. Cannot
+                        be used if value is not empty.
+                      properties:
+                        configMapKeyRef:
+                          description: Selects a key of a ConfigMap.
+                          properties:
+                            key:
+                              description: The key to select.
+                              type: string
+                            name:
+                              description: |-
+                                Name of the referent.
+                                More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names
+                                TODO: Add other useful fields. apiVersion, kind, uid?
+                              type: string
+                            optional:
+                              description: Specify whether the ConfigMap or its key
+                                must be defined
+                              type: boolean
+                          required:
+                          - key
+                          type: object
+                          x-kubernetes-map-type: atomic
+                        fieldRef:
+                          description: |-
+                            Selects a field of the pod: supports metadata.name, metadata.namespace, `metadata.labels['<KEY>']`, `metadata.annotations['<KEY>']`,
+                            spec.nodeName, spec.serviceAccountName, status.hostIP, status.podIP, status.podIPs.
+                          properties:
+                            apiVersion:
+                              description: Version of the schema the FieldPath is
+                                written in terms of, defaults to "v1".
+                              type: string
+                            fieldPath:
+                              description: Path of the field to select in the specified
+                                API version.
+                              type: string
+                          required:
+                          - fieldPath
+                          type: object
+                          x-kubernetes-map-type: atomic
+                        resourceFieldRef:
+                          description: |-
+                            Selects a resource of the container: only resources limits and requests
+                            (limits.cpu, limits.memory, limits.ephemeral-storage, requests.cpu, requests.memory and requests.ephemeral-storage) are currently supported.
+                          properties:
+                            containerName:
+                              description: 'Container name: required for volumes,
+                                optional for env vars'
+                              type: string
+                            divisor:
+                              anyOf:
+                              - type: integer
+                              - type: string
+                              description: Specifies the output format of the exposed
+                                resources, defaults to "1"
+                              pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$
+                              x-kubernetes-int-or-string: true
+                            resource:
+                              description: 'Required: resource to select'
+                              type: string
+                          required:
+                          - resource
+                          type: object
+                          x-kubernetes-map-type: atomic
+                        secretKeyRef:
+                          description: Selects a key of a secret in the pod's namespace
+                          properties:
+                            key:
+                              description: The key of the secret to select from.  Must
+                                be a valid secret key.
+                              type: string
+                            name:
+                              description: |-
+                                Name of the referent.
+                                More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names
+                                TODO: Add other useful fields. apiVersion, kind, uid?
+                              type: string
+                            optional:
+                              description: Specify whether the Secret or its key must
+                                be defined
+                              type: boolean
+                          required:
+                          - key
+                          type: object
+                          x-kubernetes-map-type: atomic
+                      type: object
+                  required:
+                  - name
+                  type: object
+                type: array
               configDaemonNodeSelector:
                 additionalProperties:
                   type: string

controllers/sriovoperatorconfig_controller.go

@@ -23,6 +23,7 @@ import (
 	"sort"
 	"strings"
 
+	"gopkg.in/yaml.v2"
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
@@ -189,6 +190,18 @@ func (r *SriovOperatorConfigReconciler) syncConfigDaemonSet(ctx context.Context,
 	}
 	data.Data["ParallelNicConfig"] = r.FeatureGate.IsEnabled(consts.ParallelNicConfigFeatureGate)
 
+	// If ConfigDaemonEnvVars has been passed we have to marshal it.
+	// The go-template will be able to print and indent it properly.
+	// As the Kubernetes API is validing the EnvVars before there should never be an error.
+	if dc.Spec.ConfigDaemonEnvVars != nil {
+		envVarsYaml, err := yaml.Marshal(dc.Spec.ConfigDaemonEnvVars)
+		if err != nil {
+			logger.Error(err, "Fail to marshal ConfigDaemonEnvVars")
+			return err
+		}
+		data.Data["ConfigDaemonEnvVars"] = string(envVarsYaml)
+	}
+
 	envCniBinPath := os.Getenv("SRIOV_CNI_BIN_PATH")
 	if envCniBinPath == "" {
 		data.Data["CNIBinPath"] = "/var/lib/cni/bin"

controllers/sriovoperatorconfig_controller_test.go

@@ -43,6 +43,7 @@ var _ = Describe("SriovOperatorConfig controller", Ordered, func() {
 			EnableOperatorWebhook:    true,
 			ConfigDaemonNodeSelector: map[string]string{},
 			LogLevel:                 2,
+			ConfigDaemonEnvVars:      []corev1.EnvVar{},
 		}
 		Expect(k8sClient.Create(context.Background(), config)).Should(Succeed())
 		DeferCleanup(func() {
@@ -475,5 +476,31 @@ var _ = Describe("SriovOperatorConfig controller", Ordered, func() {
 					To(Equal(expectedAffinity))
 			}, "3s", "1s").Should(Succeed())
 		})
+		It("should be able to update the ConfigDaemonEnvVars for the sriov-network-config-daemon", func() {
+			By("adding env vars to the SriovOperatorConfig")
+			config := &sriovnetworkv1.SriovOperatorConfig{}
+			Expect(k8sClient.Get(ctx, types.NamespacedName{Namespace: testNamespace, Name: "default"}, config)).NotTo(HaveOccurred())
+
+			config.Spec.ConfigDaemonEnvVars = []corev1.EnvVar{
+				corev1.EnvVar{Name: "MLX_PLUGIN_FW_RESET", Value: "True"},
+			}
+			err := k8sClient.Update(ctx, config)
+			Expect(err).NotTo(HaveOccurred())
+
+			daemonSet := &appsv1.DaemonSet{}
+			Eventually(func() bool {
+				// By("wait for DaemonSet NodeSelector")
+				err := k8sClient.Get(ctx, types.NamespacedName{Name: "sriov-network-config-daemon", Namespace: testNamespace}, daemonSet)
+				if err != nil {
+					return false
+				}
+				for _, envVar := range daemonSet.Spec.Template.Spec.Containers[0].Env {
+					if envVar.Name == "MLX_PLUGIN_FW_RESET" && envVar.Value == "True" {
+						return true
+					}
+				}
+				return false
+			}, util.APITimeout, util.RetryInterval).Should(BeTrue())
+		})
 	})
 })

deployment/sriov-network-operator-chart/crds/sriovnetwork.openshift.io_sriovoperatorconfigs.yaml

@@ -40,6 +40,119 @@ spec:
           spec:
             description: SriovOperatorConfigSpec defines the desired state of SriovOperatorConfig
             properties:
+              configDaemonEnvVars:
+                description: ConfigDaemonEnvVars specifies a set of environment variables
+                  to be passed to the sriov-network-config-daemon.
+                items:
+                  description: EnvVar represents an environment variable present in
+                    a Container.
+                  properties:
+                    name:
+                      description: Name of the environment variable. Must be a C_IDENTIFIER.
+                      type: string
+                    value:
+                      description: |-
+                        Variable references $(VAR_NAME) are expanded
+                        using the previously defined environment variables in the container and
+                        any service environment variables. If a variable cannot be resolved,
+                        the reference in the input string will be unchanged. Double $$ are reduced
+                        to a single $, which allows for escaping the $(VAR_NAME) syntax: i.e.
+                        "$$(VAR_NAME)" will produce the string literal "$(VAR_NAME)".
+                        Escaped references will never be expanded, regardless of whether the variable
+                        exists or not.
+                        Defaults to "".
+                      type: string
+                    valueFrom:
+                      description: Source for the environment variable's value. Cannot
+                        be used if value is not empty.
+                      properties:
+                        configMapKeyRef:
+                          description: Selects a key of a ConfigMap.
+                          properties:
+                            key:
+                              description: The key to select.
+                              type: string
+                            name:
+                              description: |-
+                                Name of the referent.
+                                More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names
+                                TODO: Add other useful fields. apiVersion, kind, uid?
+                              type: string
+                            optional:
+                              description: Specify whether the ConfigMap or its key
+                                must be defined
+                              type: boolean
+                          required:
+                          - key
+                          type: object
+                          x-kubernetes-map-type: atomic
+                        fieldRef:
+                          description: |-
+                            Selects a field of the pod: supports metadata.name, metadata.namespace, `metadata.labels['<KEY>']`, `metadata.annotations['<KEY>']`,
+                            spec.nodeName, spec.serviceAccountName, status.hostIP, status.podIP, status.podIPs.
+                          properties:
+                            apiVersion:
+                              description: Version of the schema the FieldPath is
+                                written in terms of, defaults to "v1".
+                              type: string
+                            fieldPath:
+                              description: Path of the field to select in the specified
+                                API version.
+                              type: string
+                          required:
+                          - fieldPath
+                          type: object
+                          x-kubernetes-map-type: atomic
+                        resourceFieldRef:
+                          description: |-
+                            Selects a resource of the container: only resources limits and requests
+                            (limits.cpu, limits.memory, limits.ephemeral-storage, requests.cpu, requests.memory and requests.ephemeral-storage) are currently supported.
+                          properties:
+                            containerName:
+                              description: 'Container name: required for volumes,
+                                optional for env vars'
+                              type: string
+                            divisor:
+                              anyOf:
+                              - type: integer
+                              - type: string
+                              description: Specifies the output format of the exposed
+                                resources, defaults to "1"
+                              pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$
+                              x-kubernetes-int-or-string: true
+                            resource:
+                              description: 'Required: resource to select'
+                              type: string
+                          required:
+                          - resource
+                          type: object
+                          x-kubernetes-map-type: atomic
+                        secretKeyRef:
+                          description: Selects a key of a secret in the pod's namespace
+                          properties:
+                            key:
+                              description: The key of the secret to select from.  Must
+                                be a valid secret key.
+                              type: string
+                            name:
+                              description: |-
+                                Name of the referent.
+                                More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names
+                                TODO: Add other useful fields. apiVersion, kind, uid?
+                              type: string
+                            optional:
+                              description: Specify whether the Secret or its key must
+                                be defined
+                              type: boolean
+                          required:
+                          - key
+                          type: object
+                          x-kubernetes-map-type: atomic
+                      type: object
+                  required:
+                  - name
+                  type: object
+                type: array
               configDaemonNodeSelector:
                 additionalProperties:
                   type: string

pkg/plugins/mellanox/mellanox_plugin.go

@@ -8,6 +8,7 @@ import (
 	sriovnetworkv1 "github.com/k8snetworkplumbingwg/sriov-network-operator/api/v1"
 	"github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/helper"
 	plugin "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/plugins"
+	"github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/vars"
 	mlx "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/vendors/mellanox"
 )
 
@@ -19,7 +20,7 @@ type MellanoxPlugin struct {
 	helpers     helper.HostHelpersInterface
 }
 
-var pciAddressesToChange []string
+var pciAddressesToReset []string
 var attributesToChange map[string]mlx.MlxNic
 var mellanoxNicsStatus map[string]map[string]sriovnetworkv1.InterfaceExt
 var mellanoxNicsSpec map[string]sriovnetworkv1.Interface
@@ -52,7 +53,7 @@ func (p *MellanoxPlugin) OnNodeStateChange(new *sriovnetworkv1.SriovNetworkNodeS
 	needDrain = false
 	needReboot = false
 	err = nil
-	pciAddressesToChange = []string{}
+	pciAddressesToReset = []string{}
 	attributesToChange = map[string]mlx.MlxNic{}
 	mellanoxNicsStatus = map[string]map[string]sriovnetworkv1.InterfaceExt{}
 	mellanoxNicsSpec = map[string]sriovnetworkv1.Interface{}
@@ -135,7 +136,7 @@ func (p *MellanoxPlugin) OnNodeStateChange(new *sriovnetworkv1.SriovNetworkNodeS
 		}
 
 		if needReboot {
-			pciAddressesToChange = append(pciAddressesToChange, ifaceSpec.PciAddress)
+			pciAddressesToReset = append(pciAddressesToReset, ifaceSpec.PciAddress)
 		}
 	}
 
@@ -190,9 +191,12 @@ func (p *MellanoxPlugin) Apply() error {
 		return nil
 	}
 	log.Log.Info("mellanox plugin Apply()")
-	err := p.helpers.MlxResetFW(pciAddressesToChange)
-	if err != nil {
-		return err
+	// Only mstfwreset if the featuregate is enabled.
+	if vars.MlxPluginFwReset {
+		err := p.helpers.MlxResetFW(pciAddressesToReset)
+		if err != nil {
+			return err
+		}
 	}
 	return p.helpers.MlxConfigFW(attributesToChange)
 }