Adding action field to the MBSC image spec and changing logic

accordingly

Current implementation of BuildSign controller is running Build and Sign depending
on the existence of the intermidiate images. Images are verified by the
direct HTTP call to the repo, which we are trying to depricate.
The new solution is to make MIC controller to decide what action MBSC
controller needs to execute (Build or Sign). MIC controller makes this
decision based on the status of the pull pods, and the status of the
image in the MBSC:
1) if pull pod fails - schedule Build if Build config exists in image
   Spec
2) if pull pods fails - schedule Sign , if Build config missing and Sign
   config exists in the image Spec
3) if MBSC status is BuildSuccess - schedule Sign if config exists in
   the image spec, otherwise set the image to Exists
4) if MBSC status is BuildFailed - set the image status to ImageDoesNotExist
5) if MBSC status is SignSuccess - set the image to Exists
6) if MBSC status is SignFailed - set the image to ImageDoesNotExist

The changes in PR:
1) chaning the MBSC CRD  - adding field and renaming some of the fields
2) updating MBSC package and unit-tests
3) updating MIC controller and unit-test

api/v1beta1/modulebuildsignconfig_types.go

@@ -17,9 +17,51 @@ limitations under the License.
 package v1beta1
 
 import (
+	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
+type BuildOrSignAction string
+
+const (
+	// BuildImage means that image needs to be built
+	BuildImage BuildOrSignAction = "BuildImage"
+
+	// 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"
+
+	// 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"
+)
+
+// ModuleImageSpec describes the image whose state needs to be queried
+type ModuleBuildSignSpec struct {
+	ModuleImageSpec `json:",inline"`
+
+	// +kubebuilder:validation:Enum=BuildImage;SignImage
+	Action BuildOrSignAction `json:"action"`
+}
+
+// ModuleBuildSignConfigSpec describes the images that need to be built/signed
+// More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#spec-and-status
+// +kubebuilder:validation:Required
+type ModuleBuildSignConfigSpec struct {
+	Images []ModuleBuildSignSpec `json:"images"`
+
+	// ImageRepoSecret contains pull secret for the image's repo, if needed
+	// +optional
+	ImageRepoSecret *v1.LocalObjectReference `json:"imageRepoSecret,omitempty"`
+}
+
 // +kubebuilder:object:root=true
 // +kubebuilder:subresource:status
 
@@ -30,8 +72,8 @@ type ModuleBuildSignConfig struct {
 	metav1.TypeMeta   `json:",inline"`
 	metav1.ObjectMeta `json:"metadata,omitempty"`
 
-	Spec   ModuleImagesConfigSpec   `json:"spec,omitempty"`
-	Status ModuleImagesConfigStatus `json:"status,omitempty"`
+	Spec   ModuleBuildSignConfigSpec `json:"spec,omitempty"`
+	Status ModuleImagesConfigStatus  `json:"status,omitempty"`
 }
 
 // +kubebuilder:object:root=true

api/v1beta1/moduleimagesconfig_types.go

@@ -28,12 +28,10 @@ const (
 	ImageExists ImageState = "Exists"
 	// ImageDoesNotExist means that image does not exist in the specified repo
 	ImageDoesNotExist ImageState = "DoesNotExist"
-	// ImageNeedsBuilding means that image does not exists, but has Build/Sign sections and can be built
+	// ImageNeedsBuilding means that image does not exists, but has Build section and can be built
 	ImageNeedsBuilding ImageState = "NeedsBuilding"
-	// ImageBuildFailed means that image does not exists and the Build/Sign failed
-	ImageBuildFailed ImageState = "BuildFailed"
-	// ImageBuildSucceeded means that image has been built and pushed succesfully
-	ImageBuildSucceeded ImageState = "BuildSucceeded"
+	// ImageNeedsSigning means that images needs signing, because it was pre-built, or in-cluster build succeeded
+	ImageNeedsSigning ImageState = "NeedsSigning"
 )
 
 // ModuleImageSpec describes the image whose state needs to be queried

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

@@ -41,7 +41,7 @@ spec:
             type: object
           spec:
             description: |-
-              ModuleImagesConfigSpec describes the images of the Module whose status needs to be verified
+              ModuleBuildSignConfigSpec describes the images that need to be built/signed
               More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#spec-and-status
             properties:
               imageRepoSecret:
@@ -64,6 +64,11 @@ spec:
                   description: ModuleImageSpec describes the image whose state needs
                     to be queried
                   properties:
+                    action:
+                      enum:
+                      - BuildImage
+                      - SignImage
+                      type: string
                     build:
                       description: Build contains build instructions, in case image
                         needs building
@@ -219,6 +224,7 @@ spec:
                       - keySecret
                       type: object
                   required:
+                  - action
                   - image
                   type: object
                 type: array

internal/controllers/mic_reconciler.go

@@ -128,7 +128,7 @@ func newMICReconcilerHelper(client client.Client,
 }
 
 func (mrhi *micReconcilerHelperImpl) updateStatusByPullPods(ctx context.Context, micObj *kmmv1beta1.ModuleImagesConfig, pods []v1.Pod) error {
-	logger := ctrl.LoggerFrom(ctx)
+	logger := ctrl.LoggerFrom(ctx).WithValues("mic name", micObj.Name)
 	if len(pods) == 0 {
 		return nil
 	}
@@ -139,19 +139,24 @@ func (mrhi *micReconcilerHelperImpl) updateStatusByPullPods(ctx context.Context,
 	for _, p := range pods {
 		image := p.Labels[imageLabelKey]
 		imageSpec := mrhi.micHelper.GetModuleImageSpec(micObj, image)
+		logger := logger.WithValues("image", image)
 		if imageSpec == nil {
-			logger.Info(utils.WarnString("image not present in spec during updateStatusByPods, deleting pod"), "mic", micObj.Name, "image", image)
+			logger.Info(utils.WarnString("image not present in spec during updateStatusByPods, deleting pod"))
 			podsToDelete = append(podsToDelete, p)
 			continue
 		}
 		phase := p.Status.Phase
 		switch phase {
 		case v1.PodFailed:
-			if imageSpec.Build != nil || imageSpec.Sign != nil {
-				logger.Info("failed pod with build or sign spec, updating image status to NeedsBulding")
+			switch {
+			case imageSpec.Build != nil:
+				logger.Info("pull pod failed, build exists, setting status to kmmv1beta1.ImageNeedsBuilding")
 				mrhi.micHelper.SetImageStatus(micObj, image, kmmv1beta1.ImageNeedsBuilding)
-			} else {
-				logger.Info(utils.WarnString("failed pod without build or sign spec, shoud not have happened"), "image", image)
+			case imageSpec.Sign != nil:
+				logger.Info("pull pod failed, build does not exist, sign exists, setting status to kmmv1beta1.ImageNeedsSigning")
+				mrhi.micHelper.SetImageStatus(micObj, image, kmmv1beta1.ImageNeedsSigning)
+			default:
+				logger.Info(utils.WarnString("failed pod without build or sign spec, shoud not have happened"))
 			}
 			podsToDelete = append(podsToDelete, p)
 		case v1.PodSucceeded:
@@ -194,11 +199,18 @@ func (mrhi *micReconcilerHelperImpl) updateStatusByMBSC(ctx context.Context, mic
 			// image not found in spec, ignore
 			continue
 		}
-		micImageStatus := kmmv1beta1.ImageExists
-		if imageState.Status == kmmv1beta1.ImageBuildFailed {
-			micImageStatus = kmmv1beta1.ImageDoesNotExist
+		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)
 		}
-		mrhi.micHelper.SetImageStatus(micObj, imageState.Image, micImageStatus)
 	}
 
 	return mrhi.client.Status().Patch(ctx, micObj, patchFrom)
@@ -209,13 +221,13 @@ func (mrhi *micReconcilerHelperImpl) processImagesSpecs(ctx context.Context, mic
 	for _, imageSpec := range micObj.Spec.Images {
 		imageState := mrhi.micHelper.GetImageState(micObj, imageSpec.Image)
 
+		var err error
 		switch imageState {
 		case "":
 			// image State is not set: either new image or pull pod is still running
 			if mrhi.podHelper.getPullPodForImage(pullPods, imageSpec.Image) == nil {
 				// no pull pod- create it, otherwise we wait for it to finish
-				err := mrhi.podHelper.createPullPod(ctx, &imageSpec, micObj)
-				errs = append(errs, err)
+				err = mrhi.podHelper.createPullPod(ctx, &imageSpec, micObj)
 			}
 		case kmmv1beta1.ImageDoesNotExist:
 			if imageSpec.Build == nil && imageSpec.Sign == nil {
@@ -224,9 +236,12 @@ func (mrhi *micReconcilerHelperImpl) processImagesSpecs(ctx context.Context, mic
 			fallthrough
 		case kmmv1beta1.ImageNeedsBuilding:
 			// image needs to be built - patching MBSC
-			err := mrhi.mbscHelper.CreateOrPatch(ctx, micObj.Name, micObj.Namespace, &imageSpec, micObj.Spec.ImageRepoSecret, micObj)
-			errs = append(errs, err)
+			err = mrhi.mbscHelper.CreateOrPatch(ctx, micObj, &imageSpec, kmmv1beta1.BuildImage)
+		case kmmv1beta1.ImageNeedsSigning:
+			// image needs to be signed - patching MBSC
+			err = mrhi.mbscHelper.CreateOrPatch(ctx, micObj, &imageSpec, kmmv1beta1.SignImage)
 		}
+		errs = append(errs, err)
 	}
 	return errors.Join(errs...)
 }