Start using MIC status to check if an image exists.

Now that the MIC object is being reconciled correctly then we can start
using the MIC.status in order to check if an image exists instead of
using an explicit HTTP request from the controller container.

Signed-off-by: Yoni Bettan <[email protected]>

Author: ybettan

internal/controllers/module_reconciler.go

@@ -365,18 +365,19 @@ func (mrh *moduleReconcilerHelper) handleMIC(ctx context.Context, mod *kmmv1beta
 }
 
 func (mrh *moduleReconcilerHelper) enableModuleOnNode(ctx context.Context, mld *api.ModuleLoaderData, node *v1.Node) error {
+
 	logger := log.FromContext(ctx)
 
-	if module.ShouldBeBuilt(mld) || module.ShouldBeSigned(mld) {
-		exists, err := module.ImageExists(ctx, mrh.client, mrh.registryAPI, mld, mld.Namespace, mld.ContainerImage)
-		if err != nil {
-			return fmt.Errorf("failed to verify that image %s exists: %v", mld.ContainerImage, err)
-		}
-		if !exists {
-			// skip updating NMC, reconciliation will kick in once the build pod is completed
-			logger.V(1).Info("Image does not exist, not adding to NMC", "nmc name", node.Name, "container image", mld.ContainerImage)
-			return nil
-		}
+	micObj, err := mrh.micAPI.Get(ctx, mld.Name, mld.Namespace)
+	if err != nil {
+		return fmt.Errorf("failed to get moduleImagesConfig %s: %v", mld.Name, err)
+	}
+
+	imageStatus := mrh.micAPI.GetImageState(micObj, mld.ContainerImage)
+	if imageStatus != kmmv1beta1.ImageExists {
+		// skip updating NMC, reconciliation will kick in once the build pod is completed
+		logger.V(1).Info("Image does not exist, not adding to NMC", "nmc name", node.Name, "container image", mld.ContainerImage)
+		return nil
 	}
 
 	moduleConfig := kmmv1beta1.ModuleConfig{

internal/controllers/module_reconciler_test.go

@@ -717,13 +717,15 @@ var _ = Describe("enableModuleOnNode", func() {
 	const (
 		moduleNamespace = "moduleNamespace"
 		moduleName      = "moduleName"
+		containerImage  = "containerImage"
 	)
 
 	var (
 		ctx                  context.Context
 		ctrl                 *gomock.Controller
 		clnt                 *client.MockClient
 		rgst                 *registry.MockRegistry
+		mockMIC              *mic.MockMIC
 		mrh                  moduleReconcilerHelperAPI
 		helper               *nmc.MockHelper
 		mld                  *api.ModuleLoaderData
@@ -737,7 +739,8 @@ var _ = Describe("enableModuleOnNode", func() {
 		clnt = client.NewMockClient(ctrl)
 		helper = nmc.NewMockHelper(ctrl)
 		rgst = registry.NewMockRegistry(ctrl)
-		mrh = newModuleReconcilerHelper(clnt, nil, rgst, nil, helper, scheme)
+		mockMIC = mic.NewMockMIC(ctrl)
+		mrh = newModuleReconcilerHelper(clnt, nil, rgst, mockMIC, helper, scheme)
 		node = v1.Node{
 			ObjectMeta: metav1.ObjectMeta{Name: "nodeName"},
 		}
@@ -748,7 +751,7 @@ var _ = Describe("enableModuleOnNode", func() {
 			Name:                  moduleName,
 			Namespace:             moduleNamespace,
 			InTreeModulesToRemove: []string{"InTreeModuleToRemove"},
-			ContainerImage:        "containerImage",
+			ContainerImage:        containerImage,
 		}
 
 		expectedModuleConfig = &kmmv1beta1.ModuleConfig{
@@ -759,25 +762,24 @@ var _ = Describe("enableModuleOnNode", func() {
 		}
 	})
 
-	It("Build configured and image does not exist", func() {
-		mld.Build = &kmmv1beta1.Build{}
-		rgst.EXPECT().ImageExists(ctx, mld.ContainerImage, gomock.Any(), gomock.Any()).Return(false, nil)
-		err := mrh.enableModuleOnNode(ctx, mld, &node)
-		Expect(err).NotTo(HaveOccurred())
-	})
+	It("should fail if we failed to get MIC", func() {
+
+		mockMIC.EXPECT().Get(ctx, moduleName, moduleNamespace).Return(nil, errors.New("some error"))
 
-	It("Sign configured and image does not exist", func() {
-		mld.Sign = &kmmv1beta1.Sign{}
-		rgst.EXPECT().ImageExists(ctx, mld.ContainerImage, gomock.Any(), gomock.Any()).Return(false, nil)
 		err := mrh.enableModuleOnNode(ctx, mld, &node)
-		Expect(err).NotTo(HaveOccurred())
+		Expect(err).To(HaveOccurred())
+		Expect(err.Error()).To(ContainSubstring("failed to get moduleImagesConfig"))
 	})
 
-	It("Build configured and failed to check if image exists", func() {
-		mld.Build = &kmmv1beta1.Build{}
-		rgst.EXPECT().ImageExists(ctx, mld.ContainerImage, gomock.Any(), gomock.Any()).Return(false, fmt.Errorf("some error"))
+	It("should do nothing if the image doesn't exist", func() {
+
+		gomock.InOrder(
+			mockMIC.EXPECT().Get(ctx, moduleName, moduleNamespace).Return(&kmmv1beta1.ModuleImagesConfig{}, nil),
+			mockMIC.EXPECT().GetImageState(gomock.Any(), containerImage).Return(kmmv1beta1.ImageDoesNotExist),
+		)
+
 		err := mrh.enableModuleOnNode(ctx, mld, &node)
-		Expect(err).To(HaveOccurred())
+		Expect(err).NotTo(HaveOccurred())
 	})
 
 	It("NMC does not exists", func() {
@@ -786,6 +788,8 @@ var _ = Describe("enableModuleOnNode", func() {
 		}
 
 		gomock.InOrder(
+			mockMIC.EXPECT().Get(ctx, moduleName, moduleNamespace).Return(&kmmv1beta1.ModuleImagesConfig{}, nil),
+			mockMIC.EXPECT().GetImageState(gomock.Any(), containerImage).Return(kmmv1beta1.ImageExists),
 			clnt.EXPECT().Get(ctx, gomock.Any(), gomock.Any()).Return(apierrors.NewNotFound(schema.GroupResource{}, "whatever")),
 			helper.EXPECT().SetModuleConfig(nmc, mld, expectedModuleConfig).Return(nil),
 			clnt.EXPECT().Create(ctx, gomock.Any()).Return(nil),
@@ -813,6 +817,8 @@ var _ = Describe("enableModuleOnNode", func() {
 		)
 
 		gomock.InOrder(
+			mockMIC.EXPECT().Get(ctx, moduleName, moduleNamespace).Return(&kmmv1beta1.ModuleImagesConfig{}, nil),
+			mockMIC.EXPECT().GetImageState(gomock.Any(), containerImage).Return(kmmv1beta1.ImageExists),
 			clnt.EXPECT().Get(ctx, gomock.Any(), gomock.Any()).DoAndReturn(
 				func(_ interface{}, _ interface{}, nmc *kmmv1beta1.NodeModulesConfig, _ ...ctrlclient.GetOption) error {
 					nmc.SetName(node.Name)

internal/mic/mic.go

@@ -8,6 +8,7 @@ import (
 	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/types"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 	"sigs.k8s.io/controller-runtime/pkg/log"
@@ -18,6 +19,7 @@ import (
 type MIC interface {
 	CreateOrPatch(ctx context.Context, name, ns string, images []kmmv1beta1.ModuleImageSpec,
 		imageRepoSecret *v1.LocalObjectReference, owner metav1.Object) error
+	Get(ctx context.Context, name, ns string) (*kmmv1beta1.ModuleImagesConfig, error)
 	GetModuleImageSpec(micObj *kmmv1beta1.ModuleImagesConfig, image string) *kmmv1beta1.ModuleImageSpec
 	SetImageStatus(micObj *kmmv1beta1.ModuleImagesConfig, image string, status kmmv1beta1.ImageState)
 	GetImageState(micObj *kmmv1beta1.ModuleImagesConfig, image string) kmmv1beta1.ImageState
@@ -65,6 +67,16 @@ func (mici *micImpl) CreateOrPatch(ctx context.Context, name, ns string, images
 	return nil
 }
 
+func (mici *micImpl) Get(ctx context.Context, name, ns string) (*kmmv1beta1.ModuleImagesConfig, error) {
+
+	var micObj kmmv1beta1.ModuleImagesConfig
+	if err := mici.client.Get(ctx, types.NamespacedName{Namespace: ns, Name: name}, &micObj); err != nil {
+		return nil, fmt.Errorf("could not get ModuleImagesConfig %s: %v", name, err)
+	}
+
+	return &micObj, nil
+}
+
 func (mici *micImpl) GetModuleImageSpec(micObj *kmmv1beta1.ModuleImagesConfig, image string) *kmmv1beta1.ModuleImageSpec {
 	for _, imageSpec := range micObj.Spec.Images {
 		if imageSpec.Image == image {

internal/mic/mic_test.go

@@ -19,7 +19,7 @@ import (
 	ctrlclient "sigs.k8s.io/controller-runtime/pkg/client"
 )
 
-var _ = Describe("ApplyMIC", func() {
+var _ = Describe("CreateOrPatch", func() {
 
 	const (
 		micName      = "my-name"
@@ -133,6 +133,49 @@ var _ = Describe("ApplyMIC", func() {
 	})
 })
 
+var _ = Describe("Get", func() {
+
+	const (
+		micName      = "my-name"
+		micNamespace = "my-namespace"
+	)
+
+	var (
+		ctx        context.Context
+		ctrl       *gomock.Controller
+		mockClient *client.MockClient
+		micAPI     MIC
+	)
+
+	BeforeEach(func() {
+		ctx = context.Background()
+		ctrl = gomock.NewController(GinkgoT())
+		mockClient = client.NewMockClient(ctrl)
+		micAPI = New(mockClient, scheme)
+		utilruntime.Must(v1beta1.AddToScheme(scheme))
+	})
+
+	It("should fail if we failed to get the MIC", func() {
+
+		mockClient.EXPECT().Get(ctx, gomock.Any(), gomock.Any()).Return(fmt.Errorf("some error"))
+
+		_, err := micAPI.Get(ctx, micName, micNamespace)
+
+		Expect(err).To(HaveOccurred())
+		Expect(err.Error()).To(ContainSubstring("could not get ModuleImagesConfig"))
+	})
+
+	It("should work as expected", func() {
+
+		mockClient.EXPECT().Get(ctx, gomock.Any(), gomock.Any()).Return(nil)
+
+		_, err := micAPI.Get(ctx, micName, micNamespace)
+
+		Expect(err).NotTo(HaveOccurred())
+	})
+
+})
+
 var _ = Describe("GetModuleImageSpec", func() {
 	var (
 		micAPI MIC