From 4525c5330df543ec681209626bfc785b0c9cad01 Mon Sep 17 00:00:00 2001 From: TomerNewman Date: Thu, 5 Dec 2024 16:22:07 +0200 Subject: [PATCH] bugfix/updating-operand-image Until now, when the Operator image was upgraded, the operand was not upgraded unless the user updated the CR also. Now the operand uses the updated operand-image from the operator itself. --- api/v1/nodefeaturediscovery_types.go | 1 + .../mock_nodefeaturediscovery_reconciler.go | 32 +++--- .../nodefeaturediscovery_reconciler.go | 42 ++++---- .../nodefeaturediscovery_reconciler_test.go | 97 ++++++++++++------- internal/daemonset/daemonset.go | 12 +-- internal/daemonset/daemonset_test.go | 4 +- internal/daemonset/mock_daemonset.go | 16 +-- internal/deployment/deployment.go | 12 +-- internal/deployment/deployment_test.go | 4 +- internal/deployment/mock_deployment.go | 16 +-- 10 files changed, 137 insertions(+), 99 deletions(-) diff --git a/api/v1/nodefeaturediscovery_types.go b/api/v1/nodefeaturediscovery_types.go index 5adb25b0a..a0f965ef1 100644 --- a/api/v1/nodefeaturediscovery_types.go +++ b/api/v1/nodefeaturediscovery_types.go @@ -82,6 +82,7 @@ type OperandSpec struct { // Image defines the image to pull for the // NFD operand // [defaults to registry.k8s.io/nfd/node-feature-discovery] + // +kubebuilder:validation:Optional // +kubebuilder:validation:Pattern=[a-zA-Z0-9\-]+ Image string `json:"image,omitempty"` diff --git a/internal/controllers/mock_nodefeaturediscovery_reconciler.go b/internal/controllers/mock_nodefeaturediscovery_reconciler.go index 7de3eb272..24cfc835b 100644 --- a/internal/controllers/mock_nodefeaturediscovery_reconciler.go +++ b/internal/controllers/mock_nodefeaturediscovery_reconciler.go @@ -54,31 +54,31 @@ func (mr *MocknodeFeatureDiscoveryHelperAPIMockRecorder) finalizeComponents(ctx, } // handleGC mocks base method. -func (m *MocknodeFeatureDiscoveryHelperAPI) handleGC(ctx context.Context, nfdInstance *v1.NodeFeatureDiscovery) error { +func (m *MocknodeFeatureDiscoveryHelperAPI) handleGC(ctx context.Context, nfdInstance *v1.NodeFeatureDiscovery, operatorImage string) error { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "handleGC", ctx, nfdInstance) + ret := m.ctrl.Call(m, "handleGC", ctx, nfdInstance, operatorImage) ret0, _ := ret[0].(error) return ret0 } // handleGC indicates an expected call of handleGC. -func (mr *MocknodeFeatureDiscoveryHelperAPIMockRecorder) handleGC(ctx, nfdInstance any) *gomock.Call { +func (mr *MocknodeFeatureDiscoveryHelperAPIMockRecorder) handleGC(ctx, nfdInstance, operatorImage any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "handleGC", reflect.TypeOf((*MocknodeFeatureDiscoveryHelperAPI)(nil).handleGC), ctx, nfdInstance) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "handleGC", reflect.TypeOf((*MocknodeFeatureDiscoveryHelperAPI)(nil).handleGC), ctx, nfdInstance, operatorImage) } // handleMaster mocks base method. -func (m *MocknodeFeatureDiscoveryHelperAPI) handleMaster(ctx context.Context, nfdInstance *v1.NodeFeatureDiscovery) error { +func (m *MocknodeFeatureDiscoveryHelperAPI) handleMaster(ctx context.Context, nfdInstance *v1.NodeFeatureDiscovery, operatorImage string) error { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "handleMaster", ctx, nfdInstance) + ret := m.ctrl.Call(m, "handleMaster", ctx, nfdInstance, operatorImage) ret0, _ := ret[0].(error) return ret0 } // handleMaster indicates an expected call of handleMaster. -func (mr *MocknodeFeatureDiscoveryHelperAPIMockRecorder) handleMaster(ctx, nfdInstance any) *gomock.Call { +func (mr *MocknodeFeatureDiscoveryHelperAPIMockRecorder) handleMaster(ctx, nfdInstance, operatorImage any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "handleMaster", reflect.TypeOf((*MocknodeFeatureDiscoveryHelperAPI)(nil).handleMaster), ctx, nfdInstance) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "handleMaster", reflect.TypeOf((*MocknodeFeatureDiscoveryHelperAPI)(nil).handleMaster), ctx, nfdInstance, operatorImage) } // handlePrune mocks base method. @@ -125,31 +125,31 @@ func (mr *MocknodeFeatureDiscoveryHelperAPIMockRecorder) handleStatus(ctx, nfdIn } // handleTopology mocks base method. -func (m *MocknodeFeatureDiscoveryHelperAPI) handleTopology(ctx context.Context, nfdInstance *v1.NodeFeatureDiscovery) error { +func (m *MocknodeFeatureDiscoveryHelperAPI) handleTopology(ctx context.Context, nfdInstance *v1.NodeFeatureDiscovery, operatorImage string) error { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "handleTopology", ctx, nfdInstance) + ret := m.ctrl.Call(m, "handleTopology", ctx, nfdInstance, operatorImage) ret0, _ := ret[0].(error) return ret0 } // handleTopology indicates an expected call of handleTopology. -func (mr *MocknodeFeatureDiscoveryHelperAPIMockRecorder) handleTopology(ctx, nfdInstance any) *gomock.Call { +func (mr *MocknodeFeatureDiscoveryHelperAPIMockRecorder) handleTopology(ctx, nfdInstance, operatorImage any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "handleTopology", reflect.TypeOf((*MocknodeFeatureDiscoveryHelperAPI)(nil).handleTopology), ctx, nfdInstance) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "handleTopology", reflect.TypeOf((*MocknodeFeatureDiscoveryHelperAPI)(nil).handleTopology), ctx, nfdInstance, operatorImage) } // handleWorker mocks base method. -func (m *MocknodeFeatureDiscoveryHelperAPI) handleWorker(ctx context.Context, nfdInstance *v1.NodeFeatureDiscovery) error { +func (m *MocknodeFeatureDiscoveryHelperAPI) handleWorker(ctx context.Context, nfdInstance *v1.NodeFeatureDiscovery, operatorImage string) error { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "handleWorker", ctx, nfdInstance) + ret := m.ctrl.Call(m, "handleWorker", ctx, nfdInstance, operatorImage) ret0, _ := ret[0].(error) return ret0 } // handleWorker indicates an expected call of handleWorker. -func (mr *MocknodeFeatureDiscoveryHelperAPIMockRecorder) handleWorker(ctx, nfdInstance any) *gomock.Call { +func (mr *MocknodeFeatureDiscoveryHelperAPIMockRecorder) handleWorker(ctx, nfdInstance, operatorImage any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "handleWorker", reflect.TypeOf((*MocknodeFeatureDiscoveryHelperAPI)(nil).handleWorker), ctx, nfdInstance) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "handleWorker", reflect.TypeOf((*MocknodeFeatureDiscoveryHelperAPI)(nil).handleWorker), ctx, nfdInstance, operatorImage) } // hasFinalizer mocks base method. diff --git a/internal/controllers/nodefeaturediscovery_reconciler.go b/internal/controllers/nodefeaturediscovery_reconciler.go index 7faa5868a..84a02c462 100644 --- a/internal/controllers/nodefeaturediscovery_reconciler.go +++ b/internal/controllers/nodefeaturediscovery_reconciler.go @@ -20,6 +20,7 @@ import ( "context" "errors" "fmt" + "os" "reflect" appsv1 "k8s.io/api/apps/v1" @@ -99,6 +100,13 @@ func isControlledByNFD(obj client.Object) bool { return controller.Kind == nfdKind } +func getOperandImage(nfdInstance *nfdv1.NodeFeatureDiscovery) string { + if nfdInstance.Spec.Operand.Image == "" { + return os.Getenv("NODE_FEATURE_DISCOVERY_IMAGE") + } + return nfdInstance.Spec.Operand.Image +} + // +kubebuilder:rbac:groups=apps,resources=daemonsets,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=apps,resources=deployments,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=core,resources=configmaps,verbs=get;list;watch;create;update;patch;delete @@ -115,6 +123,7 @@ func isControlledByNFD(obj client.Object) bool { func (r *nodeFeatureDiscoveryReconciler) Reconcile(ctx context.Context, nfdInstance *nfdv1.NodeFeatureDiscovery) (ctrl.Result, error) { res := ctrl.Result{} logger := ctrl.LoggerFrom(ctx).WithValues("instance namespace", nfdInstance.Namespace, "instance name", nfdInstance.Name) + operandImage := getOperandImage(nfdInstance) if nfdInstance.DeletionTimestamp != nil { // NFD CR is being deleted @@ -145,25 +154,24 @@ func (r *nodeFeatureDiscoveryReconciler) Reconcile(ctx context.Context, nfdInsta errs = append(errs, err) logger.Info("reconciling master component") - err = r.helper.handleMaster(ctx, nfdInstance) + err = r.helper.handleMaster(ctx, nfdInstance, operandImage) errs = append(errs, err) logger.Info("reconciling worker component") - err = r.helper.handleWorker(ctx, nfdInstance) + err = r.helper.handleWorker(ctx, nfdInstance, operandImage) errs = append(errs, err) logger.Info("reconciling topology components") - err = r.helper.handleTopology(ctx, nfdInstance) + err = r.helper.handleTopology(ctx, nfdInstance, operandImage) errs = append(errs, err) logger.Info("reconciling garbage collector") - err = r.helper.handleGC(ctx, nfdInstance) + err = r.helper.handleGC(ctx, nfdInstance, operandImage) errs = append(errs, err) logger.Info("reconciling NFD status") err = r.helper.handleStatus(ctx, nfdInstance) errs = append(errs, err) - return res, errors.Join(errs...) } @@ -175,10 +183,10 @@ type nodeFeatureDiscoveryHelperAPI interface { setFinalizer(ctx context.Context, instance *nfdv1.NodeFeatureDiscovery) error removeFinalizer(ctx context.Context, instance *nfdv1.NodeFeatureDiscovery) error handleSCCs(ctx context.Context, nfdInstance *nfdv1.NodeFeatureDiscovery) error - handleMaster(ctx context.Context, nfdInstance *nfdv1.NodeFeatureDiscovery) error - handleWorker(ctx context.Context, nfdInstance *nfdv1.NodeFeatureDiscovery) error - handleTopology(ctx context.Context, nfdInstance *nfdv1.NodeFeatureDiscovery) error - handleGC(ctx context.Context, nfdInstance *nfdv1.NodeFeatureDiscovery) error + handleMaster(ctx context.Context, nfdInstance *nfdv1.NodeFeatureDiscovery, operatorImage string) error + handleWorker(ctx context.Context, nfdInstance *nfdv1.NodeFeatureDiscovery, operatorImage string) error + handleTopology(ctx context.Context, nfdInstance *nfdv1.NodeFeatureDiscovery, operatorImage string) error + handleGC(ctx context.Context, nfdInstance *nfdv1.NodeFeatureDiscovery, operatorImage string) error handlePrune(ctx context.Context, nfdInstance *nfdv1.NodeFeatureDiscovery) (bool, error) handleStatus(ctx context.Context, nfdInstance *nfdv1.NodeFeatureDiscovery) error } @@ -291,12 +299,12 @@ func (nfdh *nodeFeatureDiscoveryHelper) handleSCCs(ctx context.Context, nfdInsta return nil } -func (nfdh *nodeFeatureDiscoveryHelper) handleMaster(ctx context.Context, nfdInstance *nfdv1.NodeFeatureDiscovery) error { +func (nfdh *nodeFeatureDiscoveryHelper) handleMaster(ctx context.Context, nfdInstance *nfdv1.NodeFeatureDiscovery, operandImage string) error { masterDep := appsv1.Deployment{ ObjectMeta: metav1.ObjectMeta{Name: "nfd-master", Namespace: nfdInstance.Namespace}, } opRes, err := controllerutil.CreateOrPatch(ctx, nfdh.client, &masterDep, func() error { - return nfdh.deploymentAPI.SetMasterDeploymentAsDesired(nfdInstance, &masterDep) + return nfdh.deploymentAPI.SetMasterDeploymentAsDesired(nfdInstance, &masterDep, operandImage) }) if err != nil { @@ -306,7 +314,7 @@ func (nfdh *nodeFeatureDiscoveryHelper) handleMaster(ctx context.Context, nfdIns return nil } -func (nfdh *nodeFeatureDiscoveryHelper) handleWorker(ctx context.Context, nfdInstance *nfdv1.NodeFeatureDiscovery) error { +func (nfdh *nodeFeatureDiscoveryHelper) handleWorker(ctx context.Context, nfdInstance *nfdv1.NodeFeatureDiscovery, operandImage string) error { logger := ctrl.LoggerFrom(ctx) workerCM := corev1.ConfigMap{ @@ -324,7 +332,7 @@ func (nfdh *nodeFeatureDiscoveryHelper) handleWorker(ctx context.Context, nfdIns ObjectMeta: metav1.ObjectMeta{Name: "nfd-worker", Namespace: nfdInstance.Namespace}, } opRes, err := controllerutil.CreateOrPatch(ctx, nfdh.client, &workerDS, func() error { - return nfdh.daemonsetAPI.SetWorkerDaemonsetAsDesired(ctx, nfdInstance, &workerDS) + return nfdh.daemonsetAPI.SetWorkerDaemonsetAsDesired(ctx, nfdInstance, &workerDS, operandImage) }) if err != nil { return fmt.Errorf("failed to reconcile worker DaemonSet %s/%s: %w", nfdInstance.Namespace, nfdInstance.Name, err) @@ -335,7 +343,7 @@ func (nfdh *nodeFeatureDiscoveryHelper) handleWorker(ctx context.Context, nfdIns return nil } -func (nfdh *nodeFeatureDiscoveryHelper) handleTopology(ctx context.Context, nfdInstance *nfdv1.NodeFeatureDiscovery) error { +func (nfdh *nodeFeatureDiscoveryHelper) handleTopology(ctx context.Context, nfdInstance *nfdv1.NodeFeatureDiscovery, operandImage string) error { if !nfdInstance.Spec.TopologyUpdater { return nil } @@ -343,7 +351,7 @@ func (nfdh *nodeFeatureDiscoveryHelper) handleTopology(ctx context.Context, nfdI ObjectMeta: metav1.ObjectMeta{Name: "nfd-topology-updater", Namespace: nfdInstance.Namespace}, } opRes, err := controllerutil.CreateOrPatch(ctx, nfdh.client, &topologyDS, func() error { - return nfdh.daemonsetAPI.SetTopologyDaemonsetAsDesired(ctx, nfdInstance, &topologyDS) + return nfdh.daemonsetAPI.SetTopologyDaemonsetAsDesired(ctx, nfdInstance, &topologyDS, operandImage) }) if err != nil { @@ -353,12 +361,12 @@ func (nfdh *nodeFeatureDiscoveryHelper) handleTopology(ctx context.Context, nfdI return nil } -func (nfdh *nodeFeatureDiscoveryHelper) handleGC(ctx context.Context, nfdInstance *nfdv1.NodeFeatureDiscovery) error { +func (nfdh *nodeFeatureDiscoveryHelper) handleGC(ctx context.Context, nfdInstance *nfdv1.NodeFeatureDiscovery, operandImage string) error { gcDep := appsv1.Deployment{ ObjectMeta: metav1.ObjectMeta{Name: "nfd-gc", Namespace: nfdInstance.Namespace}, } opRes, err := controllerutil.CreateOrPatch(ctx, nfdh.client, &gcDep, func() error { - return nfdh.deploymentAPI.SetGCDeploymentAsDesired(nfdInstance, &gcDep) + return nfdh.deploymentAPI.SetGCDeploymentAsDesired(nfdInstance, &gcDep, operandImage) }) if err != nil { diff --git a/internal/controllers/nodefeaturediscovery_reconciler_test.go b/internal/controllers/nodefeaturediscovery_reconciler_test.go index 456fd2770..81330c564 100644 --- a/internal/controllers/nodefeaturediscovery_reconciler_test.go +++ b/internal/controllers/nodefeaturediscovery_reconciler_test.go @@ -19,6 +19,7 @@ package new_controllers import ( "context" "fmt" + "os" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -66,10 +67,10 @@ var _ = Describe("Reconcile", func() { mockHelper.EXPECT().hasFinalizer(&nfdCR).Return(true) mockHelper.EXPECT().handleSCCs(ctx, &nfdCR).Return(nil) - mockHelper.EXPECT().handleMaster(ctx, &nfdCR).Return(nil) - mockHelper.EXPECT().handleWorker(ctx, &nfdCR).Return(nil) - mockHelper.EXPECT().handleTopology(ctx, &nfdCR).Return(nil) - mockHelper.EXPECT().handleGC(ctx, &nfdCR).Return(nil) + mockHelper.EXPECT().handleMaster(ctx, &nfdCR, nfdCR.Spec.Operand.Image).Return(nil) + mockHelper.EXPECT().handleWorker(ctx, &nfdCR, nfdCR.Spec.Operand.Image).Return(nil) + mockHelper.EXPECT().handleTopology(ctx, &nfdCR, nfdCR.Spec.Operand.Image).Return(nil) + mockHelper.EXPECT().handleGC(ctx, &nfdCR, nfdCR.Spec.Operand.Image).Return(nil) mockHelper.EXPECT().handleStatus(ctx, &nfdCR).Return(nil) res, err := nfdr.Reconcile(ctx, &nfdCR) @@ -147,10 +148,10 @@ var _ = Describe("Reconcile", func() { mockHelper.EXPECT().hasFinalizer(&nfdCR).Return(true) mockHelper.EXPECT().handleSCCs(ctx, &nfdCR).Return(handlerSCCError) - mockHelper.EXPECT().handleMaster(ctx, &nfdCR).Return(handlerMasterError) - mockHelper.EXPECT().handleWorker(ctx, &nfdCR).Return(handlerWorkerError) - mockHelper.EXPECT().handleTopology(ctx, &nfdCR).Return(handleTopologyError) - mockHelper.EXPECT().handleGC(ctx, &nfdCR).Return(handlerGCError) + mockHelper.EXPECT().handleMaster(ctx, &nfdCR, nfdCR.Spec.Operand.Image).Return(handlerMasterError) + mockHelper.EXPECT().handleWorker(ctx, &nfdCR, nfdCR.Spec.Operand.Image).Return(handlerWorkerError) + mockHelper.EXPECT().handleTopology(ctx, &nfdCR, nfdCR.Spec.Operand.Image).Return(handleTopologyError) + mockHelper.EXPECT().handleGC(ctx, &nfdCR, nfdCR.Spec.Operand.Image).Return(handlerGCError) mockHelper.EXPECT().handleStatus(ctx, &nfdCR).Return(handleStatusError) res, err := nfdr.Reconcile(ctx, &nfdCR) @@ -194,11 +195,11 @@ var _ = Describe("handleMaster", func() { nfdCR := nfdv1.NodeFeatureDiscovery{} gomock.InOrder( clnt.EXPECT().Get(ctx, gomock.Any(), gomock.Any()).Return(apierrors.NewNotFound(schema.GroupResource{}, "whatever")), - mockDeployment.EXPECT().SetMasterDeploymentAsDesired(&nfdCR, gomock.Any()).Return(nil), + mockDeployment.EXPECT().SetMasterDeploymentAsDesired(&nfdCR, gomock.Any(), nfdCR.Spec.Operand.Image).Return(nil), clnt.EXPECT().Create(ctx, gomock.Any()).Return(nil), ) - err := nfdh.handleMaster(ctx, &nfdCR) + err := nfdh.handleMaster(ctx, &nfdCR, nfdCR.Spec.Operand.Image) Expect(err).To(BeNil()) }) @@ -220,10 +221,10 @@ var _ = Describe("handleMaster", func() { return nil }, ), - mockDeployment.EXPECT().SetMasterDeploymentAsDesired(&nfdCR, &existingDeployment).Return(nil), + mockDeployment.EXPECT().SetMasterDeploymentAsDesired(&nfdCR, &existingDeployment, nfdCR.Spec.Operand.Image).Return(nil), ) - err := nfdh.handleMaster(ctx, &nfdCR) + err := nfdh.handleMaster(ctx, &nfdCR, nfdCR.Spec.Operand.Image) Expect(err).To(BeNil()) }) @@ -231,10 +232,10 @@ var _ = Describe("handleMaster", func() { nfdCR := nfdv1.NodeFeatureDiscovery{} gomock.InOrder( clnt.EXPECT().Get(ctx, gomock.Any(), gomock.Any()).Return(apierrors.NewNotFound(schema.GroupResource{}, "whatever")), - mockDeployment.EXPECT().SetMasterDeploymentAsDesired(&nfdCR, gomock.Any()).Return(fmt.Errorf("some error")), + mockDeployment.EXPECT().SetMasterDeploymentAsDesired(&nfdCR, gomock.Any(), nfdCR.Spec.Operand.Image).Return(fmt.Errorf("some error")), ) - err := nfdh.handleMaster(ctx, &nfdCR) + err := nfdh.handleMaster(ctx, &nfdCR, nfdCR.Spec.Operand.Image) Expect(err).To(HaveOccurred()) }) }) @@ -266,11 +267,11 @@ var _ = Describe("handleWorker", func() { mockCM.EXPECT().SetWorkerConfigMapAsDesired(ctx, &nfdCR, gomock.Any()).Return(nil), clnt.EXPECT().Create(ctx, gomock.Any()).Return(nil), clnt.EXPECT().Get(ctx, gomock.Any(), gomock.Any()).Return(apierrors.NewNotFound(schema.GroupResource{}, "whatever")), - mockDS.EXPECT().SetWorkerDaemonsetAsDesired(ctx, &nfdCR, gomock.Any()).Return(nil), + mockDS.EXPECT().SetWorkerDaemonsetAsDesired(ctx, &nfdCR, gomock.Any(), nfdCR.Spec.Operand.Image).Return(nil), clnt.EXPECT().Create(ctx, gomock.Any()).Return(nil), ) - err := nfdh.handleWorker(ctx, &nfdCR) + err := nfdh.handleWorker(ctx, &nfdCR, nfdCR.Spec.Operand.Image) Expect(err).To(BeNil()) }) @@ -306,10 +307,10 @@ var _ = Describe("handleWorker", func() { return nil }, ), - mockDS.EXPECT().SetWorkerDaemonsetAsDesired(ctx, &nfdCR, &existingDS).Return(nil), + mockDS.EXPECT().SetWorkerDaemonsetAsDesired(ctx, &nfdCR, &existingDS, nfdCR.Spec.Operand.Image).Return(nil), ) - err := nfdh.handleWorker(ctx, &nfdCR) + err := nfdh.handleWorker(ctx, &nfdCR, nfdCR.Spec.Operand.Image) Expect(err).To(BeNil()) }) @@ -319,7 +320,7 @@ var _ = Describe("handleWorker", func() { mockCM.EXPECT().SetWorkerConfigMapAsDesired(ctx, &nfdCR, gomock.Any()).Return(fmt.Errorf("some error")), ) - err := nfdh.handleWorker(ctx, &nfdCR) + err := nfdh.handleWorker(ctx, &nfdCR, nfdCR.Spec.Operand.Image) Expect(err).To(HaveOccurred()) }) @@ -329,10 +330,10 @@ var _ = Describe("handleWorker", func() { mockCM.EXPECT().SetWorkerConfigMapAsDesired(ctx, &nfdCR, gomock.Any()).Return(nil), clnt.EXPECT().Create(ctx, gomock.Any()).Return(nil), clnt.EXPECT().Get(ctx, gomock.Any(), gomock.Any()).Return(apierrors.NewNotFound(schema.GroupResource{}, "whatever")), - mockDS.EXPECT().SetWorkerDaemonsetAsDesired(ctx, &nfdCR, gomock.Any()).Return(fmt.Errorf("some error")), + mockDS.EXPECT().SetWorkerDaemonsetAsDesired(ctx, &nfdCR, gomock.Any(), nfdCR.Spec.Operand.Image).Return(fmt.Errorf("some error")), ) - err := nfdh.handleWorker(ctx, &nfdCR) + err := nfdh.handleWorker(ctx, &nfdCR, nfdCR.Spec.Operand.Image) Expect(err).To(HaveOccurred()) }) }) @@ -363,11 +364,11 @@ var _ = Describe("handleTopology", func() { } gomock.InOrder( clnt.EXPECT().Get(ctx, gomock.Any(), gomock.Any()).Return(apierrors.NewNotFound(schema.GroupResource{}, "whatever")), - mockDS.EXPECT().SetTopologyDaemonsetAsDesired(ctx, &nfdCR, gomock.Any()).Return(nil), + mockDS.EXPECT().SetTopologyDaemonsetAsDesired(ctx, &nfdCR, gomock.Any(), nfdCR.Spec.Operand.Image).Return(nil), clnt.EXPECT().Create(ctx, gomock.Any()).Return(nil), ) - err := nfdh.handleTopology(ctx, &nfdCR) + err := nfdh.handleTopology(ctx, &nfdCR, nfdCR.Spec.Operand.Image) Expect(err).To(BeNil()) }) @@ -392,10 +393,10 @@ var _ = Describe("handleTopology", func() { return nil }, ), - mockDS.EXPECT().SetTopologyDaemonsetAsDesired(ctx, &nfdCR, &existingDS).Return(nil), + mockDS.EXPECT().SetTopologyDaemonsetAsDesired(ctx, &nfdCR, &existingDS, nfdCR.Spec.Operand.Image).Return(nil), ) - err := nfdh.handleTopology(ctx, &nfdCR) + err := nfdh.handleTopology(ctx, &nfdCR, nfdCR.Spec.Operand.Image) Expect(err).To(BeNil()) }) @@ -407,17 +408,17 @@ var _ = Describe("handleTopology", func() { } gomock.InOrder( clnt.EXPECT().Get(ctx, gomock.Any(), gomock.Any()).Return(apierrors.NewNotFound(schema.GroupResource{}, "whatever")), - mockDS.EXPECT().SetTopologyDaemonsetAsDesired(ctx, &nfdCR, gomock.Any()).Return(fmt.Errorf("some error")), + mockDS.EXPECT().SetTopologyDaemonsetAsDesired(ctx, &nfdCR, gomock.Any(), nfdCR.Spec.Operand.Image).Return(fmt.Errorf("some error")), ) - err := nfdh.handleTopology(ctx, &nfdCR) + err := nfdh.handleTopology(ctx, &nfdCR, nfdCR.Spec.Operand.Image) Expect(err).To(HaveOccurred()) }) It("if TopologyUpdate not set - nothing to do", func() { nfdCR := nfdv1.NodeFeatureDiscovery{} - err := nfdh.handleTopology(ctx, &nfdCR) + err := nfdh.handleTopology(ctx, &nfdCR, nfdCR.Spec.Operand.Image) Expect(err).To(BeNil()) }) }) @@ -444,11 +445,11 @@ var _ = Describe("handleGC", func() { nfdCR := nfdv1.NodeFeatureDiscovery{} gomock.InOrder( clnt.EXPECT().Get(ctx, gomock.Any(), gomock.Any()).Return(apierrors.NewNotFound(schema.GroupResource{}, "whatever")), - mockDeployment.EXPECT().SetGCDeploymentAsDesired(&nfdCR, gomock.Any()).Return(nil), + mockDeployment.EXPECT().SetGCDeploymentAsDesired(&nfdCR, gomock.Any(), nfdCR.Spec.Operand.Image).Return(nil), clnt.EXPECT().Create(ctx, gomock.Any()).Return(nil), ) - err := nfdh.handleGC(ctx, &nfdCR) + err := nfdh.handleGC(ctx, &nfdCR, nfdCR.Spec.Operand.Image) Expect(err).To(BeNil()) }) @@ -470,10 +471,10 @@ var _ = Describe("handleGC", func() { return nil }, ), - mockDeployment.EXPECT().SetGCDeploymentAsDesired(&nfdCR, &existingDeployment).Return(nil), + mockDeployment.EXPECT().SetGCDeploymentAsDesired(&nfdCR, &existingDeployment, nfdCR.Spec.Operand.Image).Return(nil), ) - err := nfdh.handleGC(ctx, &nfdCR) + err := nfdh.handleGC(ctx, &nfdCR, nfdCR.Spec.Operand.Image) Expect(err).To(BeNil()) }) @@ -481,10 +482,10 @@ var _ = Describe("handleGC", func() { nfdCR := nfdv1.NodeFeatureDiscovery{} gomock.InOrder( clnt.EXPECT().Get(ctx, gomock.Any(), gomock.Any()).Return(apierrors.NewNotFound(schema.GroupResource{}, "whatever")), - mockDeployment.EXPECT().SetGCDeploymentAsDesired(&nfdCR, gomock.Any()).Return(fmt.Errorf("some error")), + mockDeployment.EXPECT().SetGCDeploymentAsDesired(&nfdCR, gomock.Any(), nfdCR.Spec.Operand.Image).Return(fmt.Errorf("some error")), ) - err := nfdh.handleGC(ctx, &nfdCR) + err := nfdh.handleGC(ctx, &nfdCR, nfdCR.Spec.Operand.Image) Expect(err).To(HaveOccurred()) }) }) @@ -880,3 +881,31 @@ var _ = Describe("handleStatus", func() { Expect(err).To(HaveOccurred()) }) }) + +var _ = Describe("getOperandImage", Ordered, func() { + var nfdCR nfdv1.NodeFeatureDiscovery + const ( + testImage = "TestRegistry/TestNamespace/TestImage:latest" + otherTestImage = "TestRegistry/TestNamespace/SomeOtherTestImage:latest" + ) + BeforeEach(func() { + nfdCR = nfdv1.NodeFeatureDiscovery{} + }) + BeforeAll(func() { + err := os.Setenv("NODE_FEATURE_DISCOVERY_IMAGE", testImage) + Expect(err).ToNot(HaveOccurred()) + }) + AfterAll(func() { + err := os.Unsetenv("NODE_FEATURE_DISCOVERY_IMAGE") + Expect(err).ToNot(HaveOccurred()) + }) + It("Should get the operand image from the env variable", func() { + operandImage := getOperandImage(&nfdCR) + Expect(operandImage).To(Equal(testImage)) + }) + It("Should get the operand image from the cr", func() { + nfdCR.Spec.Operand.Image = otherTestImage + operandImage := getOperandImage(&nfdCR) + Expect(operandImage).To(Equal(otherTestImage)) + }) +}) diff --git a/internal/daemonset/daemonset.go b/internal/daemonset/daemonset.go index 1917d2af3..f03c81bf8 100644 --- a/internal/daemonset/daemonset.go +++ b/internal/daemonset/daemonset.go @@ -34,8 +34,8 @@ import ( //go:generate mockgen -source=daemonset.go -package=daemonset -destination=mock_daemonset.go DaemonsetAPI type DaemonsetAPI interface { - SetTopologyDaemonsetAsDesired(ctx context.Context, nfdInstance *nfdv1.NodeFeatureDiscovery, topologyDS *appsv1.DaemonSet) error - SetWorkerDaemonsetAsDesired(ctx context.Context, nfdInstance *nfdv1.NodeFeatureDiscovery, workerDS *appsv1.DaemonSet) error + SetTopologyDaemonsetAsDesired(ctx context.Context, nfdInstance *nfdv1.NodeFeatureDiscovery, topologyDS *appsv1.DaemonSet, operandImage string) error + SetWorkerDaemonsetAsDesired(ctx context.Context, nfdInstance *nfdv1.NodeFeatureDiscovery, workerDS *appsv1.DaemonSet, operandImage string) error DeleteDaemonSet(ctx context.Context, namespace, name string) error GetDaemonSet(ctx context.Context, namespace, name string) (*appsv1.DaemonSet, error) } @@ -52,7 +52,7 @@ func NewDaemonsetAPI(client client.Client, scheme *runtime.Scheme) DaemonsetAPI } } -func (d *daemonset) SetTopologyDaemonsetAsDesired(ctx context.Context, nfdInstance *nfdv1.NodeFeatureDiscovery, topologyDS *appsv1.DaemonSet) error { +func (d *daemonset) SetTopologyDaemonsetAsDesired(ctx context.Context, nfdInstance *nfdv1.NodeFeatureDiscovery, topologyDS *appsv1.DaemonSet, operandImage string) error { topologyDS.ObjectMeta.Labels = map[string]string{"app": "nfd"} podLabels := map[string]string{"app": "nfd-topology-updater"} @@ -70,7 +70,7 @@ func (d *daemonset) SetTopologyDaemonsetAsDesired(ctx context.Context, nfdInstan Containers: []corev1.Container{ { Name: "nfd-topology-updater", - Image: nfdInstance.Spec.Operand.ImagePath(), + Image: operandImage, ImagePullPolicy: getImagePullPolicy(nfdInstance), Command: []string{ "nfd-topology-updater", @@ -227,7 +227,7 @@ func getVolumes() []corev1.Volume { } } -func (d *daemonset) SetWorkerDaemonsetAsDesired(ctx context.Context, nfdInstance *nfdv1.NodeFeatureDiscovery, workerDS *appsv1.DaemonSet) error { +func (d *daemonset) SetWorkerDaemonsetAsDesired(ctx context.Context, nfdInstance *nfdv1.NodeFeatureDiscovery, workerDS *appsv1.DaemonSet, operandImage string) error { workerDS.ObjectMeta.Labels = map[string]string{"app": "nfd"} workerDS.Spec = appsv1.DaemonSetSpec{ @@ -253,7 +253,7 @@ func (d *daemonset) SetWorkerDaemonsetAsDesired(ctx context.Context, nfdInstance Containers: []corev1.Container{ { Env: getWorkerEnvs(), - Image: nfdInstance.Spec.Operand.ImagePath(), + Image: operandImage, Name: "nfd-worker", Command: []string{"nfd-worker"}, Args: []string{}, diff --git a/internal/daemonset/daemonset_test.go b/internal/daemonset/daemonset_test.go index b6491f780..0796eebbd 100644 --- a/internal/daemonset/daemonset_test.go +++ b/internal/daemonset/daemonset_test.go @@ -64,7 +64,7 @@ var _ = Describe("SetTopologyDaemonsetAsDesired", func() { }, } - err := daemonsetAPI.SetTopologyDaemonsetAsDesired(ctx, &nfdCR, &topologyDS) + err := daemonsetAPI.SetTopologyDaemonsetAsDesired(ctx, &nfdCR, &topologyDS, nfdCR.Spec.Operand.Image) Expect(err).To(BeNil()) expectedYAMLFile, err := os.ReadFile("testdata/test_topology_daemonset.yaml") @@ -108,7 +108,7 @@ var _ = Describe("SetWorkerDaemonsetAsDesired", func() { }, } - err := daemonsetAPI.SetWorkerDaemonsetAsDesired(ctx, &nfdCR, &actualWorkerDS) + err := daemonsetAPI.SetWorkerDaemonsetAsDesired(ctx, &nfdCR, &actualWorkerDS, nfdCR.Spec.Operand.Image) Expect(err).To(BeNil()) expectedYAMLFile, err := os.ReadFile("testdata/test_worker_daemonset.yaml") diff --git a/internal/daemonset/mock_daemonset.go b/internal/daemonset/mock_daemonset.go index 568776bdc..a2cec06a9 100644 --- a/internal/daemonset/mock_daemonset.go +++ b/internal/daemonset/mock_daemonset.go @@ -70,29 +70,29 @@ func (mr *MockDaemonsetAPIMockRecorder) GetDaemonSet(ctx, namespace, name any) * } // SetTopologyDaemonsetAsDesired mocks base method. -func (m *MockDaemonsetAPI) SetTopologyDaemonsetAsDesired(ctx context.Context, nfdInstance *v1.NodeFeatureDiscovery, topologyDS *v10.DaemonSet) error { +func (m *MockDaemonsetAPI) SetTopologyDaemonsetAsDesired(ctx context.Context, nfdInstance *v1.NodeFeatureDiscovery, topologyDS *v10.DaemonSet, operandImage string) error { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "SetTopologyDaemonsetAsDesired", ctx, nfdInstance, topologyDS) + ret := m.ctrl.Call(m, "SetTopologyDaemonsetAsDesired", ctx, nfdInstance, topologyDS, operandImage) ret0, _ := ret[0].(error) return ret0 } // SetTopologyDaemonsetAsDesired indicates an expected call of SetTopologyDaemonsetAsDesired. -func (mr *MockDaemonsetAPIMockRecorder) SetTopologyDaemonsetAsDesired(ctx, nfdInstance, topologyDS any) *gomock.Call { +func (mr *MockDaemonsetAPIMockRecorder) SetTopologyDaemonsetAsDesired(ctx, nfdInstance, topologyDS, operandImage any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetTopologyDaemonsetAsDesired", reflect.TypeOf((*MockDaemonsetAPI)(nil).SetTopologyDaemonsetAsDesired), ctx, nfdInstance, topologyDS) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetTopologyDaemonsetAsDesired", reflect.TypeOf((*MockDaemonsetAPI)(nil).SetTopologyDaemonsetAsDesired), ctx, nfdInstance, topologyDS, operandImage) } // SetWorkerDaemonsetAsDesired mocks base method. -func (m *MockDaemonsetAPI) SetWorkerDaemonsetAsDesired(ctx context.Context, nfdInstance *v1.NodeFeatureDiscovery, workerDS *v10.DaemonSet) error { +func (m *MockDaemonsetAPI) SetWorkerDaemonsetAsDesired(ctx context.Context, nfdInstance *v1.NodeFeatureDiscovery, workerDS *v10.DaemonSet, operandImage string) error { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "SetWorkerDaemonsetAsDesired", ctx, nfdInstance, workerDS) + ret := m.ctrl.Call(m, "SetWorkerDaemonsetAsDesired", ctx, nfdInstance, workerDS, operandImage) ret0, _ := ret[0].(error) return ret0 } // SetWorkerDaemonsetAsDesired indicates an expected call of SetWorkerDaemonsetAsDesired. -func (mr *MockDaemonsetAPIMockRecorder) SetWorkerDaemonsetAsDesired(ctx, nfdInstance, workerDS any) *gomock.Call { +func (mr *MockDaemonsetAPIMockRecorder) SetWorkerDaemonsetAsDesired(ctx, nfdInstance, workerDS, operandImage any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetWorkerDaemonsetAsDesired", reflect.TypeOf((*MockDaemonsetAPI)(nil).SetWorkerDaemonsetAsDesired), ctx, nfdInstance, workerDS) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetWorkerDaemonsetAsDesired", reflect.TypeOf((*MockDaemonsetAPI)(nil).SetWorkerDaemonsetAsDesired), ctx, nfdInstance, workerDS, operandImage) } diff --git a/internal/deployment/deployment.go b/internal/deployment/deployment.go index 736a1961c..1cb7c544c 100644 --- a/internal/deployment/deployment.go +++ b/internal/deployment/deployment.go @@ -39,8 +39,8 @@ const ( //go:generate mockgen -source=deployment.go -package=deployment -destination=mock_deployment.go DeploymentAPI type DeploymentAPI interface { - SetMasterDeploymentAsDesired(nfdInstance *nfdv1.NodeFeatureDiscovery, masterDep *v1.Deployment) error - SetGCDeploymentAsDesired(nfdInstance *nfdv1.NodeFeatureDiscovery, gcDep *v1.Deployment) error + SetMasterDeploymentAsDesired(nfdInstance *nfdv1.NodeFeatureDiscovery, masterDep *v1.Deployment, operandImage string) error + SetGCDeploymentAsDesired(nfdInstance *nfdv1.NodeFeatureDiscovery, gcDep *v1.Deployment, operandImage string) error DeleteDeployment(ctx context.Context, namespace, name string) error GetDeployment(ctx context.Context, namespace, name string) (*v1.Deployment, error) } @@ -57,7 +57,7 @@ func NewDeploymentAPI(client client.Client, scheme *runtime.Scheme) DeploymentAP } } -func (d *deployment) SetMasterDeploymentAsDesired(nfdInstance *nfdv1.NodeFeatureDiscovery, masterDep *v1.Deployment) error { +func (d *deployment) SetMasterDeploymentAsDesired(nfdInstance *nfdv1.NodeFeatureDiscovery, masterDep *v1.Deployment, operandImage string) error { standartLabels := map[string]string{"app": "nfd-master"} masterDep.ObjectMeta.Labels = standartLabels @@ -79,7 +79,7 @@ func (d *deployment) SetMasterDeploymentAsDesired(nfdInstance *nfdv1.NodeFeature Containers: []corev1.Container{ { Name: "nfd-master", - Image: nfdInstance.Spec.Operand.ImagePath(), + Image: operandImage, ImagePullPolicy: getImagePullPolicy(nfdInstance), Command: []string{ "nfd-master", @@ -97,7 +97,7 @@ func (d *deployment) SetMasterDeploymentAsDesired(nfdInstance *nfdv1.NodeFeature return controllerutil.SetControllerReference(nfdInstance, masterDep, d.scheme) } -func (d *deployment) SetGCDeploymentAsDesired(nfdInstance *nfdv1.NodeFeatureDiscovery, gcDep *v1.Deployment) error { +func (d *deployment) SetGCDeploymentAsDesired(nfdInstance *nfdv1.NodeFeatureDiscovery, gcDep *v1.Deployment, operandImage string) error { gcDep.ObjectMeta.Labels = map[string]string{"app": "nfd"} matchLabels := map[string]string{"app": "nfd-gc"} gcDep.Spec = v1.DeploymentSpec{ @@ -116,7 +116,7 @@ func (d *deployment) SetGCDeploymentAsDesired(nfdInstance *nfdv1.NodeFeatureDisc Containers: []corev1.Container{ { Name: "nfd-gc", - Image: nfdInstance.Spec.Operand.ImagePath(), + Image: operandImage, ImagePullPolicy: corev1.PullAlways, Command: []string{ "nfd-gc", diff --git a/internal/deployment/deployment_test.go b/internal/deployment/deployment_test.go index dfa636f7d..56106efef 100644 --- a/internal/deployment/deployment_test.go +++ b/internal/deployment/deployment_test.go @@ -62,7 +62,7 @@ var _ = Describe("SetMasterDeploymentAsDesired", func() { }, } - err := deploymentAPI.SetMasterDeploymentAsDesired(&nfdCR, &masterDep) + err := deploymentAPI.SetMasterDeploymentAsDesired(&nfdCR, &masterDep, nfdCR.Spec.Operand.Image) Expect(err).To(BeNil()) expectedYAMLFile, err := os.ReadFile("testdata/test_master_deployment.yaml") @@ -104,7 +104,7 @@ var _ = Describe("SetGCDeploymentAsDesired", func() { }, } - err := deploymentAPI.SetGCDeploymentAsDesired(&nfdCR, &masterDep) + err := deploymentAPI.SetGCDeploymentAsDesired(&nfdCR, &masterDep, nfdCR.Spec.Operand.Image) Expect(err).To(BeNil()) expectedYAMLFile, err := os.ReadFile("testdata/test_gc_deployment.yaml") diff --git a/internal/deployment/mock_deployment.go b/internal/deployment/mock_deployment.go index c2e35871e..9f85be9e4 100644 --- a/internal/deployment/mock_deployment.go +++ b/internal/deployment/mock_deployment.go @@ -70,29 +70,29 @@ func (mr *MockDeploymentAPIMockRecorder) GetDeployment(ctx, namespace, name any) } // SetGCDeploymentAsDesired mocks base method. -func (m *MockDeploymentAPI) SetGCDeploymentAsDesired(nfdInstance *v1.NodeFeatureDiscovery, gcDep *v10.Deployment) error { +func (m *MockDeploymentAPI) SetGCDeploymentAsDesired(nfdInstance *v1.NodeFeatureDiscovery, gcDep *v10.Deployment, operandImage string) error { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "SetGCDeploymentAsDesired", nfdInstance, gcDep) + ret := m.ctrl.Call(m, "SetGCDeploymentAsDesired", nfdInstance, gcDep, operandImage) ret0, _ := ret[0].(error) return ret0 } // SetGCDeploymentAsDesired indicates an expected call of SetGCDeploymentAsDesired. -func (mr *MockDeploymentAPIMockRecorder) SetGCDeploymentAsDesired(nfdInstance, gcDep any) *gomock.Call { +func (mr *MockDeploymentAPIMockRecorder) SetGCDeploymentAsDesired(nfdInstance, gcDep, operandImage any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetGCDeploymentAsDesired", reflect.TypeOf((*MockDeploymentAPI)(nil).SetGCDeploymentAsDesired), nfdInstance, gcDep) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetGCDeploymentAsDesired", reflect.TypeOf((*MockDeploymentAPI)(nil).SetGCDeploymentAsDesired), nfdInstance, gcDep, operandImage) } // SetMasterDeploymentAsDesired mocks base method. -func (m *MockDeploymentAPI) SetMasterDeploymentAsDesired(nfdInstance *v1.NodeFeatureDiscovery, masterDep *v10.Deployment) error { +func (m *MockDeploymentAPI) SetMasterDeploymentAsDesired(nfdInstance *v1.NodeFeatureDiscovery, masterDep *v10.Deployment, operandImage string) error { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "SetMasterDeploymentAsDesired", nfdInstance, masterDep) + ret := m.ctrl.Call(m, "SetMasterDeploymentAsDesired", nfdInstance, masterDep, operandImage) ret0, _ := ret[0].(error) return ret0 } // SetMasterDeploymentAsDesired indicates an expected call of SetMasterDeploymentAsDesired. -func (mr *MockDeploymentAPIMockRecorder) SetMasterDeploymentAsDesired(nfdInstance, masterDep any) *gomock.Call { +func (mr *MockDeploymentAPIMockRecorder) SetMasterDeploymentAsDesired(nfdInstance, masterDep, operandImage any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetMasterDeploymentAsDesired", reflect.TypeOf((*MockDeploymentAPI)(nil).SetMasterDeploymentAsDesired), nfdInstance, masterDep) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetMasterDeploymentAsDesired", reflect.TypeOf((*MockDeploymentAPI)(nil).SetMasterDeploymentAsDesired), nfdInstance, masterDep, operandImage) }