diff --git a/internal/pod/workerpodmanager.go b/internal/pod/workerpodmanager.go index 8f47fcb68..942a7805d 100644 --- a/internal/pod/workerpodmanager.go +++ b/internal/pod/workerpodmanager.go @@ -166,7 +166,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) } @@ -238,11 +238,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 diff --git a/internal/pod/workerpodmanager_test.go b/internal/pod/workerpodmanager_test.go index dd0cc7048..e720854a3 100644 --- a/internal/pod/workerpodmanager_test.go +++ b/internal/pod/workerpodmanager_test.go @@ -484,6 +484,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), @@ -495,11 +502,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{ @@ -559,11 +562,6 @@ cp -R /firmware-path/* /tmp/firmware-path; MountPath: sharedFilesDir, ReadOnly: true, }, - { - Name: "modules-order", - ReadOnly: true, - MountPath: "/etc/modprobe.d", - }, }, }, }, @@ -601,21 +599,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 { diff --git a/internal/worker/worker.go b/internal/worker/worker.go index c11be0872..d9a7f3399 100644 --- a/internal/worker/worker.go +++ b/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) diff --git a/internal/worker/worker_test.go b/internal/worker/worker_test.go index d9a865cfc..0ed64f400 100644 --- a/internal/worker/worker_test.go +++ b/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,