fixing Daemonsets GC (#356)

Fixing the version labels that are checked during GC
n order to find out of device-plugin/module-loader DS needs
to be deleted

Author: yevgeny-shnaidman

internal/daemonset/daemonset.go

@@ -57,9 +57,8 @@ func NewCreator(client client.Client, kernelLabel string, scheme *runtime.Scheme
 func (dc *daemonSetGenerator) GarbageCollect(ctx context.Context, mod *kmmv1beta1.Module, existingDS []appsv1.DaemonSet, validKernels sets.Set[string]) ([]string, error) {
 	deleted := make([]string, 0)
 
-	versionLabel := utils.GetModuleVersionLabelName(mod.Namespace, mod.Name)
 	for _, ds := range existingDS {
-		if isOlderVersionUnusedDaemonset(&ds, versionLabel, mod.Spec.ModuleLoader.Container.Version) {
+		if isOlderVersionUnusedDaemonset(&ds, mod.Spec.ModuleLoader.Container.Version) {
 			if err := dc.client.Delete(ctx, &ds); err != nil {
 				return nil, fmt.Errorf("could not delete DaemonSet %s: %v", ds.Name, err)
 			}
@@ -332,8 +331,14 @@ func getDevicePluginNodeLabel(namespace, moduleName string, useDeprecatedLabel b
 	return fmt.Sprintf("kmm.node.kubernetes.io/%s.%s.device-plugin-ready", namespace, moduleName)
 }
 
-func isOlderVersionUnusedDaemonset(ds *appsv1.DaemonSet, moduleVersionLabel, moduleVersion string) bool {
-	return ds.Labels[moduleVersionLabel] != moduleVersion && ds.Status.DesiredNumberScheduled == 0
+func isOlderVersionUnusedDaemonset(ds *appsv1.DaemonSet, moduleVersion string) bool {
+	moduleName := ds.Labels[constants.ModuleNameLabel]
+	moduleNamespace := ds.Namespace
+	versionLabel := utils.GetModuleLoaderVersionLabelName(moduleNamespace, moduleName)
+	if IsDevicePluginDS(ds) {
+		versionLabel = utils.GetDevicePluginVersionLabelName(moduleNamespace, moduleName)
+	}
+	return ds.Labels[versionLabel] != moduleVersion && ds.Status.DesiredNumberScheduled == 0
 }
 
 func isModuleLoaderDaemonsetWithInvalidKernel(ds *appsv1.DaemonSet, kernelLabel string, validKernels sets.Set[string]) bool {

internal/daemonset/daemonset_test.go

@@ -550,22 +550,32 @@ var _ = Describe("GarbageCollect", func() {
 			},
 		},
 	}
-	versionLabel := utils.GetModuleVersionLabelName(mod.Namespace, mod.Name)
+	moduleLoaderVersionLabel := utils.GetModuleLoaderVersionLabelName(mod.Namespace, mod.Name)
+	devicePluginVersionLabel := utils.GetDevicePluginVersionLabelName(mod.Namespace, mod.Name)
 
-	DescribeTable("device-plugin and modue-loader GC", func(devicePluginFormerLabel, moduleLoaderFormerLabel, moduleLoaderInvalidKernel bool,
+	DescribeTable("device-plugin and module-loader GC", func(devicePluginFormerLabel, moduleLoaderFormerLabel, moduleLoaderInvalidKernel bool,
 		devicePluginFormerDesired, moduleLoaderFormerDesired int) {
 		moduleLoaderDS := appsv1.DaemonSet{
 			ObjectMeta: metav1.ObjectMeta{
 				Name:      "moduleLoader",
 				Namespace: "namespace",
-				Labels:    map[string]string{kernelLabel: legitKernelVersion, constants.DaemonSetRole: moduleLoaderRoleLabelValue, versionLabel: currentModuleVersion},
+				Labels: map[string]string{
+					kernelLabel:               legitKernelVersion,
+					constants.DaemonSetRole:   moduleLoaderRoleLabelValue,
+					moduleLoaderVersionLabel:  currentModuleVersion,
+					constants.ModuleNameLabel: mod.Name,
+				},
 			},
 		}
 		devicePluginDS := appsv1.DaemonSet{
 			ObjectMeta: metav1.ObjectMeta{
 				Name:      "devicePlugin",
 				Namespace: "namespace",
-				Labels:    map[string]string{constants.DaemonSetRole: devicePluginRoleLabelValue, versionLabel: currentModuleVersion},
+				Labels: map[string]string{
+					constants.DaemonSetRole:   devicePluginRoleLabelValue,
+					devicePluginVersionLabel:  currentModuleVersion,
+					constants.ModuleNameLabel: mod.Name,
+				},
 			},
 		}
 		devicePluginFormerVersionDS := &appsv1.DaemonSet{}
@@ -577,14 +587,14 @@ var _ = Describe("GarbageCollect", func() {
 		if devicePluginFormerLabel {
 			devicePluginFormerVersionDS = devicePluginDS.DeepCopy()
 			devicePluginFormerVersionDS.SetName("devicePluginFormer")
-			devicePluginFormerVersionDS.Labels[versionLabel] = "former label"
+			devicePluginFormerVersionDS.Labels[devicePluginVersionLabel] = "former label"
 			devicePluginFormerVersionDS.Status.DesiredNumberScheduled = int32(devicePluginFormerDesired)
 			existingDS = append(existingDS, *devicePluginFormerVersionDS)
 		}
 		if moduleLoaderFormerLabel {
 			moduleLoaderFormerVersionDS = moduleLoaderDS.DeepCopy()
 			moduleLoaderFormerVersionDS.SetName("moduleLoaderFormer")
-			moduleLoaderFormerVersionDS.Labels[versionLabel] = "former label"
+			moduleLoaderFormerVersionDS.Labels[moduleLoaderVersionLabel] = "former label"
 			moduleLoaderFormerVersionDS.Status.DesiredNumberScheduled = int32(moduleLoaderFormerDesired)
 			existingDS = append(existingDS, *moduleLoaderFormerVersionDS)
 		}
@@ -627,7 +637,7 @@ var _ = Describe("GarbageCollect", func() {
 			ObjectMeta: metav1.ObjectMeta{
 				Name:      "moduleLoader",
 				Namespace: "namespace",
-				Labels:    map[string]string{kernelLabel: notLegitKernelVersion, constants.DaemonSetRole: moduleLoaderRoleLabelValue, versionLabel: currentModuleVersion},
+				Labels:    map[string]string{kernelLabel: notLegitKernelVersion, constants.DaemonSetRole: moduleLoaderRoleLabelValue, moduleLoaderVersionLabel: currentModuleVersion},
 			},
 		}
 		clnt.EXPECT().Delete(context.Background(), &deleteDS).Return(fmt.Errorf("some error"))
@@ -637,7 +647,7 @@ var _ = Describe("GarbageCollect", func() {
 		_, err := dc.GarbageCollect(context.Background(), mod, existingDS, sets.New[string](legitKernelVersion))
 		Expect(err).To(HaveOccurred())
 
-		deleteDS.Labels[versionLabel] = "former label"
+		deleteDS.Labels[moduleLoaderVersionLabel] = "former label"
 		clnt.EXPECT().Delete(context.Background(), &deleteDS).Return(fmt.Errorf("some error"))
 
 		existingDS = []appsv1.DaemonSet{deleteDS}