diff --git a/internal/controllers/mock_nmc_reconciler.go b/internal/controllers/mock_nmc_reconciler.go index 6f82f97a1..328c5eb55 100644 --- a/internal/controllers/mock_nmc_reconciler.go +++ b/internal/controllers/mock_nmc_reconciler.go @@ -15,6 +15,8 @@ import ( v1beta1 "github.com/kubernetes-sigs/kernel-module-management/api/v1beta1" 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" client "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -83,6 +85,18 @@ func (mr *MocknmcReconcilerHelperMockRecorder) ProcessUnconfiguredModuleStatus(c return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ProcessUnconfiguredModuleStatus", reflect.TypeOf((*MocknmcReconcilerHelper)(nil).ProcessUnconfiguredModuleStatus), ctx, nmc, status) } +// RecordEvents mocks base method. +func (m *MocknmcReconcilerHelper) RecordEvents(node *v1.Node, loadedModules, unloadedModules []types.NamespacedName) { + m.ctrl.T.Helper() + m.ctrl.Call(m, "RecordEvents", node, loadedModules, unloadedModules) +} + +// RecordEvents indicates an expected call of RecordEvents. +func (mr *MocknmcReconcilerHelperMockRecorder) RecordEvents(node, loadedModules, unloadedModules any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RecordEvents", reflect.TypeOf((*MocknmcReconcilerHelper)(nil).RecordEvents), node, loadedModules, unloadedModules) +} + // RemovePodFinalizers mocks base method. func (m *MocknmcReconcilerHelper) RemovePodFinalizers(ctx context.Context, nodeName string) error { m.ctrl.T.Helper() @@ -111,18 +125,127 @@ func (mr *MocknmcReconcilerHelperMockRecorder) SyncStatus(ctx, nmc any) *gomock. return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SyncStatus", reflect.TypeOf((*MocknmcReconcilerHelper)(nil).SyncStatus), ctx, nmc) } -// UpdateNodeLabelsAndRecordEvents mocks base method. -func (m *MocknmcReconcilerHelper) UpdateNodeLabelsAndRecordEvents(ctx context.Context, nmc *v1beta1.NodeModulesConfig) error { +// UpdateNodeLabels mocks base method. +func (m *MocknmcReconcilerHelper) UpdateNodeLabels(ctx context.Context, nmc *v1beta1.NodeModulesConfig, node *v1.Node) ([]types.NamespacedName, []types.NamespacedName, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "UpdateNodeLabelsAndRecordEvents", ctx, nmc) - ret0, _ := ret[0].(error) + ret := m.ctrl.Call(m, "UpdateNodeLabels", ctx, nmc, node) + ret0, _ := ret[0].([]types.NamespacedName) + ret1, _ := ret[1].([]types.NamespacedName) + ret2, _ := ret[2].(error) + return ret0, ret1, ret2 +} + +// UpdateNodeLabels indicates an expected call of UpdateNodeLabels. +func (mr *MocknmcReconcilerHelperMockRecorder) UpdateNodeLabels(ctx, nmc, node any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateNodeLabels", reflect.TypeOf((*MocknmcReconcilerHelper)(nil).UpdateNodeLabels), ctx, nmc, node) +} + +// MocklabelPreparationHelper is a mock of labelPreparationHelper interface. +type MocklabelPreparationHelper struct { + ctrl *gomock.Controller + recorder *MocklabelPreparationHelperMockRecorder +} + +// MocklabelPreparationHelperMockRecorder is the mock recorder for MocklabelPreparationHelper. +type MocklabelPreparationHelperMockRecorder struct { + mock *MocklabelPreparationHelper +} + +// NewMocklabelPreparationHelper creates a new mock instance. +func NewMocklabelPreparationHelper(ctrl *gomock.Controller) *MocklabelPreparationHelper { + mock := &MocklabelPreparationHelper{ctrl: ctrl} + mock.recorder = &MocklabelPreparationHelperMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MocklabelPreparationHelper) EXPECT() *MocklabelPreparationHelperMockRecorder { + return m.recorder +} + +// addEqualLabels mocks base method. +func (m *MocklabelPreparationHelper) addEqualLabels(nodeModuleReadyLabels sets.Set[types.NamespacedName], specLabels, statusLabels map[types.NamespacedName]v1beta1.ModuleConfig) []types.NamespacedName { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "addEqualLabels", nodeModuleReadyLabels, specLabels, statusLabels) + ret0, _ := ret[0].([]types.NamespacedName) + return ret0 +} + +// addEqualLabels indicates an expected call of addEqualLabels. +func (mr *MocklabelPreparationHelperMockRecorder) addEqualLabels(nodeModuleReadyLabels, specLabels, statusLabels any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "addEqualLabels", reflect.TypeOf((*MocklabelPreparationHelper)(nil).addEqualLabels), nodeModuleReadyLabels, specLabels, statusLabels) +} + +// getDeprecatedKernelModuleReadyLabels mocks base method. +func (m *MocklabelPreparationHelper) getDeprecatedKernelModuleReadyLabels(node v1.Node) sets.Set[string] { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "getDeprecatedKernelModuleReadyLabels", node) + ret0, _ := ret[0].(sets.Set[string]) + return ret0 +} + +// getDeprecatedKernelModuleReadyLabels indicates an expected call of getDeprecatedKernelModuleReadyLabels. +func (mr *MocklabelPreparationHelperMockRecorder) getDeprecatedKernelModuleReadyLabels(node any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "getDeprecatedKernelModuleReadyLabels", reflect.TypeOf((*MocklabelPreparationHelper)(nil).getDeprecatedKernelModuleReadyLabels), node) +} + +// getNodeKernelModuleReadyLabels mocks base method. +func (m *MocklabelPreparationHelper) getNodeKernelModuleReadyLabels(node v1.Node) sets.Set[types.NamespacedName] { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "getNodeKernelModuleReadyLabels", node) + ret0, _ := ret[0].(sets.Set[types.NamespacedName]) + return ret0 +} + +// getNodeKernelModuleReadyLabels indicates an expected call of getNodeKernelModuleReadyLabels. +func (mr *MocklabelPreparationHelperMockRecorder) getNodeKernelModuleReadyLabels(node any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "getNodeKernelModuleReadyLabels", reflect.TypeOf((*MocklabelPreparationHelper)(nil).getNodeKernelModuleReadyLabels), node) +} + +// getSpecLabelsAndTheirConfigs mocks base method. +func (m *MocklabelPreparationHelper) getSpecLabelsAndTheirConfigs(nmc *v1beta1.NodeModulesConfig) map[types.NamespacedName]v1beta1.ModuleConfig { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "getSpecLabelsAndTheirConfigs", nmc) + ret0, _ := ret[0].(map[types.NamespacedName]v1beta1.ModuleConfig) + return ret0 +} + +// getSpecLabelsAndTheirConfigs indicates an expected call of getSpecLabelsAndTheirConfigs. +func (mr *MocklabelPreparationHelperMockRecorder) getSpecLabelsAndTheirConfigs(nmc any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "getSpecLabelsAndTheirConfigs", reflect.TypeOf((*MocklabelPreparationHelper)(nil).getSpecLabelsAndTheirConfigs), nmc) +} + +// getStatusLabelsAndTheirConfigs mocks base method. +func (m *MocklabelPreparationHelper) getStatusLabelsAndTheirConfigs(nmc *v1beta1.NodeModulesConfig) map[types.NamespacedName]v1beta1.ModuleConfig { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "getStatusLabelsAndTheirConfigs", nmc) + ret0, _ := ret[0].(map[types.NamespacedName]v1beta1.ModuleConfig) + return ret0 +} + +// getStatusLabelsAndTheirConfigs indicates an expected call of getStatusLabelsAndTheirConfigs. +func (mr *MocklabelPreparationHelperMockRecorder) getStatusLabelsAndTheirConfigs(nmc any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "getStatusLabelsAndTheirConfigs", reflect.TypeOf((*MocklabelPreparationHelper)(nil).getStatusLabelsAndTheirConfigs), nmc) +} + +// removeOrphanedLabels mocks base method. +func (m *MocklabelPreparationHelper) removeOrphanedLabels(nodeModuleReadyLabels sets.Set[types.NamespacedName], specLabels, statusLabels map[types.NamespacedName]v1beta1.ModuleConfig) []types.NamespacedName { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "removeOrphanedLabels", nodeModuleReadyLabels, specLabels, statusLabels) + ret0, _ := ret[0].([]types.NamespacedName) return ret0 } -// UpdateNodeLabelsAndRecordEvents indicates an expected call of UpdateNodeLabelsAndRecordEvents. -func (mr *MocknmcReconcilerHelperMockRecorder) UpdateNodeLabelsAndRecordEvents(ctx, nmc any) *gomock.Call { +// removeOrphanedLabels indicates an expected call of removeOrphanedLabels. +func (mr *MocklabelPreparationHelperMockRecorder) removeOrphanedLabels(nodeModuleReadyLabels, specLabels, statusLabels any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateNodeLabelsAndRecordEvents", reflect.TypeOf((*MocknmcReconcilerHelper)(nil).UpdateNodeLabelsAndRecordEvents), ctx, nmc) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "removeOrphanedLabels", reflect.TypeOf((*MocklabelPreparationHelper)(nil).removeOrphanedLabels), nodeModuleReadyLabels, specLabels, statusLabels) } // MockpodManager is a mock of podManager interface. diff --git a/internal/controllers/nmc_reconciler.go b/internal/controllers/nmc_reconciler.go index 9e89107f2..0664fcd6c 100644 --- a/internal/controllers/nmc_reconciler.go +++ b/internal/controllers/nmc_reconciler.go @@ -153,9 +153,15 @@ func (r *NMCReconciler) Reconcile(ctx context.Context, req reconcile.Request) (r if err := r.helper.GarbageCollectInUseLabels(ctx, &nmcObj); err != nil { errs = append(errs, fmt.Errorf("failed to GC in-use labels for NMC %s: %v", req.NamespacedName, err)) } + node := v1.Node{} + if err := r.client.Get(ctx, types.NamespacedName{Name: nmcObj.Name}, &node); err != nil { + return ctrl.Result{}, fmt.Errorf("could not get node %s: %v", nmcObj.Name, err) + } - if err := r.helper.UpdateNodeLabelsAndRecordEvents(ctx, &nmcObj); err != nil { + if loaded, unloaded, err := r.helper.UpdateNodeLabels(ctx, &nmcObj, &node); err != nil { errs = append(errs, fmt.Errorf("could not update node's labels for NMC %s: %v", req.NamespacedName, err)) + } else { + r.helper.RecordEvents(&node, loaded, unloaded) } return ctrl.Result{}, errors.Join(errs...) @@ -213,7 +219,8 @@ type nmcReconcilerHelper interface { ProcessUnconfiguredModuleStatus(ctx context.Context, nmc *kmmv1beta1.NodeModulesConfig, status *kmmv1beta1.NodeModuleStatus) error RemovePodFinalizers(ctx context.Context, nodeName string) error SyncStatus(ctx context.Context, nmc *kmmv1beta1.NodeModulesConfig) error - UpdateNodeLabelsAndRecordEvents(ctx context.Context, nmc *kmmv1beta1.NodeModulesConfig) error + UpdateNodeLabels(ctx context.Context, nmc *kmmv1beta1.NodeModulesConfig, node *v1.Node) ([]types.NamespacedName, []types.NamespacedName, error) + RecordEvents(node *v1.Node, loadedModules, unloadedModules []types.NamespacedName) } type nmcReconcilerHelperImpl struct { @@ -221,6 +228,7 @@ type nmcReconcilerHelperImpl struct { pm podManager recorder record.EventRecorder nodeAPI node.Node + lph labelPreparationHelper } func newNMCReconcilerHelper(client client.Client, pm podManager, recorder record.EventRecorder, nodeAPI node.Node) nmcReconcilerHelper { @@ -229,6 +237,7 @@ func newNMCReconcilerHelper(client client.Client, pm podManager, recorder record pm: pm, recorder: recorder, nodeAPI: nodeAPI, + lph: newLabelPreparationHelper(), } } @@ -555,104 +564,147 @@ func (h *nmcReconcilerHelperImpl) SyncStatus(ctx context.Context, nmcObj *kmmv1b return errors.Join(errs...) } -func (h *nmcReconcilerHelperImpl) UpdateNodeLabelsAndRecordEvents(ctx context.Context, nmc *kmmv1beta1.NodeModulesConfig) error { - node := v1.Node{} - if err := h.client.Get(ctx, types.NamespacedName{Name: nmc.Name}, &node); err != nil { - return fmt.Errorf("could not get node %s: %v", nmc.Name, err) - } +type labelPreparationHelper interface { + getDeprecatedKernelModuleReadyLabels(node v1.Node) sets.Set[string] + getNodeKernelModuleReadyLabels(node v1.Node) sets.Set[types.NamespacedName] + getSpecLabelsAndTheirConfigs(nmc *kmmv1beta1.NodeModulesConfig) map[types.NamespacedName]kmmv1beta1.ModuleConfig + getStatusLabelsAndTheirConfigs(nmc *kmmv1beta1.NodeModulesConfig) map[types.NamespacedName]kmmv1beta1.ModuleConfig + addEqualLabels(nodeModuleReadyLabels sets.Set[types.NamespacedName], + specLabels, statusLabels map[types.NamespacedName]kmmv1beta1.ModuleConfig) []types.NamespacedName + removeOrphanedLabels(nodeModuleReadyLabels sets.Set[types.NamespacedName], + specLabels, statusLabels map[types.NamespacedName]kmmv1beta1.ModuleConfig) []types.NamespacedName +} +type labelPreparationHelperImpl struct{} - // get all the kernel module ready labels of the node +func newLabelPreparationHelper() labelPreparationHelper { + return &labelPreparationHelperImpl{} +} + +func (lph *labelPreparationHelperImpl) getNodeKernelModuleReadyLabels(node v1.Node) sets.Set[types.NamespacedName] { nodeModuleReadyLabels := sets.New[types.NamespacedName]() - deprecatedNodeModuleReadyLabels := sets.New[string]() for label := range node.GetLabels() { if ok, namespace, name := utils.IsKernelModuleReadyNodeLabel(label); ok { nodeModuleReadyLabels.Insert(types.NamespacedName{Namespace: namespace, Name: name}) } + } + return nodeModuleReadyLabels +} +func (lph *labelPreparationHelperImpl) getDeprecatedKernelModuleReadyLabels(node v1.Node) sets.Set[string] { + deprecatedNodeModuleReadyLabels := sets.New[string]() + + for label := range node.GetLabels() { if utils.IsDeprecatedKernelModuleReadyNodeLabel(label) { deprecatedNodeModuleReadyLabels.Insert(label) } } + return deprecatedNodeModuleReadyLabels +} - // get spec labels and their config +func (lph *labelPreparationHelperImpl) getSpecLabelsAndTheirConfigs(nmc *kmmv1beta1.NodeModulesConfig) map[types.NamespacedName]kmmv1beta1.ModuleConfig { specLabels := make(map[types.NamespacedName]kmmv1beta1.ModuleConfig) + for _, module := range nmc.Spec.Modules { specLabels[types.NamespacedName{Namespace: module.Namespace, Name: module.Name}] = module.Config } + return specLabels +} - // get status labels and their config +func (lph *labelPreparationHelperImpl) getStatusLabelsAndTheirConfigs(nmc *kmmv1beta1.NodeModulesConfig) map[types.NamespacedName]kmmv1beta1.ModuleConfig { statusLabels := make(map[types.NamespacedName]kmmv1beta1.ModuleConfig) + for _, module := range nmc.Status.Modules { label := types.NamespacedName{Namespace: module.Namespace, Name: module.Name} statusLabels[label] = module.Config } + return statusLabels +} - unloaded := make([]types.NamespacedName, 0, len(nodeModuleReadyLabels)) - loaded := make([]types.NamespacedName, 0, len(specLabels)) +func (h *nmcReconcilerHelperImpl) UpdateNodeLabels(ctx context.Context, nmc *kmmv1beta1.NodeModulesConfig, node *v1.Node) ([]types.NamespacedName, []types.NamespacedName, error) { - patchFrom := client.MergeFrom(node.DeepCopy()) + // get all the kernel module ready labels of the node + nodeModuleReadyLabels := h.lph.getNodeKernelModuleReadyLabels(*node) + deprecatedNodeModuleReadyLabels := h.lph.getDeprecatedKernelModuleReadyLabels(*node) - // label in node but not in spec or status - should be removed - for nsn := range nodeModuleReadyLabels { - _, inSpec := specLabels[nsn] - _, inStatus := statusLabels[nsn] - if !inSpec && !inStatus { - meta.RemoveLabel( - &node, - utils.GetKernelModuleReadyNodeLabel(nsn.Namespace, nsn.Name), - ) + // get spec labels and their config + specLabels := h.lph.getSpecLabelsAndTheirConfigs(nmc) - unloaded = append(unloaded, nsn) - } - } + // get status labels and their config + statusLabels := h.lph.getStatusLabelsAndTheirConfigs(nmc) - // v1 ready labels, deprecated - should be removed - for label := range deprecatedNodeModuleReadyLabels { - meta.RemoveLabel(&node, label) - } + // label in node but not in spec or status - should be removed + nsnLabelsToBeRemoved := h.lph.removeOrphanedLabels(nodeModuleReadyLabels, specLabels, statusLabels) // label in spec and status and config equal - should be added - for nsn, specConfig := range specLabels { - statusConfig, ok := statusLabels[nsn] - if ok && reflect.DeepEqual(specConfig, statusConfig) && !nodeModuleReadyLabels.Has(nsn) { - meta.SetLabel( - &node, - utils.GetKernelModuleReadyNodeLabel(nsn.Namespace, nsn.Name), - "", - ) + nsnLabelsToBeLoaded := h.lph.addEqualLabels(nodeModuleReadyLabels, specLabels, statusLabels) - loaded = append(loaded, nsn) - } + var loadedLabels []string + unloadedLabels := deprecatedNodeModuleReadyLabels.UnsortedList() + + for _, label := range nsnLabelsToBeRemoved { + unloadedLabels = append(unloadedLabels, utils.GetKernelModuleReadyNodeLabel(label.Namespace, label.Name)) + } + for _, label := range nsnLabelsToBeLoaded { + loadedLabels = append(loadedLabels, utils.GetKernelModuleReadyNodeLabel(label.Namespace, label.Name)) } - if err := h.client.Patch(ctx, &node, patchFrom); err != nil { - return fmt.Errorf("could not patch node: %v", err) + if err := h.nodeAPI.UpdateLabels(ctx, node, loadedLabels, unloadedLabels); err != nil { + return nil, nil, fmt.Errorf("could not update labels on the node: %v", err) } - for _, nsn := range unloaded { + return nsnLabelsToBeLoaded, nsnLabelsToBeRemoved, nil +} + +func (h *nmcReconcilerHelperImpl) RecordEvents(node *v1.Node, loadedModules, unloadedModules []types.NamespacedName) { + for _, nsn := range loadedModules { h.recorder.AnnotatedEventf( - &node, + node, map[string]string{"module": nsn.String()}, v1.EventTypeNormal, - "ModuleUnloaded", - "Module %s unloaded from the kernel", + "ModuleLoaded", + "Module %s loaded into the kernel", nsn.String(), ) } - - for _, nsn := range loaded { + for _, nsn := range unloadedModules { h.recorder.AnnotatedEventf( - &node, + node, map[string]string{"module": nsn.String()}, v1.EventTypeNormal, - "ModuleLoaded", - "Module %s loaded into the kernel", + "ModuleUnloaded", + "Module %s unloaded from the kernel", nsn.String(), ) } +} - return nil +func (lph *labelPreparationHelperImpl) removeOrphanedLabels(nodeModuleReadyLabels sets.Set[types.NamespacedName], + specLabels, statusLabels map[types.NamespacedName]kmmv1beta1.ModuleConfig) []types.NamespacedName { + + unloaded := make([]types.NamespacedName, 0, len(nodeModuleReadyLabels)) + + for nsn := range nodeModuleReadyLabels { + _, inSpec := specLabels[nsn] + _, inStatus := statusLabels[nsn] + if !inSpec && !inStatus { + unloaded = append(unloaded, nsn) + } + } + return unloaded +} +func (lph *labelPreparationHelperImpl) addEqualLabels(nodeModuleReadyLabels sets.Set[types.NamespacedName], + specLabels, statusLabels map[types.NamespacedName]kmmv1beta1.ModuleConfig) []types.NamespacedName { + + loaded := make([]types.NamespacedName, 0, len(nodeModuleReadyLabels)) + + for nsn, specConfig := range specLabels { + statusConfig, ok := statusLabels[nsn] + if ok && reflect.DeepEqual(specConfig, statusConfig) && !nodeModuleReadyLabels.Has(nsn) { + loaded = append(loaded, nsn) + } + } + return loaded } const ( diff --git a/internal/controllers/nmc_reconciler_test.go b/internal/controllers/nmc_reconciler_test.go index a973082e1..6a3ec746c 100644 --- a/internal/controllers/nmc_reconciler_test.go +++ b/internal/controllers/nmc_reconciler_test.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "github.com/kubernetes-sigs/kernel-module-management/internal/node" + "k8s.io/apimachinery/pkg/util/sets" "path/filepath" "reflect" "strings" @@ -36,7 +37,15 @@ import ( "sigs.k8s.io/controller-runtime/pkg/reconcile" ) -const nmcName = "nmc" +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" +) var _ = Describe("NodeModulesConfigReconciler_Reconcile", func() { var ( @@ -101,6 +110,12 @@ var _ = Describe("NodeModulesConfigReconciler_Reconcile", func() { mod1Name = "mod1" mod2Name = "mod2" ) + var ( + loaded []types.NamespacedName + unloaded []types.NamespacedName + err error + node v1.Node + ) spec0 := kmmv1beta1.NodeModuleSpec{ ModuleItem: kmmv1beta1.ModuleItem{ Namespace: namespace, @@ -155,7 +170,9 @@ var _ = Describe("NodeModulesConfigReconciler_Reconcile", func() { wh.EXPECT().ProcessModuleSpec(contextWithValueMatch, nmc, &spec1, nil), wh.EXPECT().ProcessUnconfiguredModuleStatus(contextWithValueMatch, nmc, &status2), wh.EXPECT().GarbageCollectInUseLabels(ctx, nmc), - wh.EXPECT().UpdateNodeLabelsAndRecordEvents(ctx, nmc), + kubeClient.EXPECT().Get(ctx, types.NamespacedName{Name: nmc.Name}, &node).Return(nil), + wh.EXPECT().UpdateNodeLabels(ctx, nmc, &node).Return(loaded, unloaded, err), + wh.EXPECT().RecordEvents(&node, loaded, unloaded), ) Expect( @@ -171,6 +188,11 @@ var _ = Describe("NodeModulesConfigReconciler_Reconcile", func() { mod1Name = "mod1" mod2Name = "mod2" ) + var ( + node v1.Node + err error + ) + spec0 := kmmv1beta1.NodeModuleSpec{ ModuleItem: kmmv1beta1.ModuleItem{ Namespace: namespace, @@ -217,10 +239,11 @@ var _ = Describe("NodeModulesConfigReconciler_Reconcile", func() { wh.EXPECT().ProcessModuleSpec(contextWithValueMatch, nmc, &spec0, &status0).Return(fmt.Errorf("some error")), wh.EXPECT().ProcessUnconfiguredModuleStatus(contextWithValueMatch, nmc, &status2).Return(fmt.Errorf("some error")), wh.EXPECT().GarbageCollectInUseLabels(ctx, nmc).Return(fmt.Errorf("some error")), - wh.EXPECT().UpdateNodeLabelsAndRecordEvents(ctx, nmc).Return(fmt.Errorf("some error")), + kubeClient.EXPECT().Get(ctx, types.NamespacedName{Name: nmc.Name}, &node).Return(nil), + wh.EXPECT().UpdateNodeLabels(ctx, nmc, &node).Return(nil, nil, fmt.Errorf("some error")), ) - _, err := r.Reconcile(ctx, req) + _, err = r.Reconcile(ctx, req) Expect(err).ToNot(BeNil()) }) }) @@ -1158,39 +1181,148 @@ var _ = Describe("nmcReconcilerHelperImpl_RemovePodFinalizers", func() { const ( moduleName = "my-module" moduleNamespace = "my-module-namespace" + nodeName = "node-name" ) -var _ = Describe("nmcReconcilerHelperImpl_UpdateNodeLabelsAndRecordEvents", func() { +var kernelModuleLabelName = utils.GetKernelModuleReadyNodeLabel(moduleNamespace, moduleName) + +var _ = Describe("nmcReconcilerHelperImpl_UpdateNodeLabels", func() { var ( - ctx = context.TODO() - client *testclient.MockClient - expectedNode v1.Node - fakeRecorder *record.FakeRecorder - nmc kmmv1beta1.NodeModulesConfig - wh nmcReconcilerHelper + ctx context.Context + client *testclient.MockClient + fakeRecorder *record.FakeRecorder + n *node.MockNode + nmc kmmv1beta1.NodeModulesConfig + wh nmcReconcilerHelper + mlph *MocklabelPreparationHelper + firstLabelName string + secondLabelName string ) BeforeEach(func() { + ctx = context.TODO() ctrl := gomock.NewController(GinkgoT()) client = testclient.NewMockClient(ctrl) - expectedNode = v1.Node{ - ObjectMeta: metav1.ObjectMeta{ - Name: "node name", - Labels: map[string]string{}, - }, - } nmc = kmmv1beta1.NodeModulesConfig{ ObjectMeta: metav1.ObjectMeta{Name: "nmcName"}, + Spec: kmmv1beta1.NodeModulesConfigSpec{ + Modules: []kmmv1beta1.NodeModuleSpec{ + { + ModuleItem: kmmv1beta1.ModuleItem{ + Namespace: nsFirst, + Name: nameFirst, + }, + Config: kmmv1beta1.ModuleConfig{}, + }, + { + ModuleItem: kmmv1beta1.ModuleItem{ + Namespace: nsSecond, + Name: nameSecond, + }, + Config: kmmv1beta1.ModuleConfig{}, + }, + }, + }, } fakeRecorder = record.NewFakeRecorder(10) - wh = newNMCReconcilerHelper(client, nil, fakeRecorder, nil) + n = node.NewMockNode(ctrl) + wh = newNMCReconcilerHelper(client, nil, fakeRecorder, n) + mlph = NewMocklabelPreparationHelper(ctrl) + wh = &nmcReconcilerHelperImpl{ + client: client, + pm: nil, + recorder: fakeRecorder, + nodeAPI: n, + lph: mlph, + } + firstLabelName = fmt.Sprintf("kmm.node.kubernetes.io/%s.%s.ready", nsFirst, nameFirst) + secondLabelName = fmt.Sprintf("kmm.node.kubernetes.io/%s.%s.ready", nsSecond, nameSecond) }) - moduleConfig := kmmv1beta1.ModuleConfig{ - KernelVersion: "some version", - ContainerImage: "some image", - InTreeModulesToRemove: []string{"some kernel module"}, - } + It("failed to get node", func() { + client.EXPECT().Get(ctx, gomock.Any(), gomock.Any(), gomock.Any()).Return(fmt.Errorf("some error")) + err := client.Get(ctx, types.NamespacedName{Name: nmc.Name}, &v1.Node{}) + Expect(err).To(HaveOccurred()) + }) + It("Should fail patching node after change in labels", func() { + node := v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + firstLabelName: "", + }, + Name: nodeName, + }, + } + emptySet := sets.Set[types.NamespacedName]{} + emptyMap := map[types.NamespacedName]kmmv1beta1.ModuleConfig{} + + gomock.InOrder( + mlph.EXPECT().getNodeKernelModuleReadyLabels(node).Return(emptySet), + mlph.EXPECT().getDeprecatedKernelModuleReadyLabels(node).Return(sets.Set[string]{}), + mlph.EXPECT().getSpecLabelsAndTheirConfigs(&nmc).Return(emptyMap), + mlph.EXPECT().getStatusLabelsAndTheirConfigs(&nmc).Return(emptyMap), + mlph.EXPECT().removeOrphanedLabels(emptySet, emptyMap, emptyMap).Return([]types.NamespacedName{{Name: nameSecond, Namespace: nsSecond}}), + mlph.EXPECT().addEqualLabels(emptySet, emptyMap, emptyMap).Return([]types.NamespacedName{{Name: nameFirst, Namespace: nsFirst}}), + n.EXPECT(). + UpdateLabels( + ctx, + &node, + []string{firstLabelName}, + []string{secondLabelName}, + ).Return(fmt.Errorf("some error")), + ) + _, _, err := wh.UpdateNodeLabels(ctx, &nmc, &node) + Expect(err).To(HaveOccurred()) + }) + It("Should work as expected", func() { + node := v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + firstLabelName: "", + }, + Name: nodeName, + }, + } + emptySet := sets.Set[types.NamespacedName]{} + emptyMap := map[types.NamespacedName]kmmv1beta1.ModuleConfig{} + + gomock.InOrder( + mlph.EXPECT().getNodeKernelModuleReadyLabels(node).Return(emptySet), + mlph.EXPECT().getDeprecatedKernelModuleReadyLabels(node).Return(sets.Set[string]{}), + mlph.EXPECT().getSpecLabelsAndTheirConfigs(&nmc).Return(emptyMap), + mlph.EXPECT().getStatusLabelsAndTheirConfigs(&nmc).Return(emptyMap), + mlph.EXPECT().removeOrphanedLabels(emptySet, emptyMap, emptyMap).Return([]types.NamespacedName{{Name: nameSecond, Namespace: nsSecond}}), + mlph.EXPECT().addEqualLabels(emptySet, emptyMap, emptyMap).Return([]types.NamespacedName{{Name: nameFirst, Namespace: nsFirst}}), + n.EXPECT(). + UpdateLabels( + ctx, + &node, + []string{firstLabelName}, + []string{secondLabelName}, + ).Return(nil), + ) + _, _, err := wh.UpdateNodeLabels(ctx, &nmc, &node) + Expect(err).ToNot(HaveOccurred()) + Expect(node.Labels).To(HaveKey(firstLabelName)) + + }) +}) + +var _ = Describe("nmcReconcilerHelperImpl_RecordEvents", func() { + var ( + client *testclient.MockClient + fakeRecorder *record.FakeRecorder + n node.Node + wh nmcReconcilerHelper + ) + + BeforeEach(func() { + ctrl := gomock.NewController(GinkgoT()) + client = testclient.NewMockClient(ctrl) + n = node.NewNode(client) + fakeRecorder = record.NewFakeRecorder(10) + wh = newNMCReconcilerHelper(client, nil, fakeRecorder, n) + }) closeAndGetAllEvents := func(events chan string) []string { elems := make([]string, 0) @@ -1206,13 +1338,6 @@ var _ = Describe("nmcReconcilerHelperImpl_UpdateNodeLabelsAndRecordEvents", func _ = closeAndGetAllEvents(make(chan string)) - It("failed to get node", func() { - client.EXPECT().Get(ctx, gomock.Any(), gomock.Any()).Return(fmt.Errorf("some error")) - - err := wh.UpdateNodeLabelsAndRecordEvents(ctx, &nmc) - Expect(err).To(HaveOccurred()) - }) - type testCase struct { nodeLabelPresent bool specPresent bool @@ -1225,61 +1350,10 @@ var _ = Describe("nmcReconcilerHelperImpl_UpdateNodeLabelsAndRecordEvents", func } DescribeTable( - "nodes labels scenarios", - func(tc testCase) { - nodeLabels := map[string]string{"kmm.node.kubernetes.io/deprecated-label.ready": ""} - - if tc.nodeLabelPresent { - nodeLabels[utils.GetKernelModuleReadyNodeLabel(moduleNamespace, moduleName)] = "" - } - if tc.specPresent { - nmc.Spec.Modules = []kmmv1beta1.NodeModuleSpec{ - { - ModuleItem: kmmv1beta1.ModuleItem{ - Name: moduleName, - Namespace: moduleNamespace, - }, - Config: moduleConfig, - }, - } - } - if tc.statusPresent { - nmc.Status.Modules = []kmmv1beta1.NodeModuleStatus{ - { - ModuleItem: kmmv1beta1.ModuleItem{ - Name: moduleName, - Namespace: moduleNamespace, - }, - }, - } - if tc.statusConfigPresent { - statusConfig := moduleConfig - if !tc.configsEqual { - statusConfig.ContainerImage = "some other container image" - } - nmc.Status.Modules[0].Config = statusConfig - } - } - - if tc.resultLabelPresent { - resultLabels := map[string]string{utils.GetKernelModuleReadyNodeLabel(moduleNamespace, moduleName): ""} - expectedNode.SetLabels(resultLabels) - } - - gomock.InOrder( - client.EXPECT().Get(ctx, gomock.Any(), gomock.Any()).DoAndReturn( - func(_ interface{}, _ interface{}, node *v1.Node, _ ...ctrlclient.GetOption) error { - node.SetName("node name") - node.SetLabels(nodeLabels) - return nil - }, - ), - client.EXPECT().Patch(ctx, &expectedNode, gomock.Any()).Return(nil), - ) - - err := wh.UpdateNodeLabelsAndRecordEvents(ctx, &nmc) - Expect(err).NotTo(HaveOccurred()) + "RecordEvents different scenarios", + func(tc testCase, loaded []types.NamespacedName, unloaded []types.NamespacedName, node v1.Node) { + wh.RecordEvents(&node, loaded, unloaded) events := closeAndGetAllEvents(fakeRecorder.Events) if !tc.addsReadyLabel && !tc.removesReadyLabel { @@ -1303,6 +1377,14 @@ var _ = Describe("nmcReconcilerHelperImpl_UpdateNodeLabelsAndRecordEvents", func nodeLabelPresent: true, removesReadyLabel: true, }, + []types.NamespacedName{}, + []types.NamespacedName{{Namespace: moduleNamespace, Name: moduleName}}, + v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{}, + Name: nodeName, + }, + }, ), Entry( "node label present, spec present, status missing", @@ -1311,6 +1393,16 @@ var _ = Describe("nmcReconcilerHelperImpl_UpdateNodeLabelsAndRecordEvents", func specPresent: true, resultLabelPresent: true, }, + []types.NamespacedName{}, + []types.NamespacedName{}, + v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + kernelModuleLabelName: "", + }, + Name: nodeName, + }, + }, ), Entry( "node label present, spec present, status present, status config missing", @@ -1320,6 +1412,16 @@ var _ = Describe("nmcReconcilerHelperImpl_UpdateNodeLabelsAndRecordEvents", func statusPresent: true, resultLabelPresent: true, }, + []types.NamespacedName{}, + []types.NamespacedName{}, + v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + kernelModuleLabelName: "", + }, + Name: nodeName, + }, + }, ), Entry( "node label present, spec present, status present, status config present, configs not equal", @@ -1330,6 +1432,16 @@ var _ = Describe("nmcReconcilerHelperImpl_UpdateNodeLabelsAndRecordEvents", func statusConfigPresent: true, resultLabelPresent: true, }, + []types.NamespacedName{}, + []types.NamespacedName{}, + v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + kernelModuleLabelName: "", + }, + Name: nodeName, + }, + }, ), Entry( "node label present, spec present, status present, status config present, configs equal", @@ -1341,14 +1453,40 @@ var _ = Describe("nmcReconcilerHelperImpl_UpdateNodeLabelsAndRecordEvents", func configsEqual: true, resultLabelPresent: true, }, + []types.NamespacedName{}, + []types.NamespacedName{}, + v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + kernelModuleLabelName: "", + }, + Name: nodeName, + }, + }, ), Entry( "node label missing, spec missing, status missing", testCase{}, + []types.NamespacedName{}, + []types.NamespacedName{}, + v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{}, + Name: nodeName, + }, + }, ), Entry( "node label missing, spec present, status missing", testCase{specPresent: true}, + []types.NamespacedName{}, + []types.NamespacedName{}, + v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{}, + Name: nodeName, + }, + }, ), Entry( "node label missing, spec present, status present, status config missing", @@ -1356,6 +1494,14 @@ var _ = Describe("nmcReconcilerHelperImpl_UpdateNodeLabelsAndRecordEvents", func specPresent: true, statusPresent: true, }, + []types.NamespacedName{}, + []types.NamespacedName{}, + v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{}, + Name: nodeName, + }, + }, ), Entry( "node label missing, spec present, status present, status config present, configs not equal", @@ -1364,6 +1510,14 @@ var _ = Describe("nmcReconcilerHelperImpl_UpdateNodeLabelsAndRecordEvents", func statusPresent: true, statusConfigPresent: true, }, + []types.NamespacedName{}, + []types.NamespacedName{}, + v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{}, + Name: nodeName, + }, + }, ), Entry( "node label missing, spec present, status present, status config present, configs equal", @@ -1375,6 +1529,16 @@ var _ = Describe("nmcReconcilerHelperImpl_UpdateNodeLabelsAndRecordEvents", func resultLabelPresent: true, addsReadyLabel: true, }, + []types.NamespacedName{{Namespace: moduleNamespace, Name: moduleName}}, + []types.NamespacedName{}, + v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + kernelModuleLabelName: "", + }, + Name: nodeName, + }, + }, ), ) }) @@ -1905,3 +2069,229 @@ var _ = Describe("pullSecretHelperImpl_VolumesAndVolumeMounts", func() { Expect(resVolMounts).To(BeComparableTo(volMounts)) }) }) + +var _ = Describe("getKernelModuleReadyLabels", func() { + lph := newLabelPreparationHelper() + + DescribeTable("getKernelModuleReadyLabels different scenarios", func(labels map[string]string, + nodeModuleReadyLabelsEqual sets.Set[types.NamespacedName]) { + node := v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Labels: labels, + }, + } + + nodeModuleReadyLabels := lph.getNodeKernelModuleReadyLabels(node) + + Expect(nodeModuleReadyLabels).To(Equal(nodeModuleReadyLabelsEqual)) + }, + Entry("Should be empty", map[string]string{}, + sets.Set[types.NamespacedName]{}, + ), + + Entry("nodeModuleReadyLabels found", map[string]string{"invalid": ""}, + sets.Set[types.NamespacedName]{}, + ), + + Entry("nodeModuleReadyLabels found", map[string]string{fmt.Sprintf("kmm.node.kubernetes.io/%s.%s.ready", nsFirst, nameFirst): ""}, + sets.Set[types.NamespacedName]{ + {Namespace: nsFirst, Name: nameFirst}: {}, + }, + ), + ) +}) + +var _ = Describe("getDeprecatedKernelModuleReadyLabels", func() { + + lph := newLabelPreparationHelper() + DescribeTable("getDeprecatedKernelModuleReadyLabels different scenarios", func(labels map[string]string, + deprecatedNodeModuleReadyLabelsEqual sets.Set[string]) { + node := v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Labels: labels, + }, + } + + deprecatedNodeModuleReadyLabels := lph.getDeprecatedKernelModuleReadyLabels(node) + + Expect(deprecatedNodeModuleReadyLabels).To(Equal(deprecatedNodeModuleReadyLabelsEqual)) + }, + Entry("Should be empty", map[string]string{}, + sets.Set[string]{}, + ), + + Entry("deprecated node module ready labels not found", map[string]string{"invalid": ""}, + sets.Set[string]{}, + ), + + Entry("deprecated node module ready labels found", map[string]string{fmt.Sprintf("kmm.node.kubernetes.io/%s.ready", nameFirst): ""}, + sets.Set[string]{ + fmt.Sprintf("kmm.node.kubernetes.io/%s.ready", nameFirst): {}, + }, + ), + ) +}) + +var _ = Describe("getSpecLabelsAndTheirConfigs", func() { + + var ( + nmc kmmv1beta1.NodeModulesConfig + lph labelPreparationHelper + ) + + BeforeEach(func() { + nmc = kmmv1beta1.NodeModulesConfig{ + ObjectMeta: metav1.ObjectMeta{Name: "nmcName"}, + Spec: kmmv1beta1.NodeModulesConfigSpec{ + Modules: []kmmv1beta1.NodeModuleSpec{ + { + ModuleItem: kmmv1beta1.ModuleItem{ + Namespace: nsFirst, + Name: nameFirst, + }, + Config: kmmv1beta1.ModuleConfig{ContainerImage: imageFirst}, + }, + }, + }, + } + lph = newLabelPreparationHelper() + }) + It("Should not have module not from nmc", func() { + specLabels := lph.getSpecLabelsAndTheirConfigs(&nmc) + Expect(specLabels).ToNot(HaveKey(types.NamespacedName{Namespace: nsSecond, Name: nameSecond})) + }) + + It("Should have module from nmc", func() { + specLabels := lph.getSpecLabelsAndTheirConfigs(&nmc) + Expect(specLabels).To(HaveKeyWithValue(types.NamespacedName{Namespace: nsFirst, Name: nameFirst}, kmmv1beta1.ModuleConfig{ContainerImage: imageFirst})) + }) +}) + +var _ = Describe("getStatusLabelsAndTheirConfigs", func() { + + var ( + nmc kmmv1beta1.NodeModulesConfig + lph labelPreparationHelper + ) + + BeforeEach(func() { + nmc = kmmv1beta1.NodeModulesConfig{ + ObjectMeta: metav1.ObjectMeta{Name: "nmcName"}, + Status: kmmv1beta1.NodeModulesConfigStatus{ + Modules: []kmmv1beta1.NodeModuleStatus{ + { + ModuleItem: kmmv1beta1.ModuleItem{ + Namespace: nsFirst, + Name: nameFirst, + }, + Config: kmmv1beta1.ModuleConfig{ContainerImage: imageFirst}, + }, + }, + }, + } + lph = newLabelPreparationHelper() + }) + It("Should not have module not from nmc", func() { + statusLabels := lph.getStatusLabelsAndTheirConfigs(&nmc) + Expect(statusLabels).To(HaveKeyWithValue(types.NamespacedName{Namespace: nsFirst, Name: nameFirst}, kmmv1beta1.ModuleConfig{ContainerImage: imageFirst})) + }) + + It("Should have module from nmc", func() { + statusLabels := lph.getStatusLabelsAndTheirConfigs(&nmc) + Expect(statusLabels).ToNot(HaveKey(types.NamespacedName{Namespace: nsSecond, Name: nameSecond})) + }) + +}) + +var _ = Describe("removeOrphanedLabels", func() { + + var lph = labelPreparationHelperImpl{} + + DescribeTable("removeOrphanedLabels different scenarios", func(nodeModuleReadyLabels sets.Set[types.NamespacedName], + specLabels map[types.NamespacedName]kmmv1beta1.ModuleConfig, + statusLabels map[types.NamespacedName]kmmv1beta1.ModuleConfig, + node v1.Node, + expectedUnloaded []types.NamespacedName, + ) { + unloaded := lph.removeOrphanedLabels(nodeModuleReadyLabels, specLabels, statusLabels) + Expect(unloaded).To(Equal(expectedUnloaded)) + }, + Entry("Empty spec and status labels, should result of empty unloaded variable", + sets.Set[types.NamespacedName]{}, + map[types.NamespacedName]kmmv1beta1.ModuleConfig{}, + map[types.NamespacedName]kmmv1beta1.ModuleConfig{}, + v1.Node{}, + []types.NamespacedName{}), + Entry("ModuleConfig obj exists in specLabels so it should not be in unloaded variable", + sets.Set[types.NamespacedName]{ + {Namespace: nsFirst, Name: nameFirst}: {}, + {Namespace: nsSecond, Name: nameSecond}: {}, + }, + map[types.NamespacedName]kmmv1beta1.ModuleConfig{{Namespace: nsFirst, Name: nameFirst}: {}}, + map[types.NamespacedName]kmmv1beta1.ModuleConfig{}, + v1.Node{}, + []types.NamespacedName{{Namespace: nsSecond, Name: nameSecond}}), + + Entry("ModuleConfig obj exists in statusLabels so it should not be in unloaded variable", + sets.Set[types.NamespacedName]{ + {Namespace: nsFirst, Name: nameFirst}: {}, + {Namespace: nsSecond, Name: nameSecond}: {}, + }, + map[types.NamespacedName]kmmv1beta1.ModuleConfig{}, + map[types.NamespacedName]kmmv1beta1.ModuleConfig{{Namespace: nsFirst, Name: nameFirst}: {}}, + v1.Node{}, + []types.NamespacedName{{Namespace: nsSecond, Name: nameSecond}}), + + Entry("Both ModuleConfig obj exist in specLabels or statusLabels so they should not be in unloaded variable", + sets.Set[types.NamespacedName]{ + {Namespace: nsFirst, Name: nameFirst}: {}, + {Namespace: nsSecond, Name: nameSecond}: {}, + }, + map[types.NamespacedName]kmmv1beta1.ModuleConfig{{Namespace: nsFirst, Name: nameFirst}: {}}, + map[types.NamespacedName]kmmv1beta1.ModuleConfig{{Namespace: nsSecond, Name: nameSecond}: {}}, + v1.Node{}, + []types.NamespacedName{})) +}) + +var _ = Describe("addEqualLabels", func() { + var lph = labelPreparationHelperImpl{} + + DescribeTable("addEqualLabels different scenarios", func(nodeModuleReadyLabels sets.Set[types.NamespacedName], specLabels map[types.NamespacedName]kmmv1beta1.ModuleConfig, + statusLabels map[types.NamespacedName]kmmv1beta1.ModuleConfig, + node v1.Node, + expectedUnloaded []types.NamespacedName, + ) { + loaded := lph.addEqualLabels(nodeModuleReadyLabels, specLabels, statusLabels) + Expect(loaded).To(Equal(expectedUnloaded)) + }, + Entry("Empty spec and status labels, should result of empty loaded variable", + sets.Set[types.NamespacedName]{}, + map[types.NamespacedName]kmmv1beta1.ModuleConfig{}, + map[types.NamespacedName]kmmv1beta1.ModuleConfig{}, + v1.Node{}, + []types.NamespacedName{}), + Entry("specConfig and statusConfig are equal and nsn is not in nodeModuleReadyLabels, so nsn should be returned", + sets.Set[types.NamespacedName]{ + {Namespace: nsSecond, Name: nameSecond}: {}, + }, + map[types.NamespacedName]kmmv1beta1.ModuleConfig{{Namespace: nsFirst, Name: nameFirst}: {ContainerImage: imageFirst}}, + map[types.NamespacedName]kmmv1beta1.ModuleConfig{{Namespace: nsFirst, Name: nameFirst}: {ContainerImage: imageFirst}}, + v1.Node{}, + []types.NamespacedName{{Namespace: nsFirst, Name: nameFirst}}), + Entry("specConfig and statusConfig aren't equal so nsn shouldn't not be returned", + sets.Set[types.NamespacedName]{ + {Namespace: nsSecond, Name: nameSecond}: {}, + }, + map[types.NamespacedName]kmmv1beta1.ModuleConfig{{Namespace: nsFirst, Name: nameFirst}: {ContainerImage: imageFirst}}, + map[types.NamespacedName]kmmv1beta1.ModuleConfig{{Namespace: nsFirst, Name: nameFirst}: {ContainerImage: imageSecond}}, + v1.Node{}, + []types.NamespacedName{}), + Entry("nsn is in nodeModuleReadyLabels so nsn should not be returned", + sets.Set[types.NamespacedName]{ + {Namespace: nsFirst, Name: nameFirst}: {}, + }, + map[types.NamespacedName]kmmv1beta1.ModuleConfig{{Namespace: nsFirst, Name: nameFirst}: {ContainerImage: imageFirst}}, + map[types.NamespacedName]kmmv1beta1.ModuleConfig{{Namespace: nsFirst, Name: nameFirst}: {ContainerImage: imageFirst}}, + v1.Node{}, + []types.NamespacedName{})) +}) diff --git a/internal/node/mock_node.go b/internal/node/mock_node.go index 0db5f207e..74308443e 100644 --- a/internal/node/mock_node.go +++ b/internal/node/mock_node.go @@ -96,3 +96,17 @@ func (mr *MockNodeMockRecorder) IsNodeSchedulable(node any) *gomock.Call { mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsNodeSchedulable", reflect.TypeOf((*MockNode)(nil).IsNodeSchedulable), node) } + +// UpdateLabels mocks base method. +func (m *MockNode) UpdateLabels(ctx context.Context, node *v1.Node, toBeAdded, toBeRemoved []string) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "UpdateLabels", ctx, node, toBeAdded, toBeRemoved) + ret0, _ := ret[0].(error) + return ret0 +} + +// UpdateLabels indicates an expected call of UpdateLabels. +func (mr *MockNodeMockRecorder) UpdateLabels(ctx, node, toBeAdded, toBeRemoved any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateLabels", reflect.TypeOf((*MockNode)(nil).UpdateLabels), ctx, node, toBeAdded, toBeRemoved) +} diff --git a/internal/node/node.go b/internal/node/node.go index 3b6bfdf4b..8e959631d 100644 --- a/internal/node/node.go +++ b/internal/node/node.go @@ -3,6 +3,7 @@ package node import ( "context" "fmt" + "github.com/kubernetes-sigs/kernel-module-management/internal/meta" v1 "k8s.io/api/core/v1" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/log" @@ -13,6 +14,7 @@ type Node interface { GetNodesListBySelector(ctx context.Context, selector map[string]string) ([]v1.Node, error) GetNumTargetedNodes(ctx context.Context, selector map[string]string) (int, error) FindNodeCondition(cond []v1.NodeCondition, conditionType v1.NodeConditionType) *v1.NodeCondition + UpdateLabels(ctx context.Context, node *v1.Node, toBeAdded, toBeRemoved []string) error } type node struct { @@ -72,3 +74,34 @@ func (n *node) FindNodeCondition(cond []v1.NodeCondition, conditionType v1.NodeC return nil } + +func (n *node) UpdateLabels(ctx context.Context, node *v1.Node, toBeAdded, toBeRemoved []string) error { + patchFrom := client.MergeFrom(node.DeepCopy()) + + addLabels(node, toBeAdded) + removeLabels(node, toBeRemoved) + + if err := n.client.Patch(ctx, node, patchFrom); err != nil { + return fmt.Errorf("could not patch node: %v", err) + } + return nil +} + +func addLabels(node *v1.Node, labels []string) { + for _, label := range labels { + meta.SetLabel( + node, + label, + "", + ) + } +} + +func removeLabels(node *v1.Node, labels []string) { + for _, label := range labels { + meta.RemoveLabel( + node, + label, + ) + } +} diff --git a/internal/node/node_test.go b/internal/node/node_test.go index a6e51f72c..ab4a060a3 100644 --- a/internal/node/node_test.go +++ b/internal/node/node_test.go @@ -4,10 +4,12 @@ import ( "context" "fmt" "github.com/kubernetes-sigs/kernel-module-management/internal/client" + "github.com/kubernetes-sigs/kernel-module-management/internal/utils" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" "go.uber.org/mock/gomock" v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) var _ = Describe("IsNodeSchedulable", func() { @@ -118,6 +120,59 @@ var _ = Describe("GetNodesListBySelector", func() { }) }) +var ( + loadedKernelModuleReadyNodeLabel = utils.GetKernelModuleReadyNodeLabel("loaded-ns", "loaded-n") + unloadedKernelModuleReadyNodeLabel = utils.GetKernelModuleReadyNodeLabel("unloaded-ns", "unloaded-n") +) + +var _ = Describe("UpdateLabels", func() { + var ( + ctrl *gomock.Controller + n Node + ctx context.Context + clnt *client.MockClient + ) + + BeforeEach(func() { + ctrl = gomock.NewController(GinkgoT()) + clnt = client.NewMockClient(ctrl) + ctx = context.TODO() + n = NewNode(clnt) + }) + + It("Should work as expected", func() { + node := v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{}, + }, + } + loaded := []string{loadedKernelModuleReadyNodeLabel} + unloaded := []string{unloadedKernelModuleReadyNodeLabel} + + clnt.EXPECT().Patch(ctx, gomock.Any(), gomock.Any(), gomock.Any()).Return(nil) + + err := n.UpdateLabels(ctx, &node, loaded, unloaded) + Expect(err).ToNot(HaveOccurred()) + Expect(node.Labels).To(HaveKey(loadedKernelModuleReadyNodeLabel)) + }) + + It("Should fail to patch node", func() { + node := v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{}, + }, + } + loaded := []string{loadedKernelModuleReadyNodeLabel} + unloaded := []string{unloadedKernelModuleReadyNodeLabel} + + clnt.EXPECT().Patch(ctx, gomock.Any(), gomock.Any(), gomock.Any()).Return(fmt.Errorf("some error")) + + err := n.UpdateLabels(ctx, &node, loaded, unloaded) + Expect(err).To(HaveOccurred()) + + }) +}) + var _ = Describe("GetNumTargetedNodes", func() { var ( ctrl *gomock.Controller @@ -131,7 +186,7 @@ var _ = Describe("GetNumTargetedNodes", func() { mn = NewNode(clnt) }) - It("There are not schedulable nodes", func() { + It("There are no schedulable nodes", func() { clnt.EXPECT().List(context.Background(), gomock.Any(), gomock.Any()).Return(fmt.Errorf("some error")) numOfNodes, err := mn.GetNumTargetedNodes(context.Background(), map[string]string{}) @@ -220,3 +275,37 @@ var _ = Describe("FindNodeCondition", func() { Expect(conditionFound).To(BeNil()) }) }) + +var _ = Describe("addLabels", func() { + var node v1.Node + + BeforeEach(func() { + node = v1.Node{} + }) + + It("Should add labels", func() { + labels := []string{loadedKernelModuleReadyNodeLabel} + addLabels(&node, labels) + Expect(node.Labels).To(HaveKey(loadedKernelModuleReadyNodeLabel)) + }) +}) + +var _ = Describe("removeLabels", func() { + var node v1.Node + + BeforeEach(func() { + node = v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + loadedKernelModuleReadyNodeLabel: "", + }, + }, + } + }) + + It("Should remove labels", func() { + labels := []string{loadedKernelModuleReadyNodeLabel} + removeLabels(&node, labels) + Expect(node.Labels).ToNot(HaveKey(loadedKernelModuleReadyNodeLabel)) + }) +})