Non-schedulable nodes should not be part of targeted nodes for Module (…

…#125)

Currently ModuleLoader cannot be scheduled to run on Non-Schedulable nodes,
i.e. masters etc'. This PR fixed the extraction of schedulable nodes list,
which will also affect the status update of the Module

controllers/module_reconciler.go

@@ -241,13 +241,20 @@ func (r *ModuleReconciler) getNodesListBySelector(ctx context.Context, mod *kmmv
 	logger := log.FromContext(ctx)
 	logger.V(1).Info("Listing nodes", "selector", mod.Spec.Selector)
 
-	nodes := v1.NodeList{}
+	selectedNodes := v1.NodeList{}
 	opt := client.MatchingLabels(mod.Spec.Selector)
-	if err := r.Client.List(ctx, &nodes, opt); err != nil {
+	if err := r.Client.List(ctx, &selectedNodes, opt); err != nil {
 		logger.Error(err, "Could not list nodes")
 		return nil, fmt.Errorf("could not list nodes: %v", err)
 	}
-	return nodes.Items, nil
+	nodes := make([]v1.Node, 0, len(selectedNodes.Items))
+
+	for _, node := range selectedNodes.Items {
+		if isNodeSchedulable(&node) {
+			nodes = append(nodes, node)
+		}
+	}
+	return nodes, nil
 }
 
 func (r *ModuleReconciler) handleBuild(ctx context.Context,
@@ -397,3 +404,12 @@ func (r *ModuleReconciler) SetupWithManager(mgr ctrl.Manager, kernelLabel string
 		Named("module").
 		Complete(r)
 }
+
+func isNodeSchedulable(node *v1.Node) bool {
+	for _, taint := range node.Spec.Taints {
+		if taint.Effect == v1.TaintEffectNoSchedule {
+			return false
+		}
+	}
+	return true
+}

controllers/module_reconciler_test.go

@@ -971,3 +971,70 @@ var _ = Describe("ModuleReconciler_handleBuild", func() {
 		Expect(res).To(BeFalse())
 	})
 })
+
+var _ = Describe("ModuleReconciler_getNodesListBySelector", func() {
+	var (
+		ctrl *gomock.Controller
+		clnt *client.MockClient
+	)
+
+	BeforeEach(func() {
+		ctrl = gomock.NewController(GinkgoT())
+		clnt = client.NewMockClient(ctrl)
+	})
+
+	ctx := context.Background()
+	mod := kmmv1beta1.Module{
+		Spec: kmmv1beta1.ModuleSpec{
+			Selector: map[string]string{"key": "value"},
+		},
+	}
+
+	It("no nodes with matching labels", func() {
+		clnt.EXPECT().List(ctx, gomock.Any(), gomock.Any()).DoAndReturn(
+			func(_ interface{}, list *v1.NodeList, _ ...interface{}) error {
+				list.Items = []v1.Node{}
+				return nil
+			},
+		)
+		mr := NewModuleReconciler(clnt, nil, nil, nil, nil, nil, nil, nil, nil)
+		nodeList, err := mr.getNodesListBySelector(context.Background(), &mod)
+		Expect(err).NotTo(HaveOccurred())
+		Expect(len(nodeList)).To(Equal(0))
+	})
+
+	It("2 nodes with matching labels, all schedulable", func() {
+		clnt.EXPECT().List(ctx, gomock.Any(), gomock.Any()).DoAndReturn(
+			func(_ interface{}, list *v1.NodeList, _ ...interface{}) error {
+				list.Items = []v1.Node{v1.Node{}, v1.Node{}}
+				return nil
+			},
+		)
+		mr := NewModuleReconciler(clnt, nil, nil, nil, nil, nil, nil, nil, nil)
+		nodeList, err := mr.getNodesListBySelector(context.Background(), &mod)
+		Expect(err).NotTo(HaveOccurred())
+		Expect(len(nodeList)).To(Equal(2))
+	})
+
+	It("2 nodes with matching labels, 1 not schedulable", func() {
+		notSchedulableNode := v1.Node{
+			Spec: v1.NodeSpec{
+				Taints: []v1.Taint{
+					{
+						Effect: v1.TaintEffectNoSchedule,
+					},
+				},
+			},
+		}
+		clnt.EXPECT().List(ctx, gomock.Any(), gomock.Any()).DoAndReturn(
+			func(_ interface{}, list *v1.NodeList, _ ...interface{}) error {
+				list.Items = []v1.Node{notSchedulableNode, v1.Node{}}
+				return nil
+			},
+		)
+		mr := NewModuleReconciler(clnt, nil, nil, nil, nil, nil, nil, nil, nil)
+		nodeList, err := mr.getNodesListBySelector(context.Background(), &mod)
+		Expect(err).NotTo(HaveOccurred())
+		Expect(len(nodeList)).To(Equal(1))
+	})
+})