Fix collision between sofdep unloading and hard symbol dependency

scenario:
1.KMM Modules needs to load module_a, module_b, module_c
2. module_a has a hard symbol dependency on module_b.
3. modulesLoadingOrder defined as: [module_a, module_b, module_c]

There is no problems when loading. When unloading using softdep, the following happens
1. module_a is unloaded first
2. module_b is unloaded automatically, due to hard symbol dependency
3. modprobe looks at softdep, sees that module_b is unloaded, and does
   not continue to module_c

This commit changes the Unload flow:
1. no softdep is created.
2. modprobe -r ocmmand is provided with the list of modules defined in
   the modulesLoadingOrder

This way it will go over every module in the command and try to unload
it


Upstream-Commit: f17d16db0c5f7f7aefa5ec2bfcd46725afe43c8f

Author: yevgeny-shnaidman

internal/pod/workerpodmanager.go

@@ -165,7 +165,7 @@ func (wpmi *workerPodManagerImpl) LoaderPodTemplate(ctx context.Context, nmc cli
 		return nil, fmt.Errorf("could not create the base Pod: %v", err)
 	}
 
-	if nms.Config.Modprobe.ModulesLoadingOrder != nil {
+	if len(nms.Config.Modprobe.ModulesLoadingOrder) > 0 {
 		if err = setWorkerSofdepConfig(pod, nms.Config.Modprobe.ModulesLoadingOrder); err != nil {
 			return nil, fmt.Errorf("could not set software dependency for mulitple modules: %v", err)
 		}
@@ -237,11 +237,7 @@ func (wpmi *workerPodManagerImpl) UnloaderPodTemplate(ctx context.Context, nmc c
 		return nil, fmt.Errorf("could not set the worker Pod's security context: %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)
-		}
-	}
+	// we skip the softdep configuration, since in unload we will do it with one "modprobe -r" call
 
 	if nms.Config.Modprobe.FirmwarePath != "" {
 		firmwareHostPath := wpmi.workerCfg.FirmwareHostPath

internal/pod/workerpodmanager_test.go

@@ -485,6 +485,13 @@ cp -R /firmware-path/* /tmp/firmware-path;
 		configAnnotationValue = strings.ReplaceAll(configAnnotationValue, "firmwarePath: /firmware-path\n  ", "")
 	}
 	tolerationAnotationValue := "- effect: NoExecute\n  key: test-key\n  value: test-value\n"
+	annotations := map[string]string{
+		configAnnotationKey:      configAnnotationValue,
+		tolerationsAnnotationKey: tolerationAnotationValue,
+	}
+	if isLoaderPod {
+		annotations[modulesOrderKey] = modulesOrderValue
+	}
 	pod := v1.Pod{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      WorkerPodName(nmcName, moduleName),
@@ -496,11 +503,7 @@ cp -R /firmware-path/* /tmp/firmware-path;
 				actionLabelKey:                string(action),
 				constants.ModuleNameLabel:     moduleName,
 			},
-			Annotations: map[string]string{
-				configAnnotationKey:      configAnnotationValue,
-				modulesOrderKey:          modulesOrderValue,
-				tolerationsAnnotationKey: tolerationAnotationValue,
-			},
+			Annotations: annotations,
 		},
 		Spec: v1.PodSpec{
 			Tolerations: []v1.Toleration{
@@ -555,11 +558,6 @@ cp -R /firmware-path/* /tmp/firmware-path;
 							MountPath: sharedFilesDir,
 							ReadOnly:  true,
 						},
-						{
-							Name:      "modules-order",
-							ReadOnly:  true,
-							MountPath: "/etc/modprobe.d",
-						},
 					},
 				},
 			},
@@ -597,21 +595,31 @@ cp -R /firmware-path/* /tmp/firmware-path;
 						EmptyDir: &v1.EmptyDirVolumeSource{},
 					},
 				},
-				{
-					Name: "modules-order",
-					VolumeSource: v1.VolumeSource{
-						DownwardAPI: &v1.DownwardAPIVolumeSource{
-							Items: []v1.DownwardAPIVolumeFile{
-								{
-									Path:     "softdep.conf",
-									FieldRef: &v1.ObjectFieldSelector{FieldPath: fmt.Sprintf("metadata.annotations['%s']", modulesOrderKey)},
-								},
-							},
+			},
+		},
+	}
+
+	if isLoaderPod {
+		softDepVolumeMount := v1.VolumeMount{
+			Name:      "modules-order",
+			ReadOnly:  true,
+			MountPath: "/etc/modprobe.d",
+		}
+		pod.Spec.Containers[0].VolumeMounts = append(pod.Spec.Containers[0].VolumeMounts, softDepVolumeMount)
+		softdepVolume := v1.Volume{
+			Name: "modules-order",
+			VolumeSource: v1.VolumeSource{
+				DownwardAPI: &v1.DownwardAPIVolumeSource{
+					Items: []v1.DownwardAPIVolumeFile{
+						{
+							Path:     "softdep.conf",
+							FieldRef: &v1.ObjectFieldSelector{FieldPath: fmt.Sprintf("metadata.annotations['%s']", modulesOrderKey)},
 						},
 					},
 				},
 			},
-		},
+		}
+		pod.Spec.Volumes = append(pod.Spec.Volumes, softdepVolume)
 	}
 
 	if withFirmware {

internal/worker/worker.go

@@ -146,14 +146,19 @@ func (w *worker) UnloadKmod(ctx context.Context, cfg *kmmv1beta1.ModuleConfig, f
 		if cfg.Modprobe.Args != nil {
 			args = append(args, cfg.Modprobe.Args.Unload...)
 		}
-
-		args = append(args, moduleName)
+		if len(cfg.Modprobe.ModulesLoadingOrder) > 0 {
+			w.logger.Info("Preparing to unload multiple modules", "modules", cfg.Modprobe.ModulesLoadingOrder)
+			args = append(args, cfg.Modprobe.ModulesLoadingOrder...)
+		} else {
+			w.logger.Info("Preparing to unload single module", "moduleName", moduleName)
+			args = append(args, moduleName)
+		}
 	}
 
-	w.logger.Info("Unloading module", "name", moduleName)
+	w.logger.Info("Starting unloading", "args", args)
 
 	if err := w.mr.Run(ctx, args...); err != nil {
-		return fmt.Errorf("could not unload module %s: %v", moduleName, err)
+		return fmt.Errorf("failed to unload module %q: %v", args, err)
 	}
 
 	//remove firmware files only (no directories)

internal/worker/worker_test.go

@@ -332,6 +332,25 @@ var _ = Describe("worker_UnloadKmod", func() {
 		)
 	})
 
+	It("should use ModulesLoadingOrder when set", func() {
+		cfg := v1beta1.ModuleConfig{
+			ContainerImage: imageName,
+			Modprobe: v1beta1.ModprobeSpec{
+				ModuleName:          moduleName,
+				DirName:             dirName,
+				ModulesLoadingOrder: []string{"a", "b", "c"},
+			},
+		}
+
+		mr.EXPECT().Run(ctx, "-rvd", filepath.Join(sharedFilesDir, dirName), "a", "b", "c")
+
+		Expect(
+			w.UnloadKmod(ctx, &cfg, ""),
+		).NotTo(
+			HaveOccurred(),
+		)
+	})
+
 	It("should remove all firmware file only", func() {
 		cfg := v1beta1.ModuleConfig{
 			ContainerImage: imageName,