Introducing Module-to-NMC reconciliation logic (#495) (#688)

NMC package will update/create module's NMC entry based on the mld configuration. Currently it is a standalone package with unit test. Later on it will be called by the module controller instead of HandleDriverContainer. GarbageCollecto scenario is not handled by this PR: in case module does not contain mapping for the node's kernel version, but the NMC of the node does contain module version. Decision flow: 1) based on node's kernel version, taints and labels, the module is either flag as should be running on the node or should not be running on the node 2) in case it should be running: the module config data of the module for that node's NMC is updated/created. In case NMC does not exists - it is creates first 3) in case it should not be running: if NMC does not exists, nothing to do,otherwise remove modules data from NMC spec
rh-ecosystem-edge · Aug 8, 2023 · 50cee77 · 50cee77
1 parent 8e208d2
commit 50cee77
Show file tree

Hide file tree

Showing 13 changed files with 707 additions and 54 deletions.
diff --git a/api/v1beta1/nodemodulesconfig_types.go b/api/v1beta1/nodemodulesconfig_types.go
@@ -21,6 +21,7 @@ import (
 )
 
 type ModuleConfig struct {
+	KernelVersion  string `json:"kernelVersion"`
 	ContainerImage string `json:"containerImage"`
 	// When InsecurePull is true, the container image can be pulled without TLS.
 	InsecurePull bool `json:"insecurePull"`

diff --git a/bundle/manifests/kernel-module-management.clusterserviceversion.yaml b/bundle/manifests/kernel-module-management.clusterserviceversion.yaml
@@ -211,6 +211,23 @@ spec:
           - get
           - patch
           - update
+        - apiGroups:
+          - kmm.sigs.x-k8s.io
+          resources:
+          - nodemodulesconfigs
+          verbs:
+          - create
+          - get
+          - list
+          - patch
+          - update
+          - watch
+        - apiGroups:
+          - kmm.sigs.x-k8s.io
+          resources:
+          - nodemodulesconfigs/status
+          verbs:
+          - get
         - apiGroups:
           - kmm.sigs.x-k8s.io
           resources:

diff --git a/config/crd/bases/kmm.sigs.x-k8s.io_nodemodulesconfig.yaml b/config/crd/bases/kmm.sigs.x-k8s.io_nodemodulesconfig.yaml
@@ -53,6 +53,8 @@ spec:
                           description: When InsecurePull is true, the container image
                             can be pulled without TLS.
                           type: boolean
+                        kernelVersion:
+                          type: string
                         kmodLoaded:
                           type: boolean
                         modprobe:
@@ -142,6 +144,7 @@ spec:
                       required:
                       - containerImage
                       - insecurePull
+                      - kernelVersion
                       - kmodLoaded
                       - modprobe
                       type: object

diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml
@@ -131,6 +131,23 @@ rules:
   - get
   - patch
   - update
+- apiGroups:
+  - kmm.sigs.x-k8s.io
+  resources:
+  - nodemodulesconfigs
+  verbs:
+  - create
+  - get
+  - list
+  - patch
+  - update
+  - watch
+- apiGroups:
+  - kmm.sigs.x-k8s.io
+  resources:
+  - nodemodulesconfigs/status
+  verbs:
+  - get
 - apiGroups:
   - kmm.sigs.x-k8s.io
   resources:

diff --git a/controllers/module_reconciler.go b/controllers/module_reconciler.go
@@ -94,6 +94,8 @@ func NewModuleReconciler(
 //+kubebuilder:rbac:groups="core",resources=serviceaccounts,verbs=get;list;watch
 //+kubebuilder:rbac:groups="build.openshift.io",resources=builds,verbs=get;list;create;delete;watch;patch
 //+kubebuilder:rbac:groups="batch",resources=jobs,verbs=create;list;watch;delete
+//+kubebuilder:rbac:groups=kmm.sigs.x-k8s.io,resources=nodemodulesconfigs,verbs=get;list;watch;update;patch;create
+//+kubebuilder:rbac:groups=kmm.sigs.x-k8s.io,resources=nodemodulesconfigs/status,verbs=get;
 
 // Reconcile lists all nodes and looks for kernels that match its mappings.
 // For each mapping that matches at least one node in the cluster, it creates a DaemonSet running the container image

diff --git a/internal/filter/filter.go b/internal/filter/filter.go
@@ -7,8 +7,6 @@ import (
 	"github.com/go-logr/logr"
 	imagev1 "github.com/openshift/api/image/v1"
 	v1 "k8s.io/api/core/v1"
-	"k8s.io/apimachinery/pkg/labels"
-	"k8s.io/apimachinery/pkg/selection"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/kubectl/pkg/util/podutils"
 	clusterv1 "open-cluster-management.io/api/cluster/v1"
@@ -171,28 +169,18 @@ func (f *Filter) FindModulesForNode(ctx context.Context, node client.Object) []r
 
 	logger.Info("Listed modules", "count", len(mods.Items))
 
-	nodeLabelsSet := labels.Set(node.GetLabels())
-
 	for _, mod := range mods.Items {
 		logger := logger.WithValues("module name", mod.Name)
 
 		logger.V(1).Info("Processing module")
 
-		sel := labels.NewSelector()
-
-		for k, v := range mod.Spec.Selector {
-			logger.V(1).Info("Processing selector item", "key", k, "value", v)
-
-			requirement, err := labels.NewRequirement(k, selection.Equals, []string{v})
-			if err != nil {
-				logger.Error(err, "could not generate requirement")
-				return reqs
-			}
-
-			sel = sel.Add(*requirement)
+		moduleSelectorMatchNode, err := utils.IsObjectSelectedByLabels(node.GetLabels(), mod.Spec.Selector)
+		if err != nil {
+			logger.Error(err, "could not determine if node is selected by module", "node", node.GetName(), "module", mod.Name)
+			return reqs
 		}
 
-		if !sel.Matches(nodeLabelsSet) {
+		if !moduleSelectorMatchNode {
 			logger.V(1).Info("Node labels do not match the module's selector; skipping")
 			continue
 		}
@@ -224,28 +212,18 @@ func (f *Filter) FindManagedClusterModulesForCluster(ctx context.Context, cluste
 
 	logger.Info("Listed ManagedClusterModules", "count", len(mods.Items))
 
-	clusterLabelsSet := labels.Set(cluster.GetLabels())
-
 	for _, mod := range mods.Items {
 		logger := logger.WithValues("ManagedClusterModule name", mod.Name)
 
 		logger.V(1).Info("Processing ManagedClusterModule")
 
-		sel := labels.NewSelector()
-
-		for k, v := range mod.Spec.Selector {
-			logger.V(1).Info("Processing selector item", "key", k, "value", v)
-
-			requirement, err := labels.NewRequirement(k, selection.Equals, []string{v})
-			if err != nil {
-				logger.Error(err, "could not generate requirement")
-				return reqs
-			}
-
-			sel = sel.Add(*requirement)
+		mcmSelectorMatchCluster, err := utils.IsObjectSelectedByLabels(cluster.GetLabels(), mod.Spec.Selector)
+		if err != nil {
+			logger.Error(err, "could not determine if cluster is selected by ManagedClusterModule", "node", cluster.GetName(), "module", mod.Name)
+			return reqs
 		}
 
-		if !sel.Matches(clusterLabelsSet) {
+		if !mcmSelectorMatchCluster {
 			logger.V(1).Info("Cluster labels do not match the ManagedClusterModule's selector; skipping")
 			continue
 		}

diff --git a/internal/nmc/helper.go b/internal/nmc/helper.go
@@ -15,7 +15,8 @@ import (
 
 type Helper interface {
 	Get(ctx context.Context, name string) (*kmmv1beta1.NodeModulesConfig, error)
-	SetNMCAsDesired(ctx context.Context, nmc *kmmv1beta1.NodeModulesConfig, namespace, name string, moduleConfig *kmmv1beta1.ModuleConfig) error
+	SetModuleConfig(ctx context.Context, nmc *kmmv1beta1.NodeModulesConfig, namespace, name string, moduleConfig *kmmv1beta1.ModuleConfig) error
+	RemoveModuleConfig(ctx context.Context, nmc *kmmv1beta1.NodeModulesConfig, namespace, name string) error
 	GetModuleEntry(nmc *kmmv1beta1.NodeModulesConfig, modNamespace, modName string) (*kmmv1beta1.NodeModuleSpec, int)
 }
 
@@ -41,21 +42,13 @@ func (h *helper) Get(ctx context.Context, name string) (*kmmv1beta1.NodeModulesC
 	return &nmc, nil
 }
 
-func (h *helper) SetNMCAsDesired(ctx context.Context,
+func (h *helper) SetModuleConfig(ctx context.Context,
 	nmc *kmmv1beta1.NodeModulesConfig,
 	namespace string,
 	name string,
 	moduleConfig *kmmv1beta1.ModuleConfig) error {
-	foundEntry, index := h.GetModuleEntry(nmc, namespace, name)
-	// remove entry
-	if moduleConfig == nil {
-		if foundEntry != nil {
-			nmc.Spec.Modules = append(nmc.Spec.Modules[:index], nmc.Spec.Modules[index+1:]...)
-		}
-		return nil
-	}
 
-	// add/change entry
+	foundEntry, _ := h.GetModuleEntry(nmc, namespace, name)
 	if foundEntry == nil {
 		nmc.Spec.Modules = append(nmc.Spec.Modules, kmmv1beta1.NodeModuleSpec{Name: name, Namespace: namespace})
 		foundEntry = &nmc.Spec.Modules[len(nmc.Spec.Modules)-1]
@@ -65,6 +58,14 @@ func (h *helper) SetNMCAsDesired(ctx context.Context,
 	return nil
 }
 
+func (h *helper) RemoveModuleConfig(ctx context.Context, nmc *kmmv1beta1.NodeModulesConfig, namespace, name string) error {
+	foundEntry, index := h.GetModuleEntry(nmc, namespace, name)
+	if foundEntry != nil {
+		nmc.Spec.Modules = append(nmc.Spec.Modules[:index], nmc.Spec.Modules[index+1:]...)
+	}
+	return nil
+}
+
 func (h *helper) GetModuleEntry(nmc *kmmv1beta1.NodeModulesConfig, modNamespace, modName string) (*kmmv1beta1.NodeModuleSpec, int) {
 	for i, moduleSpec := range nmc.Spec.Modules {
 		if moduleSpec.Namespace == modNamespace && moduleSpec.Name == modName {

diff --git a/internal/nmc/helper_test.go b/internal/nmc/helper_test.go
@@ -68,7 +68,7 @@ var _ = Describe("Get", func() {
 
 })
 
-var _ = Describe("SetNMCAsDesired", func() {
+var _ = Describe("SetModuleConfig", func() {
 	var (
 		ctx       context.Context
 		nmcHelper Helper
@@ -92,7 +92,7 @@ var _ = Describe("SetNMCAsDesired", func() {
 
 		moduleConfig := kmmv1beta1.ModuleConfig{InTreeModuleToRemove: "in-tree-module"}
 
-		err := nmcHelper.SetNMCAsDesired(ctx, &nmc, namespace, name, &moduleConfig)
+		err := nmcHelper.SetModuleConfig(ctx, &nmc, namespace, name, &moduleConfig)
 
 		Expect(err).NotTo(HaveOccurred())
 		Expect(len(nmc.Spec.Modules)).To(Equal(3))
@@ -114,12 +114,29 @@ var _ = Describe("SetNMCAsDesired", func() {
 
 		moduleConfig := kmmv1beta1.ModuleConfig{InTreeModuleToRemove: "in-tree-module"}
 
-		err := nmcHelper.SetNMCAsDesired(ctx, &nmc, namespace, name, &moduleConfig)
+		err := nmcHelper.SetModuleConfig(ctx, &nmc, namespace, name, &moduleConfig)
 
 		Expect(err).NotTo(HaveOccurred())
 		Expect(len(nmc.Spec.Modules)).To(Equal(2))
 		Expect(nmc.Spec.Modules[1].Config.InTreeModuleToRemove).To(Equal("in-tree-module"))
 	})
+})
+
+var _ = Describe("RemoveModuleConfig", func() {
+	var (
+		ctx       context.Context
+		nmcHelper Helper
+	)
+
+	BeforeEach(func() {
+		ctx = context.Background()
+		nmcHelper = NewHelper(nil)
+	})
+
+	namespace := "test_namespace"
+	name := "test_name"
+
+	nmc := kmmv1beta1.NodeModulesConfig{}
 
 	It("deleting non-existent module", func() {
 		nmc.Spec.Modules = []kmmv1beta1.NodeModuleSpec{
@@ -135,7 +152,7 @@ var _ = Describe("SetNMCAsDesired", func() {
 			},
 		}
 
-		err := nmcHelper.SetNMCAsDesired(ctx, &nmc, namespace, name, nil)
+		err := nmcHelper.RemoveModuleConfig(ctx, &nmc, namespace, name)
 
 		Expect(err).NotTo(HaveOccurred())
 		Expect(len(nmc.Spec.Modules)).To(Equal(2))
@@ -157,7 +174,7 @@ var _ = Describe("SetNMCAsDesired", func() {
 			},
 		}
 
-		err := nmcHelper.SetNMCAsDesired(ctx, &nmc, namespace, name, nil)
+		err := nmcHelper.RemoveModuleConfig(ctx, &nmc, namespace, name)
 
 		Expect(err).NotTo(HaveOccurred())
 		Expect(len(nmc.Spec.Modules)).To(Equal(1))

diff --git a/internal/nmc/mock_helper.go b/internal/nmc/mock_helper.go