Changing MSBC image status structure

Currently MBSC image's spec contains the action that MBSC controller
should execute(sign or build). But since the action does not exists in
the image's status, the MIC controller cannot know if the success status
refers to the previous action (i.e build) or the current requested
action( i.e sign). In addition, since action will appear in status,
there is no need to have explicit statuses like BuildFailed: we change
it to ActionFailed or ActionSuccess.
This PR contains the following:
1) adding action field to the MBSC image' status
2) chaning the type of Status field: now it will be only ActionFailure
   or ActionSuccess
2) update the MIC controller MBSC lookup function to decide what is the
   current status of image in MIC, based on MBSC image's action and
status fields
3) unit-tests

Author: yevgeny-shnaidman

api/v1beta1/modulebuildsignconfig_types.go

@@ -22,6 +22,7 @@ import (
 )
 
 type BuildOrSignAction string
+type BuildOrSignStatus string
 
 const (
 	// BuildImage means that image needs to be built
@@ -30,20 +31,14 @@ const (
 	// SignImage means that image needs to be built
 	SignImage BuildOrSignAction = "SignImage"
 
-	// ImageBuildFailed means that image does not exists and the Build failed
-	ImageBuildFailed ImageState = "BuildFailed"
+	// ActionSuccess means that action (sign or build, depending on the action field) has succeeded
+	ActionSuccess BuildOrSignStatus = "Success"
 
-	// ImageBuildSucceeded means that image has been built and pushed succesfully
-	ImageBuildSucceeded ImageState = "BuildSucceeded"
-
-	// ImageSignFailed means that image does not exists and the Sign failed
-	ImageSignFailed ImageState = "SignFailed"
-
-	// ImageSignSucceeded means that image has been signed and pushed succesfully
-	ImageSignSucceeded ImageState = "SignSucceeded"
+	// ActionFailure means that action (sign or build, depending on the action field) has failed
+	ActionFailure BuildOrSignStatus = "Failure"
 )
 
-// ModuleImageSpec describes the image whose state needs to be queried
+// ModuleBuildSignSpec describes the image whose state needs to be queried
 type ModuleBuildSignSpec struct {
 	ModuleImageSpec `json:",inline"`
 
@@ -62,6 +57,23 @@ type ModuleBuildSignConfigSpec struct {
 	ImageRepoSecret *v1.LocalObjectReference `json:"imageRepoSecret,omitempty"`
 }
 
+// BuildSignImageState contains the status of the image that was requested to be built/signed
+type BuildSignImageState struct {
+	Image string `json:"image"`
+
+	// +kubebuilder:validation:Enum=Success;Failure
+	Status BuildOrSignStatus `json:"status"`
+
+	// +kubebuilder:validation:Enum=BuildImage;SignImage
+	Action BuildOrSignAction `json:"action"`
+}
+
+// ModuleBuildSignConfigStatus describes the status of the images that needed to be built/signed
+// More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#spec-and-status
+type ModuleBuildSignConfigStatus struct {
+	Images []BuildSignImageState `json:"images"`
+}
+
 // +kubebuilder:object:root=true
 // +kubebuilder:subresource:status
 
@@ -72,8 +84,8 @@ type ModuleBuildSignConfig struct {
 	metav1.TypeMeta   `json:",inline"`
 	metav1.ObjectMeta `json:"metadata,omitempty"`
 
-	Spec   ModuleBuildSignConfigSpec `json:"spec,omitempty"`
-	Status ModuleImagesConfigStatus  `json:"status,omitempty"`
+	Spec   ModuleBuildSignConfigSpec   `json:"spec,omitempty"`
+	Status ModuleBuildSignConfigStatus `json:"status,omitempty"`
 }
 
 // +kubebuilder:object:root=true

api/v1beta1/zz_generated.deepcopy.go

@@ -61,8 +61,8 @@ spec:
                 x-kubernetes-map-type: atomic
               images:
                 items:
-                  description: ModuleImageSpec describes the image whose state needs
-                    to be queried
+                  description: ModuleBuildSignSpec describes the image whose state
+                    needs to be queried
                   properties:
                     action:
                       enum:
@@ -233,27 +233,34 @@ spec:
             type: object
           status:
             description: |-
-              ModuleImagesConfigStatus describes the status of the images that need to be verified (defined in the spec)
+              ModuleBuildSignConfigStatus describes the status of the images that needed to be built/signed
               More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#spec-and-status
             properties:
-              imagesStates:
+              images:
                 items:
+                  description: BuildSignImageState contains the status of the image
+                    that was requested to be built/signed
                   properties:
+                    action:
+                      enum:
+                      - BuildImage
+                      - SignImage
+                      type: string
                     image:
-                      description: image
                       type: string
                     status:
-                      description: |-
-                        status of the image
-                        one of: Exists, notExists
+                      enum:
+                      - Success
+                      - Failure
                       type: string
                   required:
+                  - action
                   - image
                   - status
                   type: object
                 type: array
             required:
-            - imagesStates
+            - images
             type: object
         type: object
     served: true

config/crd/bases/kmm.sigs.x-k8s.io_modulebuildsignconfigs.yaml

@@ -188,23 +188,29 @@ func (mrhi *micReconcilerHelperImpl) updateStatusByMBSC(ctx context.Context, mic
 	}
 
 	patchFrom := client.MergeFrom(micObj.DeepCopy())
-	for _, imageState := range mbsc.Status.ImagesStates {
-		imageSpec := mrhi.micHelper.GetModuleImageSpec(micObj, imageState.Image)
-		if imageSpec == nil {
-			// image not found in spec, ignore
+	for _, mbscImageState := range mbsc.Status.Images {
+		micImageSpec := mrhi.micHelper.GetModuleImageSpec(micObj, mbscImageState.Image)
+		if micImageSpec == nil {
+			// spec for image does not exists in MIC, meaning it was already deleted - ignore status
 			continue
 		}
-		switch imageState.Status {
-		case kmmv1beta1.ImageBuildFailed, kmmv1beta1.ImageSignFailed:
-			mrhi.micHelper.SetImageStatus(micObj, imageState.Image, kmmv1beta1.ImageDoesNotExist)
-		case kmmv1beta1.ImageBuildSucceeded:
-			if imageSpec.Sign != nil {
-				mrhi.micHelper.SetImageStatus(micObj, imageState.Image, kmmv1beta1.ImageNeedsSigning)
-			} else {
-				mrhi.micHelper.SetImageStatus(micObj, imageState.Image, kmmv1beta1.ImageExists)
-			}
-		case kmmv1beta1.ImageSignSucceeded:
-			mrhi.micHelper.SetImageStatus(micObj, imageState.Image, kmmv1beta1.ImageExists)
+
+		mbscStatus := mbscImageState.Status
+		mbscAction := mbscImageState.Action
+
+		switch {
+		case mbscStatus == kmmv1beta1.ActionFailure:
+			// any failure (build or sign) - image does not exists
+			mrhi.micHelper.SetImageStatus(micObj, mbscImageState.Image, kmmv1beta1.ImageDoesNotExist)
+		case mbscStatus == kmmv1beta1.ActionSuccess && mbscAction == kmmv1beta1.SignImage:
+			// sign action succeeded - image exists, nothing more to do
+			mrhi.micHelper.SetImageStatus(micObj, mbscImageState.Image, kmmv1beta1.ImageExists)
+		case mbscStatus == kmmv1beta1.ActionSuccess && micImageSpec.Sign != nil:
+			// build succeeded and sign exists - image needs to be signed
+			mrhi.micHelper.SetImageStatus(micObj, mbscImageState.Image, kmmv1beta1.ImageNeedsSigning)
+		case mbscStatus == kmmv1beta1.ActionSuccess:
+			// build succeeded, no sign - image exists
+			mrhi.micHelper.SetImageStatus(micObj, mbscImageState.Image, kmmv1beta1.ImageExists)
 		}
 	}
 

internal/controllers/mic_reconciler.go

@@ -272,8 +272,8 @@ var _ = Describe("updateStatusByMBSC", func() {
 
 	It("Image in MBSC status does not exists in MIC spec", func() {
 		testMBSC := kmmv1beta1.ModuleBuildSignConfig{
-			Status: kmmv1beta1.ModuleImagesConfigStatus{
-				ImagesStates: []kmmv1beta1.ModuleImageState{
+			Status: kmmv1beta1.ModuleBuildSignConfigStatus{
+				Images: []kmmv1beta1.BuildSignImageState{
 					{
 						Image: "some image",
 					},
@@ -282,22 +282,24 @@ var _ = Describe("updateStatusByMBSC", func() {
 		}
 		gomock.InOrder(
 			mbscHelper.EXPECT().Get(ctx, testMic.Name, testMic.Namespace).Return(&testMBSC, nil),
-			micHelper.EXPECT().GetModuleImageSpec(&testMic, testMBSC.Status.ImagesStates[0].Image).Return(nil),
+			micHelper.EXPECT().GetModuleImageSpec(&testMic, "some image").Return(nil),
 			clnt.EXPECT().Status().Return(statusWriter),
 			statusWriter.EXPECT().Patch(ctx, &testMic, gomock.Any()),
 		)
 		err := mrh.updateStatusByMBSC(ctx, &testMic)
 		Expect(err).To(BeNil())
 	})
 
-	DescribeTable("images in MBSC status exist in MIC spec",
-		func(signExists bool, mbscImageStatus kmmv1beta1.ImageState, expectedImageState kmmv1beta1.ImageState) {
+	DescribeTable("image has status in MBSC and spec in MIC",
+		func(signExists bool, mbscImageAction kmmv1beta1.BuildOrSignAction, mbscImageStatus kmmv1beta1.BuildOrSignStatus,
+			expectedMICImageState kmmv1beta1.ImageState) {
 			testMBSC := kmmv1beta1.ModuleBuildSignConfig{
-				Status: kmmv1beta1.ModuleImagesConfigStatus{
-					ImagesStates: []kmmv1beta1.ModuleImageState{
+				Status: kmmv1beta1.ModuleBuildSignConfigStatus{
+					Images: []kmmv1beta1.BuildSignImageState{
 						{
 							Image:  "some image",
 							Status: mbscImageStatus,
+							Action: mbscImageAction,
 						},
 					},
 				},
@@ -309,20 +311,22 @@ var _ = Describe("updateStatusByMBSC", func() {
 			gomock.InOrder(
 				mbscHelper.EXPECT().Get(ctx, testMic.Name, testMic.Namespace).Return(&testMBSC, nil),
 				micHelper.EXPECT().GetModuleImageSpec(&testMic, "some image").Return(&imageSpec),
-				micHelper.EXPECT().SetImageStatus(&testMic, "some image", expectedImageState),
+				micHelper.EXPECT().SetImageStatus(&testMic, "some image", expectedMICImageState),
 				clnt.EXPECT().Status().Return(statusWriter),
 				statusWriter.EXPECT().Patch(ctx, &testMic, gomock.Any()),
 			)
 
 			err := mrh.updateStatusByMBSC(ctx, &testMic)
 			Expect(err).To(BeNil())
 		},
-		Entry("sign config does not exists, status is ImageBuildFailed", false, kmmv1beta1.ImageBuildFailed, kmmv1beta1.ImageDoesNotExist),
-		Entry("sign config does not exists, status is ImageBuildSucceeded", false, kmmv1beta1.ImageBuildSucceeded, kmmv1beta1.ImageExists),
-		Entry("sign config exists, status is ImageBuildFailed", true, kmmv1beta1.ImageBuildFailed, kmmv1beta1.ImageDoesNotExist),
-		Entry("sign config exists, status is ImageBuildSucceeded", true, kmmv1beta1.ImageBuildSucceeded, kmmv1beta1.ImageNeedsSigning),
-		Entry("status is ImageSignFailed", false, kmmv1beta1.ImageSignFailed, kmmv1beta1.ImageDoesNotExist),
-		Entry("status is ImageSignSucceeded", false, kmmv1beta1.ImageSignSucceeded, kmmv1beta1.ImageExists),
+		Entry("sign config does not exists, action Build, status Failed", false, kmmv1beta1.BuildImage, kmmv1beta1.ActionFailure, kmmv1beta1.ImageDoesNotExist),
+		Entry("sign config does not exists, action Sign, status Failed", false, kmmv1beta1.SignImage, kmmv1beta1.ActionFailure, kmmv1beta1.ImageDoesNotExist),
+		Entry("sign config does not exists, action Build, status Succeeded", false, kmmv1beta1.BuildImage, kmmv1beta1.ActionSuccess, kmmv1beta1.ImageExists),
+		Entry("sign config does not exists, action Sign, status Succeeded", false, kmmv1beta1.SignImage, kmmv1beta1.ActionSuccess, kmmv1beta1.ImageExists),
+		Entry("sign config exists, action Build, status Failed", true, kmmv1beta1.BuildImage, kmmv1beta1.ActionFailure, kmmv1beta1.ImageDoesNotExist),
+		Entry("sign config exists, action Sign, status Failed", true, kmmv1beta1.SignImage, kmmv1beta1.ActionFailure, kmmv1beta1.ImageDoesNotExist),
+		Entry("sign config exists, action Build, status Succeeded", true, kmmv1beta1.BuildImage, kmmv1beta1.ActionSuccess, kmmv1beta1.ImageNeedsSigning),
+		Entry("sign config exists, action Sign, status Succeeded", true, kmmv1beta1.SignImage, kmmv1beta1.ActionSuccess, kmmv1beta1.ImageExists),
 	)
 })