Set a default value for setFirmwareClassPath in the worker config. (#…

…1170)

When firmware is added to the driver-cotnainer there are 2 actions that
were taken:

1. Copy the firmware files onto the node at `/var/lib/firmware`
2. Add `/var/lib/firmware` to `/sys/module/firmware_class/parameters/path`
   in order to update the kernel lookup directories for firmware files.

In order to make this feature configurable, we will allow the user to
choose the path on the nodes to write the firmware files to.

In order to make it work with no additional changes, when firmware files are
added to the driver-container, we added a default directory to add the
firmware files to on the nodes.

To keep it simple, we will also always add this path to the kernel lookup
directories.

Signed-off-by: Yoni Bettan <[email protected]>

config/manager/controller_config.yaml

@@ -13,3 +13,5 @@ metrics:
 worker:
   runAsUser: 0
   seLinuxType: spc_t
+  #FIXME: rename
+  setFirmwareClassPath: /var/lib/firmware

docs/mkdocs/documentation/deploy_kmod.md

@@ -124,7 +124,9 @@ spec:
 
         dirName: /opt  # Optional
 
-        # Optional. Will copy /firmware/* into /var/lib/firmware/ on the node.
+        # Optional. Will copy /firmware/* on the node into the path specified
+        # in the `kmm-operator-manager-config` at `worker.setFirmwareClassPath`
+        # before `modprobe` is called to insert the kernel module..
         firmwarePath: /firmware
 
         parameters:  # Optional

docs/mkdocs/documentation/firmwares.md

@@ -3,8 +3,9 @@
 Kernel modules sometimes need to load firmware files from the filesystem.
 KMM supports copying firmware files from the [kmod image](kmod_image.md)
 to the node's filesystem.  
-The contents of `.spec.moduleLoader.container.modprobe.firmwarePath` are copied into `/var/lib/firmware` on the node
-before `modprobe` is called to insert the kernel module.  
+The contents of `.spec.moduleLoader.container.modprobe.firmwarePath` are copied
+on the node into the path specified in the `kmm-operator-manager-config` configMap
+at `worker.setFirmwareClassPath` before `modprobe` is called to insert the kernel module.
 All files and empty directories are removed from that location before `modprobe -r` is called to unload the kernel
 module, when the pod is terminated.
 
@@ -42,7 +43,9 @@ spec:
       modprobe:
         moduleName: my-kmod  # Required
 
-        # Optional. Will copy /firmware/* into /var/lib/firmware/ on the node.
+        # Optional. Will copy /firmware/* on the node into the path specified
+        # in the `kmm-operator-manager-config` at `worker.setFirmwareClassPath`
+        # before `modprobe` is called to insert the kernel module..
         firmwarePath: /firmware
 
         # Add kernel mappings
@@ -55,5 +58,6 @@ spec:
 The Linux kernel accepts the `firmware_class.path` parameter as a
 [search path for firmwares](https://www.kernel.org/doc/html/latest/driver-api/firmware/fw_search_path.html).
 Since version 2.0.0, KMM workers can set that value on nodes by writing to sysfs, before attempting to load kmods.  
-To enable that feature, set `worker.setFirmwareClassPath` to `/var/lib/firmware` in the
+To enable that feature, set `worker.setFirmwareClassPath` in the
 [operator configuration](configure.md#workersetfirmwareclasspath).  
+The default value is `/var/lib/firmware`

internal/controllers/nmc_reconciler.go

@@ -839,12 +839,28 @@ func (p *podManagerImpl) LoaderPodTemplate(ctx context.Context, nmc client.Objec
 		return nil, fmt.Errorf("could not create the base Pod: %v", err)
 	}
 
+	if nms.Config.Modprobe.ModulesLoadingOrder != nil {
+		if err = setWorkerSofdepConfig(pod, nms.Config.Modprobe.ModulesLoadingOrder); err != nil {
+			return nil, fmt.Errorf("could not set software dependency for mulitple modules: %v", err)
+		}
+	}
+
 	args := []string{"kmod", "load", configFullPath}
 
 	privileged := false
+	if nms.Config.Modprobe.FirmwarePath != "" {
 
-	if p.workerCfg.SetFirmwareClassPath != nil {
-		args = append(args, "--"+worker.FlagFirmwareClassPath, *p.workerCfg.SetFirmwareClassPath)
+		firmwareClassPath := p.workerCfg.SetFirmwareClassPath
+		if firmwareClassPath == nil {
+			return nil, fmt.Errorf("firmwarePath was set but firmwareClassPath wasn't set")
+		}
+
+		args = append(args, "--"+worker.FlagFirmwareMountPath, *firmwareClassPath)
+		if err = setFirmwareVolume(pod, firmwareClassPath); err != nil {
+			return nil, fmt.Errorf("could not map host volume needed for firmware loading: %v", err)
+		}
+
+		args = append(args, "--"+worker.FlagFirmwareClassPath, *firmwareClassPath)
 		privileged = true
 	}
 
@@ -856,19 +872,6 @@ func (p *podManagerImpl) LoaderPodTemplate(ctx context.Context, nmc client.Objec
 		return nil, fmt.Errorf("could not set the worker Pod as privileged: %v", err)
 	}
 
-	if nms.Config.Modprobe.ModulesLoadingOrder != nil {
-		if err = setWorkerSofdepConfig(pod, nms.Config.Modprobe.ModulesLoadingOrder); err != nil {
-			return nil, fmt.Errorf("could not set software dependency for mulitple modules: %v", err)
-		}
-	}
-
-	if nms.Config.Modprobe.FirmwarePath != "" {
-		args = append(args, "--"+worker.FlagFirmwareMountPath, worker.FirmwareMountPath)
-		if err = setFirmwareVolume(pod, p.workerCfg.SetFirmwareClassPath); err != nil {
-			return nil, fmt.Errorf("could not map host volume needed for firmware loading: %v", err)
-		}
-	}
-
 	if err = setWorkerContainerArgs(pod, args); err != nil {
 		return nil, fmt.Errorf("could not set worker container args: %v", err)
 	}
@@ -901,8 +904,12 @@ func (p *podManagerImpl) UnloaderPodTemplate(ctx context.Context, nmc client.Obj
 	}
 
 	if nms.Config.Modprobe.FirmwarePath != "" {
-		args = append(args, "--"+worker.FlagFirmwareMountPath, worker.FirmwareMountPath)
-		if err = setFirmwareVolume(pod, p.workerCfg.SetFirmwareClassPath); err != nil {
+		firmwareClassPath := p.workerCfg.SetFirmwareClassPath
+		if firmwareClassPath == nil {
+			return nil, fmt.Errorf("firmwarePath was set but firmwareClassPath wasn't set")
+		}
+		args = append(args, "--"+worker.FlagFirmwareMountPath, *firmwareClassPath)
+		if err = setFirmwareVolume(pod, firmwareClassPath); err != nil {
 			return nil, fmt.Errorf("could not map host volume needed for firmware loading: %v", err)
 		}
 	}
@@ -1199,22 +1206,21 @@ func setFirmwareVolume(pod *v1.Pod, hostFirmwarePath *string) error {
 		return errors.New("could not find the worker container")
 	}
 
-	firmwareVolumeMount := v1.VolumeMount{
-		Name:      volNameVarLibFirmware,
-		MountPath: worker.FirmwareMountPath,
+	if hostFirmwarePath == nil {
+		return errors.New("hostFirmwarePath must be set")
 	}
 
-	hostMountPath := "/var/lib/firmware"
-	if hostFirmwarePath != nil {
-		hostMountPath = *hostFirmwarePath
+	firmwareVolumeMount := v1.VolumeMount{
+		Name:      volNameVarLibFirmware,
+		MountPath: *hostFirmwarePath,
 	}
 
 	hostPathDirectoryOrCreate := v1.HostPathDirectoryOrCreate
 	firmwareVolume := v1.Volume{
 		Name: volNameVarLibFirmware,
 		VolumeSource: v1.VolumeSource{
 			HostPath: &v1.HostPathVolumeSource{
-				Path: hostMountPath,
+				Path: *hostFirmwarePath,
 				Type: &hostPathDirectoryOrCreate,
 			},
 		},