diff --git a/api/v1beta1/nodemodulesconfig_types.go b/api/v1beta1/nodemodulesconfig_types.go index f9d1f7e5f..830743434 100644 --- a/api/v1beta1/nodemodulesconfig_types.go +++ b/api/v1beta1/nodemodulesconfig_types.go @@ -31,9 +31,10 @@ type ModuleConfig struct { } type NodeModuleSpec struct { - Name string `json:"name"` - Namespace string `json:"namespace"` - Config ModuleConfig `json:"config"` + Name string `json:"name"` + Namespace string `json:"namespace"` + Config ModuleConfig `json:"config"` + ServiceAccountName string `json:"serviceAccountName"` } // NodeModulesConfigSpec describes the desired state of modules on the node diff --git a/cmd/manager/main.go b/cmd/manager/main.go index a52b37124..623dbccce 100644 --- a/cmd/manager/main.go +++ b/cmd/manager/main.go @@ -22,8 +22,6 @@ import ( "os" "strconv" - "github.com/kubernetes-sigs/kernel-module-management/internal/build/pod" - "github.com/kubernetes-sigs/kernel-module-management/internal/config" "k8s.io/apimachinery/pkg/runtime" utilruntime "k8s.io/apimachinery/pkg/util/runtime" clientgoscheme "k8s.io/client-go/kubernetes/scheme" @@ -40,7 +38,9 @@ import ( v1beta12 "github.com/kubernetes-sigs/kernel-module-management/api/v1beta1" "github.com/kubernetes-sigs/kernel-module-management/controllers" "github.com/kubernetes-sigs/kernel-module-management/internal/build" + "github.com/kubernetes-sigs/kernel-module-management/internal/build/pod" "github.com/kubernetes-sigs/kernel-module-management/internal/cmd" + "github.com/kubernetes-sigs/kernel-module-management/internal/config" "github.com/kubernetes-sigs/kernel-module-management/internal/constants" "github.com/kubernetes-sigs/kernel-module-management/internal/daemonset" "github.com/kubernetes-sigs/kernel-module-management/internal/filter" @@ -86,6 +86,7 @@ func main() { } operatorNamespace := cmd.GetEnvOrFatalError(constants.OperatorNamespaceEnvVar, setupLogger) + workerImage := cmd.GetEnvOrFatalError("RELATED_IMAGES_WORKER", setupLogger) managed, err := GetBoolEnv("KMM_MANAGED") if err != nil { @@ -163,6 +164,17 @@ func main() { cmd.FatalError(setupLogger, err, "unable to create controller", "name", controllers.ModuleNMCReconcilerName) } + workerHelper := controllers.NewWorkerHelper( + client, + controllers.NewPodManager(client, workerImage, scheme), + ) + + ctx := ctrl.SetupSignalHandler() + + if err = controllers.NewNodeModulesConfigReconciler(client, workerHelper).SetupWithManager(ctx, mgr); err != nil { + cmd.FatalError(setupLogger, err, "unable to create controller", "name", controllers.NodeModulesConfigReconcilerName) + } + nodeKernelReconciler := controllers.NewNodeKernelReconciler(client, constants.KernelLabel, filterAPI) if err = nodeKernelReconciler.SetupWithManager(mgr); err != nil { @@ -210,7 +222,7 @@ func main() { } setupLogger.Info("starting manager") - if err = mgr.Start(ctrl.SetupSignalHandler()); err != nil { + if err = mgr.Start(ctx); err != nil { cmd.FatalError(setupLogger, err, "problem running manager") } } 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 7a9019359..d26315d70 100644 --- a/config/crd/bases/kmm.sigs.x-k8s.io_nodemodulesconfigs.yaml +++ b/config/crd/bases/kmm.sigs.x-k8s.io_nodemodulesconfigs.yaml @@ -149,10 +149,13 @@ spec: type: string namespace: type: string + serviceAccountName: + type: string required: - config - name - namespace + - serviceAccountName type: object type: array type: object diff --git a/config/manager/kustomization.yaml b/config/manager/kustomization.yaml index b8cc12ec0..0fcb40137 100644 --- a/config/manager/kustomization.yaml +++ b/config/manager/kustomization.yaml @@ -4,10 +4,16 @@ kind: Kustomization resources: - ../manager-base +patches: +- path: manager_worker_image_patch.yaml + images: - name: controller newName: gcr.io/k8s-staging-kmm/kernel-module-management-operator newTag: latest +- name: worker + newName: gcr.io/k8s-staging-kmm/kernel-module-management-worker + newTag: latest configMapGenerator: - files: diff --git a/config/manager/manager_worker_image_patch.yaml b/config/manager/manager_worker_image_patch.yaml new file mode 100644 index 000000000..84760f775 --- /dev/null +++ b/config/manager/manager_worker_image_patch.yaml @@ -0,0 +1,13 @@ +apiVersion: apps/v1 +kind: Deployment +metadata: + name: controller-manager + namespace: system +spec: + template: + spec: + containers: + - name: manager + env: + - name: RELATED_IMAGES_WORKER + value: worker diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index f983a92d7..fc179b1a5 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -98,6 +98,12 @@ rules: - list - patch - watch +- apiGroups: + - kmm.sigs.x-k8s.io + resources: + - nodemodulesconfigs/status + verbs: + - patch - apiGroups: - kmm.sigs.x-k8s.io resources: diff --git a/controllers/mock_nodemodulesconfig_reconciler.go b/controllers/mock_nodemodulesconfig_reconciler.go new file mode 100644 index 000000000..204aef5e6 --- /dev/null +++ b/controllers/mock_nodemodulesconfig_reconciler.go @@ -0,0 +1,174 @@ +// Code generated by MockGen. DO NOT EDIT. +// Source: nodemodulesconfig_reconciler.go + +// Package controllers is a generated GoMock package. +package controllers + +import ( + context "context" + reflect "reflect" + + v1beta1 "github.com/kubernetes-sigs/kernel-module-management/api/v1beta1" + gomock "go.uber.org/mock/gomock" + v1 "k8s.io/api/core/v1" + client "sigs.k8s.io/controller-runtime/pkg/client" +) + +// MockWorkerHelper is a mock of WorkerHelper interface. +type MockWorkerHelper struct { + ctrl *gomock.Controller + recorder *MockWorkerHelperMockRecorder +} + +// MockWorkerHelperMockRecorder is the mock recorder for MockWorkerHelper. +type MockWorkerHelperMockRecorder struct { + mock *MockWorkerHelper +} + +// NewMockWorkerHelper creates a new mock instance. +func NewMockWorkerHelper(ctrl *gomock.Controller) *MockWorkerHelper { + mock := &MockWorkerHelper{ctrl: ctrl} + mock.recorder = &MockWorkerHelperMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockWorkerHelper) EXPECT() *MockWorkerHelperMockRecorder { + return m.recorder +} + +// ProcessModuleSpec mocks base method. +func (m *MockWorkerHelper) ProcessModuleSpec(ctx context.Context, nmc *v1beta1.NodeModulesConfig, spec *v1beta1.NodeModuleSpec, status *v1beta1.NodeModuleStatus) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ProcessModuleSpec", ctx, nmc, spec, status) + ret0, _ := ret[0].(error) + return ret0 +} + +// ProcessModuleSpec indicates an expected call of ProcessModuleSpec. +func (mr *MockWorkerHelperMockRecorder) ProcessModuleSpec(ctx, nmc, spec, status interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ProcessModuleSpec", reflect.TypeOf((*MockWorkerHelper)(nil).ProcessModuleSpec), ctx, nmc, spec, status) +} + +// ProcessOrphanModuleStatus mocks base method. +func (m *MockWorkerHelper) ProcessOrphanModuleStatus(ctx context.Context, nmc *v1beta1.NodeModulesConfig, status *v1beta1.NodeModuleStatus) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ProcessOrphanModuleStatus", ctx, nmc, status) + ret0, _ := ret[0].(error) + return ret0 +} + +// ProcessOrphanModuleStatus indicates an expected call of ProcessOrphanModuleStatus. +func (mr *MockWorkerHelperMockRecorder) ProcessOrphanModuleStatus(ctx, nmc, status interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ProcessOrphanModuleStatus", reflect.TypeOf((*MockWorkerHelper)(nil).ProcessOrphanModuleStatus), ctx, nmc, status) +} + +// RemoveOrphanFinalizers mocks base method. +func (m *MockWorkerHelper) RemoveOrphanFinalizers(ctx context.Context, nodeName string) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "RemoveOrphanFinalizers", ctx, nodeName) + ret0, _ := ret[0].(error) + return ret0 +} + +// RemoveOrphanFinalizers indicates an expected call of RemoveOrphanFinalizers. +func (mr *MockWorkerHelperMockRecorder) RemoveOrphanFinalizers(ctx, nodeName interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RemoveOrphanFinalizers", reflect.TypeOf((*MockWorkerHelper)(nil).RemoveOrphanFinalizers), ctx, nodeName) +} + +// SyncStatus mocks base method. +func (m *MockWorkerHelper) SyncStatus(ctx context.Context, nmc *v1beta1.NodeModulesConfig) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "SyncStatus", ctx, nmc) + ret0, _ := ret[0].(error) + return ret0 +} + +// SyncStatus indicates an expected call of SyncStatus. +func (mr *MockWorkerHelperMockRecorder) SyncStatus(ctx, nmc interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SyncStatus", reflect.TypeOf((*MockWorkerHelper)(nil).SyncStatus), ctx, nmc) +} + +// MockPodManager is a mock of PodManager interface. +type MockPodManager struct { + ctrl *gomock.Controller + recorder *MockPodManagerMockRecorder +} + +// MockPodManagerMockRecorder is the mock recorder for MockPodManager. +type MockPodManagerMockRecorder struct { + mock *MockPodManager +} + +// NewMockPodManager creates a new mock instance. +func NewMockPodManager(ctrl *gomock.Controller) *MockPodManager { + mock := &MockPodManager{ctrl: ctrl} + mock.recorder = &MockPodManagerMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockPodManager) EXPECT() *MockPodManagerMockRecorder { + return m.recorder +} + +// CreateLoaderPod mocks base method. +func (m *MockPodManager) CreateLoaderPod(ctx context.Context, nmc client.Object, nms *v1beta1.NodeModuleSpec) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "CreateLoaderPod", ctx, nmc, nms) + ret0, _ := ret[0].(error) + return ret0 +} + +// CreateLoaderPod indicates an expected call of CreateLoaderPod. +func (mr *MockPodManagerMockRecorder) CreateLoaderPod(ctx, nmc, nms interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateLoaderPod", reflect.TypeOf((*MockPodManager)(nil).CreateLoaderPod), ctx, nmc, nms) +} + +// CreateUnloaderPod mocks base method. +func (m *MockPodManager) CreateUnloaderPod(ctx context.Context, nmc client.Object, nms *v1beta1.NodeModuleStatus) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "CreateUnloaderPod", ctx, nmc, nms) + ret0, _ := ret[0].(error) + return ret0 +} + +// CreateUnloaderPod indicates an expected call of CreateUnloaderPod. +func (mr *MockPodManagerMockRecorder) CreateUnloaderPod(ctx, nmc, nms interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateUnloaderPod", reflect.TypeOf((*MockPodManager)(nil).CreateUnloaderPod), ctx, nmc, nms) +} + +// DeletePod mocks base method. +func (m *MockPodManager) DeletePod(ctx context.Context, pod *v1.Pod) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "DeletePod", ctx, pod) + ret0, _ := ret[0].(error) + return ret0 +} + +// DeletePod indicates an expected call of DeletePod. +func (mr *MockPodManagerMockRecorder) DeletePod(ctx, pod interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeletePod", reflect.TypeOf((*MockPodManager)(nil).DeletePod), ctx, pod) +} + +// ListWorkerPodsOnNode mocks base method. +func (m *MockPodManager) ListWorkerPodsOnNode(ctx context.Context, nodeName string) ([]v1.Pod, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ListWorkerPodsOnNode", ctx, nodeName) + ret0, _ := ret[0].([]v1.Pod) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// ListWorkerPodsOnNode indicates an expected call of ListWorkerPodsOnNode. +func (mr *MockPodManagerMockRecorder) ListWorkerPodsOnNode(ctx, nodeName interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListWorkerPodsOnNode", reflect.TypeOf((*MockPodManager)(nil).ListWorkerPodsOnNode), ctx, nodeName) +} diff --git a/controllers/module_nmc_reconciler.go b/controllers/module_nmc_reconciler.go index fa09f4937..3254598c1 100644 --- a/controllers/module_nmc_reconciler.go +++ b/controllers/module_nmc_reconciler.go @@ -178,6 +178,10 @@ func (mnrh *moduleNMCReconcilerHelper) enableModuleOnNode(ctx context.Context, m Modprobe: mld.Modprobe, } + if tls := mld.RegistryTLS; tls != nil { + moduleConfig.InsecurePull = tls.Insecure || tls.InsecureSkipTLSVerify + } + nmc := &kmmv1beta1.NodeModulesConfig{ ObjectMeta: metav1.ObjectMeta{Name: nodeName}, } diff --git a/controllers/nodemodulesconfig_reconciler.go b/controllers/nodemodulesconfig_reconciler.go new file mode 100644 index 000000000..627aa8bda --- /dev/null +++ b/controllers/nodemodulesconfig_reconciler.go @@ -0,0 +1,696 @@ +package controllers + +import ( + "context" + "errors" + "fmt" + "reflect" + + kmmv1beta1 "github.com/kubernetes-sigs/kernel-module-management/api/v1beta1" + "github.com/kubernetes-sigs/kernel-module-management/internal/constants" + "github.com/kubernetes-sigs/kernel-module-management/internal/filter" + "github.com/kubernetes-sigs/kernel-module-management/internal/nmc" + v1 "k8s.io/api/core/v1" + k8serrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + "k8s.io/kubectl/pkg/cmd/util/podcmd" + "k8s.io/utils/pointer" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/builder" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + "sigs.k8s.io/controller-runtime/pkg/handler" + "sigs.k8s.io/controller-runtime/pkg/manager" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + "sigs.k8s.io/yaml" +) + +type WorkerAction string + +const ( + WorkerActionLoad = "Load" + WorkerActionUnload = "Unload" + + NodeModulesConfigReconcilerName = "NodeModulesConfig" + + actionLabelKey = "kmm.node.kubernetes.io/worker-action" + configAnnotationKey = "kmm.node.kubernetes.io/worker-config" + nodeModulesConfigFinalizer = "kmm.node.kubernetes.io/nodemodulesconfig-reconciler" + volumeNameConfig = "config" + workerContainerName = "worker" +) + +//+kubebuilder:rbac:groups=kmm.sigs.x-k8s.io,resources=nodemodulesconfigs,verbs=get;list;watch +//+kubebuilder:rbac:groups=kmm.sigs.x-k8s.io,resources=nodemodulesconfigs/status,verbs=patch +//+kubebuilder:rbac:groups="core",resources=pods,verbs=create;delete;get;list;watch +//+kubebuilder:rbac:groups="core",resources=nodes,verbs=get;list;watch + +type NodeModulesConfigReconciler struct { + client client.Client + helper WorkerHelper +} + +func NewNodeModulesConfigReconciler(client client.Client, helper WorkerHelper) *NodeModulesConfigReconciler { + return &NodeModulesConfigReconciler{ + client: client, + helper: helper, + } +} + +func (r *NodeModulesConfigReconciler) Reconcile(ctx context.Context, req reconcile.Request) (reconcile.Result, error) { + logger := ctrl.LoggerFrom(ctx) + + nmcObj := kmmv1beta1.NodeModulesConfig{} + + if err := r.client.Get(ctx, req.NamespacedName, &nmcObj); err != nil { + if k8serrors.IsNotFound(err) { + // Pods are owned by the NMC, so the GC will have deleted them already. + // Remove the finalizer if we did not have a chance to do it before NMC deletion. + logger.Info("Clearing worker Pod finalizers") + + if err = r.helper.RemoveOrphanFinalizers(ctx, req.Name); err != nil { + return reconcile.Result{}, fmt.Errorf("could not clear all Pod finalizers for NMC %s: %v", req.Name, err) + } + + return reconcile.Result{}, nil + } + + return reconcile.Result{}, fmt.Errorf("could not get NodeModuleState %s: %v", req.NamespacedName, err) + } + + if err := r.helper.SyncStatus(ctx, &nmcObj); err != nil { + return reconcile.Result{}, fmt.Errorf("could not reconcile status for NodeModulesConfig %s: %v", nmcObj.Name, err) + } + + // Statuses are now up-to-date. + + statusMap := make(map[string]*kmmv1beta1.NodeModuleStatus, len(nmcObj.Status.Modules)) + + for i := 0; i < len(nmcObj.Status.Modules); i++ { + status := nmcObj.Status.Modules[i] + statusMap[status.Namespace+"/"+status.Name] = &nmcObj.Status.Modules[i] + } + + errs := make([]error, 0, len(nmcObj.Spec.Modules)+len(nmcObj.Status.Modules)) + + for _, mod := range nmcObj.Spec.Modules { + moduleNameKey := mod.Namespace + "/" + mod.Name + + logger := logger.WithValues("module", moduleNameKey) + + if err := r.helper.ProcessModuleSpec(ctrl.LoggerInto(ctx, logger), &nmcObj, &mod, statusMap[moduleNameKey]); err != nil { + errs = append( + errs, + fmt.Errorf("error processing Module %s: %v", moduleNameKey, err), + ) + } + + delete(statusMap, moduleNameKey) + } + + // We have processed all module specs. + // Now, go through the remaining, "orphan" statuses that do not have a corresponding spec; those must be unloaded. + + for statusNameKey, status := range statusMap { + logger := logger.WithValues("status", statusNameKey) + + if err := r.helper.ProcessOrphanModuleStatus(ctrl.LoggerInto(ctx, logger), &nmcObj, status); err != nil { + errs = append( + errs, + fmt.Errorf("erorr processing orphan status for Module %s: %v", statusNameKey, err), + ) + } + } + + return ctrl.Result{}, errors.Join(errs...) +} + +func (r *NodeModulesConfigReconciler) SetupWithManager(ctx context.Context, mgr manager.Manager) error { + // Cache pods by the name of the node they run on. + // Because NMC name == node name, we can efficiently reconcile the NMC status by listing all pods currently running + // or completed for it. + err := mgr.GetCache().IndexField(ctx, &v1.Pod{}, ".spec.nodeName", func(o client.Object) []string { + return []string{o.(*v1.Pod).Spec.NodeName} + }) + if err != nil { + return fmt.Errorf("could not start the worker Pod indexer: %v", err) + } + + nodeToNMCMapFunc := func(_ context.Context, o client.Object) []reconcile.Request { + return []reconcile.Request{ + {NamespacedName: types.NamespacedName{Name: o.GetName()}}, + } + } + + return ctrl.NewControllerManagedBy(mgr). + Named(NodeModulesConfigReconcilerName). + For(&kmmv1beta1.NodeModulesConfig{}). + Owns(&v1.Pod{}). + // TODO maybe replace this with Owns() if we make nodes the owners of NodeModulesConfigs. + Watches( + &v1.Node{}, + handler.EnqueueRequestsFromMapFunc(nodeToNMCMapFunc), + builder.WithPredicates(filter.SkipDeletions()), + ). + Complete(r) +} + +func workerPodName(nodeName, moduleName string) string { + return fmt.Sprintf("kmm-worker-%s-%s", nodeName, moduleName) +} + +func GetContainerStatus(statuses []v1.ContainerStatus, name string) v1.ContainerStatus { + for i := range statuses { + if statuses[i].Name == name { + return statuses[i] + } + } + + return v1.ContainerStatus{} +} + +func FindNodeCondition(cond []v1.NodeCondition, conditionType v1.NodeConditionType) *v1.NodeCondition { + for i := 0; i < len(cond); i++ { + c := cond[i] + + if c.Type == conditionType { + return &c + } + } + + return nil +} + +//go:generate mockgen -source=nodemodulesconfig_reconciler.go -package=controllers -destination=mock_nodemodulesconfig_reconciler.go WorkerHelper + +type WorkerHelper interface { + ProcessModuleSpec(ctx context.Context, nmc *kmmv1beta1.NodeModulesConfig, spec *kmmv1beta1.NodeModuleSpec, status *kmmv1beta1.NodeModuleStatus) error + ProcessOrphanModuleStatus(ctx context.Context, nmc *kmmv1beta1.NodeModulesConfig, status *kmmv1beta1.NodeModuleStatus) error + SyncStatus(ctx context.Context, nmc *kmmv1beta1.NodeModulesConfig) error + RemoveOrphanFinalizers(ctx context.Context, nodeName string) error +} + +type workerHelper struct { + client client.Client + pm PodManager +} + +func NewWorkerHelper(client client.Client, pm PodManager) WorkerHelper { + return &workerHelper{ + client: client, + pm: pm, + } +} + +// ProcessModuleSpec determines if a worker Pod should be created for a Module entry in a +// NodeModulesConfig .spec.modules. +// A loading worker pod is created when: +// - there is no corresponding entry in the NodeModulesConfig's .status.modules list; +// - the lastTransitionTime property in the .status.modules entry is older that the last transition time +// of the Ready condition on the node. This makes sure that we always load modules after maintenance operations +// that would make a node not Ready, such as a reboot. +// +// An unloading worker Pod is created when the entry in .spec.modules has a different config compared to the entry in +// .status.modules. +func (w *workerHelper) ProcessModuleSpec( + ctx context.Context, + nmc *kmmv1beta1.NodeModulesConfig, + spec *kmmv1beta1.NodeModuleSpec, + status *kmmv1beta1.NodeModuleStatus, +) error { + logger := ctrl.LoggerFrom(ctx) + + if status == nil { + logger.Info("Missing status; creating loader Pod") + + return w.pm.CreateLoaderPod(ctx, nmc, spec) + } + + if status.InProgress { + logger.Info("Worker pod is running; skipping") + return nil + } + + if !reflect.DeepEqual(spec.Config, *status.Config) { + logger.Info("Outdated config in status; creating unloader Pod") + + return w.pm.CreateUnloaderPod(ctx, nmc, status) + } + + node := v1.Node{} + + if err := w.client.Get(ctx, types.NamespacedName{Name: nmc.Name}, &node); err != nil { + return fmt.Errorf("could not get node %s: %v", nmc.Name, err) + } + + readyCondition := FindNodeCondition(node.Status.Conditions, v1.NodeReady) + if readyCondition == nil { + return fmt.Errorf("node %s has no Ready condition", nmc.Name) + } + + if readyCondition.Status == v1.ConditionTrue && status.LastTransitionTime.Before(&readyCondition.LastTransitionTime) { + logger.Info("Outdated last transition time status; creating unloader Pod") + + return w.pm.CreateLoaderPod(ctx, nmc, spec) + } + + logger.Info("Spec and status in sync; nothing to do") + + return nil +} + +func (w *workerHelper) ProcessOrphanModuleStatus( + ctx context.Context, + nmc *kmmv1beta1.NodeModulesConfig, + status *kmmv1beta1.NodeModuleStatus, +) error { + logger := ctrl.LoggerFrom(ctx) + + if status.InProgress { + logger.Info("Sync status is in progress; skipping") + return nil + } + + logger.Info("Creating unloader Pod") + + return w.pm.CreateUnloaderPod(ctx, nmc, status) +} + +func (w *workerHelper) RemoveOrphanFinalizers(ctx context.Context, nodeName string) error { + pods, err := w.pm.ListWorkerPodsOnNode(ctx, nodeName) + if err != nil { + return fmt.Errorf("could not delete orphan worker Pods on node %s: %v", nodeName, err) + } + + errs := make([]error, 0, len(pods)) + + for i := 0; i < len(pods); i++ { + pod := &pods[i] + + mergeFrom := client.MergeFrom(pod.DeepCopy()) + + if controllerutil.RemoveFinalizer(pod, nodeModulesConfigFinalizer) { + if err = w.client.Patch(ctx, pod, mergeFrom); err != nil { + errs = append( + errs, + fmt.Errorf("could not patch Pod %s/%s: %v", pod.Namespace, pod.Name, err), + ) + + continue + } + } + } + + return errors.Join(errs...) +} + +func (w *workerHelper) SyncStatus(ctx context.Context, nmcObj *kmmv1beta1.NodeModulesConfig) error { + logger := ctrl.LoggerFrom(ctx) + + logger.Info("Syncing status") + + pods, err := w.pm.ListWorkerPodsOnNode(ctx, nmcObj.Name) + if err != nil { + return fmt.Errorf("could not list worker pods for NodeModulesConfig %s: %v", nmcObj.Name, err) + } + + logger.V(1).Info("List worker Pods", "count", len(pods)) + + if len(pods) == 0 { + return nil + } + + patchFrom := client.MergeFrom(nmcObj.DeepCopy()) + errs := make([]error, 0, len(pods)) + + for _, p := range pods { + podNSN := types.NamespacedName{Namespace: p.Namespace, Name: p.Name} + + modNamespace := p.Namespace + modName := p.Labels[constants.ModuleNameLabel] + phase := p.Status.Phase + + logger := logger.WithValues("pod name", p.Name, "pod phase", p.Status.Phase) + + logger.Info("Processing worker Pod") + + status := kmmv1beta1.NodeModuleStatus{ + Namespace: modNamespace, + Name: modName, + } + + deletePod := false + statusDeleted := false + + switch phase { + case v1.PodFailed: + deletePod = true + case v1.PodSucceeded: + deletePod = true + + if p.Labels[actionLabelKey] == WorkerActionUnload { + nmc.RemoveModuleStatus(&nmcObj.Status.Modules, modNamespace, modName) + deletePod = true + statusDeleted = true + break + } + + config := kmmv1beta1.ModuleConfig{} + + if err = yaml.UnmarshalStrict([]byte(p.Annotations[configAnnotationKey]), &config); err != nil { + errs = append( + errs, + fmt.Errorf("%s: could not unmarshal the ModuleConfig from YAML: %v", podNSN, err), + ) + + continue + } + + status.Config = &config + + podLTT := GetContainerStatus(p.Status.ContainerStatuses, workerContainerName). + State. + Terminated. + FinishedAt + + status.LastTransitionTime = &podLTT + + deletePod = true + case v1.PodPending, v1.PodRunning: + status.InProgress = true + // TODO: if the NMC's spec changed compared to the Pod's config, recreate the Pod + default: + errs = append( + errs, + fmt.Errorf("%s: unhandled Pod phase %q", podNSN, phase), + ) + } + + if deletePod { + if err = w.pm.DeletePod(ctx, &p); err != nil { + errs = append( + errs, + fmt.Errorf("could not delete worker Pod %s: %v", podNSN, errs), + ) + + continue + } + } + + if !statusDeleted { + nmc.SetModuleStatus(&nmcObj.Status.Modules, status) + } + } + + if err = errors.Join(errs...); err != nil { + return fmt.Errorf("encountered errors while reconciling NMC %s status: %v", nmcObj.Name, err) + } + + if err = w.client.Status().Patch(ctx, nmcObj, patchFrom); err != nil { + return fmt.Errorf("could not patch NodeModulesConfig %s status: %v", nmcObj.Name, err) + } + + return nil +} + +const ( + configFileName = "config.yaml" + configFullPath = volMountPoingConfig + "/" + configFileName + + volNameConfig = "config" + volMountPoingConfig = "/etc/kmm-worker" +) + +//go:generate mockgen -source=nodemodulesconfig_reconciler.go -package=controllers -destination=mock_nodemodulesconfig_reconciler.go PodManager + +type PodManager interface { + CreateLoaderPod(ctx context.Context, nmc client.Object, nms *kmmv1beta1.NodeModuleSpec) error + CreateUnloaderPod(ctx context.Context, nmc client.Object, nms *kmmv1beta1.NodeModuleStatus) error + DeletePod(ctx context.Context, pod *v1.Pod) error + ListWorkerPodsOnNode(ctx context.Context, nodeName string) ([]v1.Pod, error) +} + +type podManager struct { + client client.Client + scheme *runtime.Scheme + workerImage string +} + +func NewPodManager(client client.Client, workerImage string, scheme *runtime.Scheme) PodManager { + return &podManager{ + client: client, + scheme: scheme, + workerImage: workerImage, + } +} + +func (p *podManager) CreateLoaderPod(ctx context.Context, nmc client.Object, nms *kmmv1beta1.NodeModuleSpec) error { + pod, err := p.baseWorkerPod(nmc.GetName(), nms.Namespace, nms.Name, nms.ServiceAccountName, nmc) + if err != nil { + return fmt.Errorf("could not create the base Pod: %v", err) + } + + if err = setWorkerContainerArgs(pod, []string{"kmod", "load", configFullPath}); err != nil { + return fmt.Errorf("could not set worker container args: %v", err) + } + + if err = setWorkerConfigAnnotation(pod, nms.Config); err != nil { + return fmt.Errorf("could not set worker config: %v", err) + } + + setWorkerActionLabel(pod, WorkerActionLoad) + pod.Spec.RestartPolicy = v1.RestartPolicyNever + + return p.client.Create(ctx, pod) +} + +func (p *podManager) CreateUnloaderPod(ctx context.Context, nmc client.Object, nms *kmmv1beta1.NodeModuleStatus) error { + pod, err := p.baseWorkerPod(nmc.GetName(), nms.Namespace, nms.Name, nms.ServiceAccountName, nmc) + if err != nil { + return fmt.Errorf("could not create the base Pod: %v", err) + } + + if err = setWorkerContainerArgs(pod, []string{"kmod", "unload", configFullPath}); err != nil { + return fmt.Errorf("could not set worker container args: %v", err) + } + + if err = setWorkerConfigAnnotation(pod, *nms.Config); err != nil { + return fmt.Errorf("could not set worker config: %v", err) + } + + setWorkerActionLabel(pod, WorkerActionUnload) + + return p.client.Create(ctx, pod) +} + +func (p *podManager) DeletePod(ctx context.Context, pod *v1.Pod) error { + logger := ctrl.LoggerFrom(ctx) + + logger.Info("Removing Pod finalizer") + + podPatch := client.MergeFrom(pod.DeepCopy()) + + controllerutil.RemoveFinalizer(pod, nodeModulesConfigFinalizer) + + if err := p.client.Patch(ctx, pod, podPatch); err != nil { + return fmt.Errorf("could not patch Pod %s/%s: %v", pod.Namespace, pod.Name, err) + } + + if pod.DeletionTimestamp == nil { + logger.Info("DeletionTimestamp not set; deleting Pod") + + if err := p.client.Delete(ctx, pod); client.IgnoreNotFound(err) != nil { + return fmt.Errorf("could not delete Pod %s/%s: %v", pod.Namespace, pod.Name, err) + } + } else { + logger.Info("DeletionTimestamp set; not deleting Pod") + } + + return nil +} + +func (p *podManager) ListWorkerPodsOnNode(ctx context.Context, nodeName string) ([]v1.Pod, error) { + logger := ctrl.LoggerFrom(ctx).WithValues("node name", nodeName) + + pl := v1.PodList{} + + hl := client.HasLabels{actionLabelKey} + mf := client.MatchingFields{".spec.nodeName": nodeName} + + logger.V(1).Info("Listing worker Pods") + + if err := p.client.List(ctx, &pl, hl, mf); err != nil { + return nil, fmt.Errorf("could not list worker pods for node %s: %v", nodeName, err) + } + + return pl.Items, nil +} + +func (p *podManager) PodExists(ctx context.Context, nodeName, modName, modNamespace string) (bool, error) { + pod := v1.Pod{} + + nsn := types.NamespacedName{ + Namespace: modNamespace, + Name: workerPodName(nodeName, modName), + } + + if err := p.client.Get(ctx, nsn, &pod); err != nil { + if k8serrors.IsNotFound(err) { + return false, nil + } + + return false, fmt.Errorf("error getting Pod %s: %v", nsn, err) + } + + return true, nil +} + +func (p *podManager) baseWorkerPod(nodeName, namespace, modName, serviceAccountName string, owner client.Object) (*v1.Pod, error) { + const ( + volNameLibModules = "lib-modules" + volNameUsrLibModules = "usr-lib-modules" + volNameVarLibFirmware = "var-lib-firmware" + ) + + hostPathDirectory := v1.HostPathDirectory + hostPathDirectoryOrCreate := v1.HostPathDirectoryOrCreate + + pod := v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: namespace, + Name: workerPodName(nodeName, modName), + Labels: map[string]string{constants.ModuleNameLabel: modName}, + }, + Spec: v1.PodSpec{ + Containers: []v1.Container{ + { + Name: workerContainerName, + Image: p.workerImage, + VolumeMounts: []v1.VolumeMount{ + { + Name: volNameConfig, + MountPath: volMountPoingConfig, + ReadOnly: true, + }, + { + Name: volNameLibModules, + MountPath: "/lib/modules", + ReadOnly: true, + }, + { + Name: volNameUsrLibModules, + MountPath: "/usr/lib/modules", + ReadOnly: true, + }, + { + Name: volNameVarLibFirmware, + MountPath: "/var/lib/firmware", + }, + }, + SecurityContext: &v1.SecurityContext{ + Privileged: pointer.Bool(true), + }, + }, + }, + NodeName: nodeName, + RestartPolicy: v1.RestartPolicyNever, + ServiceAccountName: serviceAccountName, + Volumes: []v1.Volume{ + { + Name: volumeNameConfig, + VolumeSource: v1.VolumeSource{ + DownwardAPI: &v1.DownwardAPIVolumeSource{ + Items: []v1.DownwardAPIVolumeFile{ + { + Path: configFileName, + FieldRef: &v1.ObjectFieldSelector{ + FieldPath: fmt.Sprintf("metadata.annotations['%s']", configAnnotationKey), + }, + }, + }, + }, + }, + }, + { + Name: volNameLibModules, + VolumeSource: v1.VolumeSource{ + HostPath: &v1.HostPathVolumeSource{ + Path: "/lib/modules", + Type: &hostPathDirectory, + }, + }, + }, + { + Name: volNameUsrLibModules, + VolumeSource: v1.VolumeSource{ + HostPath: &v1.HostPathVolumeSource{ + Path: "/usr/lib/modules", + Type: &hostPathDirectory, + }, + }, + }, + { + Name: volNameVarLibFirmware, + VolumeSource: v1.VolumeSource{ + HostPath: &v1.HostPathVolumeSource{ + Path: "/var/lib/firmware", + Type: &hostPathDirectoryOrCreate, + }, + }, + }, + }, + }, + } + + if err := ctrl.SetControllerReference(owner, &pod, p.scheme); err != nil { + return nil, fmt.Errorf("could not set the owner as controller: %v", err) + } + + controllerutil.AddFinalizer(&pod, nodeModulesConfigFinalizer) + + return &pod, nil +} + +func setWorkerActionLabel(pod *v1.Pod, action WorkerAction) { + labels := pod.GetLabels() + + if labels == nil { + labels = make(map[string]string) + } + + labels[actionLabelKey] = string(action) + + pod.SetLabels(labels) +} + +func setWorkerConfigAnnotation(pod *v1.Pod, cfg kmmv1beta1.ModuleConfig) error { + b, err := yaml.Marshal(cfg) + if err != nil { + return fmt.Errorf("could not marshal the ModuleConfig to YAML: %v", err) + } + + annotations := pod.GetAnnotations() + + if annotations == nil { + annotations = make(map[string]string) + } + + annotations[configAnnotationKey] = string(b) + + pod.SetAnnotations(annotations) + + return nil +} + +func setWorkerContainerArgs(pod *v1.Pod, args []string) error { + container, _ := podcmd.FindContainerByName(pod, workerContainerName) + if container == nil { + return errors.New("could not find the worker container") + } + + container.Args = args + + return nil +} diff --git a/controllers/nodemodulesconfig_reconciler_test.go b/controllers/nodemodulesconfig_reconciler_test.go new file mode 100644 index 000000000..b7d938e4a --- /dev/null +++ b/controllers/nodemodulesconfig_reconciler_test.go @@ -0,0 +1,915 @@ +package controllers + +import ( + "context" + "errors" + "fmt" + "reflect" + "time" + + "github.com/budougumi0617/cmpmock" + kmmv1beta1 "github.com/kubernetes-sigs/kernel-module-management/api/v1beta1" + testclient "github.com/kubernetes-sigs/kernel-module-management/internal/client" + "github.com/kubernetes-sigs/kernel-module-management/internal/constants" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "go.uber.org/mock/gomock" + v1 "k8s.io/api/core/v1" + k8serrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/types" + "k8s.io/utils/pointer" + ctrl "sigs.k8s.io/controller-runtime" + ctrlclient "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + "sigs.k8s.io/controller-runtime/pkg/reconcile" +) + +const nmcName = "nmc" + +var _ = Describe("NodeModulesConfigReconciler_Reconcile", func() { + var ( + kubeClient *testclient.MockClient + wh *MockWorkerHelper + + r *NodeModulesConfigReconciler + + ctx = context.TODO() + nmcNsn = types.NamespacedName{Name: nmcName} + req = reconcile.Request{NamespacedName: nmcNsn} + ) + + BeforeEach(func() { + ctrl := gomock.NewController(GinkgoT()) + kubeClient = testclient.NewMockClient(ctrl) + wh = NewMockWorkerHelper(ctrl) + r = NewNodeModulesConfigReconciler(kubeClient, wh) + }) + + It("should clean worker Pod finalizers and return if the NMC does not exist", func() { + gomock.InOrder( + kubeClient. + EXPECT(). + Get(ctx, nmcNsn, &kmmv1beta1.NodeModulesConfig{}). + Return(k8serrors.NewNotFound(schema.GroupResource{}, nmcName)), + wh.EXPECT().RemoveOrphanFinalizers(ctx, nmcName), + ) + + Expect( + r.Reconcile(ctx, req), + ).To( + Equal(ctrl.Result{}), + ) + }) + + It("should fail if we could not synchronize the NMC status", func() { + nmc := &kmmv1beta1.NodeModulesConfig{ + ObjectMeta: metav1.ObjectMeta{Name: nmcName}, + } + + gomock.InOrder( + kubeClient. + EXPECT(). + Get(ctx, nmcNsn, &kmmv1beta1.NodeModulesConfig{}). + Do(func(_ context.Context, _ types.NamespacedName, kubeNmc ctrlclient.Object, _ ...ctrlclient.Options) { + *kubeNmc.(*kmmv1beta1.NodeModulesConfig) = *nmc + }), + wh.EXPECT().SyncStatus(ctx, nmc).Return(errors.New("random error")), + ) + + _, err := r.Reconcile(ctx, req) + Expect(err).To(HaveOccurred()) + }) + + It("should process spec entries and orphan statuses", func() { + const ( + mod0Name = "mod0" + mod1Name = "mod1" + mod2Name = "mod2" + ) + spec0 := kmmv1beta1.NodeModuleSpec{ + Namespace: namespace, + Name: mod0Name, + } + + spec1 := kmmv1beta1.NodeModuleSpec{ + Namespace: namespace, + Name: mod1Name, + } + + status0 := kmmv1beta1.NodeModuleStatus{ + Namespace: namespace, + Name: mod0Name, + } + + status2 := kmmv1beta1.NodeModuleStatus{ + Namespace: namespace, + Name: mod2Name, + } + + nmc := &kmmv1beta1.NodeModulesConfig{ + ObjectMeta: metav1.ObjectMeta{Name: nmcName}, + Spec: kmmv1beta1.NodeModulesConfigSpec{ + Modules: []kmmv1beta1.NodeModuleSpec{spec0, spec1}, + }, + Status: kmmv1beta1.NodeModulesConfigStatus{ + Modules: []kmmv1beta1.NodeModuleStatus{status0, status2}, + }, + } + + contextWithValueMatch := gomock.AssignableToTypeOf( + reflect.TypeOf((*context.Context)(nil)).Elem(), + ) + + gomock.InOrder( + kubeClient. + EXPECT(). + Get(ctx, nmcNsn, &kmmv1beta1.NodeModulesConfig{}). + Do(func(_ context.Context, _ types.NamespacedName, kubeNmc ctrlclient.Object, _ ...ctrlclient.Options) { + *kubeNmc.(*kmmv1beta1.NodeModulesConfig) = *nmc + }), + wh.EXPECT().SyncStatus(ctx, nmc), + wh.EXPECT().ProcessModuleSpec(contextWithValueMatch, nmc, &spec0, &status0), + wh.EXPECT().ProcessModuleSpec(contextWithValueMatch, nmc, &spec1, nil), + wh.EXPECT().ProcessOrphanModuleStatus(contextWithValueMatch, nmc, &status2), + ) + + Expect( + r.Reconcile(ctx, req), + ).To( + BeZero(), + ) + }) +}) + +var moduleConfig = kmmv1beta1.ModuleConfig{ + KernelVersion: "kernel version", + ContainerImage: "container image", + InsecurePull: true, + InTreeModuleToRemove: "intree", + Modprobe: kmmv1beta1.ModprobeSpec{ + ModuleName: "test", + Parameters: []string{"a", "b"}, + DirName: "/dir", + Args: nil, + RawArgs: nil, + FirmwarePath: "/firmware-path", + ModulesLoadingOrder: []string{"a", "b", "c"}, + }, +} + +var _ = Describe("workerHelper_ProcessModuleSpec", func() { + const ( + name = "name" + namespace = "namespace" + ) + + var ( + ctx = context.TODO() + + client *testclient.MockClient + pm *MockPodManager + wh WorkerHelper + ) + + BeforeEach(func() { + ctrl := gomock.NewController(GinkgoT()) + client = testclient.NewMockClient(ctrl) + pm = NewMockPodManager(ctrl) + wh = NewWorkerHelper(client, pm) + }) + + It("should create a loader Pod if the corresponding status is missing", func() { + nmc := &kmmv1beta1.NodeModulesConfig{} + spec := &kmmv1beta1.NodeModuleSpec{Name: name, Namespace: namespace} + + pm.EXPECT().CreateLoaderPod(ctx, nmc, spec) + + Expect( + wh.ProcessModuleSpec(ctx, nmc, spec, nil), + ).NotTo( + HaveOccurred(), + ) + }) + + It("should create an unloader Pod if the spec is different from the status", func() { + nmc := &kmmv1beta1.NodeModulesConfig{} + + spec := &kmmv1beta1.NodeModuleSpec{ + Name: name, + Namespace: namespace, + Config: kmmv1beta1.ModuleConfig{ContainerImage: "old-container-image"}, + } + + status := &kmmv1beta1.NodeModuleStatus{ + Name: name, + Namespace: namespace, + Config: &kmmv1beta1.ModuleConfig{ContainerImage: "new-container-image"}, + } + + pm.EXPECT().CreateUnloaderPod(ctx, nmc, status) + + Expect( + wh.ProcessModuleSpec(ctx, nmc, spec, status), + ).NotTo( + HaveOccurred(), + ) + }) + + It("should do nothing if InProgress is true, even though the config is different", func() { + spec := &kmmv1beta1.NodeModuleSpec{ + Name: name, + Namespace: namespace, + Config: kmmv1beta1.ModuleConfig{ContainerImage: "old-container-image"}, + } + + status := &kmmv1beta1.NodeModuleStatus{ + Name: name, + Namespace: namespace, + Config: &kmmv1beta1.ModuleConfig{ContainerImage: "new-container-image"}, + InProgress: true, + } + + Expect( + wh.ProcessModuleSpec(ctx, &kmmv1beta1.NodeModulesConfig{}, spec, status), + ).NotTo( + HaveOccurred(), + ) + }) + + It("should return an error if the node has no ready condition", func() { + nmc := &kmmv1beta1.NodeModulesConfig{ + ObjectMeta: metav1.ObjectMeta{Name: nmcName}, + } + + spec := &kmmv1beta1.NodeModuleSpec{ + Config: moduleConfig, + Name: name, + Namespace: namespace, + } + + now := metav1.Now() + + status := &kmmv1beta1.NodeModuleStatus{ + Config: &moduleConfig, + Name: name, + Namespace: namespace, + LastTransitionTime: &now, + } + + client.EXPECT().Get(ctx, types.NamespacedName{Name: nmcName}, &v1.Node{}) + + Expect( + wh.ProcessModuleSpec(ctx, nmc, spec, status), + ).To( + HaveOccurred(), + ) + }) + + nmc := &kmmv1beta1.NodeModulesConfig{ + ObjectMeta: metav1.ObjectMeta{Name: nmcName}, + } + + spec := &kmmv1beta1.NodeModuleSpec{ + Config: moduleConfig, + Name: name, + Namespace: namespace, + } + + now := metav1.Now() + + status := &kmmv1beta1.NodeModuleStatus{ + Config: &moduleConfig, + Name: name, + Namespace: namespace, + LastTransitionTime: &metav1.Time{Time: now.Add(-1 * time.Minute)}, + } + + DescribeTable( + "should create a loader Pod if status is older than the Ready condition", + func(cs v1.ConditionStatus, shouldCreate bool) { + getNode := client. + EXPECT(). + Get(ctx, types.NamespacedName{Name: nmcName}, &v1.Node{}). + Do(func(_ context.Context, _ types.NamespacedName, node *v1.Node, _ ...ctrl.Options) { + node.Status.Conditions = []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: cs, + LastTransitionTime: now, + }, + } + }) + + if shouldCreate { + pm.EXPECT().CreateLoaderPod(ctx, nmc, spec).After(getNode) + } + + Expect( + wh.ProcessModuleSpec(ctx, nmc, spec, status), + ).NotTo( + HaveOccurred(), + ) + }, + Entry(nil, v1.ConditionFalse, false), + Entry(nil, v1.ConditionTrue, true), + ) +}) + +var _ = Describe("workerHelper_ProcessOrphanModuleStatus", func() { + ctx := context.TODO() + + It("should do nothing if the status sync is in progress", func() { + nmc := &kmmv1beta1.NodeModulesConfig{} + status := &kmmv1beta1.NodeModuleStatus{InProgress: true} + + Expect( + NewWorkerHelper(nil, nil).ProcessOrphanModuleStatus(ctx, nmc, status), + ).NotTo( + HaveOccurred(), + ) + }) + + It("should create an unloader Pod", func() { + ctrl := gomock.NewController(GinkgoT()) + client := testclient.NewMockClient(ctrl) + pm := NewMockPodManager(ctrl) + wh := NewWorkerHelper(client, pm) + + nmc := &kmmv1beta1.NodeModulesConfig{} + status := &kmmv1beta1.NodeModuleStatus{} + + pm.EXPECT().CreateUnloaderPod(ctx, nmc, status) + + Expect( + wh.ProcessOrphanModuleStatus(ctx, nmc, status), + ).NotTo( + HaveOccurred(), + ) + }) +}) + +var _ = Describe("workerHelper_SyncStatus", func() { + var ( + ctx = context.TODO() + + ctrl *gomock.Controller + kubeClient *testclient.MockClient + pm *MockPodManager + wh WorkerHelper + sw *testclient.MockStatusWriter + ) + + BeforeEach(func() { + ctrl = gomock.NewController(GinkgoT()) + kubeClient = testclient.NewMockClient(ctrl) + pm = NewMockPodManager(ctrl) + wh = NewWorkerHelper(kubeClient, pm) + sw = testclient.NewMockStatusWriter(ctrl) + }) + + It("should do nothing if there are no running pods for this NMC", func() { + pm.EXPECT().ListWorkerPodsOnNode(ctx, nmcName) + + nmc := &kmmv1beta1.NodeModulesConfig{ + ObjectMeta: metav1.ObjectMeta{Name: nmcName}, + } + + Expect( + wh.SyncStatus(ctx, nmc), + ).NotTo( + HaveOccurred(), + ) + }) + + It("should delete the pod if it is failed", func() { + const ( + podName = "pod-name" + podNamespace = "pod-namespace" + ) + + pod := v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: podNamespace, + Name: podName, + }, + Status: v1.PodStatus{Phase: v1.PodFailed}, + } + + nmc := &kmmv1beta1.NodeModulesConfig{ + ObjectMeta: metav1.ObjectMeta{Name: nmcName}, + } + + gomock.InOrder( + pm.EXPECT().ListWorkerPodsOnNode(ctx, nmcName).Return([]v1.Pod{pod}, nil), + pm.EXPECT().DeletePod(ctx, &pod), + kubeClient.EXPECT().Status().Return(sw), + sw.EXPECT().Patch(ctx, nmc, gomock.Any()), + ) + + Expect( + wh.SyncStatus(ctx, nmc), + ).NotTo( + HaveOccurred(), + ) + }) + + It("should set the in progress status if pods are pending or running", func() { + nmc := &kmmv1beta1.NodeModulesConfig{ + ObjectMeta: metav1.ObjectMeta{Name: nmcName}, + } + + pods := []v1.Pod{ + { + Status: v1.PodStatus{Phase: v1.PodRunning}, + }, + { + Status: v1.PodStatus{Phase: v1.PodPending}, + }, + } + + gomock.InOrder( + pm.EXPECT().ListWorkerPodsOnNode(ctx, nmcName).Return(pods, nil), + kubeClient.EXPECT().Status().Return(sw), + sw.EXPECT().Patch(ctx, nmc, gomock.Any()), + ) + + Expect( + wh.SyncStatus(ctx, nmc), + ).NotTo( + HaveOccurred(), + ) + + Expect(nmc.Status.Modules).To(HaveLen(1)) + Expect(nmc.Status.Modules[0].InProgress).To(BeTrue()) + }) + + It("should remove the status if an unloader pod was successful", func() { + const ( + modName = "module" + modNamespace = "namespace" + ) + + nmc := &kmmv1beta1.NodeModulesConfig{ + ObjectMeta: metav1.ObjectMeta{Name: nmcName}, + Status: kmmv1beta1.NodeModulesConfigStatus{ + Modules: []kmmv1beta1.NodeModuleStatus{ + { + Name: modName, + Namespace: modNamespace, + }, + }, + }, + } + + pod := v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: modNamespace, + Labels: map[string]string{ + actionLabelKey: WorkerActionUnload, + constants.ModuleNameLabel: modName, + }, + }, + Status: v1.PodStatus{Phase: v1.PodSucceeded}, + } + + gomock.InOrder( + pm.EXPECT().ListWorkerPodsOnNode(ctx, nmcName).Return([]v1.Pod{pod}, nil), + pm.EXPECT().DeletePod(ctx, &pod), + kubeClient.EXPECT().Status().Return(sw), + sw.EXPECT().Patch(ctx, nmc, gomock.Any()), + ) + + Expect( + wh.SyncStatus(ctx, nmc), + ).NotTo( + HaveOccurred(), + ) + + Expect(nmc.Status.Modules).To(BeEmpty()) + }) + + It("should add the status if a loader pod was successful", func() { + const ( + modName = "module" + modNamespace = "namespace" + ) + + nmc := &kmmv1beta1.NodeModulesConfig{ + ObjectMeta: metav1.ObjectMeta{Name: nmcName}, + } + + now := metav1.Now() + + cfg := kmmv1beta1.ModuleConfig{ + KernelVersion: "some-kernel-version", + ContainerImage: "some-container-image", + InsecurePull: true, + InTreeModuleToRemove: "intree", + Modprobe: kmmv1beta1.ModprobeSpec{ModuleName: "test"}, + } + + pod := v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: modNamespace, + Labels: map[string]string{ + actionLabelKey: WorkerActionLoad, + constants.ModuleNameLabel: modName, + }, + }, + Status: v1.PodStatus{ + Phase: v1.PodSucceeded, + ContainerStatuses: []v1.ContainerStatus{ + { + Name: "worker", + State: v1.ContainerState{ + Terminated: &v1.ContainerStateTerminated{FinishedAt: now}, + }, + }, + }, + }, + } + + Expect( + setWorkerConfigAnnotation(&pod, cfg), + ).NotTo( + HaveOccurred(), + ) + + gomock.InOrder( + pm.EXPECT().ListWorkerPodsOnNode(ctx, nmcName).Return([]v1.Pod{pod}, nil), + pm.EXPECT().DeletePod(ctx, &pod), + kubeClient.EXPECT().Status().Return(sw), + sw.EXPECT().Patch(ctx, nmc, gomock.Any()), + ) + + Expect( + wh.SyncStatus(ctx, nmc), + ).NotTo( + HaveOccurred(), + ) + + Expect(nmc.Status.Modules).To(HaveLen(1)) + + expectedStatus := kmmv1beta1.NodeModuleStatus{ + Config: &cfg, + LastTransitionTime: &now, + Name: modName, + Namespace: modNamespace, + } + + Expect(nmc.Status.Modules[0]).To(BeComparableTo(expectedStatus)) + }) +}) + +var _ = Describe("workerHelper_RemoveOrphanFinalizers", func() { + const nodeName = "node-name" + + var ( + ctx = context.TODO() + + kubeClient *testclient.MockClient + pm *MockPodManager + wh WorkerHelper + ) + + BeforeEach(func() { + ctrl := gomock.NewController(GinkgoT()) + kubeClient = testclient.NewMockClient(ctrl) + pm = NewMockPodManager(ctrl) + wh = NewWorkerHelper(kubeClient, pm) + }) + + It("should do nothing if no pods are present", func() { + pm.EXPECT().ListWorkerPodsOnNode(ctx, nodeName) + + Expect( + wh.RemoveOrphanFinalizers(ctx, nodeName), + ).NotTo( + HaveOccurred(), + ) + }) + + It("should patch to remove the finalizer if it is set", func() { + const ( + name = "my-pod" + namespace = "my-namespace" + ) + + podWithFinalizer := v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + Finalizers: []string{nodeModulesConfigFinalizer}, + }, + } + + podWithoutFinalizer := podWithFinalizer + podWithoutFinalizer.Finalizers = []string{} + + gomock.InOrder( + pm.EXPECT().ListWorkerPodsOnNode(ctx, nodeName).Return([]v1.Pod{podWithFinalizer, {}}, nil), + kubeClient.EXPECT().Patch(ctx, &podWithoutFinalizer, gomock.Any()), + ) + + Expect( + wh.RemoveOrphanFinalizers(ctx, nodeName), + ).NotTo( + HaveOccurred(), + ) + }) +}) + +const ( + moduleName = "my-module" + serviceAccountName = "some-sa" + workerImage = "worker-image" +) + +var _ = Describe("podManager_CreateLoaderPod", func() { + It("should work as expected", func() { + ctrl := gomock.NewController(GinkgoT()) + client := testclient.NewMockClient(ctrl) + + nmc := &kmmv1beta1.NodeModulesConfig{ + ObjectMeta: metav1.ObjectMeta{Name: nmcName}, + } + + nms := &kmmv1beta1.NodeModuleSpec{ + Name: moduleName, + Namespace: namespace, + Config: moduleConfig, + ServiceAccountName: serviceAccountName, + } + + expected := getBaseWorkerPod("load", WorkerActionLoad, nmc) + + Expect( + controllerutil.SetControllerReference(nmc, expected, scheme), + ).NotTo( + HaveOccurred(), + ) + + controllerutil.AddFinalizer(expected, nodeModulesConfigFinalizer) + + ctx := context.TODO() + + client.EXPECT().Create(ctx, cmpmock.DiffEq(expected)) + + Expect( + NewPodManager(client, workerImage, scheme).CreateLoaderPod(ctx, nmc, nms), + ).NotTo( + HaveOccurred(), + ) + }) +}) + +var _ = Describe("podManager_CreateUnloaderPod", func() { + It("should work as expected", func() { + ctrl := gomock.NewController(GinkgoT()) + client := testclient.NewMockClient(ctrl) + + nmc := &kmmv1beta1.NodeModulesConfig{ + ObjectMeta: metav1.ObjectMeta{Name: nmcName}, + } + + status := &kmmv1beta1.NodeModuleStatus{ + Name: moduleName, + Namespace: namespace, + Config: &moduleConfig, + ServiceAccountName: serviceAccountName, + } + + expected := getBaseWorkerPod("unload", WorkerActionUnload, nmc) + + ctx := context.TODO() + + client.EXPECT().Create(ctx, cmpmock.DiffEq(expected)) + + Expect( + NewPodManager(client, workerImage, scheme).CreateUnloaderPod(ctx, nmc, status), + ).NotTo( + HaveOccurred(), + ) + }) +}) + +var _ = Describe("podManager_DeletePod", func() { + ctx := context.TODO() + now := metav1.Now() + + DescribeTable( + "should work as expected", + func(deletionTimestamp *metav1.Time) { + ctrl := gomock.NewController(GinkgoT()) + kubeclient := testclient.NewMockClient(ctrl) + + pod := &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + DeletionTimestamp: deletionTimestamp, + Finalizers: []string{nodeModulesConfigFinalizer}, + }, + } + + patchedPod := pod + patchedPod.Finalizers = nil + + patch := kubeclient.EXPECT().Patch(ctx, patchedPod, gomock.Any()) + + if deletionTimestamp == nil { + kubeclient.EXPECT().Delete(ctx, patchedPod).After(patch) + } + + Expect( + NewPodManager(kubeclient, workerImage, scheme).DeletePod(ctx, patchedPod), + ).NotTo( + HaveOccurred(), + ) + }, + Entry("deletionTimestamp not set", nil), + Entry("deletionTimestamp set", &now), + ) +}) + +var _ = Describe("podManager_ListWorkerPodsOnNode", func() { + const nodeName = "some-node" + + var ( + ctx = context.TODO() + + kubeClient *testclient.MockClient + pm PodManager + ) + + BeforeEach(func() { + ctrl := gomock.NewController(GinkgoT()) + kubeClient = testclient.NewMockClient(ctrl) + pm = NewPodManager(kubeClient, workerImage, scheme) + }) + + opts := []interface{}{ + ctrlclient.HasLabels{actionLabelKey}, + ctrlclient.MatchingFields{".spec.nodeName": nodeName}, + } + + It("should return an error if the kube client encountered one", func() { + kubeClient.EXPECT().List(ctx, &v1.PodList{}, opts...).Return(errors.New("random error")) + + _, err := pm.ListWorkerPodsOnNode(ctx, nodeName) + Expect(err).To(HaveOccurred()) + }) + + It("should the list of pods", func() { + pods := []v1.Pod{ + { + ObjectMeta: metav1.ObjectMeta{Name: "pod-0"}, + }, + { + ObjectMeta: metav1.ObjectMeta{Name: "pod-1"}, + }, + } + + kubeClient. + EXPECT(). + List(ctx, &v1.PodList{}, opts...). + Do(func(_ context.Context, pl *v1.PodList, _ ...ctrlclient.ListOption) { + pl.Items = pods + }) + + Expect( + pm.ListWorkerPodsOnNode(ctx, nodeName), + ).To( + Equal(pods), + ) + }) +}) + +func getBaseWorkerPod(subcommand string, action WorkerAction, owner ctrlclient.Object) *v1.Pod { + GinkgoHelper() + + const ( + volNameLibModules = "lib-modules" + volNameUsrLibModules = "usr-lib-modules" + volNameVarLibFirmware = "var-lib-firmware" + ) + + hostPathDirectory := v1.HostPathDirectory + hostPathDirectoryOrCreate := v1.HostPathDirectoryOrCreate + + pod := v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: workerPodName(nmcName, moduleName), + Namespace: namespace, + Labels: map[string]string{ + actionLabelKey: string(action), + constants.ModuleNameLabel: moduleName, + }, + Annotations: map[string]string{configAnnotationKey: `containerImage: container image +inTreeModuleToRemove: intree +insecurePull: true +kernelVersion: kernel version +modprobe: + dirName: /dir + firmwarePath: /firmware-path + moduleName: test + modulesLoadingOrder: + - a + - b + - c + parameters: + - a + - b +`, + }, + }, + Spec: v1.PodSpec{ + Containers: []v1.Container{ + { + Name: "worker", + Image: workerImage, + Args: []string{"kmod", subcommand, "/etc/kmm-worker/config.yaml"}, + SecurityContext: &v1.SecurityContext{Privileged: pointer.Bool(true)}, + VolumeMounts: []v1.VolumeMount{ + { + Name: volNameConfig, + MountPath: "/etc/kmm-worker", + ReadOnly: true, + }, + { + Name: volNameLibModules, + MountPath: "/lib/modules", + ReadOnly: true, + }, + { + Name: volNameUsrLibModules, + MountPath: "/usr/lib/modules", + ReadOnly: true, + }, + { + Name: volNameVarLibFirmware, + MountPath: "/var/lib/firmware", + }, + }, + }, + }, + NodeName: nmcName, + RestartPolicy: v1.RestartPolicyNever, + ServiceAccountName: serviceAccountName, + Volumes: []v1.Volume{ + { + Name: volumeNameConfig, + VolumeSource: v1.VolumeSource{ + DownwardAPI: &v1.DownwardAPIVolumeSource{ + Items: []v1.DownwardAPIVolumeFile{ + { + Path: "config.yaml", + FieldRef: &v1.ObjectFieldSelector{ + FieldPath: fmt.Sprintf("metadata.annotations['%s']", configAnnotationKey), + }, + }, + }, + }, + }, + }, + { + Name: volNameLibModules, + VolumeSource: v1.VolumeSource{ + HostPath: &v1.HostPathVolumeSource{ + Path: "/lib/modules", + Type: &hostPathDirectory, + }, + }, + }, + { + Name: volNameUsrLibModules, + VolumeSource: v1.VolumeSource{ + HostPath: &v1.HostPathVolumeSource{ + Path: "/usr/lib/modules", + Type: &hostPathDirectory, + }, + }, + }, + { + Name: volNameVarLibFirmware, + VolumeSource: v1.VolumeSource{ + HostPath: &v1.HostPathVolumeSource{ + Path: "/var/lib/firmware", + Type: &hostPathDirectoryOrCreate, + }, + }, + }, + }, + }, + } + + Expect( + controllerutil.SetControllerReference(owner, &pod, scheme), + ).NotTo( + HaveOccurred(), + ) + + controllerutil.AddFinalizer(&pod, nodeModulesConfigFinalizer) + + return &pod +} diff --git a/go.mod b/go.mod index 81d12adc0..2e995ed38 100644 --- a/go.mod +++ b/go.mod @@ -4,6 +4,7 @@ go 1.20 require ( github.com/a8m/envsubst v1.4.2 + github.com/budougumi0617/cmpmock v0.0.4 github.com/go-logr/logr v1.2.4 github.com/google/go-cmp v0.5.9 github.com/google/go-containerregistry v0.16.1 @@ -24,6 +25,7 @@ require ( k8s.io/utils v0.0.0-20230505201702-9f6742963106 open-cluster-management.io/api v0.11.0 sigs.k8s.io/controller-runtime v0.15.1 + sigs.k8s.io/yaml v1.3.0 ) require ( @@ -44,6 +46,7 @@ require ( github.com/go-task/slim-sprig v0.0.0-20230315185526-52ccab3ef572 // indirect github.com/gogo/protobuf v1.3.2 // indirect github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect + github.com/golang/mock v1.5.0 // indirect github.com/golang/protobuf v1.5.3 // indirect github.com/google/gnostic v0.6.9 // indirect github.com/google/gofuzz v1.2.0 // indirect @@ -87,5 +90,4 @@ require ( k8s.io/kube-openapi v0.0.0-20230515203736-54b630e78af5 // indirect sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect sigs.k8s.io/structured-merge-diff/v4 v4.2.3 // indirect - sigs.k8s.io/yaml v1.3.0 // indirect ) diff --git a/go.sum b/go.sum index cff57ade5..1da43d420 100644 --- a/go.sum +++ b/go.sum @@ -8,6 +8,8 @@ github.com/a8m/envsubst v1.4.2/go.mod h1:MVUTQNGQ3tsjOOtKCNd+fl8RzhsXcDvvAEzkhGt github.com/antihax/optional v1.0.0/go.mod h1:uupD/76wgC+ih3iEmQUL+0Ugr19nfwCT1kdvxnR2qWY= github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM= github.com/beorn7/perks v1.0.1/go.mod h1:G2ZrVWU2WbWT9wwq4/hrbKbnv/1ERSJQ0ibhJ6rlkpw= +github.com/budougumi0617/cmpmock v0.0.4 h1:WSSSkN8zh57MLFsMbHe0svW3sP7ZksDyqg4j8tHijdw= +github.com/budougumi0617/cmpmock v0.0.4/go.mod h1:x5H3AmbT7AipA7u65AFnHxC+bYom+txnkkSH1X+ZIIw= github.com/buger/jsonparser v1.1.1/go.mod h1:6RYKKt7H4d4+iWqouImQ9R2FZql3VbhNgx27UK13J/0= github.com/census-instrumentation/opencensus-proto v0.2.1/go.mod h1:f6KPmirojxKA12rnyqOA5BBL4O983OfeGPqjHWSTneU= github.com/cespare/xxhash v1.1.0/go.mod h1:XrSqR1VqqWfGrhpAt58auRo0WTKS1nRRg3ghfAqPWnc= @@ -70,6 +72,8 @@ github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b/go.mod h1:SBH7ygxi8pfU github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da h1:oI5xCqsCo564l8iNU+DwB5epxmsaqB+rhGL0m5jtYqE= github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da/go.mod h1:cIg4eruTrX1D+g88fzRXU5OdNfaM+9IcxsU14FzY7Hc= github.com/golang/mock v1.1.1/go.mod h1:oTYuIxOrZwtPieC+H1uAHpcLFnEyAGVDL/k47Jfbm0A= +github.com/golang/mock v1.5.0 h1:jlYHihg//f7RRwuPfptm04yp4s7O6Kw8EZiVYIGcH0g= +github.com/golang/mock v1.5.0/go.mod h1:CWnOUgYIOo4TcNZ0wHX3YZCqsaM1I1Jvs6v3mP3KVu8= github.com/golang/protobuf v1.2.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= github.com/golang/protobuf v1.3.1/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= github.com/golang/protobuf v1.3.2/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= diff --git a/internal/cmd/cmdutils.go b/internal/cmd/cmdutils.go index 6fb5e87bb..2c4ef64ec 100644 --- a/internal/cmd/cmdutils.go +++ b/internal/cmd/cmdutils.go @@ -17,7 +17,7 @@ func FatalError(l logr.Logger, err error, msg string, fields ...interface{}) { func GetEnvOrFatalError(name string, logger logr.Logger) string { val := os.Getenv(name) if val == "" { - FatalError(logger, errors.New("empty value"), "Could not get the environment variable", "name", val) + FatalError(logger, errors.New("empty value"), "Could not get the environment variable", "name", name) } return val diff --git a/internal/filter/filter.go b/internal/filter/filter.go index e7f599efe..c6f10b799 100644 --- a/internal/filter/filter.go +++ b/internal/filter/filter.go @@ -395,3 +395,7 @@ func NodeLabelModuleVersionUpdatePredicate(logger logr.Logger) predicate.Predica }, } } + +func SkipDeletions() predicate.Predicate { + return skipDeletions +}