Removing labels when node not ready

Until now nodes kept their kmod labels when became not ready.
Today we are deleting them when they are not ready due to potential race condition.

internal/controllers/nmc_reconciler.go

@@ -127,8 +127,19 @@ func (r *NMCReconciler) Reconcile(ctx context.Context, req reconcile.Request) (r
 		return ctrl.Result{}, fmt.Errorf("could not get node %s: %v", nmcObj.Name, err)
 	}
 
-	// skipping handling NMC spec, labelling, events until node becomes ready
+	// skipping handling NMC spec, events until node becomes ready
+	// removing label of loaded kmods
 	if !r.nodeAPI.IsNodeSchedulable(&node) {
+		var labelsToRemove []string
+		for label := range node.GetLabels() {
+			if ok, _, _ := utils.IsKernelModuleReadyNodeLabel(label); ok ||
+				utils.IsDeprecatedKernelModuleReadyNodeLabel(label) {
+				labelsToRemove = append(labelsToRemove, label)
+			}
+		}
+		if err := r.nodeAPI.UpdateLabels(ctx, &node, []string{}, labelsToRemove); err != nil {
+			return ctrl.Result{}, fmt.Errorf("could update node %s labels: %v", node.Name, err)
+		}
 		return ctrl.Result{}, nil
 	}
 

internal/controllers/nmc_reconciler_test.go

@@ -37,13 +37,15 @@ import (
 )
 
 const (
-	nmcName     = "nmc"
-	nsFirst     = "example-ns-1"
-	nsSecond    = "example-ns-2"
-	nameFirst   = "example-name-1"
-	nameSecond  = "example-name-2"
-	imageFirst  = "example-image-1"
-	imageSecond = "example-image-2"
+	nmcName          = "nmc"
+	nsFirst          = "example-ns-1"
+	nsSecond         = "example-ns-2"
+	nameFirst        = "example-name-1"
+	nameSecond       = "example-name-2"
+	imageFirst       = "example-image-1"
+	imageSecond      = "example-image-2"
+	kmodName         = "kmm.node.kubernetes.io/test-ns.test-module.ready"
+	labelNotToRemove = "example.node.kubernetes.io/label-not-to-be-removed"
 )
 
 var _ = Describe("NodeModulesConfigReconciler_Reconcile", func() {
@@ -126,11 +128,25 @@ var _ = Describe("NodeModulesConfigReconciler_Reconcile", func() {
 		Expect(err).To(HaveOccurred())
 	})
 
-	It("should not continue if node is not schedulable", func() {
+	It("should remove kmod labels and not continue if node is not schedulable", func() {
 		nmc := &kmmv1beta1.NodeModulesConfig{
 			ObjectMeta: metav1.ObjectMeta{Name: nmcName},
 		}
-		node := v1.Node{}
+		node := v1.Node{
+			ObjectMeta: metav1.ObjectMeta{
+				Labels: map[string]string{
+					kmodName:         "",
+					labelNotToRemove: "",
+				},
+			},
+		}
+		expectedNode := v1.Node{
+			ObjectMeta: metav1.ObjectMeta{
+				Labels: map[string]string{
+					labelNotToRemove: "",
+				},
+			},
+		}
 		gomock.InOrder(
 			kubeClient.
 				EXPECT().
@@ -139,12 +155,26 @@ var _ = Describe("NodeModulesConfigReconciler_Reconcile", func() {
 					*kubeNmc.(*kmmv1beta1.NodeModulesConfig) = *nmc
 				}),
 			wh.EXPECT().SyncStatus(ctx, nmc),
-			kubeClient.EXPECT().Get(ctx, types.NamespacedName{Name: nmc.Name}, &node).Return(nil),
+			kubeClient.EXPECT().Get(ctx, types.NamespacedName{Name: nmc.Name}, &v1.Node{}).DoAndReturn(
+				func(_ context.Context, _ types.NamespacedName, fetchedNode *v1.Node, _ ...ctrlclient.Options) error {
+					*fetchedNode = node
+					return nil
+				},
+			),
 			nm.EXPECT().IsNodeSchedulable(&node).Return(false),
+			nm.EXPECT().UpdateLabels(ctx, &node, []string{}, []string{kmodName}).DoAndReturn(
+				func(_ context.Context, obj ctrlclient.Object, _ []string, labelsToRemove []string) error {
+					for _, label := range labelsToRemove {
+						delete(node.ObjectMeta.Labels, label)
+					}
+					return nil
+				},
+			),
 		)
 
 		_, err := r.Reconcile(ctx, req)
 		Expect(err).To(BeNil())
+		Expect(node).To(Equal(expectedNode))
 	})
 
 	It("should process spec entries and orphan statuses", func() {