diff --git a/internal/constants/constants.go b/internal/constants/constants.go index e5a545fc3..a5bef635a 100644 --- a/internal/constants/constants.go +++ b/internal/constants/constants.go @@ -14,7 +14,8 @@ const ( DevicePluginVersionLabelPrefix = "beta.kmm.node.kubernetes.io/version-device-plugin" ModuleVersionLabelPrefix = "kmm.node.kubernetes.io/version-module" - ModuleFinalizer = "kmm.node.kubernetes.io/module-finalizer" + ModuleFinalizer = "kmm.node.kubernetes.io/module-finalizer" + ModuleNMCLabelPrefix = "beta.kmm.node.kubernetes.io/nmc" ManagedClusterModuleNameLabel = "kmm.node.kubernetes.io/managedclustermodule.name" KernelVersionsClusterClaimName = "kernel-versions.kmm.node.kubernetes.io" diff --git a/internal/controllers/mock_module_nmc_reconciler.go b/internal/controllers/mock_module_nmc_reconciler.go index ef7c1404a..3e01b677b 100644 --- a/internal/controllers/mock_module_nmc_reconciler.go +++ b/internal/controllers/mock_module_nmc_reconciler.go @@ -13,6 +13,7 @@ import ( gomock "go.uber.org/mock/gomock" v1 "k8s.io/api/core/v1" types "k8s.io/apimachinery/pkg/types" + sets "k8s.io/apimachinery/pkg/util/sets" ) // MockmoduleNMCReconcilerHelperAPI is a mock of moduleNMCReconcilerHelperAPI interface. @@ -53,17 +54,17 @@ func (mr *MockmoduleNMCReconcilerHelperAPIMockRecorder) disableModuleOnNode(ctx, } // enableModuleOnNode mocks base method. -func (m *MockmoduleNMCReconcilerHelperAPI) enableModuleOnNode(ctx context.Context, mld *api.ModuleLoaderData, node *v1.Node, kernelVersion string) error { +func (m *MockmoduleNMCReconcilerHelperAPI) enableModuleOnNode(ctx context.Context, mld *api.ModuleLoaderData, node *v1.Node) error { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "enableModuleOnNode", ctx, mld, node, kernelVersion) + ret := m.ctrl.Call(m, "enableModuleOnNode", ctx, mld, node) ret0, _ := ret[0].(error) return ret0 } // enableModuleOnNode indicates an expected call of enableModuleOnNode. -func (mr *MockmoduleNMCReconcilerHelperAPIMockRecorder) enableModuleOnNode(ctx, mld, node, kernelVersion interface{}) *gomock.Call { +func (mr *MockmoduleNMCReconcilerHelperAPIMockRecorder) enableModuleOnNode(ctx, mld, node interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "enableModuleOnNode", reflect.TypeOf((*MockmoduleNMCReconcilerHelperAPI)(nil).enableModuleOnNode), ctx, mld, node, kernelVersion) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "enableModuleOnNode", reflect.TypeOf((*MockmoduleNMCReconcilerHelperAPI)(nil).enableModuleOnNode), ctx, mld, node) } // finalizeModule mocks base method. @@ -80,19 +81,34 @@ func (mr *MockmoduleNMCReconcilerHelperAPIMockRecorder) finalizeModule(ctx, mod return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "finalizeModule", reflect.TypeOf((*MockmoduleNMCReconcilerHelperAPI)(nil).finalizeModule), ctx, mod) } -// getNodesList mocks base method. -func (m *MockmoduleNMCReconcilerHelperAPI) getNodesList(ctx context.Context) ([]v1.Node, error) { +// getNMCsByModuleSet mocks base method. +func (m *MockmoduleNMCReconcilerHelperAPI) getNMCsByModuleSet(ctx context.Context, mod *v1beta1.Module) (sets.Set[string], error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "getNodesList", ctx) + ret := m.ctrl.Call(m, "getNMCsByModuleSet", ctx, mod) + ret0, _ := ret[0].(sets.Set[string]) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// getNMCsByModuleSet indicates an expected call of getNMCsByModuleSet. +func (mr *MockmoduleNMCReconcilerHelperAPIMockRecorder) getNMCsByModuleSet(ctx, mod interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "getNMCsByModuleSet", reflect.TypeOf((*MockmoduleNMCReconcilerHelperAPI)(nil).getNMCsByModuleSet), ctx, mod) +} + +// getNodesListBySelector mocks base method. +func (m *MockmoduleNMCReconcilerHelperAPI) getNodesListBySelector(ctx context.Context, mod *v1beta1.Module) ([]v1.Node, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "getNodesListBySelector", ctx, mod) ret0, _ := ret[0].([]v1.Node) ret1, _ := ret[1].(error) return ret0, ret1 } -// getNodesList indicates an expected call of getNodesList. -func (mr *MockmoduleNMCReconcilerHelperAPIMockRecorder) getNodesList(ctx interface{}) *gomock.Call { +// getNodesListBySelector indicates an expected call of getNodesListBySelector. +func (mr *MockmoduleNMCReconcilerHelperAPIMockRecorder) getNodesListBySelector(ctx, mod interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "getNodesList", reflect.TypeOf((*MockmoduleNMCReconcilerHelperAPI)(nil).getNodesList), ctx) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "getNodesListBySelector", reflect.TypeOf((*MockmoduleNMCReconcilerHelperAPI)(nil).getNodesListBySelector), ctx, mod) } // getRequestedModule mocks base method. @@ -110,6 +126,21 @@ func (mr *MockmoduleNMCReconcilerHelperAPIMockRecorder) getRequestedModule(ctx, return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "getRequestedModule", reflect.TypeOf((*MockmoduleNMCReconcilerHelperAPI)(nil).getRequestedModule), ctx, namespacedName) } +// prepareSchedulingData mocks base method. +func (m *MockmoduleNMCReconcilerHelperAPI) prepareSchedulingData(ctx context.Context, mod *v1beta1.Module, targetedNodes []v1.Node, currentNMCs sets.Set[string]) (map[string]schedulingData, []error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "prepareSchedulingData", ctx, mod, targetedNodes, currentNMCs) + ret0, _ := ret[0].(map[string]schedulingData) + ret1, _ := ret[1].([]error) + return ret0, ret1 +} + +// prepareSchedulingData indicates an expected call of prepareSchedulingData. +func (mr *MockmoduleNMCReconcilerHelperAPIMockRecorder) prepareSchedulingData(ctx, mod, targetedNodes, currentNMCs interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "prepareSchedulingData", reflect.TypeOf((*MockmoduleNMCReconcilerHelperAPI)(nil).prepareSchedulingData), ctx, mod, targetedNodes, currentNMCs) +} + // setFinalizer mocks base method. func (m *MockmoduleNMCReconcilerHelperAPI) setFinalizer(ctx context.Context, mod *v1beta1.Module) error { m.ctrl.T.Helper() @@ -123,18 +154,3 @@ func (mr *MockmoduleNMCReconcilerHelperAPIMockRecorder) setFinalizer(ctx, mod in mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "setFinalizer", reflect.TypeOf((*MockmoduleNMCReconcilerHelperAPI)(nil).setFinalizer), ctx, mod) } - -// shouldModuleRunOnNode mocks base method. -func (m *MockmoduleNMCReconcilerHelperAPI) shouldModuleRunOnNode(node v1.Node, mld *api.ModuleLoaderData) (bool, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "shouldModuleRunOnNode", node, mld) - ret0, _ := ret[0].(bool) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// shouldModuleRunOnNode indicates an expected call of shouldModuleRunOnNode. -func (mr *MockmoduleNMCReconcilerHelperAPIMockRecorder) shouldModuleRunOnNode(node, mld interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "shouldModuleRunOnNode", reflect.TypeOf((*MockmoduleNMCReconcilerHelperAPI)(nil).shouldModuleRunOnNode), node, mld) -} diff --git a/internal/controllers/module_nmc_reconciler.go b/internal/controllers/module_nmc_reconciler.go index 5ea0a07fb..0987ecaf6 100644 --- a/internal/controllers/module_nmc_reconciler.go +++ b/internal/controllers/module_nmc_reconciler.go @@ -22,6 +22,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/sets" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" @@ -37,8 +38,13 @@ const ( ModuleNMCReconcilerName = "ModuleNMCReconciler" ) +type schedulingData struct { + mld *api.ModuleLoaderData + node *v1.Node + nmcExists bool +} + type ModuleNMCReconciler struct { - kernelAPI module.KernelMapper filter *filter.Filter reconHelper moduleNMCReconcilerHelperAPI } @@ -50,9 +56,8 @@ func NewModuleNMCReconciler(client client.Client, filter *filter.Filter, authFactory auth.RegistryAuthGetterFactory, scheme *runtime.Scheme) *ModuleNMCReconciler { - reconHelper := newModuleNMCReconcilerHelper(client, registryAPI, nmcHelper, authFactory, scheme) + reconHelper := newModuleNMCReconcilerHelper(client, kernelAPI, registryAPI, nmcHelper, authFactory, scheme) return &ModuleNMCReconciler{ - kernelAPI: kernelAPI, filter: filter, reconHelper: reconHelper, } @@ -85,31 +90,26 @@ func (mnr *ModuleNMCReconciler) Reconcile(ctx context.Context, req ctrl.Request) return ctrl.Result{}, fmt.Errorf("failed to set finalizer on %s Module: %v", req.NamespacedName, err) } - // get all nodes - nodes, err := mnr.reconHelper.getNodesList(ctx) + // get nodes targeted by selector + targetedNodes, err := mnr.reconHelper.getNodesListBySelector(ctx, mod) + if err != nil { + return ctrl.Result{}, fmt.Errorf("failed to get list of nodes by selector: %v", err) + } + + currentNMCs, err := mnr.reconHelper.getNMCsByModuleSet(ctx, mod) if err != nil { - return ctrl.Result{}, fmt.Errorf("failed to get nodes list: %v", err) + return ctrl.Result{}, fmt.Errorf("failed to get NMCs for %s Module: %v", req.NamespacedName, err) } + sdMap, prepareErrs := mnr.reconHelper.prepareSchedulingData(ctx, mod, targetedNodes, currentNMCs) var sumErr *multierror.Error - for _, node := range nodes { - kernelVersion := strings.TrimSuffix(node.Status.NodeInfo.KernelVersion, "+") - mld, err := mnr.kernelAPI.GetModuleLoaderDataForKernel(mod, kernelVersion) - if err != nil && !errors.Is(err, module.ErrNoMatchingKernelMapping) { - logger.Info(utils.WarnString(fmt.Sprintf("internal errors while fetching kernel mapping for version %s: %v", kernelVersion, err))) - sumErr = multierror.Append(sumErr, err) - continue - } - shouldBeOnNode, err := mnr.reconHelper.shouldModuleRunOnNode(node, mld) - if err != nil { - logger.Info(utils.WarnString(fmt.Sprintf("failed to determine if module %s/%s should be on node %s: %v", mld.Namespace, mld.Name, node.Name, err))) - sumErr = multierror.Append(sumErr, err) - continue - } - if shouldBeOnNode { - err = mnr.reconHelper.enableModuleOnNode(ctx, mld, &node, kernelVersion) - } else { - err = mnr.reconHelper.disableModuleOnNode(ctx, mod.Namespace, mod.Name, node.Name) + sumErr = multierror.Append(sumErr, prepareErrs...) + + for nodeName, sd := range sdMap { + if sd.mld != nil { + err = mnr.reconHelper.enableModuleOnNode(ctx, sd.mld, sd.node) + } else if sd.nmcExists { + err = mnr.reconHelper.disableModuleOnNode(ctx, mod.Namespace, mod.Name, nodeName) } sumErr = multierror.Append(sumErr, err) } @@ -125,16 +125,18 @@ func (mnr *ModuleNMCReconciler) Reconcile(ctx context.Context, req ctrl.Request) type moduleNMCReconcilerHelperAPI interface { setFinalizer(ctx context.Context, mod *kmmv1beta1.Module) error - getRequestedModule(ctx context.Context, namespacedName types.NamespacedName) (*kmmv1beta1.Module, error) - getNodesList(ctx context.Context) ([]v1.Node, error) finalizeModule(ctx context.Context, mod *kmmv1beta1.Module) error - shouldModuleRunOnNode(node v1.Node, mld *api.ModuleLoaderData) (bool, error) - enableModuleOnNode(ctx context.Context, mld *api.ModuleLoaderData, node *v1.Node, kernelVersion string) error + getRequestedModule(ctx context.Context, namespacedName types.NamespacedName) (*kmmv1beta1.Module, error) + getNodesListBySelector(ctx context.Context, mod *kmmv1beta1.Module) ([]v1.Node, error) + getNMCsByModuleSet(ctx context.Context, mod *kmmv1beta1.Module) (sets.Set[string], error) + prepareSchedulingData(ctx context.Context, mod *kmmv1beta1.Module, targetedNodes []v1.Node, currentNMCs sets.Set[string]) (map[string]schedulingData, []error) + enableModuleOnNode(ctx context.Context, mld *api.ModuleLoaderData, node *v1.Node) error disableModuleOnNode(ctx context.Context, modNamespace, modName, nodeName string) error } type moduleNMCReconcilerHelper struct { client client.Client + kernelAPI module.KernelMapper registryAPI registry.Registry nmcHelper nmc.Helper authFactory auth.RegistryAuthGetterFactory @@ -142,12 +144,14 @@ type moduleNMCReconcilerHelper struct { } func newModuleNMCReconcilerHelper(client client.Client, + kernelAPI module.KernelMapper, registryAPI registry.Registry, nmcHelper nmc.Helper, authFactory auth.RegistryAuthGetterFactory, scheme *runtime.Scheme) moduleNMCReconcilerHelperAPI { return &moduleNMCReconcilerHelper{ client: client, + kernelAPI: kernelAPI, registryAPI: registryAPI, nmcHelper: nmcHelper, authFactory: authFactory, @@ -205,33 +209,82 @@ func (mnrh *moduleNMCReconcilerHelper) finalizeModule(ctx context.Context, mod * return mnrh.client.Patch(ctx, mod, client.MergeFrom(modCopy)) } -func (mnrh *moduleNMCReconcilerHelper) getNodesList(ctx context.Context) ([]v1.Node, error) { - nodes := v1.NodeList{} - err := mnrh.client.List(ctx, &nodes) - if err != nil { - return nil, fmt.Errorf("failed to get list of nodes: %v", err) +func (mnrh *moduleNMCReconcilerHelper) getNodesListBySelector(ctx context.Context, mod *kmmv1beta1.Module) ([]v1.Node, error) { + logger := log.FromContext(ctx) + logger.V(1).Info("Listing nodes", "selector", mod.Spec.Selector) + + selectedNodes := v1.NodeList{} + opt := client.MatchingLabels(mod.Spec.Selector) + if err := mnrh.client.List(ctx, &selectedNodes, opt); err != nil { + return nil, fmt.Errorf("could not list nodes: %v", err) } - return nodes.Items, nil -} + nodes := make([]v1.Node, 0, len(selectedNodes.Items)) -func (mnrh *moduleNMCReconcilerHelper) shouldModuleRunOnNode(node v1.Node, mld *api.ModuleLoaderData) (bool, error) { - if mld == nil { - return false, nil + for _, node := range selectedNodes.Items { + if utils.IsNodeSchedulable(&node) { + nodes = append(nodes, node) + } } + return nodes, nil +} - nodeKernelVersion := strings.TrimSuffix(node.Status.NodeInfo.KernelVersion, "+") - if nodeKernelVersion != mld.KernelVersion { - return false, nil +func (mnrh *moduleNMCReconcilerHelper) getNMCsByModuleSet(ctx context.Context, mod *kmmv1beta1.Module) (sets.Set[string], error) { + nmcNamesList, err := mnrh.getNMCsNamesForModule(ctx, mod) + if err != nil { + return nil, fmt.Errorf("failed to get list of %s/%s module's NMC for map: %v", mod.Namespace, mod.Name, err) } - if !utils.IsNodeSchedulable(&node) { - return false, nil + return sets.New[string](nmcNamesList...), nil +} + +func (mnrh *moduleNMCReconcilerHelper) getNMCsNamesForModule(ctx context.Context, mod *kmmv1beta1.Module) ([]string, error) { + logger := log.FromContext(ctx) + moduleNMCLabel := utils.GetModuleNMCLabel(mod.Namespace, mod.Name) + logger.V(1).Info("Listing nmcs", "selector", moduleNMCLabel) + selectedNMCs := kmmv1beta1.NodeModulesConfigList{} + opt := client.MatchingLabels(map[string]string{moduleNMCLabel: ""}) + if err := mnrh.client.List(ctx, &selectedNMCs, opt); err != nil { + return nil, fmt.Errorf("could not list NMCs: %v", err) + } + result := make([]string, len(selectedNMCs.Items)) + for i := range selectedNMCs.Items { + result[i] = selectedNMCs.Items[i].Name } + return result, nil +} + +// prepareSchedulingData prepare data needed to scheduling enable/disable module per node +// in case there is an error during handling one of the nodes, function continues to the next node +// It returns the map of scheduling data per successfully processed node, and slice of errors +// per unsuccessfuly processed nodes +func (mnrh *moduleNMCReconcilerHelper) prepareSchedulingData(ctx context.Context, + mod *kmmv1beta1.Module, + targetedNodes []v1.Node, + currentNMCs sets.Set[string]) (map[string]schedulingData, []error) { - return utils.IsObjectSelectedByLabels(node.GetLabels(), mld.Selector) + logger := log.FromContext(ctx) + result := make(map[string]schedulingData) + errs := make([]error, 0, len(targetedNodes)) + for _, node := range targetedNodes { + kernelVersion := strings.TrimSuffix(node.Status.NodeInfo.KernelVersion, "+") + mld, err := mnrh.kernelAPI.GetModuleLoaderDataForKernel(mod, kernelVersion) + if err != nil && !errors.Is(err, module.ErrNoMatchingKernelMapping) { + // deleting earlier, so as not to change NMC in case we failed to determine mld + currentNMCs.Delete(node.Name) + logger.Info(utils.WarnString(fmt.Sprintf("internal errors while fetching kernel mapping for version %s: %v", kernelVersion, err))) + errs = append(errs, err) + continue + } + result[node.Name] = schedulingData{mld: mld, node: &node, nmcExists: currentNMCs.Has(node.Name)} + currentNMCs.Delete(node.Name) + } + for _, nmcName := range currentNMCs.UnsortedList() { + result[nmcName] = schedulingData{mld: nil, nmcExists: true} + } + return result, errs } -func (mnrh *moduleNMCReconcilerHelper) enableModuleOnNode(ctx context.Context, mld *api.ModuleLoaderData, node *v1.Node, kernelVersion string) error { +func (mnrh *moduleNMCReconcilerHelper) enableModuleOnNode(ctx context.Context, mld *api.ModuleLoaderData, node *v1.Node) error { logger := log.FromContext(ctx) exists, err := module.ImageExists(ctx, mnrh.authFactory, mnrh.registryAPI, mld, mld.ContainerImage) if err != nil { @@ -242,7 +295,7 @@ func (mnrh *moduleNMCReconcilerHelper) enableModuleOnNode(ctx context.Context, m return nil } moduleConfig := kmmv1beta1.ModuleConfig{ - KernelVersion: kernelVersion, + KernelVersion: mld.KernelVersion, ContainerImage: mld.ContainerImage, InTreeModuleToRemove: mld.InTreeModuleToRemove, Modprobe: mld.Modprobe, @@ -272,13 +325,8 @@ func (mnrh *moduleNMCReconcilerHelper) enableModuleOnNode(ctx context.Context, m } func (mnrh *moduleNMCReconcilerHelper) disableModuleOnNode(ctx context.Context, modNamespace, modName, nodeName string) error { - nmc, err := mnrh.nmcHelper.Get(ctx, nodeName) - if err != nil { - if k8serrors.IsNotFound(err) { - // NodeModulesConfig does not exists, module was never running on the node, we are good - return nil - } - return fmt.Errorf("failed to get the NodeModulesConfig for node %s: %v", nodeName, err) + nmc := &kmmv1beta1.NodeModulesConfig{ + ObjectMeta: metav1.ObjectMeta{Name: nodeName}, } return mnrh.removeModuleFromNMC(ctx, nmc, modNamespace, modName) diff --git a/internal/controllers/module_nmc_reconciler_test.go b/internal/controllers/module_nmc_reconciler_test.go index 68913933f..e850e9d93 100644 --- a/internal/controllers/module_nmc_reconciler_test.go +++ b/internal/controllers/module_nmc_reconciler_test.go @@ -20,32 +20,30 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/sets" ctrlclient "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "sigs.k8s.io/controller-runtime/pkg/reconcile" ) -var _ = Describe("ModuleNMCReconciler_Reconcile", func() { +var _ = Describe("Reconcile", func() { var ( ctrl *gomock.Controller - mockKernel *module.MockKernelMapper mockReconHelper *MockmoduleNMCReconcilerHelperAPI mnr *ModuleNMCReconciler ) BeforeEach(func() { ctrl = gomock.NewController(GinkgoT()) - mockKernel = module.NewMockKernelMapper(ctrl) mockReconHelper = NewMockmoduleNMCReconcilerHelperAPI(ctrl) mnr = &ModuleNMCReconciler{ - kernelAPI: mockKernel, reconHelper: mockReconHelper, } }) const moduleName = "test-module" - const kernelVersion = "kernel version" + const nodeName = "nodeName" nsn := types.NamespacedName{ Name: moduleName, @@ -55,6 +53,16 @@ var _ = Describe("ModuleNMCReconciler_Reconcile", func() { req := reconcile.Request{NamespacedName: nsn} ctx := context.Background() + mod := kmmv1beta1.Module{} + node := v1.Node{ + ObjectMeta: metav1.ObjectMeta{Name: nodeName}, + } + targetedNodes := []v1.Node{node} + currentNMCs := sets.New[string](nodeName) + mld := api.ModuleLoaderData{KernelVersion: "some version"} + enableSchedulingData := schedulingData{mld: &mld, node: &node} + disableSchedulingData := schedulingData{mld: nil, nmcExists: true} + disableSchedulingDataNoNMC := schedulingData{mld: nil, nmcExists: false} It("should return ok if module has been deleted", func() { mockReconHelper.EXPECT().getRequestedModule(ctx, nsn).Return(nil, apierrors.NewNotFound(schema.GroupResource{}, "whatever")) @@ -82,17 +90,14 @@ var _ = Describe("ModuleNMCReconciler_Reconcile", func() { DescribeTable("check error flows", func(getModuleError, setFinalizerError, getNodesError, - getMLDError, - shouldRunOnNodeError, + getNMCsMapError, + prepareSchedulingError, shouldBeOnNode bool) { - mod := kmmv1beta1.Module{} - node := v1.Node{ - ObjectMeta: metav1.ObjectMeta{Name: moduleName}, - Status: v1.NodeStatus{ - NodeInfo: v1.NodeSystemInfo{KernelVersion: kernelVersion}, - }, + + nmcMLDConfigs := map[string]schedulingData{"nodeName": disableSchedulingData} + if shouldBeOnNode { + nmcMLDConfigs = map[string]schedulingData{"nodeName": enableSchedulingData} } - mld := api.ModuleLoaderData{KernelVersion: kernelVersion} returnedError := fmt.Errorf("some error") if getModuleError { mockReconHelper.EXPECT().getRequestedModule(ctx, nsn).Return(nil, returnedError) @@ -105,22 +110,22 @@ var _ = Describe("ModuleNMCReconciler_Reconcile", func() { } mockReconHelper.EXPECT().setFinalizer(ctx, &mod).Return(nil) if getNodesError { - mockReconHelper.EXPECT().getNodesList(ctx).Return(nil, returnedError) + mockReconHelper.EXPECT().getNodesListBySelector(ctx, &mod).Return(nil, returnedError) goto executeTestFunction } - mockReconHelper.EXPECT().getNodesList(ctx).Return([]v1.Node{node}, nil) - if getMLDError { - mockKernel.EXPECT().GetModuleLoaderDataForKernel(&mod, kernelVersion).Return(nil, returnedError) + mockReconHelper.EXPECT().getNodesListBySelector(ctx, &mod).Return(targetedNodes, nil) + if getNMCsMapError { + mockReconHelper.EXPECT().getNMCsByModuleSet(ctx, &mod).Return(nil, returnedError) goto executeTestFunction } - mockKernel.EXPECT().GetModuleLoaderDataForKernel(&mod, kernelVersion).Return(&mld, nil) - if shouldRunOnNodeError { - mockReconHelper.EXPECT().shouldModuleRunOnNode(node, &mld).Return(shouldBeOnNode, returnedError) + mockReconHelper.EXPECT().getNMCsByModuleSet(ctx, &mod).Return(currentNMCs, nil) + if prepareSchedulingError { + mockReconHelper.EXPECT().prepareSchedulingData(ctx, &mod, targetedNodes, currentNMCs).Return(nil, []error{returnedError}) goto executeTestFunction } - mockReconHelper.EXPECT().shouldModuleRunOnNode(node, &mld).Return(shouldBeOnNode, nil) + mockReconHelper.EXPECT().prepareSchedulingData(ctx, &mod, targetedNodes, currentNMCs).Return(nmcMLDConfigs, []error{}) if shouldBeOnNode { - mockReconHelper.EXPECT().enableModuleOnNode(ctx, &mld, &node, kernelVersion).Return(returnedError) + mockReconHelper.EXPECT().enableModuleOnNode(ctx, &mld, &node).Return(returnedError) } else { mockReconHelper.EXPECT().disableModuleOnNode(ctx, mod.Namespace, mod.Name, node.Name).Return(returnedError) } @@ -134,28 +139,22 @@ var _ = Describe("ModuleNMCReconciler_Reconcile", func() { }, Entry("getRequestedModule failed", true, false, false, false, false, false), Entry("setFinalizer failed", false, true, false, false, false, false), - Entry("getNodesList failed", false, false, true, false, false, false), - Entry("getModuleLoaderDataForKernel failed", false, false, false, true, false, false), - Entry("shouldModuleRunOnNode failed", false, false, false, false, true, false), + Entry("getNodesListBySelector failed", false, false, true, false, false, false), + Entry("getNMCsByModuleMap failed", false, false, false, true, false, false), + Entry("prepareSchedulingData failed", false, false, false, false, true, false), Entry("enableModuleOnNode failed", false, false, false, false, false, true), Entry("disableModuleOnNode failed", false, false, false, false, false, false), ) - It("Good flow, kernel mapping exists, should run on node", func() { - mod := kmmv1beta1.Module{} - node := v1.Node{ - Status: v1.NodeStatus{ - NodeInfo: v1.NodeSystemInfo{KernelVersion: kernelVersion}, - }, - } - mld := api.ModuleLoaderData{KernelVersion: kernelVersion} + It("Good flow, should run on node", func() { + nmcMLDConfigs := map[string]schedulingData{nodeName: enableSchedulingData} gomock.InOrder( mockReconHelper.EXPECT().getRequestedModule(ctx, nsn).Return(&mod, nil), mockReconHelper.EXPECT().setFinalizer(ctx, &mod).Return(nil), - mockReconHelper.EXPECT().getNodesList(ctx).Return([]v1.Node{node}, nil), - mockKernel.EXPECT().GetModuleLoaderDataForKernel(&mod, kernelVersion).Return(&mld, nil), - mockReconHelper.EXPECT().shouldModuleRunOnNode(node, &mld).Return(true, nil), - mockReconHelper.EXPECT().enableModuleOnNode(ctx, &mld, &node, kernelVersion).Return(nil), + mockReconHelper.EXPECT().getNodesListBySelector(ctx, &mod).Return(targetedNodes, nil), + mockReconHelper.EXPECT().getNMCsByModuleSet(ctx, &mod).Return(currentNMCs, nil), + mockReconHelper.EXPECT().prepareSchedulingData(ctx, &mod, targetedNodes, currentNMCs).Return(nmcMLDConfigs, nil), + mockReconHelper.EXPECT().enableModuleOnNode(ctx, &mld, &node).Return(nil), ) res, err := mnr.Reconcile(ctx, req) @@ -164,19 +163,14 @@ var _ = Describe("ModuleNMCReconciler_Reconcile", func() { Expect(err).NotTo(HaveOccurred()) }) - It("Good flow, kernel mapping missing, should not run on node", func() { - mod := kmmv1beta1.Module{} - node := v1.Node{ - Status: v1.NodeStatus{ - NodeInfo: v1.NodeSystemInfo{KernelVersion: kernelVersion}, - }, - } + It("Good flow, should not run on node, nmc exists", func() { + nmcMLDConfigs := map[string]schedulingData{nodeName: disableSchedulingData} gomock.InOrder( mockReconHelper.EXPECT().getRequestedModule(ctx, nsn).Return(&mod, nil), mockReconHelper.EXPECT().setFinalizer(ctx, &mod).Return(nil), - mockReconHelper.EXPECT().getNodesList(ctx).Return([]v1.Node{node}, nil), - mockKernel.EXPECT().GetModuleLoaderDataForKernel(&mod, kernelVersion).Return(nil, module.ErrNoMatchingKernelMapping), - mockReconHelper.EXPECT().shouldModuleRunOnNode(node, nil).Return(false, nil), + mockReconHelper.EXPECT().getNodesListBySelector(ctx, &mod).Return(targetedNodes, nil), + mockReconHelper.EXPECT().getNMCsByModuleSet(ctx, &mod).Return(currentNMCs, nil), + mockReconHelper.EXPECT().prepareSchedulingData(ctx, &mod, targetedNodes, currentNMCs).Return(nmcMLDConfigs, nil), mockReconHelper.EXPECT().disableModuleOnNode(ctx, mod.Namespace, mod.Name, node.Name).Return(nil), ) @@ -186,9 +180,25 @@ var _ = Describe("ModuleNMCReconciler_Reconcile", func() { Expect(err).NotTo(HaveOccurred()) }) + It("Good flow, should not run on node, nmc does not exist", func() { + nmcMLDConfigs := map[string]schedulingData{nodeName: disableSchedulingDataNoNMC} + gomock.InOrder( + mockReconHelper.EXPECT().getRequestedModule(ctx, nsn).Return(&mod, nil), + mockReconHelper.EXPECT().setFinalizer(ctx, &mod).Return(nil), + mockReconHelper.EXPECT().getNodesListBySelector(ctx, &mod).Return(targetedNodes, nil), + mockReconHelper.EXPECT().getNMCsByModuleSet(ctx, &mod).Return(currentNMCs, nil), + mockReconHelper.EXPECT().prepareSchedulingData(ctx, &mod, targetedNodes, currentNMCs).Return(nmcMLDConfigs, nil), + ) + + res, err := mnr.Reconcile(ctx, req) + + Expect(res).To(Equal(reconcile.Result{})) + Expect(err).NotTo(HaveOccurred()) + }) + }) -var _ = Describe("ModuleReconciler_getRequestedModule", func() { +var _ = Describe("getRequestedModule", func() { var ( ctrl *gomock.Controller clnt *client.MockClient @@ -198,7 +208,7 @@ var _ = Describe("ModuleReconciler_getRequestedModule", func() { BeforeEach(func() { ctrl = gomock.NewController(GinkgoT()) clnt = client.NewMockClient(ctrl) - mnrh = newModuleNMCReconcilerHelper(clnt, nil, nil, nil, scheme) + mnrh = newModuleNMCReconcilerHelper(clnt, nil, nil, nil, nil, scheme) }) ctx := context.Background() @@ -230,7 +240,7 @@ var _ = Describe("ModuleReconciler_getRequestedModule", func() { }) -var _ = Describe("ModuleReconciler_setFinalizer", func() { +var _ = Describe("setFinalizer", func() { var ( ctrl *gomock.Controller clnt *client.MockClient @@ -241,7 +251,7 @@ var _ = Describe("ModuleReconciler_setFinalizer", func() { BeforeEach(func() { ctrl = gomock.NewController(GinkgoT()) clnt = client.NewMockClient(ctrl) - mnrh = newModuleNMCReconcilerHelper(clnt, nil, nil, nil, scheme) + mnrh = newModuleNMCReconcilerHelper(clnt, nil, nil, nil, nil, scheme) mod = kmmv1beta1.Module{} }) @@ -268,7 +278,7 @@ var _ = Describe("ModuleReconciler_setFinalizer", func() { }) }) -var _ = Describe("ModuleReconciler_getNodes", func() { +var _ = Describe("getNodesListBySelector", func() { var ( ctrl *gomock.Controller clnt *client.MockClient @@ -278,7 +288,7 @@ var _ = Describe("ModuleReconciler_getNodes", func() { BeforeEach(func() { ctrl = gomock.NewController(GinkgoT()) clnt = client.NewMockClient(ctrl) - mnrh = newModuleNMCReconcilerHelper(clnt, nil, nil, nil, scheme) + mnrh = newModuleNMCReconcilerHelper(clnt, nil, nil, nil, nil, scheme) }) ctx := context.Background() @@ -286,7 +296,7 @@ var _ = Describe("ModuleReconciler_getNodes", func() { It("list failed", func() { clnt.EXPECT().List(ctx, gomock.Any(), gomock.Any()).Return(fmt.Errorf("some error")) - nodes, err := mnrh.getNodesList(ctx) + nodes, err := mnrh.getNodesListBySelector(ctx, &kmmv1beta1.Module{}) Expect(err).To(HaveOccurred()) Expect(nodes).To(BeNil()) @@ -302,7 +312,7 @@ var _ = Describe("ModuleReconciler_getNodes", func() { return nil }, ) - nodes, err := mnrh.getNodesList(ctx) + nodes, err := mnrh.getNodesListBySelector(ctx, &kmmv1beta1.Module{}) Expect(err).NotTo(HaveOccurred()) Expect(nodes).To(Equal([]v1.Node{node1, node2, node3})) @@ -325,7 +335,7 @@ var _ = Describe("finalizeModule", func() { ctrl = gomock.NewController(GinkgoT()) clnt = client.NewMockClient(ctrl) helper = nmc.NewMockHelper(ctrl) - mnrh = newModuleNMCReconcilerHelper(clnt, nil, helper, nil, scheme) + mnrh = newModuleNMCReconcilerHelper(clnt, nil, nil, helper, nil, scheme) mod = &kmmv1beta1.Module{ ObjectMeta: metav1.ObjectMeta{Name: "moduleName", Namespace: "moduleNamespace"}, } @@ -397,90 +407,141 @@ var _ = Describe("finalizeModule", func() { }) }) -var _ = Describe("shouldModuleRunOnNode", func() { +var _ = Describe("getNMCsByModuleSet", func() { var ( - mnrh moduleNMCReconcilerHelperAPI - kernelVersion string + ctrl *gomock.Controller + clnt *client.MockClient + mnrh moduleNMCReconcilerHelperAPI ) BeforeEach(func() { - mnrh = newModuleNMCReconcilerHelper(nil, nil, nil, nil, scheme) - kernelVersion = "some version" + ctrl = gomock.NewController(GinkgoT()) + clnt = client.NewMockClient(ctrl) + mnrh = newModuleNMCReconcilerHelper(clnt, nil, nil, nil, nil, scheme) }) - It("kernel version not equal", func() { - node := v1.Node{ - Status: v1.NodeStatus{ - NodeInfo: v1.NodeSystemInfo{ - KernelVersion: kernelVersion, - }, - }, + ctx := context.Background() + + It("list failed", func() { + clnt.EXPECT().List(ctx, gomock.Any(), gomock.Any()).Return(fmt.Errorf("some error")) + + nodes, err := mnrh.getNMCsByModuleSet(ctx, &kmmv1beta1.Module{}) + + Expect(err).To(HaveOccurred()) + Expect(nodes).To(BeNil()) + }) + + It("Return NMCs", func() { + nmc1 := kmmv1beta1.NodeModulesConfig{ + ObjectMeta: metav1.ObjectMeta{Name: "nmc1"}, } - mld := &api.ModuleLoaderData{ - KernelVersion: "other kernelVersion", + nmc2 := kmmv1beta1.NodeModulesConfig{ + ObjectMeta: metav1.ObjectMeta{Name: "nmc2"}, } + nmc3 := kmmv1beta1.NodeModulesConfig{ + ObjectMeta: metav1.ObjectMeta{Name: "nmc3"}, + } + clnt.EXPECT().List(ctx, gomock.Any(), gomock.Any()).DoAndReturn( + func(_ interface{}, list *kmmv1beta1.NodeModulesConfigList, _ ...interface{}) error { + list.Items = []kmmv1beta1.NodeModulesConfig{nmc1, nmc2, nmc3} + return nil + }, + ) + + nmcsSet, err := mnrh.getNMCsByModuleSet(ctx, &kmmv1beta1.Module{}) + + expectedSet := sets.New[string]([]string{"nmc1", "nmc2", "nmc3"}...) - res, err := mnrh.shouldModuleRunOnNode(node, mld) Expect(err).NotTo(HaveOccurred()) - Expect(res).To(BeFalse()) + Expect(nmcsSet.Equal(expectedSet)).To(BeTrue()) + }) +}) - It("node not schedulable", func() { - node := v1.Node{ - Spec: v1.NodeSpec{ - Taints: []v1.Taint{ - v1.Taint{ - Effect: v1.TaintEffectNoSchedule, - }, - }, - }, - Status: v1.NodeStatus{ - NodeInfo: v1.NodeSystemInfo{ - KernelVersion: kernelVersion, - }, - }, - } +var _ = Describe("prepareSchedulingData", func() { + var ( + ctrl *gomock.Controller + clnt *client.MockClient + mockKernel *module.MockKernelMapper + mockHelper *nmc.MockHelper + mnrh moduleNMCReconcilerHelperAPI + ) - mld := &api.ModuleLoaderData{ - KernelVersion: kernelVersion, - } + BeforeEach(func() { + ctrl = gomock.NewController(GinkgoT()) + clnt = client.NewMockClient(ctrl) + mockKernel = module.NewMockKernelMapper(ctrl) + mockHelper = nmc.NewMockHelper(ctrl) + mnrh = newModuleNMCReconcilerHelper(clnt, mockKernel, nil, mockHelper, nil, scheme) + }) - res, err := mnrh.shouldModuleRunOnNode(node, mld) - Expect(err).NotTo(HaveOccurred()) - Expect(res).To(BeFalse()) + const kernelVersion = "some kernel version" + const nodeName = "nodeName" + + ctx := context.Background() + mod := kmmv1beta1.Module{} + node := v1.Node{ + ObjectMeta: metav1.ObjectMeta{Name: nodeName}, + Status: v1.NodeStatus{ + NodeInfo: v1.NodeSystemInfo{KernelVersion: kernelVersion}, + }, + } + targetedNodes := []v1.Node{node} + mld := api.ModuleLoaderData{KernelVersion: "some version"} + + It("failed to determine mld", func() { + currentNMCs := sets.New[string](nodeName) + mockKernel.EXPECT().GetModuleLoaderDataForKernel(&mod, kernelVersion).Return(nil, fmt.Errorf("some error")) + + scheduleData, errs := mnrh.prepareSchedulingData(ctx, &mod, targetedNodes, currentNMCs) + + Expect(len(errs)).To(Equal(1)) + Expect(scheduleData).To(Equal(map[string]schedulingData{})) }) - DescribeTable("selector vs node labels verification", func(mldSelector, nodeLabels map[string]string, expectedResult bool) { - mld := &api.ModuleLoaderData{ - KernelVersion: kernelVersion, - Selector: mldSelector, - } - node := v1.Node{ - Status: v1.NodeStatus{ - NodeInfo: v1.NodeSystemInfo{ - KernelVersion: kernelVersion, - }, - }, - } - node.SetLabels(nodeLabels) + It("mld for kernel version does not exists", func() { + currentNMCs := sets.New[string](nodeName) + mockKernel.EXPECT().GetModuleLoaderDataForKernel(&mod, kernelVersion).Return(nil, module.ErrNoMatchingKernelMapping) - res, err := mnrh.shouldModuleRunOnNode(node, mld) - Expect(err).NotTo(HaveOccurred()) - Expect(res).To(Equal(expectedResult)) - }, - Entry("selector label not present in nodes'", - map[string]string{"label_1": "label_1_value", "label_2": "label_2_value"}, - map[string]string{"label_1": "label_1_value", "label_3": "label_3_value"}, - false), - Entry("selector labels present in nodes'", - map[string]string{"label_1": "label_1_value", "label_2": "label_2_value"}, - map[string]string{"label_1": "label_1_value", "label_2": "label_2_value"}, - true), - Entry("no selector labels'", - nil, - map[string]string{"label_1": "label_1_value", "label_2": "label_2_value"}, - true), - ) + scheduleData, errs := mnrh.prepareSchedulingData(ctx, &mod, targetedNodes, currentNMCs) + + expectedScheduleData := map[string]schedulingData{nodeName: schedulingData{mld: nil, node: &node, nmcExists: true}} + Expect(errs).To(BeEmpty()) + Expect(scheduleData).To(Equal(expectedScheduleData)) + }) + + It("mld exists", func() { + currentNMCs := sets.New[string](nodeName) + mockKernel.EXPECT().GetModuleLoaderDataForKernel(&mod, kernelVersion).Return(&mld, nil) + + scheduleData, errs := mnrh.prepareSchedulingData(ctx, &mod, targetedNodes, currentNMCs) + + expectedScheduleData := map[string]schedulingData{nodeName: schedulingData{mld: &mld, node: &node, nmcExists: true}} + Expect(errs).To(BeEmpty()) + Expect(scheduleData).To(Equal(expectedScheduleData)) + }) + + It("mld exists, nmc exists for other node", func() { + currentNMCs := sets.New[string]("some other node") + mockKernel.EXPECT().GetModuleLoaderDataForKernel(&mod, kernelVersion).Return(&mld, nil) + + scheduleData, errs := mnrh.prepareSchedulingData(ctx, &mod, targetedNodes, currentNMCs) + + Expect(errs).To(BeEmpty()) + Expect(scheduleData).To(HaveKeyWithValue(nodeName, schedulingData{mld: &mld, node: &node, nmcExists: false})) + Expect(scheduleData).To(HaveKeyWithValue("some other node", schedulingData{mld: nil, nmcExists: true})) + }) + + It("failed to determine mld for one of the nodes/nmcs", func() { + currentNMCs := sets.New[string]("some other node") + mockKernel.EXPECT().GetModuleLoaderDataForKernel(&mod, kernelVersion).Return(nil, fmt.Errorf("some error")) + + scheduleData, errs := mnrh.prepareSchedulingData(ctx, &mod, targetedNodes, currentNMCs) + + Expect(errs).NotTo(BeEmpty()) + expectedScheduleData := map[string]schedulingData{"some other node": schedulingData{mld: nil, nmcExists: true}} + Expect(scheduleData).To(Equal(expectedScheduleData)) + }) }) var _ = Describe("enableModuleOnNode", func() { @@ -504,7 +565,7 @@ var _ = Describe("enableModuleOnNode", func() { helper = nmc.NewMockHelper(ctrl) rgst = registry.NewMockRegistry(ctrl) authFactory = auth.NewMockRegistryAuthGetterFactory(ctrl) - mnrh = newModuleNMCReconcilerHelper(clnt, rgst, helper, authFactory, scheme) + mnrh = newModuleNMCReconcilerHelper(clnt, nil, rgst, helper, authFactory, scheme) node = v1.Node{ ObjectMeta: metav1.ObjectMeta{Name: "nodeName"}, } @@ -532,7 +593,7 @@ var _ = Describe("enableModuleOnNode", func() { authFactory.EXPECT().NewRegistryAuthGetterFrom(mld).Return(authGetter), rgst.EXPECT().ImageExists(ctx, mld.ContainerImage, gomock.Any(), authGetter).Return(false, nil), ) - err := mnrh.enableModuleOnNode(ctx, mld, &node, kernelVersion) + err := mnrh.enableModuleOnNode(ctx, mld, &node) Expect(err).NotTo(HaveOccurred()) }) @@ -542,7 +603,7 @@ var _ = Describe("enableModuleOnNode", func() { authFactory.EXPECT().NewRegistryAuthGetterFrom(mld).Return(authGetter), rgst.EXPECT().ImageExists(ctx, mld.ContainerImage, gomock.Any(), authGetter).Return(false, fmt.Errorf("some error")), ) - err := mnrh.enableModuleOnNode(ctx, mld, &node, kernelVersion) + err := mnrh.enableModuleOnNode(ctx, mld, &node) Expect(err).To(HaveOccurred()) }) @@ -560,7 +621,7 @@ var _ = Describe("enableModuleOnNode", func() { clnt.EXPECT().Create(ctx, gomock.Any()).Return(nil), ) - err := mnrh.enableModuleOnNode(ctx, mld, &node, kernelVersion) + err := mnrh.enableModuleOnNode(ctx, mld, &node) Expect(err).NotTo(HaveOccurred()) }) @@ -582,7 +643,7 @@ var _ = Describe("enableModuleOnNode", func() { clnt.EXPECT().Patch(ctx, gomock.Any(), gomock.Any()).Return(nil), ) - err := mnrh.enableModuleOnNode(ctx, mld, &node, kernelVersion) + err := mnrh.enableModuleOnNode(ctx, mld, &node) Expect(err).NotTo(HaveOccurred()) }) }) @@ -604,32 +665,17 @@ var _ = Describe("disableModuleOnNode", func() { ctrl = gomock.NewController(GinkgoT()) clnt = client.NewMockClient(ctrl) helper = nmc.NewMockHelper(ctrl) - mnrh = newModuleNMCReconcilerHelper(clnt, nil, helper, nil, scheme) + mnrh = newModuleNMCReconcilerHelper(clnt, nil, nil, helper, nil, scheme) nodeName = "node name" moduleName = "moduleName" moduleNamespace = "moduleNamespace" }) - It("NMC does not exists", func() { - helper.EXPECT().Get(ctx, nodeName).Return(nil, apierrors.NewNotFound(schema.GroupResource{}, nodeName)) - - err := mnrh.disableModuleOnNode(ctx, moduleNamespace, moduleName, nodeName) - Expect(err).NotTo(HaveOccurred()) - }) - - It("failed to get NMC", func() { - helper.EXPECT().Get(ctx, nodeName).Return(nil, fmt.Errorf("some error")) - - err := mnrh.disableModuleOnNode(ctx, moduleNamespace, moduleName, nodeName) - Expect(err).To(HaveOccurred()) - }) - It("NMC exists", func() { nmc := &kmmv1beta1.NodeModulesConfig{ ObjectMeta: metav1.ObjectMeta{Name: nodeName}, } gomock.InOrder( - helper.EXPECT().Get(ctx, nodeName).Return(nmc, nil), clnt.EXPECT().Get(ctx, gomock.Any(), gomock.Any()).DoAndReturn( func(_ interface{}, _ interface{}, nmc *kmmv1beta1.NodeModulesConfig, _ ...ctrlclient.GetOption) error { nmc.SetName(nodeName) diff --git a/internal/nmc/helper.go b/internal/nmc/helper.go index b98a6e03a..0b1258ed1 100644 --- a/internal/nmc/helper.go +++ b/internal/nmc/helper.go @@ -6,6 +6,7 @@ import ( kmmv1beta1 "github.com/rh-ecosystem-edge/kernel-module-management/api/v1beta1" "github.com/rh-ecosystem-edge/kernel-module-management/internal/api" + "github.com/rh-ecosystem-edge/kernel-module-management/internal/utils" k8serrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" @@ -61,6 +62,7 @@ func (h *helper) SetModuleConfig( nmc.Spec.Modules = append(nmc.Spec.Modules, nms) foundEntry = &nmc.Spec.Modules[len(nmc.Spec.Modules)-1] } + setLabel(nmc, mld.Namespace, mld.Name) foundEntry.Config = *moduleConfig return nil @@ -71,6 +73,7 @@ func (h *helper) RemoveModuleConfig(nmc *kmmv1beta1.NodeModulesConfig, namespace if foundEntry != nil { nmc.Spec.Modules = append(nmc.Spec.Modules[:index], nmc.Spec.Modules[index+1:]...) } + removeLabel(nmc, namespace, name) return nil } @@ -124,3 +127,22 @@ func SetModuleStatus(statuses *[]kmmv1beta1.NodeModuleStatus, status kmmv1beta1. *statuses = append(*statuses, status) } } + +func setLabel(nmc *kmmv1beta1.NodeModulesConfig, namespace, name string) { + moduleNMCLabel := utils.GetModuleNMCLabel(namespace, name) + nmcLabels := nmc.GetLabels() + if nmcLabels == nil { + nmcLabels = map[string]string{} + } + nmcLabels[moduleNMCLabel] = "" + nmc.SetLabels(nmcLabels) +} + +func removeLabel(nmc *kmmv1beta1.NodeModulesConfig, namespace, name string) { + moduleNMCLabel := utils.GetModuleNMCLabel(namespace, name) + nmcLabels := nmc.GetLabels() + if nmcLabels != nil { + delete(nmcLabels, moduleNMCLabel) + nmc.SetLabels(nmcLabels) + } +} diff --git a/internal/nmc/helper_test.go b/internal/nmc/helper_test.go index f0b5e3a24..41bf676e1 100644 --- a/internal/nmc/helper_test.go +++ b/internal/nmc/helper_test.go @@ -9,6 +9,7 @@ import ( kmmv1beta1 "github.com/rh-ecosystem-edge/kernel-module-management/api/v1beta1" "github.com/rh-ecosystem-edge/kernel-module-management/internal/api" "github.com/rh-ecosystem-edge/kernel-module-management/internal/client" + "github.com/rh-ecosystem-edge/kernel-module-management/internal/utils" "go.uber.org/mock/gomock" k8serrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -98,6 +99,7 @@ var _ = Describe("SetModuleConfig", func() { Expect(err).NotTo(HaveOccurred()) Expect(len(nmc.Spec.Modules)).To(Equal(3)) Expect(nmc.Spec.Modules[2].Config.InTreeModuleToRemove).To(Equal("in-tree-module")) + Expect(nmc.GetLabels()).To(HaveKeyWithValue(utils.GetModuleNMCLabel(namespace, name), "")) }) It("changing existing module config", func() { @@ -124,6 +126,7 @@ var _ = Describe("SetModuleConfig", func() { Expect(err).NotTo(HaveOccurred()) Expect(len(nmc.Spec.Modules)).To(Equal(2)) Expect(nmc.Spec.Modules[1].Config.InTreeModuleToRemove).To(Equal("in-tree-module")) + Expect(nmc.GetLabels()).To(HaveKeyWithValue(utils.GetModuleNMCLabel(namespace, name), "")) }) }) @@ -163,9 +166,11 @@ var _ = Describe("RemoveModuleConfig", func() { Expect(len(nmc.Spec.Modules)).To(Equal(2)) Expect(nmc.Spec.Modules[0].Config.InTreeModuleToRemove).To(Equal("some-in-tree-module-1")) Expect(nmc.Spec.Modules[1].Config.InTreeModuleToRemove).To(Equal("some-in-tree-module-2")) + Expect(nmc.GetLabels()).NotTo(HaveKeyWithValue(utils.GetModuleNMCLabel(namespace, name), "")) }) It("deleting existing module", func() { + nmc.SetLabels(map[string]string{utils.GetModuleNMCLabel(namespace, name): ""}) nmc.Spec.Modules = []kmmv1beta1.NodeModuleSpec{ { ModuleItem: kmmv1beta1.ModuleItem{ @@ -188,6 +193,7 @@ var _ = Describe("RemoveModuleConfig", func() { Expect(err).NotTo(HaveOccurred()) Expect(len(nmc.Spec.Modules)).To(Equal(1)) Expect(nmc.Spec.Modules[0].Config.InTreeModuleToRemove).To(Equal("some-in-tree-module-1")) + Expect(nmc.GetLabels()).NotTo(HaveKeyWithValue(utils.GetModuleNMCLabel(namespace, name), "")) }) }) diff --git a/internal/utils/moduleversionlabels.go b/internal/utils/kmmlabels.go similarity index 92% rename from internal/utils/moduleversionlabels.go rename to internal/utils/kmmlabels.go index 55c509a73..3bd4c89ad 100644 --- a/internal/utils/moduleversionlabels.go +++ b/internal/utils/kmmlabels.go @@ -54,3 +54,7 @@ func GetNodesVersionLabels(nodeLabels map[string]string) map[string]string { } return versionLabels } + +func GetModuleNMCLabel(namespace, name string) string { + return fmt.Sprintf("%s.%s.%s", constants.ModuleNMCLabelPrefix, namespace, name) +} diff --git a/internal/utils/moduleversionlabels_test.go b/internal/utils/kmmlabels_test.go similarity index 100% rename from internal/utils/moduleversionlabels_test.go rename to internal/utils/kmmlabels_test.go