diff --git a/controllers/module_reconciler.go b/controllers/module_reconciler.go index 154ae64a9..139101197 100644 --- a/controllers/module_reconciler.go +++ b/controllers/module_reconciler.go @@ -21,11 +21,13 @@ import ( "fmt" kmmv1beta1 "github.com/kubernetes-sigs/kernel-module-management/api/v1beta1" + "github.com/kubernetes-sigs/kernel-module-management/internal/auth" "github.com/kubernetes-sigs/kernel-module-management/internal/build" "github.com/kubernetes-sigs/kernel-module-management/internal/daemonset" "github.com/kubernetes-sigs/kernel-module-management/internal/filter" "github.com/kubernetes-sigs/kernel-module-management/internal/metrics" "github.com/kubernetes-sigs/kernel-module-management/internal/module" + "github.com/kubernetes-sigs/kernel-module-management/internal/registry" "github.com/kubernetes-sigs/kernel-module-management/internal/statusupdater" appsv1 "k8s.io/api/apps/v1" batchv1 "k8s.io/api/batch/v1" @@ -52,6 +54,7 @@ type ModuleReconciler struct { daemonAPI daemonset.DaemonSetCreator kernelAPI module.KernelMapper metricsAPI metrics.Metrics + registry registry.Registry filter *filter.Filter statusUpdaterAPI statusupdater.ModuleStatusUpdater } @@ -63,6 +66,7 @@ func NewModuleReconciler( kernelAPI module.KernelMapper, metricsAPI metrics.Metrics, filter *filter.Filter, + registry registry.Registry, statusUpdaterAPI statusupdater.ModuleStatusUpdater) *ModuleReconciler { return &ModuleReconciler{ Client: client, @@ -71,6 +75,7 @@ func NewModuleReconciler( kernelAPI: kernelAPI, metricsAPI: metricsAPI, filter: filter, + registry: registry, statusUpdaterAPI: statusUpdaterAPI, } } @@ -229,8 +234,14 @@ func (r *ModuleReconciler) handleBuild(ctx context.Context, if mod.Spec.ModuleLoader.Container.Build == nil && km.Build == nil { return false, nil } + exists, err := r.checkImageExists(ctx, mod, km) + if err != nil { + return false, fmt.Errorf("failed to check image existence for kernel %s: %w", kernelVersion, err) + } + if exists { + return false, nil + } - // [TODO] check access to the image - execute build only if needed (image is inaccessible) logger := log.FromContext(ctx).WithValues("kernel version", kernelVersion, "image", km.ContainerImage) buildCtx := log.IntoContext(ctx, logger) @@ -249,6 +260,23 @@ func (r *ModuleReconciler) handleBuild(ctx context.Context, return buildRes.Requeue, nil } +func (r *ModuleReconciler) checkImageExists(ctx context.Context, mod *kmmv1beta1.Module, km *kmmv1beta1.KernelMapping) (bool, error) { + var registryAuthGetter auth.RegistryAuthGetter + if irs := mod.Spec.ImageRepoSecret; irs != nil { + namespacedName := types.NamespacedName{ + Name: irs.Name, + Namespace: mod.Namespace, + } + registryAuthGetter = auth.NewRegistryAuthGetter(r.Client, namespacedName) + } + pullOptions := module.GetRelevantPullOptions(mod, km) + imageAvailable, err := r.registry.ImageExists(ctx, km.ContainerImage, pullOptions, registryAuthGetter) + if err != nil { + return false, fmt.Errorf("could not check if the image is available: %v", err) + } + return imageAvailable, nil +} + func (r *ModuleReconciler) handleDriverContainer(ctx context.Context, mod *kmmv1beta1.Module, km *kmmv1beta1.KernelMapping, diff --git a/controllers/module_reconciler_test.go b/controllers/module_reconciler_test.go index 9661208d1..9f54c951a 100644 --- a/controllers/module_reconciler_test.go +++ b/controllers/module_reconciler_test.go @@ -5,11 +5,13 @@ import ( "github.com/golang/mock/gomock" kmmv1beta1 "github.com/kubernetes-sigs/kernel-module-management/api/v1beta1" + "github.com/kubernetes-sigs/kernel-module-management/internal/auth" "github.com/kubernetes-sigs/kernel-module-management/internal/build" "github.com/kubernetes-sigs/kernel-module-management/internal/client" "github.com/kubernetes-sigs/kernel-module-management/internal/daemonset" "github.com/kubernetes-sigs/kernel-module-management/internal/metrics" "github.com/kubernetes-sigs/kernel-module-management/internal/module" + "github.com/kubernetes-sigs/kernel-module-management/internal/registry" "github.com/kubernetes-sigs/kernel-module-management/internal/statusupdater" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -27,430 +29,582 @@ const ( namespace = "namespace" ) -var _ = Describe("ModuleReconciler", func() { - Describe("Reconcile", func() { - var ( - ctrl *gomock.Controller - clnt *client.MockClient - mockBM *build.MockManager - mockDC *daemonset.MockDaemonSetCreator - mockKM *module.MockKernelMapper - mockMetrics *metrics.MockMetrics - mockSU *statusupdater.MockModuleStatusUpdater - ) +var _ = Describe("ModuleReconciler_Reconcile", func() { + var ( + ctrl *gomock.Controller + clnt *client.MockClient + mockBM *build.MockManager + mockDC *daemonset.MockDaemonSetCreator + mockKM *module.MockKernelMapper + mockMetrics *metrics.MockMetrics + mockSU *statusupdater.MockModuleStatusUpdater + mockRegistry *registry.MockRegistry + ) + + BeforeEach(func() { + ctrl = gomock.NewController(GinkgoT()) + clnt = client.NewMockClient(ctrl) + mockBM = build.NewMockManager(ctrl) + mockDC = daemonset.NewMockDaemonSetCreator(ctrl) + mockKM = module.NewMockKernelMapper(ctrl) + mockMetrics = metrics.NewMockMetrics(ctrl) + mockSU = statusupdater.NewMockModuleStatusUpdater(ctrl) + mockRegistry = registry.NewMockRegistry(ctrl) + }) - BeforeEach(func() { - ctrl = gomock.NewController(GinkgoT()) - clnt = client.NewMockClient(ctrl) - mockBM = build.NewMockManager(ctrl) - mockDC = daemonset.NewMockDaemonSetCreator(ctrl) - mockKM = module.NewMockKernelMapper(ctrl) - mockMetrics = metrics.NewMockMetrics(ctrl) - mockSU = statusupdater.NewMockModuleStatusUpdater(ctrl) - }) - - const moduleName = "test-module" - - nsn := types.NamespacedName{ - Name: moduleName, - Namespace: namespace, - } + const moduleName = "test-module" - req := reconcile.Request{NamespacedName: nsn} + nsn := types.NamespacedName{ + Name: moduleName, + Namespace: namespace, + } - ctx := context.Background() + req := reconcile.Request{NamespacedName: nsn} - It("should do nothing if the Module is not available anymore", func() { - clnt. - EXPECT(). - Get(ctx, nsn, &kmmv1beta1.Module{}). - Return( - apierrors.NewNotFound(schema.GroupResource{}, moduleName), - ) + ctx := context.Background() - mr := NewModuleReconciler(clnt, mockBM, mockDC, mockKM, mockMetrics, nil, mockSU) - Expect( - mr.Reconcile(ctx, req), - ).To( - Equal(reconcile.Result{}), + It("should do nothing if the Module is not available anymore", func() { + clnt. + EXPECT(). + Get(ctx, nsn, &kmmv1beta1.Module{}). + Return( + apierrors.NewNotFound(schema.GroupResource{}, moduleName), ) - }) - It("should do nothing when no nodes match the selector", func() { - mod := kmmv1beta1.Module{ - ObjectMeta: metav1.ObjectMeta{ - Name: moduleName, - Namespace: namespace, + mr := NewModuleReconciler(clnt, mockBM, mockDC, mockKM, mockMetrics, nil, mockRegistry, mockSU) + Expect( + mr.Reconcile(ctx, req), + ).To( + Equal(reconcile.Result{}), + ) + }) + + It("should do nothing when no nodes match the selector", func() { + mod := kmmv1beta1.Module{ + ObjectMeta: metav1.ObjectMeta{ + Name: moduleName, + Namespace: namespace, + }, + Spec: kmmv1beta1.ModuleSpec{ + Selector: map[string]string{"key": "value"}, + }, + } + + gomock.InOrder( + clnt.EXPECT().Get(ctx, req.NamespacedName, gomock.Any()).DoAndReturn( + func(_ interface{}, _ interface{}, m *kmmv1beta1.Module) error { + m.ObjectMeta = mod.ObjectMeta + m.Spec = mod.Spec + return nil }, - Spec: kmmv1beta1.ModuleSpec{ - Selector: map[string]string{"key": "value"}, + ), + clnt.EXPECT().List(ctx, gomock.Any(), gomock.Any()).DoAndReturn( + func(_ interface{}, list *kmmv1beta1.ModuleList, _ ...interface{}) error { + list.Items = []kmmv1beta1.Module{mod} + return nil }, - } - - gomock.InOrder( - clnt.EXPECT().Get(ctx, req.NamespacedName, gomock.Any()).DoAndReturn( - func(_ interface{}, _ interface{}, m *kmmv1beta1.Module) error { - m.ObjectMeta = mod.ObjectMeta - m.Spec = mod.Spec - return nil - }, - ), - clnt.EXPECT().List(ctx, gomock.Any(), gomock.Any()).DoAndReturn( - func(_ interface{}, list *kmmv1beta1.ModuleList, _ ...interface{}) error { - list.Items = []kmmv1beta1.Module{mod} - return nil - }, - ), - mockMetrics.EXPECT().SetExistingKMMOModules(1), - clnt.EXPECT().List(ctx, gomock.Any(), gomock.Any()).DoAndReturn( - func(_ interface{}, list *v1.NodeList, _ ...interface{}) error { - list.Items = []v1.Node{} - return nil - }, - ), - ) + ), + mockMetrics.EXPECT().SetExistingKMMOModules(1), + clnt.EXPECT().List(ctx, gomock.Any(), gomock.Any()).DoAndReturn( + func(_ interface{}, list *v1.NodeList, _ ...interface{}) error { + list.Items = []v1.Node{} + return nil + }, + ), + ) - mr := NewModuleReconciler(clnt, mockBM, mockDC, mockKM, mockMetrics, nil, mockSU) + mr := NewModuleReconciler(clnt, mockBM, mockDC, mockKM, mockMetrics, nil, mockRegistry, mockSU) - dsByKernelVersion := make(map[string]*appsv1.DaemonSet) + dsByKernelVersion := make(map[string]*appsv1.DaemonSet) - gomock.InOrder( - mockDC.EXPECT().ModuleDaemonSetsByKernelVersion(ctx, moduleName, namespace).Return(dsByKernelVersion, nil), - mockDC.EXPECT().GarbageCollect(ctx, dsByKernelVersion, sets.NewString()), - mockSU.EXPECT().ModuleUpdateStatus(ctx, &mod, []v1.Node{}, []v1.Node{}, dsByKernelVersion).Return(nil), - ) + gomock.InOrder( + mockDC.EXPECT().ModuleDaemonSetsByKernelVersion(ctx, moduleName, namespace).Return(dsByKernelVersion, nil), + mockDC.EXPECT().GarbageCollect(ctx, dsByKernelVersion, sets.NewString()), + mockSU.EXPECT().ModuleUpdateStatus(ctx, &mod, []v1.Node{}, []v1.Node{}, dsByKernelVersion).Return(nil), + ) - res, err := mr.Reconcile(context.Background(), req) - Expect(err).NotTo(HaveOccurred()) - Expect(res).To(Equal(reconcile.Result{})) - }) + res, err := mr.Reconcile(context.Background(), req) + Expect(err).NotTo(HaveOccurred()) + Expect(res).To(Equal(reconcile.Result{})) + }) + + It("should remove obsolete DaemonSets when no nodes match the selector", func() { + const kernelVersion = "1.2.3" + + mod := kmmv1beta1.Module{ + ObjectMeta: metav1.ObjectMeta{ + Name: moduleName, + Namespace: namespace, + }, + Spec: kmmv1beta1.ModuleSpec{ + Selector: map[string]string{"key": "value"}, + }, + } - It("should remove obsolete DaemonSets when no nodes match the selector", func() { - const kernelVersion = "1.2.3" + ds := appsv1.DaemonSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: "some-daemonset", + Namespace: namespace, + }, + } - mod := kmmv1beta1.Module{ - ObjectMeta: metav1.ObjectMeta{ - Name: moduleName, - Namespace: namespace, + gomock.InOrder( + clnt.EXPECT().Get(ctx, req.NamespacedName, gomock.Any()).DoAndReturn( + func(_ interface{}, _ interface{}, m *kmmv1beta1.Module) error { + m.ObjectMeta = mod.ObjectMeta + m.Spec = mod.Spec + return nil }, - Spec: kmmv1beta1.ModuleSpec{ - Selector: map[string]string{"key": "value"}, + ), + clnt.EXPECT().List(ctx, gomock.Any(), gomock.Any()).DoAndReturn( + func(_ interface{}, list *kmmv1beta1.ModuleList, _ ...interface{}) error { + list.Items = []kmmv1beta1.Module{mod} + return nil }, - } - - ds := appsv1.DaemonSet{ - ObjectMeta: metav1.ObjectMeta{ - Name: "some-daemonset", - Namespace: namespace, + ), + mockMetrics.EXPECT().SetExistingKMMOModules(1), + clnt.EXPECT().List(ctx, gomock.Any(), gomock.Any()).DoAndReturn( + func(_ interface{}, list *v1.NodeList, _ ...interface{}) error { + list.Items = []v1.Node{} + return nil }, - } - - gomock.InOrder( - clnt.EXPECT().Get(ctx, req.NamespacedName, gomock.Any()).DoAndReturn( - func(_ interface{}, _ interface{}, m *kmmv1beta1.Module) error { - m.ObjectMeta = mod.ObjectMeta - m.Spec = mod.Spec - return nil - }, - ), - clnt.EXPECT().List(ctx, gomock.Any(), gomock.Any()).DoAndReturn( - func(_ interface{}, list *kmmv1beta1.ModuleList, _ ...interface{}) error { - list.Items = []kmmv1beta1.Module{mod} - return nil - }, - ), - mockMetrics.EXPECT().SetExistingKMMOModules(1), - clnt.EXPECT().List(ctx, gomock.Any(), gomock.Any()).DoAndReturn( - func(_ interface{}, list *v1.NodeList, _ ...interface{}) error { - list.Items = []v1.Node{} - return nil - }, - ), - ) + ), + ) - mr := NewModuleReconciler(clnt, mockBM, mockDC, mockKM, mockMetrics, nil, mockSU) + mr := NewModuleReconciler(clnt, mockBM, mockDC, mockKM, mockMetrics, nil, mockRegistry, mockSU) - dsByKernelVersion := map[string]*appsv1.DaemonSet{kernelVersion: &ds} + dsByKernelVersion := map[string]*appsv1.DaemonSet{kernelVersion: &ds} - gomock.InOrder( - mockDC.EXPECT().ModuleDaemonSetsByKernelVersion(ctx, moduleName, namespace).Return(dsByKernelVersion, nil), - mockDC.EXPECT().GarbageCollect(ctx, dsByKernelVersion, sets.NewString()), - mockSU.EXPECT().ModuleUpdateStatus(ctx, &mod, []v1.Node{}, []v1.Node{}, dsByKernelVersion).Return(nil), - ) + gomock.InOrder( + mockDC.EXPECT().ModuleDaemonSetsByKernelVersion(ctx, moduleName, namespace).Return(dsByKernelVersion, nil), + mockDC.EXPECT().GarbageCollect(ctx, dsByKernelVersion, sets.NewString()), + mockSU.EXPECT().ModuleUpdateStatus(ctx, &mod, []v1.Node{}, []v1.Node{}, dsByKernelVersion).Return(nil), + ) - res, err := mr.Reconcile(context.Background(), req) - Expect(err).NotTo(HaveOccurred()) - Expect(res).To(Equal(reconcile.Result{})) - }) + res, err := mr.Reconcile(context.Background(), req) + Expect(err).NotTo(HaveOccurred()) + Expect(res).To(Equal(reconcile.Result{})) + }) - It("should create a DaemonSet when a node matches the selector", func() { - const ( - imageName = "test-image" - kernelVersion = "1.2.3" - ) + It("should create a DaemonSet when a node matches the selector", func() { + const ( + imageName = "test-image" + kernelVersion = "1.2.3" + ) - mappings := []kmmv1beta1.KernelMapping{ + mappings := []kmmv1beta1.KernelMapping{ + { + ContainerImage: imageName, + Literal: kernelVersion, + }, + } + + osConfig := module.NodeOSConfig{} + + mod := kmmv1beta1.Module{ + ObjectMeta: metav1.ObjectMeta{ + Name: moduleName, + Namespace: namespace, + }, + Spec: kmmv1beta1.ModuleSpec{ + ModuleLoader: kmmv1beta1.ModuleLoaderSpec{ + Container: kmmv1beta1.ModuleLoaderContainerSpec{ + KernelMappings: mappings, + }, + }, + Selector: map[string]string{"key": "value"}, + }, + } + + nodeList := v1.NodeList{ + Items: []v1.Node{ { - ContainerImage: imageName, - Literal: kernelVersion, + ObjectMeta: metav1.ObjectMeta{ + Name: "node1", + Labels: map[string]string{"key": "value"}, + }, + Status: v1.NodeStatus{ + NodeInfo: v1.NodeSystemInfo{KernelVersion: kernelVersion}, + }, }, - } + }, + } - osConfig := module.NodeOSConfig{} + dsByKernelVersion := make(map[string]*appsv1.DaemonSet) - mod := kmmv1beta1.Module{ - ObjectMeta: metav1.ObjectMeta{ - Name: moduleName, - Namespace: namespace, + mr := NewModuleReconciler(clnt, mockBM, mockDC, mockKM, mockMetrics, nil, mockRegistry, mockSU) + + ds := appsv1.DaemonSet{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: moduleName + "-", + Namespace: namespace, + }, + } + + gomock.InOrder( + clnt.EXPECT().Get(ctx, req.NamespacedName, gomock.Any()).DoAndReturn( + func(_ interface{}, _ interface{}, m *kmmv1beta1.Module) error { + m.ObjectMeta = mod.ObjectMeta + m.Spec = mod.Spec + return nil }, - Spec: kmmv1beta1.ModuleSpec{ - ModuleLoader: kmmv1beta1.ModuleLoaderSpec{ - Container: kmmv1beta1.ModuleLoaderContainerSpec{ - KernelMappings: mappings, - }, - }, - Selector: map[string]string{"key": "value"}, + ), + clnt.EXPECT().List(ctx, gomock.Any(), gomock.Any()).DoAndReturn( + func(_ interface{}, list *kmmv1beta1.ModuleList, _ ...interface{}) error { + return nil }, - } - - nodeList := v1.NodeList{ - Items: []v1.Node{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "node1", - Labels: map[string]string{"key": "value"}, - }, - Status: v1.NodeStatus{ - NodeInfo: v1.NodeSystemInfo{KernelVersion: kernelVersion}, - }, - }, + ), + mockMetrics.EXPECT().SetExistingKMMOModules(0), + clnt.EXPECT().List(ctx, gomock.Any(), gomock.Any()).DoAndReturn( + func(_ interface{}, list *v1.NodeList, _ ...interface{}) error { + list.Items = nodeList.Items + return nil }, - } + ), + mockKM.EXPECT().GetNodeOSConfig(&nodeList.Items[0]).Return(&osConfig), + mockKM.EXPECT().FindMappingForKernel(mappings, kernelVersion).Return(&mappings[0], nil), + mockKM.EXPECT().PrepareKernelMapping(&mappings[0], &osConfig).Return(&mappings[0], nil), + mockDC.EXPECT().ModuleDaemonSetsByKernelVersion(ctx, moduleName, namespace).Return(dsByKernelVersion, nil), + clnt.EXPECT().Get(ctx, gomock.Any(), gomock.Any()).Return(apierrors.NewNotFound(schema.GroupResource{}, "whatever")), + mockDC.EXPECT().SetDriverContainerAsDesired(context.Background(), &ds, imageName, gomock.AssignableToTypeOf(mod), kernelVersion), + clnt.EXPECT().Create(ctx, gomock.Any()).Return(nil), + mockMetrics.EXPECT().SetCompletedStage(moduleName, namespace, kernelVersion, metrics.ModuleLoaderStage, false), + mockDC.EXPECT().GarbageCollect(ctx, dsByKernelVersion, sets.NewString(kernelVersion)), + mockSU.EXPECT().ModuleUpdateStatus(ctx, &mod, nodeList.Items, nodeList.Items, dsByKernelVersion).Return(nil), + ) - dsByKernelVersion := make(map[string]*appsv1.DaemonSet) + res, err := mr.Reconcile(context.Background(), req) + Expect(err).NotTo(HaveOccurred()) + Expect(res).To(Equal(reconcile.Result{})) + }) - mr := NewModuleReconciler(clnt, mockBM, mockDC, mockKM, mockMetrics, nil, mockSU) + It("should patch the DaemonSet when it already exists", func() { + const ( + imageName = "test-image" + kernelVersion = "1.2.3" + ) - ds := appsv1.DaemonSet{ - ObjectMeta: metav1.ObjectMeta{ - GenerateName: moduleName + "-", - Namespace: namespace, - }, - } - - gomock.InOrder( - clnt.EXPECT().Get(ctx, req.NamespacedName, gomock.Any()).DoAndReturn( - func(_ interface{}, _ interface{}, m *kmmv1beta1.Module) error { - m.ObjectMeta = mod.ObjectMeta - m.Spec = mod.Spec - return nil + osConfig := module.NodeOSConfig{} + + mappings := []kmmv1beta1.KernelMapping{ + { + ContainerImage: imageName, + Literal: kernelVersion, + }, + } + + nodeLabels := map[string]string{"key": "value"} + + mod := kmmv1beta1.Module{ + ObjectMeta: metav1.ObjectMeta{ + Name: moduleName, + Namespace: namespace, + }, + Spec: kmmv1beta1.ModuleSpec{ + ModuleLoader: kmmv1beta1.ModuleLoaderSpec{ + Container: kmmv1beta1.ModuleLoaderContainerSpec{ + KernelMappings: mappings, }, - ), - clnt.EXPECT().List(ctx, gomock.Any(), gomock.Any()).DoAndReturn( - func(_ interface{}, list *kmmv1beta1.ModuleList, _ ...interface{}) error { - return nil + }, + Selector: nodeLabels, + }, + } + + nodeList := v1.NodeList{ + Items: []v1.Node{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "node1", + Labels: nodeLabels, }, - ), - mockMetrics.EXPECT().SetExistingKMMOModules(0), - clnt.EXPECT().List(ctx, gomock.Any(), gomock.Any()).DoAndReturn( - func(_ interface{}, list *v1.NodeList, _ ...interface{}) error { - list.Items = nodeList.Items - return nil + Status: v1.NodeStatus{ + NodeInfo: v1.NodeSystemInfo{KernelVersion: kernelVersion}, }, - ), - mockKM.EXPECT().GetNodeOSConfig(&nodeList.Items[0]).Return(&osConfig), - mockKM.EXPECT().FindMappingForKernel(mappings, kernelVersion).Return(&mappings[0], nil), - mockKM.EXPECT().PrepareKernelMapping(&mappings[0], &osConfig).Return(&mappings[0], nil), - mockDC.EXPECT().ModuleDaemonSetsByKernelVersion(ctx, moduleName, namespace).Return(dsByKernelVersion, nil), - clnt.EXPECT().Get(ctx, gomock.Any(), gomock.Any()).Return(apierrors.NewNotFound(schema.GroupResource{}, "whatever")), - mockDC.EXPECT().SetDriverContainerAsDesired(context.Background(), &ds, imageName, gomock.AssignableToTypeOf(mod), kernelVersion), - clnt.EXPECT().Create(ctx, gomock.Any()).Return(nil), - mockMetrics.EXPECT().SetCompletedStage(moduleName, namespace, kernelVersion, metrics.ModuleLoaderStage, false), - mockDC.EXPECT().GarbageCollect(ctx, dsByKernelVersion, sets.NewString(kernelVersion)), - mockSU.EXPECT().ModuleUpdateStatus(ctx, &mod, nodeList.Items, nodeList.Items, dsByKernelVersion).Return(nil), - ) + }, + }, + } - res, err := mr.Reconcile(context.Background(), req) - Expect(err).NotTo(HaveOccurred()) - Expect(res).To(Equal(reconcile.Result{})) - }) + const ( + dsName = "some-daemonset" + dsNamespace = "test-namespace" + ) - It("should patch the DaemonSet when it already exists", func() { - const ( - imageName = "test-image" - kernelVersion = "1.2.3" - ) + ds := appsv1.DaemonSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: dsName, + Namespace: dsNamespace, + }, + } + + gomock.InOrder( + clnt.EXPECT().Get(ctx, req.NamespacedName, gomock.Any()).DoAndReturn( + func(_ interface{}, _ interface{}, m *kmmv1beta1.Module) error { + m.ObjectMeta = mod.ObjectMeta + m.Spec = mod.Spec + return nil + }, + ), + clnt.EXPECT().List(ctx, gomock.Any(), gomock.Any()).DoAndReturn( + func(_ interface{}, list *kmmv1beta1.ModuleList, _ ...interface{}) error { + list.Items = []kmmv1beta1.Module{mod} + return nil + }, + ), + mockMetrics.EXPECT().SetExistingKMMOModules(1), + clnt.EXPECT().List(ctx, gomock.Any(), gomock.Any()).DoAndReturn( + func(_ interface{}, list *v1.NodeList, _ ...interface{}) error { + list.Items = nodeList.Items + return nil + }, + ), + clnt.EXPECT().Get(ctx, gomock.Any(), gomock.Any()), + clnt.EXPECT().Patch(ctx, gomock.Any(), gomock.Any()), + ) - osConfig := module.NodeOSConfig{} + mr := NewModuleReconciler(clnt, mockBM, mockDC, mockKM, mockMetrics, nil, mockRegistry, mockSU) + + dsByKernelVersion := map[string]*appsv1.DaemonSet{kernelVersion: &ds} + + gomock.InOrder( + mockKM.EXPECT().GetNodeOSConfig(&nodeList.Items[0]).Return(&osConfig), + mockKM.EXPECT().FindMappingForKernel(mappings, kernelVersion).Return(&mappings[0], nil), + mockKM.EXPECT().PrepareKernelMapping(&mappings[0], &osConfig).Return(&mappings[0], nil), + mockDC.EXPECT().ModuleDaemonSetsByKernelVersion(ctx, moduleName, namespace).Return(dsByKernelVersion, nil), + mockDC.EXPECT().SetDriverContainerAsDesired(context.Background(), &ds, imageName, gomock.AssignableToTypeOf(mod), kernelVersion).Do( + func(ctx context.Context, d *appsv1.DaemonSet, _ string, _ kmmv1beta1.Module, _ string) { + d.SetLabels(map[string]string{"test": "test"}) + }), + mockDC.EXPECT().GarbageCollect(ctx, dsByKernelVersion, sets.NewString(kernelVersion)), + mockSU.EXPECT().ModuleUpdateStatus(ctx, &mod, nodeList.Items, nodeList.Items, dsByKernelVersion).Return(nil), + ) - mappings := []kmmv1beta1.KernelMapping{ - { - ContainerImage: imageName, - Literal: kernelVersion, + res, err := mr.Reconcile(context.Background(), req) + Expect(err).NotTo(HaveOccurred()) + Expect(res).To(Equal(reconcile.Result{})) + }) + + It("should create a Device plugin if defined in the module", func() { + const ( + imageName = "test-image" + kernelVersion = "1.2.3" + ) + + mappings := []kmmv1beta1.KernelMapping{ + { + ContainerImage: imageName, + Literal: kernelVersion, + }, + } + + mod := kmmv1beta1.Module{ + ObjectMeta: metav1.ObjectMeta{ + Name: moduleName, + Namespace: namespace, + }, + Spec: kmmv1beta1.ModuleSpec{ + DevicePlugin: &kmmv1beta1.DevicePluginSpec{}, + ModuleLoader: kmmv1beta1.ModuleLoaderSpec{ + Container: kmmv1beta1.ModuleLoaderContainerSpec{ + KernelMappings: mappings, + }, }, - } + Selector: map[string]string{"key": "value"}, + }, + } - nodeLabels := map[string]string{"key": "value"} + mr := NewModuleReconciler(clnt, mockBM, mockDC, mockKM, mockMetrics, nil, mockRegistry, mockSU) + + ds := appsv1.DaemonSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: moduleName + "-device-plugin", + Namespace: namespace, + }, + } - mod := kmmv1beta1.Module{ - ObjectMeta: metav1.ObjectMeta{ - Name: moduleName, - Namespace: namespace, + gomock.InOrder( + clnt.EXPECT().Get(ctx, req.NamespacedName, gomock.Any()).DoAndReturn( + func(_ interface{}, _ interface{}, m *kmmv1beta1.Module) error { + m.ObjectMeta = mod.ObjectMeta + m.Spec = mod.Spec + return nil }, - Spec: kmmv1beta1.ModuleSpec{ - ModuleLoader: kmmv1beta1.ModuleLoaderSpec{ - Container: kmmv1beta1.ModuleLoaderContainerSpec{ - KernelMappings: mappings, - }, - }, - Selector: nodeLabels, + ), + clnt.EXPECT().List(ctx, gomock.Any(), gomock.Any()).DoAndReturn( + func(_ interface{}, list *kmmv1beta1.ModuleList, _ ...interface{}) error { + return nil }, - } - - nodeList := v1.NodeList{ - Items: []v1.Node{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "node1", - Labels: nodeLabels, - }, - Status: v1.NodeStatus{ - NodeInfo: v1.NodeSystemInfo{KernelVersion: kernelVersion}, - }, - }, + ), + mockMetrics.EXPECT().SetExistingKMMOModules(0), + clnt.EXPECT().List(ctx, gomock.Any(), gomock.Any()).DoAndReturn( + func(_ interface{}, list *v1.NodeList, _ ...interface{}) error { + list.Items = []v1.Node{} + return nil }, - } + ), + mockDC.EXPECT().ModuleDaemonSetsByKernelVersion(ctx, moduleName, namespace).Return(nil, nil), + clnt.EXPECT().Get(ctx, gomock.Any(), gomock.Any()).Return(apierrors.NewNotFound(schema.GroupResource{}, "whatever")), + clnt.EXPECT().Get(ctx, gomock.Any(), gomock.Any()).Return(apierrors.NewNotFound(schema.GroupResource{}, "whatever")), + mockDC.EXPECT().SetDevicePluginAsDesired(context.Background(), &ds, gomock.AssignableToTypeOf(&mod)), + clnt.EXPECT().Create(ctx, gomock.Any()).Return(nil), + mockMetrics.EXPECT().SetCompletedStage(moduleName, namespace, "", metrics.DevicePluginStage, false), + mockDC.EXPECT().GarbageCollect(ctx, nil, sets.NewString()), + mockSU.EXPECT().ModuleUpdateStatus(ctx, &mod, []v1.Node{}, []v1.Node{}, nil).Return(nil), + ) - const ( - dsName = "some-daemonset" - dsNamespace = "test-namespace" - ) + res, err := mr.Reconcile(context.Background(), req) + Expect(err).NotTo(HaveOccurred()) + Expect(res).To(Equal(reconcile.Result{})) + }) +}) - ds := appsv1.DaemonSet{ - ObjectMeta: metav1.ObjectMeta{ - Name: dsName, - Namespace: dsNamespace, - }, - } - - gomock.InOrder( - clnt.EXPECT().Get(ctx, req.NamespacedName, gomock.Any()).DoAndReturn( - func(_ interface{}, _ interface{}, m *kmmv1beta1.Module) error { - m.ObjectMeta = mod.ObjectMeta - m.Spec = mod.Spec - return nil - }, - ), - clnt.EXPECT().List(ctx, gomock.Any(), gomock.Any()).DoAndReturn( - func(_ interface{}, list *kmmv1beta1.ModuleList, _ ...interface{}) error { - list.Items = []kmmv1beta1.Module{mod} - return nil - }, - ), - mockMetrics.EXPECT().SetExistingKMMOModules(1), - clnt.EXPECT().List(ctx, gomock.Any(), gomock.Any()).DoAndReturn( - func(_ interface{}, list *v1.NodeList, _ ...interface{}) error { - list.Items = nodeList.Items - return nil - }, - ), - clnt.EXPECT().Get(ctx, gomock.Any(), gomock.Any()), - clnt.EXPECT().Patch(ctx, gomock.Any(), gomock.Any()), - ) +var _ = Describe("ModuleReconciler_handleBuild", func() { + var ( + ctrl *gomock.Controller + clnt *client.MockClient + mockBM *build.MockManager + mockDC *daemonset.MockDaemonSetCreator + mockKM *module.MockKernelMapper + mockMetrics *metrics.MockMetrics + mockSU *statusupdater.MockModuleStatusUpdater + mockRegistry *registry.MockRegistry + ) + + BeforeEach(func() { + ctrl = gomock.NewController(GinkgoT()) + clnt = client.NewMockClient(ctrl) + mockBM = build.NewMockManager(ctrl) + mockDC = daemonset.NewMockDaemonSetCreator(ctrl) + mockKM = module.NewMockKernelMapper(ctrl) + mockMetrics = metrics.NewMockMetrics(ctrl) + mockSU = statusupdater.NewMockModuleStatusUpdater(ctrl) + mockRegistry = registry.NewMockRegistry(ctrl) + }) - mr := NewModuleReconciler(clnt, mockBM, mockDC, mockKM, mockMetrics, nil, mockSU) - - dsByKernelVersion := map[string]*appsv1.DaemonSet{kernelVersion: &ds} - - gomock.InOrder( - mockKM.EXPECT().GetNodeOSConfig(&nodeList.Items[0]).Return(&osConfig), - mockKM.EXPECT().FindMappingForKernel(mappings, kernelVersion).Return(&mappings[0], nil), - mockKM.EXPECT().PrepareKernelMapping(&mappings[0], &osConfig).Return(&mappings[0], nil), - mockDC.EXPECT().ModuleDaemonSetsByKernelVersion(ctx, moduleName, namespace).Return(dsByKernelVersion, nil), - mockDC.EXPECT().SetDriverContainerAsDesired(context.Background(), &ds, imageName, gomock.AssignableToTypeOf(mod), kernelVersion).Do( - func(ctx context.Context, d *appsv1.DaemonSet, _ string, _ kmmv1beta1.Module, _ string) { - d.SetLabels(map[string]string{"test": "test"}) - }), - mockDC.EXPECT().GarbageCollect(ctx, dsByKernelVersion, sets.NewString(kernelVersion)), - mockSU.EXPECT().ModuleUpdateStatus(ctx, &mod, nodeList.Items, nodeList.Items, dsByKernelVersion).Return(nil), - ) + const ( + moduleName = "test-module" + kernelVersion = "1.2.3" + imageName = "test-image" + ) - res, err := mr.Reconcile(context.Background(), req) - Expect(err).NotTo(HaveOccurred()) - Expect(res).To(Equal(reconcile.Result{})) - }) + It("Build missing in module and in kernel mapping", func() { + km := &kmmv1beta1.KernelMapping{ + ContainerImage: imageName, + Literal: kernelVersion, + } - It("should create a Device plugin if defined in the module", func() { - const ( - imageName = "test-image" - kernelVersion = "1.2.3" - ) + mod := &kmmv1beta1.Module{} - mappings := []kmmv1beta1.KernelMapping{ - { - ContainerImage: imageName, - Literal: kernelVersion, - }, - } + mr := NewModuleReconciler(clnt, mockBM, mockDC, mockKM, mockMetrics, nil, mockRegistry, mockSU) - mod := kmmv1beta1.Module{ - ObjectMeta: metav1.ObjectMeta{ - Name: moduleName, - Namespace: namespace, - }, - Spec: kmmv1beta1.ModuleSpec{ - DevicePlugin: &kmmv1beta1.DevicePluginSpec{}, - ModuleLoader: kmmv1beta1.ModuleLoaderSpec{ - Container: kmmv1beta1.ModuleLoaderContainerSpec{ - KernelMappings: mappings, - }, - }, - Selector: map[string]string{"key": "value"}, - }, - } + res, err := mr.handleBuild(context.Background(), mod, km, kernelVersion) + Expect(err).NotTo(HaveOccurred()) + Expect(res).To(BeFalse()) + }) - mr := NewModuleReconciler(clnt, mockBM, mockDC, mockKM, mockMetrics, nil, mockSU) + It("Build present, image exists", func() { + km := &kmmv1beta1.KernelMapping{ + ContainerImage: imageName, + Literal: kernelVersion, + Build: &kmmv1beta1.Build{}, + } + mod := &kmmv1beta1.Module{ + ObjectMeta: metav1.ObjectMeta{ + Name: moduleName, + Namespace: namespace, + }, + Spec: kmmv1beta1.ModuleSpec{ + ImageRepoSecret: &v1.LocalObjectReference{Name: "pull-push-secret"}, + }, + } - ds := appsv1.DaemonSet{ - ObjectMeta: metav1.ObjectMeta{ - Name: moduleName + "-device-plugin", - Namespace: namespace, - }, - } - - gomock.InOrder( - clnt.EXPECT().Get(ctx, req.NamespacedName, gomock.Any()).DoAndReturn( - func(_ interface{}, _ interface{}, m *kmmv1beta1.Module) error { - m.ObjectMeta = mod.ObjectMeta - m.Spec = mod.Spec - return nil - }, - ), - clnt.EXPECT().List(ctx, gomock.Any(), gomock.Any()).DoAndReturn( - func(_ interface{}, list *kmmv1beta1.ModuleList, _ ...interface{}) error { - return nil - }, - ), - mockMetrics.EXPECT().SetExistingKMMOModules(0), - clnt.EXPECT().List(ctx, gomock.Any(), gomock.Any()).DoAndReturn( - func(_ interface{}, list *v1.NodeList, _ ...interface{}) error { - list.Items = []v1.Node{} - return nil - }, - ), - mockDC.EXPECT().ModuleDaemonSetsByKernelVersion(ctx, moduleName, namespace).Return(nil, nil), - clnt.EXPECT().Get(ctx, gomock.Any(), gomock.Any()).Return(apierrors.NewNotFound(schema.GroupResource{}, "whatever")), - clnt.EXPECT().Get(ctx, gomock.Any(), gomock.Any()).Return(apierrors.NewNotFound(schema.GroupResource{}, "whatever")), - mockDC.EXPECT().SetDevicePluginAsDesired(context.Background(), &ds, gomock.AssignableToTypeOf(&mod)), - clnt.EXPECT().Create(ctx, gomock.Any()).Return(nil), - mockMetrics.EXPECT().SetCompletedStage(moduleName, namespace, "", metrics.DevicePluginStage, false), - mockDC.EXPECT().GarbageCollect(ctx, nil, sets.NewString()), - mockSU.EXPECT().ModuleUpdateStatus(ctx, &mod, []v1.Node{}, []v1.Node{}, nil).Return(nil), - ) + mockRegistry.EXPECT().ImageExists(context.Background(), imageName, nil, gomock.Any()).DoAndReturn( + func(_ interface{}, _ interface{}, _ interface{}, registryAuthGetter auth.RegistryAuthGetter) (bool, error) { + Expect(registryAuthGetter).ToNot(BeNil()) + return true, nil + }, + ) + + mr := NewModuleReconciler(clnt, mockBM, mockDC, mockKM, mockMetrics, nil, mockRegistry, mockSU) + res, err := mr.handleBuild(context.Background(), mod, km, kernelVersion) + Expect(err).NotTo(HaveOccurred()) + Expect(res).To(BeFalse()) + }) + + It("Build present, image does not exist, job created", func() { + km := &kmmv1beta1.KernelMapping{ + ContainerImage: imageName, + Literal: kernelVersion, + Build: &kmmv1beta1.Build{}, + } + mod := &kmmv1beta1.Module{ + ObjectMeta: metav1.ObjectMeta{ + Name: moduleName, + Namespace: namespace, + }, + Spec: kmmv1beta1.ModuleSpec{}, + } + buildRes := build.Result{Requeue: true, Status: build.StatusCreated} + gomock.InOrder( + mockRegistry.EXPECT().ImageExists(context.Background(), imageName, nil, gomock.Any()).Return(false, nil), + mockBM.EXPECT().Sync(gomock.Any(), *mod, *km, gomock.Any(), true).Return(buildRes, nil), + mockMetrics.EXPECT().SetCompletedStage(mod.Name, mod.Namespace, kernelVersion, metrics.BuildStage, false), + ) + + mr := NewModuleReconciler(clnt, mockBM, mockDC, mockKM, mockMetrics, nil, mockRegistry, mockSU) + res, err := mr.handleBuild(context.Background(), mod, km, kernelVersion) + Expect(err).NotTo(HaveOccurred()) + Expect(res).To(BeTrue()) + }) + + It("Build present, image does not exist, job created", func() { + km := &kmmv1beta1.KernelMapping{ + ContainerImage: imageName, + Literal: kernelVersion, + Build: &kmmv1beta1.Build{}, + } + mod := &kmmv1beta1.Module{ + ObjectMeta: metav1.ObjectMeta{ + Name: moduleName, + Namespace: namespace, + }, + Spec: kmmv1beta1.ModuleSpec{}, + } + buildRes := build.Result{Requeue: true, Status: build.StatusCreated} + gomock.InOrder( + mockRegistry.EXPECT().ImageExists(context.Background(), imageName, nil, gomock.Any()).Return(false, nil), + mockBM.EXPECT().Sync(gomock.Any(), *mod, *km, gomock.Any(), true).Return(buildRes, nil), + mockMetrics.EXPECT().SetCompletedStage(mod.Name, mod.Namespace, kernelVersion, metrics.BuildStage, false), + ) + + mr := NewModuleReconciler(clnt, mockBM, mockDC, mockKM, mockMetrics, nil, mockRegistry, mockSU) + res, err := mr.handleBuild(context.Background(), mod, km, kernelVersion) + Expect(err).NotTo(HaveOccurred()) + Expect(res).To(BeTrue()) + }) + + It("Build present, image does not exist, job completed", func() { + km := &kmmv1beta1.KernelMapping{ + ContainerImage: imageName, + Literal: kernelVersion, + Build: &kmmv1beta1.Build{}, + } + mod := &kmmv1beta1.Module{ + ObjectMeta: metav1.ObjectMeta{ + Name: moduleName, + Namespace: namespace, + }, + Spec: kmmv1beta1.ModuleSpec{}, + } + buildRes := build.Result{Requeue: false, Status: build.StatusCompleted} + gomock.InOrder( + mockRegistry.EXPECT().ImageExists(context.Background(), imageName, nil, gomock.Any()).Return(false, nil), + mockBM.EXPECT().Sync(gomock.Any(), *mod, *km, gomock.Any(), true).Return(buildRes, nil), + mockMetrics.EXPECT().SetCompletedStage(mod.Name, mod.Namespace, kernelVersion, metrics.BuildStage, true), + ) - res, err := mr.Reconcile(context.Background(), req) - Expect(err).NotTo(HaveOccurred()) - Expect(res).To(Equal(reconcile.Result{})) - }) + mr := NewModuleReconciler(clnt, mockBM, mockDC, mockKM, mockMetrics, nil, mockRegistry, mockSU) + res, err := mr.handleBuild(context.Background(), mod, km, kernelVersion) + Expect(err).NotTo(HaveOccurred()) + Expect(res).To(BeFalse()) }) }) diff --git a/controllers/node_kernel_reconciler_test.go b/controllers/node_kernel_reconciler_test.go index 6e633ce25..9fb52115a 100644 --- a/controllers/node_kernel_reconciler_test.go +++ b/controllers/node_kernel_reconciler_test.go @@ -14,96 +14,94 @@ import ( runtimectrl "sigs.k8s.io/controller-runtime" ) -var _ = Describe("NodeKernelReconciler", func() { - Describe("Reconcile", func() { - var ( - gCtrl *gomock.Controller - clnt *client.MockClient - ) - BeforeEach(func() { - gCtrl = gomock.NewController(GinkgoT()) - clnt = client.NewMockClient(gCtrl) - }) - const ( - kernelVersion = "1.2.3" - labelName = "label-name" - nodeName = "node-name" - ) +var _ = Describe("NodeKernelReconciler_Reconcile", func() { + var ( + gCtrl *gomock.Controller + clnt *client.MockClient + ) + BeforeEach(func() { + gCtrl = gomock.NewController(GinkgoT()) + clnt = client.NewMockClient(gCtrl) + }) + const ( + kernelVersion = "1.2.3" + labelName = "label-name" + nodeName = "node-name" + ) - It("should return an error if the node cannot be found anymore", func() { - ctx := context.Background() - clnt.EXPECT().Get(ctx, gomock.Any(), gomock.Any()).Return(errors.New("some error")) + It("should return an error if the node cannot be found anymore", func() { + ctx := context.Background() + clnt.EXPECT().Get(ctx, gomock.Any(), gomock.Any()).Return(errors.New("some error")) - nkr := NewNodeKernelReconciler(clnt, labelName, nil) - req := runtimectrl.Request{ - NamespacedName: types.NamespacedName{Name: nodeName}, - } + nkr := NewNodeKernelReconciler(clnt, labelName, nil) + req := runtimectrl.Request{ + NamespacedName: types.NamespacedName{Name: nodeName}, + } + + _, err := nkr.Reconcile(ctx, req) + Expect(err).To(HaveOccurred()) + }) - _, err := nkr.Reconcile(ctx, req) - Expect(err).To(HaveOccurred()) - }) + It("should set the label if it does not exist", func() { + node := v1.Node{ + ObjectMeta: metav1.ObjectMeta{Name: nodeName}, + Status: v1.NodeStatus{ + NodeInfo: v1.NodeSystemInfo{KernelVersion: kernelVersion}, + }, + } - It("should set the label if it does not exist", func() { - node := v1.Node{ - ObjectMeta: metav1.ObjectMeta{Name: nodeName}, - Status: v1.NodeStatus{ - NodeInfo: v1.NodeSystemInfo{KernelVersion: kernelVersion}, + ctx := context.Background() + gomock.InOrder( + clnt.EXPECT().Get(ctx, gomock.Any(), gomock.Any()).DoAndReturn( + func(_ interface{}, _ interface{}, n *v1.Node) error { + n.ObjectMeta = node.ObjectMeta + n.Status = node.Status + return nil }, - } + ), + clnt.EXPECT().Patch(ctx, gomock.Any(), gomock.Any()), + ) - ctx := context.Background() - gomock.InOrder( - clnt.EXPECT().Get(ctx, gomock.Any(), gomock.Any()).DoAndReturn( - func(_ interface{}, _ interface{}, n *v1.Node) error { - n.ObjectMeta = node.ObjectMeta - n.Status = node.Status - return nil - }, - ), - clnt.EXPECT().Patch(ctx, gomock.Any(), gomock.Any()), - ) + nkr := NewNodeKernelReconciler(clnt, labelName, nil) + req := runtimectrl.Request{ + NamespacedName: types.NamespacedName{Name: nodeName}, + } - nkr := NewNodeKernelReconciler(clnt, labelName, nil) - req := runtimectrl.Request{ - NamespacedName: types.NamespacedName{Name: nodeName}, - } + res, err := nkr.Reconcile(ctx, req) + Expect(err).NotTo(HaveOccurred()) + Expect(res).To(Equal(res)) + }) - res, err := nkr.Reconcile(ctx, req) - Expect(err).NotTo(HaveOccurred()) - Expect(res).To(Equal(res)) - }) + It("should set the label if it already exists", func() { + node := v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: nodeName, + Labels: map[string]string{kernelVersion: "4.5.6"}, + }, + Status: v1.NodeStatus{ + NodeInfo: v1.NodeSystemInfo{KernelVersion: kernelVersion}, + }, + } - It("should set the label if it already exists", func() { - node := v1.Node{ - ObjectMeta: metav1.ObjectMeta{ - Name: nodeName, - Labels: map[string]string{kernelVersion: "4.5.6"}, - }, - Status: v1.NodeStatus{ - NodeInfo: v1.NodeSystemInfo{KernelVersion: kernelVersion}, + ctx := context.Background() + gomock.InOrder( + clnt.EXPECT().Get(ctx, gomock.Any(), gomock.Any()).DoAndReturn( + func(_ interface{}, _ interface{}, n *v1.Node) error { + n.ObjectMeta = node.ObjectMeta + n.Status = node.Status + return nil }, - } - - ctx := context.Background() - gomock.InOrder( - clnt.EXPECT().Get(ctx, gomock.Any(), gomock.Any()).DoAndReturn( - func(_ interface{}, _ interface{}, n *v1.Node) error { - n.ObjectMeta = node.ObjectMeta - n.Status = node.Status - return nil - }, - ), - clnt.EXPECT().Patch(ctx, gomock.Any(), gomock.Any()), - ) + ), + clnt.EXPECT().Patch(ctx, gomock.Any(), gomock.Any()), + ) - nkr := NewNodeKernelReconciler(clnt, labelName, nil) - req := runtimectrl.Request{ - NamespacedName: types.NamespacedName{Name: nodeName}, - } + nkr := NewNodeKernelReconciler(clnt, labelName, nil) + req := runtimectrl.Request{ + NamespacedName: types.NamespacedName{Name: nodeName}, + } - res, err := nkr.Reconcile(ctx, req) - Expect(err).NotTo(HaveOccurred()) - Expect(res).To(Equal(res)) - }) + res, err := nkr.Reconcile(ctx, req) + Expect(err).NotTo(HaveOccurred()) + Expect(res).To(Equal(res)) }) }) diff --git a/internal/build/helper.go b/internal/build/helper.go index 312109286..f0924296e 100644 --- a/internal/build/helper.go +++ b/internal/build/helper.go @@ -12,7 +12,6 @@ import ( type Helper interface { ApplyBuildArgOverrides(args []v1beta1.BuildArg, overrides ...v1beta1.BuildArg) []v1beta1.BuildArg GetRelevantBuild(mod kmmv1beta1.Module, km kmmv1beta1.KernelMapping) *kmmv1beta1.Build - GetRelevantPullOptions(mod kmmv1beta1.Module, km kmmv1beta1.KernelMapping) *kmmv1beta1.PullOptions } type helper struct{} @@ -68,10 +67,3 @@ func (m *helper) GetRelevantBuild(mod kmmv1beta1.Module, km kmmv1beta1.KernelMap buildConfig.Secrets = append(buildConfig.Secrets, km.Build.Secrets...) return buildConfig } - -func (m *helper) GetRelevantPullOptions(mod kmmv1beta1.Module, km kmmv1beta1.KernelMapping) *kmmv1beta1.PullOptions { - if km.Pull != nil { - return km.Pull - } - return mod.Spec.ModuleLoader.Container.Pull -} diff --git a/internal/build/job/manager.go b/internal/build/job/manager.go index 0e79a4b01..d5cb1c50b 100644 --- a/internal/build/job/manager.go +++ b/internal/build/job/manager.go @@ -6,12 +6,9 @@ import ( "fmt" kmmv1beta1 "github.com/kubernetes-sigs/kernel-module-management/api/v1beta1" - "github.com/kubernetes-sigs/kernel-module-management/internal/auth" "github.com/kubernetes-sigs/kernel-module-management/internal/build" "github.com/kubernetes-sigs/kernel-module-management/internal/constants" - "github.com/kubernetes-sigs/kernel-module-management/internal/registry" batchv1 "k8s.io/api/batch/v1" - "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/log" ) @@ -19,18 +16,16 @@ import ( var errNoMatchingBuild = errors.New("no matching build") type jobManager struct { - client client.Client - registry registry.Registry - maker Maker - helper build.Helper + client client.Client + maker Maker + helper build.Helper } -func NewBuildManager(client client.Client, registry registry.Registry, maker Maker, helper build.Helper) *jobManager { +func NewBuildManager(client client.Client, maker Maker, helper build.Helper) *jobManager { return &jobManager{ - client: client, - registry: registry, - maker: maker, - helper: helper, + client: client, + maker: maker, + helper: helper, } } @@ -65,29 +60,9 @@ func (jbm *jobManager) getJob(ctx context.Context, mod kmmv1beta1.Module, target func (jbm *jobManager) Sync(ctx context.Context, mod kmmv1beta1.Module, m kmmv1beta1.KernelMapping, targetKernel string, pushImage bool) (build.Result, error) { logger := log.FromContext(ctx) - buildConfig := jbm.helper.GetRelevantBuild(mod, m) - imagePullOptions := jbm.helper.GetRelevantPullOptions(mod, m) - - var registryAuthGetter auth.RegistryAuthGetter - - if irs := mod.Spec.ImageRepoSecret; irs != nil { - namespacedName := types.NamespacedName{ - Name: irs.Name, - Namespace: mod.Namespace, - } - registryAuthGetter = auth.NewRegistryAuthGetter(jbm.client, namespacedName) - } - imageAvailable, err := jbm.registry.ImageExists(ctx, m.ContainerImage, imagePullOptions, registryAuthGetter) - if err != nil { - return build.Result{}, fmt.Errorf("could not check if the image is available: %v", err) - } - - if imageAvailable { - return build.Result{Status: build.StatusCompleted, Requeue: false}, nil - } - - logger.Info("Image not pull-able; building in-cluster") + logger.Info("Building in-cluster") + buildConfig := jbm.helper.GetRelevantBuild(mod, m) job, err := jbm.getJob(ctx, mod, targetKernel) if err != nil { if !errors.Is(err, errNoMatchingBuild) { diff --git a/internal/build/job/manager_test.go b/internal/build/job/manager_test.go index c17a02f53..c420015c7 100644 --- a/internal/build/job/manager_test.go +++ b/internal/build/job/manager_test.go @@ -6,15 +6,12 @@ import ( "github.com/golang/mock/gomock" kmmv1beta1 "github.com/kubernetes-sigs/kernel-module-management/api/v1beta1" - "github.com/kubernetes-sigs/kernel-module-management/internal/auth" "github.com/kubernetes-sigs/kernel-module-management/internal/build" "github.com/kubernetes-sigs/kernel-module-management/internal/client" "github.com/kubernetes-sigs/kernel-module-management/internal/constants" - registrypkg "github.com/kubernetes-sigs/kernel-module-management/internal/registry" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" batchv1 "k8s.io/api/batch/v1" - v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -40,11 +37,10 @@ var _ = Describe("JobManager", func() { Describe("Sync", func() { var ( - ctrl *gomock.Controller - clnt *client.MockClient - registry *registrypkg.MockRegistry - maker *MockMaker - helper *build.MockHelper + ctrl *gomock.Controller + clnt *client.MockClient + maker *MockMaker + helper *build.MockHelper ) const ( @@ -55,7 +51,6 @@ var _ = Describe("JobManager", func() { BeforeEach(func() { ctrl = gomock.NewController(GinkgoT()) clnt = client.NewMockClient(ctrl) - registry = registrypkg.NewMockRegistry(ctrl) maker = NewMockMaker(ctrl) helper = build.NewMockHelper(ctrl) }) @@ -67,37 +62,6 @@ var _ = Describe("JobManager", func() { ContainerImage: imageName, } - It("should return an error if there was an error getting the image", func() { - ctx := context.Background() - gomock.InOrder( - helper.EXPECT().GetRelevantBuild(gomock.Any(), km).Return(km.Build), - helper.EXPECT().GetRelevantPullOptions(gomock.Any(), km).Return(po), - registry.EXPECT().ImageExists(ctx, imageName, po, gomock.Any()).Return(false, errors.New("random error")), - ) - mgr := NewBuildManager(nil, registry, maker, helper) - - _, err := mgr.Sync(ctx, kmmv1beta1.Module{}, km, "", true) - Expect(err).To(HaveOccurred()) - }) - - It("should return StatusCompleted if the image already exists", func() { - ctx := context.Background() - - gomock.InOrder( - helper.EXPECT().GetRelevantBuild(gomock.Any(), km).Return(km.Build), - helper.EXPECT().GetRelevantPullOptions(gomock.Any(), km).Return(po), - registry.EXPECT().ImageExists(ctx, imageName, po, gomock.Any()).Return(true, nil), - ) - - mgr := NewBuildManager(nil, registry, maker, helper) - - Expect( - mgr.Sync(ctx, kmmv1beta1.Module{}, km, "", true), - ).To( - Equal(build.Result{Status: build.StatusCompleted}), - ) - }) - const ( moduleName = "module-name" kernelVersion = "1.2.3" @@ -118,20 +82,18 @@ var _ = Describe("JobManager", func() { Status: s, } ctx := context.Background() - clnt.EXPECT().List(ctx, gomock.Any(), gomock.Any()).DoAndReturn( - func(_ interface{}, list *batchv1.JobList, _ ...interface{}) error { - list.Items = []batchv1.Job{j} - return nil - }, - ) gomock.InOrder( helper.EXPECT().GetRelevantBuild(mod, km).Return(km.Build), - helper.EXPECT().GetRelevantPullOptions(gomock.Any(), km).Return(po), - registry.EXPECT().ImageExists(ctx, imageName, po, gomock.Any()).Return(false, nil), + clnt.EXPECT().List(ctx, gomock.Any(), gomock.Any()).DoAndReturn( + func(_ interface{}, list *batchv1.JobList, _ ...interface{}) error { + list.Items = []batchv1.Job{j} + return nil + }, + ), ) - mgr := NewBuildManager(clnt, registry, maker, helper) + mgr := NewBuildManager(clnt, maker, helper) res, err := mgr.Sync(ctx, mod, km, kernelVersion, true) @@ -152,13 +114,11 @@ var _ = Describe("JobManager", func() { gomock.InOrder( helper.EXPECT().GetRelevantBuild(mod, km).Return(km.Build), - helper.EXPECT().GetRelevantPullOptions(gomock.Any(), km).Return(po), - registry.EXPECT().ImageExists(ctx, imageName, po, gomock.Any()), maker.EXPECT().MakeJob(mod, km.Build, kernelVersion, km.ContainerImage, true).Return(nil, errors.New("random error")), ) clnt.EXPECT().List(ctx, gomock.Any(), gomock.Any(), gomock.Any()) - mgr := NewBuildManager(clnt, registry, maker, helper) + mgr := NewBuildManager(clnt, maker, helper) Expect( mgr.Sync(ctx, mod, km, kernelVersion, true), @@ -183,8 +143,6 @@ var _ = Describe("JobManager", func() { gomock.InOrder( helper.EXPECT().GetRelevantBuild(mod, km).Return(km.Build), - helper.EXPECT().GetRelevantPullOptions(gomock.Any(), km).Return(po), - registry.EXPECT().ImageExists(ctx, imageName, po, gomock.Any()), maker.EXPECT().MakeJob(mod, km.Build, kernelVersion, km.ContainerImage, true).Return(&j, nil), ) @@ -193,47 +151,7 @@ var _ = Describe("JobManager", func() { clnt.EXPECT().Create(ctx, &j), ) - mgr := NewBuildManager(clnt, registry, maker, helper) - - Expect( - mgr.Sync(ctx, mod, km, kernelVersion, true), - ).To( - Equal(build.Result{Requeue: true, Status: build.StatusCreated}), - ) - }) - - It("should use a non-nil RegistryAuthGetter if the imagePullSecret is set in the module", func() { - - ctx := context.Background() - - j := batchv1.Job{ - TypeMeta: metav1.TypeMeta{ - APIVersion: "batch/v1", - Kind: "Job", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: jobName, - Namespace: namespace, - }, - } - - mod.Spec.ImageRepoSecret = &v1.LocalObjectReference{Name: "pull-push-secret"} - - gomock.InOrder( - helper.EXPECT().GetRelevantBuild(mod, km).Return(km.Build), - helper.EXPECT().GetRelevantPullOptions(gomock.Any(), km).Return(po), - registry.EXPECT().ImageExists(ctx, imageName, po, gomock.Any()).DoAndReturn( - func(_ interface{}, _ interface{}, _ interface{}, registryAuthGetter auth.RegistryAuthGetter) (bool, error) { - Expect(registryAuthGetter).ToNot(BeNil()) - return false, nil - }, - ), - clnt.EXPECT().List(ctx, gomock.Any(), gomock.Any(), gomock.Any()), - maker.EXPECT().MakeJob(mod, km.Build, kernelVersion, km.ContainerImage, true).Return(&j, nil), - clnt.EXPECT().Create(ctx, &j), - ) - - mgr := NewBuildManager(clnt, registry, maker, helper) + mgr := NewBuildManager(clnt, maker, helper) Expect( mgr.Sync(ctx, mod, km, kernelVersion, true), diff --git a/internal/build/mock_helper.go b/internal/build/mock_helper.go index 0142beb24..df104d5b8 100644 --- a/internal/build/mock_helper.go +++ b/internal/build/mock_helper.go @@ -66,17 +66,3 @@ func (mr *MockHelperMockRecorder) GetRelevantBuild(mod, km interface{}) *gomock. mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetRelevantBuild", reflect.TypeOf((*MockHelper)(nil).GetRelevantBuild), mod, km) } - -// GetRelevantPullOptions mocks base method. -func (m *MockHelper) GetRelevantPullOptions(mod v1beta1.Module, km v1beta1.KernelMapping) *v1beta1.PullOptions { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetRelevantPullOptions", mod, km) - ret0, _ := ret[0].(*v1beta1.PullOptions) - return ret0 -} - -// GetRelevantPullOptions indicates an expected call of GetRelevantPullOptions. -func (mr *MockHelperMockRecorder) GetRelevantPullOptions(mod, km interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetRelevantPullOptions", reflect.TypeOf((*MockHelper)(nil).GetRelevantPullOptions), mod, km) -} diff --git a/internal/module/helper.go b/internal/module/helper.go new file mode 100644 index 000000000..2880b25ff --- /dev/null +++ b/internal/module/helper.go @@ -0,0 +1,12 @@ +package module + +import ( + kmmv1beta1 "github.com/kubernetes-sigs/kernel-module-management/api/v1beta1" +) + +func GetRelevantPullOptions(mod *kmmv1beta1.Module, km *kmmv1beta1.KernelMapping) *kmmv1beta1.PullOptions { + if km.Pull != nil { + return km.Pull + } + return mod.Spec.ModuleLoader.Container.Pull +} diff --git a/main.go b/main.go index 686b1d9f4..28d541ff4 100644 --- a/main.go +++ b/main.go @@ -123,14 +123,14 @@ func main() { registryAPI := registry.NewRegistry() helperAPI := build.NewHelper() makerAPI := job.NewMaker(helperAPI, scheme) - buildAPI := job.NewBuildManager(client, registryAPI, makerAPI, helperAPI) + buildAPI := job.NewBuildManager(client, makerAPI, helperAPI) daemonAPI := daemonset.NewCreator(client, kernelLabel, scheme) kernelAPI := module.NewKernelMapper() moduleStatusUpdaterAPI := statusupdater.NewModuleStatusUpdater(client, daemonAPI, metricsAPI) preflightStatusUpdaterAPI := statusupdater.NewPreflightStatusUpdater(client) preflightAPI := preflight.NewPreflightAPI(client, registryAPI, kernelAPI) - mc := controllers.NewModuleReconciler(client, buildAPI, daemonAPI, kernelAPI, metricsAPI, filter, moduleStatusUpdaterAPI) + mc := controllers.NewModuleReconciler(client, buildAPI, daemonAPI, kernelAPI, metricsAPI, filter, registryAPI, moduleStatusUpdaterAPI) if err = mc.SetupWithManager(mgr, kernelLabel); err != nil { setupLogger.Error(err, "unable to create controller", "controller", "Module")