diff --git a/api/v1beta1/nodemodulesconfig_types.go b/api/v1beta1/nodemodulesconfig_types.go index 0448775f9..4c681ee8e 100644 --- a/api/v1beta1/nodemodulesconfig_types.go +++ b/api/v1beta1/nodemodulesconfig_types.go @@ -44,6 +44,9 @@ type ModuleItem struct { //+optional // tolerations define which tolerations should be added for every load/unload pod running on the node Tolerations []v1.Toleration `json:"tolerations,omitempty"` + //+optional + // Version is the version of the kernel module that should be loaded + Version string `json:"version,omitempty"` } type NodeModuleSpec struct { diff --git a/config/crd-hub/bases/kmm.sigs.x-k8s.io_nodemodulesconfigs.yaml b/config/crd-hub/bases/kmm.sigs.x-k8s.io_nodemodulesconfigs.yaml index b9ea7e274..c92ab7cdd 100644 --- a/config/crd-hub/bases/kmm.sigs.x-k8s.io_nodemodulesconfigs.yaml +++ b/config/crd-hub/bases/kmm.sigs.x-k8s.io_nodemodulesconfigs.yaml @@ -222,6 +222,10 @@ spec: type: string type: object type: array + version: + description: Version is the version of the kernel module that + should be loaded + type: string required: - config - name @@ -415,6 +419,10 @@ spec: type: string type: object type: array + version: + description: Version is the version of the kernel module that + should be loaded + type: string required: - name - namespace diff --git a/config/crd/bases/kmm.sigs.x-k8s.io_nodemodulesconfigs.yaml b/config/crd/bases/kmm.sigs.x-k8s.io_nodemodulesconfigs.yaml index b9ea7e274..c92ab7cdd 100644 --- a/config/crd/bases/kmm.sigs.x-k8s.io_nodemodulesconfigs.yaml +++ b/config/crd/bases/kmm.sigs.x-k8s.io_nodemodulesconfigs.yaml @@ -222,6 +222,10 @@ spec: type: string type: object type: array + version: + description: Version is the version of the kernel module that + should be loaded + type: string required: - config - name @@ -415,6 +419,10 @@ spec: type: string type: object type: array + version: + description: Version is the version of the kernel module that + should be loaded + type: string required: - name - namespace diff --git a/config/samples/kmm.sigs.x-k8s.io_modulebuildsignconfigs.yaml b/config/samples/kmm.sigs.x-k8s.io_modulebuildsignconfigs.yaml index 94e84a3e0..f1744ce12 100644 --- a/config/samples/kmm.sigs.x-k8s.io_modulebuildsignconfigs.yaml +++ b/config/samples/kmm.sigs.x-k8s.io_modulebuildsignconfigs.yaml @@ -4,7 +4,7 @@ metadata: name: modulebuildsignconfig-sample spec: images: - - image: quay.io/myorg/my-kernel-module:latest + - image: kernelVersion: 4.18.0-372.32.1.el8_6.x86_64 action: BuildImage build: diff --git a/config/samples/kmm.sigs.x-k8s.io_moduleimagesconfigs.yaml b/config/samples/kmm.sigs.x-k8s.io_moduleimagesconfigs.yaml index 90df4efbe..70f6b0dc8 100644 --- a/config/samples/kmm.sigs.x-k8s.io_moduleimagesconfigs.yaml +++ b/config/samples/kmm.sigs.x-k8s.io_moduleimagesconfigs.yaml @@ -4,7 +4,7 @@ metadata: name: moduleimagesconfig-sample spec: images: - - image: quay.io/myorg/my-kernel-module:v1.0.0 + - image: kernelVersion: 4.18.0-372.32.1.el8_6.x86_64 imagePullPolicy: IfNotPresent pushBuiltImage: false diff --git a/config/samples/kmm.sigs.x-k8s.io_nodemodulesconfigs.yaml b/config/samples/kmm.sigs.x-k8s.io_nodemodulesconfigs.yaml index 26418e88d..175c0a85a 100644 --- a/config/samples/kmm.sigs.x-k8s.io_nodemodulesconfigs.yaml +++ b/config/samples/kmm.sigs.x-k8s.io_nodemodulesconfigs.yaml @@ -9,7 +9,7 @@ spec: serviceAccountName: kmm-operator-controller config: kernelVersion: 4.18.0-372.32.1.el8_6.x86_64 - containerImage: quay.io/myorg/my-kernel-module:latest + containerImage: imagePullPolicy: IfNotPresent insecurePull: false modprobe: diff --git a/docs/mkdocs/documentation/deploy_kmod.md b/docs/mkdocs/documentation/deploy_kmod.md index 1152a8a29..3c8d52be7 100644 --- a/docs/mkdocs/documentation/deploy_kmod.md +++ b/docs/mkdocs/documentation/deploy_kmod.md @@ -14,6 +14,11 @@ on the target node(s) to run the necessary action. The operator monitors the outcome of those Pods and records that information. It uses it to label `Node` objects when the module was successfully loaded, and to run the device plugin (if configured). +The label can be found in the node's label with the following format: +``` +kmm.node.kubernetes.io/..ready +``` +but it is strongly recommended to use `GetKernelModuleReadyNodeLabel` function from the `labels` package in order to construct the correct label ### Worker Pods diff --git a/docs/mkdocs/documentation/ordered_upgrade.md b/docs/mkdocs/documentation/ordered_upgrade.md index 6b8087f9a..8ec8e2b69 100644 --- a/docs/mkdocs/documentation/ordered_upgrade.md +++ b/docs/mkdocs/documentation/ordered_upgrade.md @@ -50,6 +50,16 @@ Steps 3, 4 and 5 are then unified into one step: update the `kmm.node.kubernetes.io/version-module..` label value to new `$moduleVersion` as set in the `Module`. +It is strongly recommended to use `GetModuleVersionLabelName` function from the `labels` package in order to construct the correct label used in the step 1 +of the upgrade flow + +### Indicator that the new version is ready to be used + +The operator will label the node with a "version.ready" label to indicate that the new version of the kernel module is loaded +and ready to be used: +`kmm.node.kubernetes.io/..version.ready=` +It is strongly recommended to use `GetKernelModuleVersionReadyNodeLabel` function from the `labels` package in order to construct the correct label + ## Implementation details ### Components diff --git a/internal/controllers/mock_nmc_reconciler.go b/internal/controllers/mock_nmc_reconciler.go index 3c6071334..022d3a1bf 100644 --- a/internal/controllers/mock_nmc_reconciler.go +++ b/internal/controllers/mock_nmc_reconciler.go @@ -192,10 +192,10 @@ func (mr *MocklabelPreparationHelperMockRecorder) addEqualLabels(nodeModuleReady } // getDeprecatedKernelModuleReadyLabels mocks base method. -func (m *MocklabelPreparationHelper) getDeprecatedKernelModuleReadyLabels(node v1.Node) sets.Set[string] { +func (m *MocklabelPreparationHelper) getDeprecatedKernelModuleReadyLabels(node v1.Node) map[string]string { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "getDeprecatedKernelModuleReadyLabels", node) - ret0, _ := ret[0].(sets.Set[string]) + ret0, _ := ret[0].(map[string]string) return ret0 } @@ -247,6 +247,20 @@ func (mr *MocklabelPreparationHelperMockRecorder) getStatusLabelsAndTheirConfigs return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "getStatusLabelsAndTheirConfigs", reflect.TypeOf((*MocklabelPreparationHelper)(nil).getStatusLabelsAndTheirConfigs), nmc) } +// getStatusVersions mocks base method. +func (m *MocklabelPreparationHelper) getStatusVersions(nmc *v1beta1.NodeModulesConfig) map[types.NamespacedName]string { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "getStatusVersions", nmc) + ret0, _ := ret[0].(map[types.NamespacedName]string) + return ret0 +} + +// getStatusVersions indicates an expected call of getStatusVersions. +func (mr *MocklabelPreparationHelperMockRecorder) getStatusVersions(nmc any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "getStatusVersions", reflect.TypeOf((*MocklabelPreparationHelper)(nil).getStatusVersions), 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() diff --git a/internal/controllers/nmc_reconciler.go b/internal/controllers/nmc_reconciler.go index a58fa8a75..1f1fc1fe8 100644 --- a/internal/controllers/nmc_reconciler.go +++ b/internal/controllers/nmc_reconciler.go @@ -102,7 +102,7 @@ func (r *NMCReconciler) Reconcile(ctx context.Context, req reconcile.Request) (r } errs := make([]error, 0, len(nmcObj.Spec.Modules)+len(nmcObj.Status.Modules)) - var readyLabelsToRemove []string + readyLabelsToRemove := make(map[string]string) for _, mod := range nmcObj.Spec.Modules { moduleNameKey := mod.Namespace + "/" + mod.Name @@ -110,7 +110,8 @@ func (r *NMCReconciler) Reconcile(ctx context.Context, req reconcile.Request) (r // skipping handling NMC spec module until node is ready if !r.nodeAPI.IsNodeSchedulable(&node, mod.Tolerations) { - readyLabelsToRemove = append(readyLabelsToRemove, utils.GetKernelModuleReadyNodeLabel(mod.Namespace, mod.Name)) + readyLabelsToRemove[utils.GetKernelModuleReadyNodeLabel(mod.Namespace, mod.Name)] = "" + readyLabelsToRemove[utils.GetKernelModuleVersionReadyNodeLabel(mod.Namespace, mod.Name)] = "" delete(statusMap, moduleNameKey) continue } @@ -141,7 +142,7 @@ func (r *NMCReconciler) Reconcile(ctx context.Context, req reconcile.Request) (r } // removing label of loaded kmods - if readyLabelsToRemove != nil { + if len(readyLabelsToRemove) != 0 { if err := r.nodeAPI.UpdateLabels(ctx, &node, nil, readyLabelsToRemove); err != nil { return ctrl.Result{}, fmt.Errorf("could remove node %s labels: %v", node.Name, err) } @@ -574,6 +575,8 @@ func (h *nmcReconcilerHelperImpl) SyncStatus(ctx context.Context, nmcObj *kmmv1b status.BootId = node.Status.NodeInfo.BootID + status.Version = h.podManager.GetModuleVersionAnnotation(&p) + nmc.SetModuleStatus(&nmcObj.Status.Modules, *status) podsToDelete = append(podsToDelete, p) @@ -599,63 +602,6 @@ func (h *nmcReconcilerHelperImpl) SyncStatus(ctx context.Context, nmcObj *kmmv1b return errors.Join(errs...) } -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{} - -func newLabelPreparationHelper() labelPreparationHelper { - return &labelPreparationHelperImpl{} -} - -func (lph *labelPreparationHelperImpl) getNodeKernelModuleReadyLabels(node v1.Node) sets.Set[types.NamespacedName] { - nodeModuleReadyLabels := sets.New[types.NamespacedName]() - - 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 -} - -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 -} - -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 -} - func (h *nmcReconcilerHelperImpl) UpdateNodeLabels(ctx context.Context, nmc *kmmv1beta1.NodeModulesConfig, node *v1.Node) ([]types.NamespacedName, []types.NamespacedName, error) { // get all the kernel module ready labels of the node @@ -668,20 +614,30 @@ func (h *nmcReconcilerHelperImpl) UpdateNodeLabels(ctx context.Context, nmc *kmm // get status labels and their config statusLabels := h.lph.getStatusLabelsAndTheirConfigs(nmc) + // get the versions per the name/namespace of the module + statusVersions := h.lph.getStatusVersions(nmc) + // 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 nsnLabelsToBeLoaded := h.lph.addEqualLabels(nodeModuleReadyLabels, specLabels, statusLabels) - var loadedLabels []string - unloadedLabels := deprecatedNodeModuleReadyLabels.UnsortedList() + loadedLabels := make(map[string]string) + unloadedLabels := deprecatedNodeModuleReadyLabels for _, label := range nsnLabelsToBeRemoved { - unloadedLabels = append(unloadedLabels, utils.GetKernelModuleReadyNodeLabel(label.Namespace, label.Name)) + unloadedLabels[utils.GetKernelModuleReadyNodeLabel(label.Namespace, label.Name)] = "" + // unload the kernel ready version label also. if it does not exists, that's ok, the code won't fail.It also means + // that the version label will not be in labelsToAdd, since status and spec are missing + unloadedLabels[utils.GetKernelModuleVersionReadyNodeLabel(label.Namespace, label.Name)] = "" } + for _, label := range nsnLabelsToBeLoaded { - loadedLabels = append(loadedLabels, utils.GetKernelModuleReadyNodeLabel(label.Namespace, label.Name)) + loadedLabels[utils.GetKernelModuleReadyNodeLabel(label.Namespace, label.Name)] = "" + if version, ok := statusVersions[label]; ok { + loadedLabels[utils.GetKernelModuleVersionReadyNodeLabel(label.Namespace, label.Name)] = version + } } if err := h.nodeAPI.UpdateLabels(ctx, node, loadedLabels, unloadedLabels); err != nil { @@ -714,6 +670,75 @@ func (h *nmcReconcilerHelperImpl) RecordEvents(node *v1.Node, loadedModules, unl } } +type labelPreparationHelper interface { + getDeprecatedKernelModuleReadyLabels(node v1.Node) map[string]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 + getStatusVersions(nmc *kmmv1beta1.NodeModulesConfig) map[types.NamespacedName]string + 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{} + +func newLabelPreparationHelper() labelPreparationHelper { + return &labelPreparationHelperImpl{} +} + +func (lph *labelPreparationHelperImpl) getNodeKernelModuleReadyLabels(node v1.Node) sets.Set[types.NamespacedName] { + nodeModuleReadyLabels := sets.New[types.NamespacedName]() + + 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) map[string]string { + deprecatedLabels := make(map[string]string) + + for key, val := range node.GetLabels() { + if utils.IsDeprecatedKernelModuleReadyNodeLabel(key) { + deprecatedLabels[key] = val + } + } + return deprecatedLabels +} + +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 +} + +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 +} + +func (lph *labelPreparationHelperImpl) getStatusVersions(nmc *kmmv1beta1.NodeModulesConfig) map[types.NamespacedName]string { + versions := make(map[types.NamespacedName]string) + + for _, module := range nmc.Status.Modules { + if module.Version != "" { + versions[types.NamespacedName{Namespace: module.Namespace, Name: module.Name}] = module.Version + } + } + return versions +} + func (lph *labelPreparationHelperImpl) removeOrphanedLabels(nodeModuleReadyLabels sets.Set[types.NamespacedName], specLabels, statusLabels map[types.NamespacedName]kmmv1beta1.ModuleConfig) []types.NamespacedName { diff --git a/internal/controllers/nmc_reconciler_test.go b/internal/controllers/nmc_reconciler_test.go index 2109e2067..59f10aa99 100644 --- a/internal/controllers/nmc_reconciler_test.go +++ b/internal/controllers/nmc_reconciler_test.go @@ -31,15 +31,16 @@ import ( ) const ( - nmcName = "nmc" - nsFirst = "example-ns-1" - nsSecond = "example-ns-2" - nameFirst = "example-name-1" - nameSecond = "example-name-2" - imageFirst = "example-image-1" - imageSecond = "example-image-2" - kmodName = "kmm.node.kubernetes.io/test-ns.test-module.ready" - labelNotToRemove = "example.node.kubernetes.io/label-not-to-be-removed" + 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" + kmodReadyLabel = "kmm.node.kubernetes.io/test-ns.test-module.ready" + kmodVersionReadyLabel = "kmm.node.kubernetes.io/test-ns.test-module.version.ready" + labelNotToRemove = "example.node.kubernetes.io/label-not-to-be-removed" ) var _ = Describe("NodeModulesConfigReconciler_Reconcile", func() { @@ -138,7 +139,7 @@ var _ = Describe("NodeModulesConfigReconciler_Reconcile", func() { node := v1.Node{ ObjectMeta: metav1.ObjectMeta{ Labels: map[string]string{ - kmodName: "", + kmodReadyLabel: "", labelNotToRemove: "", }, }, @@ -165,9 +166,10 @@ var _ = Describe("NodeModulesConfigReconciler_Reconcile", func() { ), wh.EXPECT().SyncStatus(ctx, nmc, &node), nm.EXPECT().IsNodeSchedulable(&node, nil).Return(false), - nm.EXPECT().UpdateLabels(ctx, &node, nil, []string{kmodName}).DoAndReturn( - func(_ context.Context, obj ctrlclient.Object, _, _ []string) error { - delete(node.ObjectMeta.Labels, kmodName) + nm.EXPECT().UpdateLabels(ctx, &node, nil, map[string]string{kmodReadyLabel: "", kmodVersionReadyLabel: ""}).DoAndReturn( + func(_ context.Context, obj ctrlclient.Object, _, _ map[string]string) error { + delete(node.ObjectMeta.Labels, kmodReadyLabel) + delete(node.ObjectMeta.Labels, kmodVersionReadyLabel) return nil }, ), @@ -194,8 +196,9 @@ var _ = Describe("NodeModulesConfigReconciler_Reconcile", func() { node := v1.Node{ ObjectMeta: metav1.ObjectMeta{ Labels: map[string]string{ - kmodName: "", - labelNotToRemove: "", + kmodReadyLabel: "", + kmodVersionReadyLabel: "2", + labelNotToRemove: "", }, }, } @@ -221,8 +224,8 @@ var _ = Describe("NodeModulesConfigReconciler_Reconcile", func() { ), wh.EXPECT().SyncStatus(ctx, nmc, &node), nm.EXPECT().IsNodeSchedulable(&node, nil).Return(false), - nm.EXPECT().UpdateLabels(ctx, &node, nil, []string{kmodName}).DoAndReturn( - func(_ context.Context, obj ctrlclient.Object, _, _ []string) error { + nm.EXPECT().UpdateLabels(ctx, &node, nil, map[string]string{kmodReadyLabel: "", kmodVersionReadyLabel: ""}).DoAndReturn( + func(_ context.Context, obj ctrlclient.Object, _, _ map[string]string) error { return fmt.Errorf("some error") }, ), @@ -1237,6 +1240,7 @@ var _ = Describe("nmcReconcilerHelperImpl_SyncStatus", func() { mockWorkerPodManager.EXPECT().IsUnloaderPod(&p).Return(false), mockWorkerPodManager.EXPECT().GetConfigAnnotation(&p).Return(string(b)), mockWorkerPodManager.EXPECT().GetTolerationsAnnotation(&p).Return(string(tolerations)), + mockWorkerPodManager.EXPECT().GetModuleVersionAnnotation(&p).Return("some version"), kubeClient.EXPECT().Status().Return(sw), sw.EXPECT().Patch(ctx, nmc, gomock.Any()), mockWorkerPodManager.EXPECT().DeletePod(ctx, &p), @@ -1257,6 +1261,7 @@ var _ = Describe("nmcReconcilerHelperImpl_SyncStatus", func() { Namespace: modNamespace, ServiceAccountName: serviceAccountName, Tolerations: []v1.Toleration{testToleration}, + Version: "some version", }, Config: cfg, } @@ -1377,15 +1382,17 @@ var kernelModuleLabelName = utils.GetKernelModuleReadyNodeLabel(moduleNamespace, var _ = Describe("nmcReconcilerHelperImpl_UpdateNodeLabels", func() { var ( - ctx context.Context - client *testclient.MockClient - fakeRecorder *record.FakeRecorder - n *node.MockNode - nmc kmmv1beta1.NodeModulesConfig - wh nmcReconcilerHelper - mlph *MocklabelPreparationHelper - firstLabelName string - secondLabelName string + ctx context.Context + client *testclient.MockClient + fakeRecorder *record.FakeRecorder + n *node.MockNode + nmc kmmv1beta1.NodeModulesConfig + wh nmcReconcilerHelper + mlph *MocklabelPreparationHelper + firstLabelName string + secondLabelName string + firstVersionLabelName string + secondVersionLabelName string ) BeforeEach(func() { @@ -1426,6 +1433,8 @@ var _ = Describe("nmcReconcilerHelperImpl_UpdateNodeLabels", func() { } firstLabelName = fmt.Sprintf("kmm.node.kubernetes.io/%s.%s.ready", nsFirst, nameFirst) secondLabelName = fmt.Sprintf("kmm.node.kubernetes.io/%s.%s.ready", nsSecond, nameSecond) + firstVersionLabelName = fmt.Sprintf("kmm.node.kubernetes.io/%s.%s.version.ready", nsFirst, nameFirst) + secondVersionLabelName = fmt.Sprintf("kmm.node.kubernetes.io/%s.%s.version.ready", nsSecond, nameSecond) }) It("failed to get node", func() { @@ -1447,17 +1456,18 @@ var _ = Describe("nmcReconcilerHelperImpl_UpdateNodeLabels", func() { gomock.InOrder( mlph.EXPECT().getNodeKernelModuleReadyLabels(node).Return(emptySet), - mlph.EXPECT().getDeprecatedKernelModuleReadyLabels(node).Return(sets.Set[string]{}), + mlph.EXPECT().getDeprecatedKernelModuleReadyLabels(node).Return(map[string]string{}), mlph.EXPECT().getSpecLabelsAndTheirConfigs(&nmc).Return(emptyMap), mlph.EXPECT().getStatusLabelsAndTheirConfigs(&nmc).Return(emptyMap), + mlph.EXPECT().getStatusVersions(&nmc).Return(map[types.NamespacedName]string{}), 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}, + map[string]string{firstLabelName: ""}, + map[string]string{secondLabelName: "", secondVersionLabelName: ""}, ).Return(fmt.Errorf("some error")), ) _, _, err := wh.UpdateNodeLabels(ctx, &nmc, &node) @@ -1474,20 +1484,23 @@ var _ = Describe("nmcReconcilerHelperImpl_UpdateNodeLabels", func() { } emptySet := sets.Set[types.NamespacedName]{} emptyMap := map[types.NamespacedName]kmmv1beta1.ModuleConfig{} + firstNN := types.NamespacedName{Name: nameFirst, Namespace: nsFirst} + secondNN := types.NamespacedName{Name: nameSecond, Namespace: nsSecond} gomock.InOrder( mlph.EXPECT().getNodeKernelModuleReadyLabels(node).Return(emptySet), - mlph.EXPECT().getDeprecatedKernelModuleReadyLabels(node).Return(sets.Set[string]{}), + mlph.EXPECT().getDeprecatedKernelModuleReadyLabels(node).Return(map[string]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}}), + mlph.EXPECT().getStatusVersions(&nmc).Return(map[types.NamespacedName]string{firstNN: "some version"}), + mlph.EXPECT().removeOrphanedLabels(emptySet, emptyMap, emptyMap).Return([]types.NamespacedName{secondNN}), + mlph.EXPECT().addEqualLabels(emptySet, emptyMap, emptyMap).Return([]types.NamespacedName{firstNN}), n.EXPECT(). UpdateLabels( ctx, &node, - []string{firstLabelName}, - []string{secondLabelName}, + map[string]string{firstLabelName: "", firstVersionLabelName: "some version"}, + map[string]string{secondLabelName: "", secondVersionLabelName: ""}, ).Return(nil), ) _, _, err := wh.UpdateNodeLabels(ctx, &nmc, &node) @@ -1767,7 +1780,7 @@ var _ = Describe("getDeprecatedKernelModuleReadyLabels", func() { lph := newLabelPreparationHelper() DescribeTable("getDeprecatedKernelModuleReadyLabels different scenarios", func(labels map[string]string, - deprecatedNodeModuleReadyLabelsEqual sets.Set[string]) { + deprecatedNodeModuleReadyLabelsEqual map[string]string) { node := v1.Node{ ObjectMeta: metav1.ObjectMeta{ Labels: labels, @@ -1779,16 +1792,16 @@ var _ = Describe("getDeprecatedKernelModuleReadyLabels", func() { Expect(deprecatedNodeModuleReadyLabels).To(Equal(deprecatedNodeModuleReadyLabelsEqual)) }, Entry("Should be empty", map[string]string{}, - sets.Set[string]{}, + map[string]string{}, ), Entry("deprecated node module ready labels not found", map[string]string{"invalid": ""}, - sets.Set[string]{}, + map[string]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): {}, + map[string]string{ + fmt.Sprintf("kmm.node.kubernetes.io/%s.ready", nameFirst): "", }, ), ) diff --git a/internal/nmc/helper.go b/internal/nmc/helper.go index 31bb7e002..cd25082ae 100644 --- a/internal/nmc/helper.go +++ b/internal/nmc/helper.go @@ -70,6 +70,7 @@ func (h *helper) SetModuleConfig( foundEntry.ImageRepoSecret = mld.ImageRepoSecret foundEntry.ServiceAccountName = saName foundEntry.Tolerations = mld.Tolerations + foundEntry.Version = mld.ModuleVersion return nil } diff --git a/internal/node/mock_node.go b/internal/node/mock_node.go index 4fde9dc8a..b3ddeb253 100644 --- a/internal/node/mock_node.go +++ b/internal/node/mock_node.go @@ -98,7 +98,7 @@ func (mr *MockNodeMockRecorder) IsNodeSchedulable(node, tolerations any) *gomock } // UpdateLabels mocks base method. -func (m *MockNode) UpdateLabels(ctx context.Context, node *v1.Node, toBeAdded, toBeRemoved []string) error { +func (m *MockNode) UpdateLabels(ctx context.Context, node *v1.Node, toBeAdded, toBeRemoved map[string]string) error { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "UpdateLabels", ctx, node, toBeAdded, toBeRemoved) ret0, _ := ret[0].(error) diff --git a/internal/node/node.go b/internal/node/node.go index 7093ec625..2735c894a 100644 --- a/internal/node/node.go +++ b/internal/node/node.go @@ -15,7 +15,7 @@ type Node interface { IsNodeSchedulable(node *v1.Node, tolerations []v1.Toleration) bool GetNodesListBySelector(ctx context.Context, selector map[string]string, tolerations []v1.Toleration) ([]v1.Node, error) GetNumTargetedNodes(ctx context.Context, selector map[string]string, tolerations []v1.Toleration) (int, error) - UpdateLabels(ctx context.Context, node *v1.Node, toBeAdded, toBeRemoved []string) error + UpdateLabels(ctx context.Context, node *v1.Node, toBeAdded, toBeRemoved map[string]string) error IsNodeRebooted(node *v1.Node, statusBootId string) bool } @@ -72,7 +72,7 @@ func (n *node) GetNumTargetedNodes(ctx context.Context, selector map[string]stri return len(targetedNode), nil } -func (n *node) UpdateLabels(ctx context.Context, node *v1.Node, toBeAdded, toBeRemoved []string) error { +func (n *node) UpdateLabels(ctx context.Context, node *v1.Node, toBeAdded, toBeRemoved map[string]string) error { patchFrom := client.MergeFrom(node.DeepCopy()) addLabels(node, toBeAdded) @@ -95,18 +95,18 @@ func (n *node) IsNodeRebooted(node *v1.Node, statusBootId string) bool { return false } -func addLabels(node *v1.Node, labels []string) { - for _, label := range labels { +func addLabels(node *v1.Node, labels map[string]string) { + for label, value := range labels { meta.SetLabel( node, label, - "", + value, ) } } -func removeLabels(node *v1.Node, labels []string) { - for _, label := range labels { +func removeLabels(node *v1.Node, labels map[string]string) { + for label := range labels { meta.RemoveLabel( node, label, diff --git a/internal/node/node_test.go b/internal/node/node_test.go index 59e7ab977..6f465d686 100644 --- a/internal/node/node_test.go +++ b/internal/node/node_test.go @@ -186,8 +186,8 @@ var _ = Describe("UpdateLabels", func() { Labels: map[string]string{}, }, } - loaded := []string{firstloadedKernelModuleReadyNodeLabel} - unloaded := []string{unloadedKernelModuleReadyNodeLabel} + loaded := map[string]string{firstloadedKernelModuleReadyNodeLabel: ""} + unloaded := map[string]string{unloadedKernelModuleReadyNodeLabel: ""} clnt.EXPECT().Patch(ctx, gomock.Any(), gomock.Any(), gomock.Any()).Return(nil) @@ -202,8 +202,8 @@ var _ = Describe("UpdateLabels", func() { Labels: map[string]string{}, }, } - loaded := []string{firstloadedKernelModuleReadyNodeLabel} - unloaded := []string{unloadedKernelModuleReadyNodeLabel} + loaded := map[string]string{firstloadedKernelModuleReadyNodeLabel: ""} + unloaded := map[string]string{unloadedKernelModuleReadyNodeLabel: ""} clnt.EXPECT().Patch(ctx, gomock.Any(), gomock.Any(), gomock.Any()).Return(fmt.Errorf("some error")) @@ -329,7 +329,7 @@ var _ = Describe("removeLabels", func() { }) It("Should remove labels", func() { - labels := []string{firstloadedKernelModuleReadyNodeLabel} + labels := map[string]string{firstloadedKernelModuleReadyNodeLabel: ""} removeLabels(&node, labels) Expect(node.Labels).ToNot(HaveKey(firstloadedKernelModuleReadyNodeLabel)) }) diff --git a/internal/pod/mock_workerpodmanager.go b/internal/pod/mock_workerpodmanager.go index 2d73ca448..6c7291b3e 100644 --- a/internal/pod/mock_workerpodmanager.go +++ b/internal/pod/mock_workerpodmanager.go @@ -97,6 +97,20 @@ func (mr *MockWorkerPodManagerMockRecorder) GetConfigAnnotation(p any) *gomock.C return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetConfigAnnotation", reflect.TypeOf((*MockWorkerPodManager)(nil).GetConfigAnnotation), p) } +// GetModuleVersionAnnotation mocks base method. +func (m *MockWorkerPodManager) GetModuleVersionAnnotation(p *v1.Pod) string { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetModuleVersionAnnotation", p) + ret0, _ := ret[0].(string) + return ret0 +} + +// GetModuleVersionAnnotation indicates an expected call of GetModuleVersionAnnotation. +func (mr *MockWorkerPodManagerMockRecorder) GetModuleVersionAnnotation(p any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetModuleVersionAnnotation", reflect.TypeOf((*MockWorkerPodManager)(nil).GetModuleVersionAnnotation), p) +} + // GetTolerationsAnnotation mocks base method. func (m *MockWorkerPodManager) GetTolerationsAnnotation(p *v1.Pod) string { m.ctrl.T.Helper() diff --git a/internal/pod/workerpodmanager.go b/internal/pod/workerpodmanager.go index 7b7c1cb4e..a67bd00ef 100644 --- a/internal/pod/workerpodmanager.go +++ b/internal/pod/workerpodmanager.go @@ -41,27 +41,29 @@ type WorkerPodManager interface { GetConfigAnnotation(p *v1.Pod) string HashAnnotationDiffer(p1, p2 *v1.Pod) bool GetTolerationsAnnotation(p *v1.Pod) string + GetModuleVersionAnnotation(p *v1.Pod) string } const ( NodeModulesConfigFinalizer = "kmm.node.kubernetes.io/nodemodulesconfig-reconciler" WorkerContainerName = "worker" - volMountPointConfig = "/etc/kmm-worker" - configFileName = "config.yaml" - configFullPath = volMountPointConfig + "/" + configFileName - volNameConfig = "config" - sharedFilesDir = "/tmp" - volNameTmp = "tmp" - volumeNameConfig = "config" - initContainerName = "image-extractor" - modulesOrderKey = "kmm.node.kubernetes.io/modules-order" - workerActionLoad = "Load" - workerActionUnload = "Unload" - actionLabelKey = "kmm.node.kubernetes.io/worker-action" - configAnnotationKey = "kmm.node.kubernetes.io/worker-config" - hashAnnotationKey = "kmm.node.kubernetes.io/worker-hash" - tolerationsAnnotationKey = "kmm.node.kubernetes.io/worker-tolerations" + volMountPointConfig = "/etc/kmm-worker" + configFileName = "config.yaml" + configFullPath = volMountPointConfig + "/" + configFileName + volNameConfig = "config" + sharedFilesDir = "/tmp" + volNameTmp = "tmp" + volumeNameConfig = "config" + initContainerName = "image-extractor" + modulesOrderKey = "kmm.node.kubernetes.io/modules-order" + workerActionLoad = "Load" + workerActionUnload = "Unload" + actionLabelKey = "kmm.node.kubernetes.io/worker-action" + configAnnotationKey = "kmm.node.kubernetes.io/worker-config" + hashAnnotationKey = "kmm.node.kubernetes.io/worker-hash" + tolerationsAnnotationKey = "kmm.node.kubernetes.io/worker-tolerations" + moduleVersionAnnotationKey = "kmm.node.kubernetes.io/worker-module-version" ) var ( @@ -201,6 +203,8 @@ func (wpmi *workerPodManagerImpl) LoaderPodTemplate(ctx context.Context, nmc cli return nil, fmt.Errorf("could not set worker tolerations: %v", err) } + setWorkerModuleVersionAnnotation(pod, nms.Version) + if err = setWorkerSecurityContext(pod, wpmi.workerCfg, privileged); err != nil { return nil, fmt.Errorf("could not set the worker Pod as privileged: %v", err) } @@ -292,6 +296,7 @@ func (wpmi *workerPodManagerImpl) GetConfigAnnotation(p *v1.Pod) string { return p.Annotations[configAnnotationKey] } + func (wpmi *workerPodManagerImpl) GetTolerationsAnnotation(p *v1.Pod) string { if p == nil { @@ -301,6 +306,15 @@ func (wpmi *workerPodManagerImpl) GetTolerationsAnnotation(p *v1.Pod) string { return p.Annotations[tolerationsAnnotationKey] } +func (wpmi *workerPodManagerImpl) GetModuleVersionAnnotation(p *v1.Pod) string { + + if p == nil { + return "" + } + + return p.Annotations[moduleVersionAnnotationKey] +} + func (wpmi *workerPodManagerImpl) HashAnnotationDiffer(p1, p2 *v1.Pod) bool { if p1 == nil && p2 == nil { @@ -483,6 +497,12 @@ func setWorkerTolerationsAnnotation(pod *v1.Pod, tolerations []v1.Toleration) er return nil } +func setWorkerModuleVersionAnnotation(pod *v1.Pod, moduleVersion string) { + if moduleVersion != "" { + meta.SetAnnotation(pod, moduleVersionAnnotationKey, moduleVersion) + } +} + func setWorkerSofdepConfig(pod *v1.Pod, modulesLoadingOrder []string) error { softdepAnnotationValue := getModulesOrderAnnotationValue(modulesLoadingOrder) meta.SetAnnotation(pod, modulesOrderKey, softdepAnnotationValue) diff --git a/internal/utils/kmmlabels.go b/internal/utils/kmmlabels.go index c24abf4ef..ef9e5e62f 100644 --- a/internal/utils/kmmlabels.go +++ b/internal/utils/kmmlabels.go @@ -76,6 +76,10 @@ func GetKernelModuleReadyNodeLabel(namespace, moduleName string) string { return fmt.Sprintf("kmm.node.kubernetes.io/%s.%s.ready", namespace, moduleName) } +func GetKernelModuleVersionReadyNodeLabel(namespace, moduleName string) string { + return fmt.Sprintf("kmm.node.kubernetes.io/%s.%s.version.ready", namespace, moduleName) +} + func GetDevicePluginNodeLabel(namespace, moduleName string) string { return fmt.Sprintf("kmm.node.kubernetes.io/%s.%s.device-plugin-ready", namespace, moduleName) } diff --git a/pkg/labels/labels.go b/pkg/labels/labels.go index ac840242d..201b234b2 100644 --- a/pkg/labels/labels.go +++ b/pkg/labels/labels.go @@ -11,3 +11,11 @@ func GetKernelModuleReadyNodeLabel(namespace, moduleName string) string { func GetDevicePluginNodeLabel(namespace, moduleName string) string { return utils.GetDevicePluginNodeLabel(namespace, moduleName) } + +func GetModuleVersionLabelName(namespace, name string) string { + return utils.GetModuleVersionLabelName(namespace, name) +} + +func GetKernelModuleVersionReadyNodeLabel(namespace, moduleName string) string { + return utils.GetKernelModuleVersionReadyNodeLabel(namespace, moduleName) +} diff --git a/pkg/labels/labels_test.go b/pkg/labels/labels_test.go index 6afe7cc2c..f6c1f8ce6 100644 --- a/pkg/labels/labels_test.go +++ b/pkg/labels/labels_test.go @@ -15,4 +15,15 @@ var _ = Describe("GetModuleReadyAndDevicePluginReadyLabels", func() { res := GetDevicePluginNodeLabel("some-namespace", "some-module") Expect(res).To(Equal("kmm.node.kubernetes.io/some-namespace.some-module.device-plugin-ready")) }) + + It("module version label", func() { + res := GetModuleVersionLabelName("some-namespace", "some-module") + Expect(res).To(Equal("kmm.node.kubernetes.io/version-module.some-namespace.some-module")) + }) + + It("module version ready label", func() { + res := GetKernelModuleVersionReadyNodeLabel("some-namespace", "some-module") + Expect(res).To(Equal("kmm.node.kubernetes.io/some-namespace.some-module.version.ready")) + }) + })