Simplifying firmware flags between NMC controller and worker pod

Currently NMC controller passes 2 flag to worker pod:
firmware path mount point in pod and firmware search path.
Since in our implementation those 2 values are always equal,
we can use only one flag to pass these values: --firmware-path,
and let worker pod decide when to update the kernel configuration
for firmware search

cmd/worker/funcs_kmod.go

@@ -30,16 +30,15 @@ func kmodLoadFunc(cmd *cobra.Command, args []string) error {
 		return fmt.Errorf("could not read config file %s: %v", cfgPath, err)
 	}
 
-	if flag := cmd.Flags().Lookup(worker.FlagFirmwareClassPath); flag.Changed {
-		logger.V(1).Info(worker.FlagFirmwareClassPath + " set, setting firmware_class.path")
+	mountPathFlag := cmd.Flags().Lookup(worker.FlagFirmwarePath)
+	if mountPathFlag.Changed {
+		logger.V(1).Info(worker.FlagFirmwarePath + " set, setting firmware_class.path")
 
-		if err := w.SetFirmwareClassPath(flag.Value.String()); err != nil {
+		if err := w.SetFirmwareClassPath(mountPathFlag.Value.String()); err != nil {
 			return fmt.Errorf("could not set the firmware_class.path parameter: %v", err)
 		}
 	}
 
-	mountPathFlag := cmd.Flags().Lookup(worker.FlagFirmwareMountPath)
-
 	return w.LoadKmod(cmd.Context(), cfg, mountPathFlag.Value.String())
 }
 
@@ -53,22 +52,14 @@ func kmodUnloadFunc(cmd *cobra.Command, args []string) error {
 		return fmt.Errorf("could not read config file %s: %v", cfgPath, err)
 	}
 
-	mountPathFlag := cmd.Flags().Lookup(worker.FlagFirmwareMountPath)
-
-	return w.UnloadKmod(cmd.Context(), cfg, mountPathFlag.Value.String())
+	return w.UnloadKmod(cmd.Context(), cfg, cmd.Flags().Lookup(worker.FlagFirmwarePath).Value.String())
 }
 
 func setCommandsFlags() {
 	kmodLoadCmd.Flags().String(
-		worker.FlagFirmwareClassPath,
+		worker.FlagFirmwarePath,
 		"",
-		"if set, this value will be written to "+worker.FirmwareClassPathLocation,
-	)
-
-	kmodLoadCmd.Flags().String(
-		worker.FlagFirmwareMountPath,
-		"",
-		"if set, this the value that firmware host path is mounted to")
+		"if set, this value will be written to "+worker.FirmwareClassPathLocation+" and it is also the value that firmware host path is mounted to")
 
 	kmodLoadCmd.Flags().Bool(
 		"tarball",
@@ -77,7 +68,7 @@ func setCommandsFlags() {
 	)
 
 	kmodUnloadCmd.Flags().String(
-		worker.FlagFirmwareMountPath,
+		worker.FlagFirmwarePath,
 		"",
 		"if set, this the value that firmware host path is mounted to")
 

cmd/worker/funcs_kmod_test.go

@@ -47,38 +47,30 @@ var _ = Describe("kmodLoadFunc", func() {
 
 	DescribeTable(
 		"set firmware class and firmware mount path if defined",
-		func(flagFirmwareClassPathValue, flagFirmwareMountPath *string) {
+		func(flagFirmwarePath *string) {
 			cfg := &kmmv1beta1.ModuleConfig{}
 			ctx := context.TODO()
 
 			cmd := &cobra.Command{}
 			cmd.SetContext(ctx)
-			cmd.Flags().String(worker.FlagFirmwareClassPath, "", "")
-			cmd.Flags().String(worker.FlagFirmwareMountPath, "", "")
+			cmd.Flags().String(worker.FlagFirmwarePath, "", "")
 
-			readConfig := ch.EXPECT().ReadConfigFile(configPath).Return(cfg, nil)
-			var loadKmod *gomock.Call
-			if flagFirmwareMountPath != nil {
+			if flagFirmwarePath != nil {
 				Expect(
-					cmd.Flags().Set(worker.FlagFirmwareMountPath, *flagFirmwareMountPath),
+					cmd.Flags().Set(worker.FlagFirmwarePath, *flagFirmwarePath),
 				).NotTo(
 					HaveOccurred(),
 				)
-				loadKmod = wo.EXPECT().LoadKmod(ctx, cfg, *flagFirmwareMountPath).After(readConfig)
+				gomock.InOrder(
+					ch.EXPECT().ReadConfigFile(configPath).Return(cfg, nil),
+					wo.EXPECT().SetFirmwareClassPath(*flagFirmwarePath),
+					wo.EXPECT().LoadKmod(ctx, cfg, *flagFirmwarePath),
+				)
 			} else {
-				loadKmod = wo.EXPECT().LoadKmod(ctx, cfg, "").After(readConfig)
-			}
-
-			if flagFirmwareClassPathValue != nil {
-				Expect(
-					cmd.Flags().Set(worker.FlagFirmwareClassPath, *flagFirmwareClassPathValue),
-				).NotTo(
-					HaveOccurred(),
+				gomock.InOrder(
+					ch.EXPECT().ReadConfigFile(configPath).Return(cfg, nil),
+					wo.EXPECT().LoadKmod(ctx, cfg, ""),
 				)
-
-				setFirmwarePath := wo.EXPECT().SetFirmwareClassPath(*flagFirmwareClassPathValue)
-				setFirmwarePath.After(readConfig)
-				loadKmod.After(setFirmwarePath)
 			}
 
 			Expect(
@@ -87,10 +79,8 @@ var _ = Describe("kmodLoadFunc", func() {
 				HaveOccurred(),
 			)
 		},
-		Entry("flags not defined", nil, nil),
-		Entry("class path defined and empty, mount path not defined", ptr.To(""), nil),
-		Entry("class path defined and not empty, mount path not defined", ptr.To("/some/path"), nil),
-		Entry("class path defined and empty, mount path defined", ptr.To(""), ptr.To("some mount path")),
-		Entry("class path defined and not empty, mount path defined", ptr.To("/some/path"), ptr.To("some mount path")),
+		Entry("fimrwarePath not defined", nil),
+		Entry("fimrwarePath path defined and empty", ptr.To("")),
+		Entry("firmwarePath defined", ptr.To("/some/path")),
 	)
 })

internal/controllers/nmc_reconciler.go

@@ -855,12 +855,11 @@ func (p *podManagerImpl) LoaderPodTemplate(ctx context.Context, nmc client.Objec
 			return nil, fmt.Errorf("firmwareHostPath wasn't set, while the Module requires firmware loading")
 		}
 
-		args = append(args, "--"+worker.FlagFirmwareMountPath, *firmwareHostPath)
+		args = append(args, "--"+worker.FlagFirmwarePath, *firmwareHostPath)
 		if err = setFirmwareVolume(pod, firmwareHostPath); err != nil {
 			return nil, fmt.Errorf("could not map host volume needed for firmware loading: %v", err)
 		}
 
-		args = append(args, "--"+worker.FlagFirmwareClassPath, *firmwareHostPath)
 		privileged = true
 	}
 
@@ -908,7 +907,7 @@ func (p *podManagerImpl) UnloaderPodTemplate(ctx context.Context, nmc client.Obj
 		if firmwareHostPath == nil {
 			return nil, fmt.Errorf("firmwareHostPath was not set while the Module requires firmware unloading")
 		}
-		args = append(args, "--"+worker.FlagFirmwareMountPath, *firmwareHostPath)
+		args = append(args, "--"+worker.FlagFirmwarePath, *firmwareHostPath)
 		if err = setFirmwareVolume(pod, firmwareHostPath); err != nil {
 			return nil, fmt.Errorf("could not map host volume needed for firmware unloading: %v", err)
 		}

internal/controllers/nmc_reconciler_test.go

@@ -1965,10 +1965,7 @@ softdep b pre: c
 
 	args := []string{"kmod", subcommand, "/etc/kmm-worker/config.yaml"}
 	if withFirmware {
-		args = append(args, "--set-firmware-mount-path", *firmwareHostPath)
-		if isLoaderPod && firmwareHostPath != nil {
-			args = append(args, "--set-firmware-class-path", *firmwareHostPath)
-		}
+		args = append(args, "--firmware-path", *firmwareHostPath)
 	} else {
 		configAnnotationValue = strings.ReplaceAll(configAnnotationValue, "firmwarePath: /firmware-path\n  ", "")
 	}

internal/worker/constants.go

@@ -1,8 +1,7 @@
 package worker
 
 const (
-	FlagFirmwareClassPath = "set-firmware-class-path"
-	FlagFirmwareMountPath = "set-firmware-mount-path"
+	FlagFirmwarePath = "firmware-path"
 
 	FirmwareClassPathLocation = "/sys/module/firmware_class/parameters/path"
 	ImagesDir                 = "/var/run/kmm/images"