Refactor build / sign managers

Create a new internal/imgbuild package that brings a new Manager type
covering the needs of both features, as well as a JobMaker interface.
Implement that interface in internal/{build,sign} packages.
Consolidate ModuleLoaderData generation code into the internal/api
package.
Move utils functions into their own package and remove duplicate code.

Author: qbarrand

cmd/manager-hub/main.go

@@ -19,6 +19,8 @@ package main
 import (
 	"flag"
 
+	"github.com/kubernetes-sigs/kernel-module-management/internal/api"
+	"github.com/kubernetes-sigs/kernel-module-management/internal/imgbuild"
 	"k8s.io/apimachinery/pkg/runtime"
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
 	clientgoscheme "k8s.io/client-go/kubernetes/scheme"
@@ -36,19 +38,15 @@ import (
 	"github.com/kubernetes-sigs/kernel-module-management/api-hub/v1beta1"
 	"github.com/kubernetes-sigs/kernel-module-management/controllers/hub"
 	"github.com/kubernetes-sigs/kernel-module-management/internal/build"
-	"github.com/kubernetes-sigs/kernel-module-management/internal/build/job"
 	"github.com/kubernetes-sigs/kernel-module-management/internal/cluster"
 	"github.com/kubernetes-sigs/kernel-module-management/internal/cmd"
 	"github.com/kubernetes-sigs/kernel-module-management/internal/constants"
 	"github.com/kubernetes-sigs/kernel-module-management/internal/filter"
 	"github.com/kubernetes-sigs/kernel-module-management/internal/manifestwork"
 	"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/sign"
-	signjob "github.com/kubernetes-sigs/kernel-module-management/internal/sign/job"
 	"github.com/kubernetes-sigs/kernel-module-management/internal/statusupdater"
-	"github.com/kubernetes-sigs/kernel-module-management/internal/utils"
 	//+kubebuilder:scaffold:imports
 )
 
@@ -105,20 +103,26 @@ func main() {
 	metricsAPI.Register()
 
 	registryAPI := registry.NewRegistry()
-	jobHelperAPI := utils.NewJobHelper(client)
-	buildHelper := build.NewHelper()
+	jobHelperAPI := imgbuild.NewJobHelper(client)
+	builderImage := cmd.GetEnvOrFatalError(constants.BuildImageEnvVar, setupLogger)
 
-	buildAPI := job.NewBuildManager(
+	buildAPI := imgbuild.NewBuildManager(
 		client,
-		job.NewMaker(client, buildHelper, jobHelperAPI, scheme),
 		jobHelperAPI,
+		build.NewJobMaker(client, builderImage, jobHelperAPI, scheme),
 		registryAPI,
 	)
 
-	signAPI := signjob.NewSignJobManager(
+	signAPI := imgbuild.NewSignManager(
 		client,
-		signjob.NewSigner(client, scheme, jobHelperAPI),
 		jobHelperAPI,
+		sign.NewJobMaker(
+			client,
+			jobHelperAPI,
+			builderImage,
+			cmd.GetEnvOrFatalError(constants.SignImageEnvVar, setupLogger),
+			scheme,
+		),
 		registryAPI,
 	)
 
@@ -130,7 +134,7 @@ func main() {
 	mcmr := hub.NewManagedClusterModuleReconciler(
 		client,
 		manifestwork.NewCreator(client, scheme),
-		cluster.NewClusterAPI(client, module.NewKernelMapper(buildHelper, sign.NewSignerHelper()), buildAPI, signAPI, operatorNamespace),
+		cluster.NewClusterAPI(client, api.NewModuleLoaderDataFactory(), buildAPI, signAPI, operatorNamespace),
 		statusupdater.NewManagedClusterModuleStatusUpdater(client),
 		filterAPI,
 	)

cmd/manager/main.go

@@ -22,6 +22,8 @@ import (
 	"os"
 	"strconv"
 
+	"github.com/kubernetes-sigs/kernel-module-management/internal/api"
+	"github.com/kubernetes-sigs/kernel-module-management/internal/imgbuild"
 	"k8s.io/apimachinery/pkg/runtime"
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
 	clientgoscheme "k8s.io/client-go/kubernetes/scheme"
@@ -38,19 +40,15 @@ import (
 	v1beta12 "github.com/kubernetes-sigs/kernel-module-management/api/v1beta1"
 	"github.com/kubernetes-sigs/kernel-module-management/controllers"
 	"github.com/kubernetes-sigs/kernel-module-management/internal/build"
-	"github.com/kubernetes-sigs/kernel-module-management/internal/build/job"
 	"github.com/kubernetes-sigs/kernel-module-management/internal/cmd"
 	"github.com/kubernetes-sigs/kernel-module-management/internal/constants"
 	"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/preflight"
 	"github.com/kubernetes-sigs/kernel-module-management/internal/registry"
 	"github.com/kubernetes-sigs/kernel-module-management/internal/sign"
-	signjob "github.com/kubernetes-sigs/kernel-module-management/internal/sign/job"
 	"github.com/kubernetes-sigs/kernel-module-management/internal/statusupdater"
-	"github.com/kubernetes-sigs/kernel-module-management/internal/utils"
 	//+kubebuilder:scaffold:imports
 )
 
@@ -113,32 +111,38 @@ func main() {
 	metricsAPI.Register()
 
 	registryAPI := registry.NewRegistry()
-	jobHelperAPI := utils.NewJobHelper(client)
-	buildHelperAPI := build.NewHelper()
+	jobHelperAPI := imgbuild.NewJobHelper(client)
+	buildImage := cmd.GetEnvOrFatalError(constants.BuildImageEnvVar, setupLogger)
 
-	buildAPI := job.NewBuildManager(
+	buildAPI := imgbuild.NewBuildManager(
 		client,
-		job.NewMaker(client, buildHelperAPI, jobHelperAPI, scheme),
 		jobHelperAPI,
+		build.NewJobMaker(client, buildImage, jobHelperAPI, scheme),
 		registryAPI,
 	)
 
-	signAPI := signjob.NewSignJobManager(
+	signAPI := imgbuild.NewSignManager(
 		client,
-		signjob.NewSigner(client, scheme, jobHelperAPI),
 		jobHelperAPI,
+		sign.NewJobMaker(
+			client,
+			jobHelperAPI,
+			buildImage,
+			cmd.GetEnvOrFatalError(constants.SignImageEnvVar, setupLogger),
+			scheme,
+		),
 		registryAPI,
 	)
 
 	daemonAPI := daemonset.NewCreator(client, constants.KernelLabel, scheme)
-	kernelAPI := module.NewKernelMapper(buildHelperAPI, sign.NewSignerHelper())
+	mldFactory := api.NewModuleLoaderDataFactory()
 
 	mc := controllers.NewModuleReconciler(
 		client,
 		buildAPI,
 		signAPI,
 		daemonAPI,
-		kernelAPI,
+		mldFactory,
 		metricsAPI,
 		filterAPI,
 		statusupdater.NewModuleStatusUpdater(client),
@@ -160,7 +164,7 @@ func main() {
 	}
 
 	preflightStatusUpdaterAPI := statusupdater.NewPreflightStatusUpdater(client)
-	preflightAPI := preflight.NewPreflightAPI(client, buildAPI, signAPI, registryAPI, preflightStatusUpdaterAPI, kernelAPI)
+	preflightAPI := preflight.NewPreflightAPI(client, buildAPI, signAPI, registryAPI, preflightStatusUpdaterAPI, mldFactory)
 
 	if err = controllers.NewPreflightValidationReconciler(client, filterAPI, metricsAPI, preflightStatusUpdaterAPI, preflightAPI).SetupWithManager(mgr); err != nil {
 		cmd.FatalError(setupLogger, err, "unable to create controller", "name", controllers.PreflightValidationReconcilerName)

controllers/module_reconciler.go

@@ -26,8 +26,8 @@ import (
 	"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/imgbuild"
 	"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/sign"
 	"github.com/kubernetes-sigs/kernel-module-management/internal/statusupdater"
 	"github.com/kubernetes-sigs/kernel-module-management/internal/utils"
@@ -64,9 +64,9 @@ type ModuleReconciler struct {
 func NewModuleReconciler(
 	client client.Client,
 	buildAPI build.Manager,
-	signAPI sign.SignManager,
+	signAPI sign.Manager,
 	daemonAPI daemonset.DaemonSetCreator,
-	kernelAPI module.KernelMapper,
+	kernelAPI api.ModuleLoaderDataFactory,
 	metricsAPI metrics.Metrics,
 	filter *filter.Filter,
 	statusUpdaterAPI statusupdater.ModuleStatusUpdater,
@@ -198,17 +198,17 @@ type hashData struct {
 type moduleReconcilerHelper struct {
 	client     client.Client
 	buildAPI   build.Manager
-	signAPI    sign.SignManager
+	signAPI    sign.Manager
 	daemonAPI  daemonset.DaemonSetCreator
-	kernelAPI  module.KernelMapper
+	kernelAPI  api.ModuleLoaderDataFactory
 	metricsAPI metrics.Metrics
 }
 
 func newModuleReconcilerHelper(client client.Client,
 	buildAPI build.Manager,
-	signAPI sign.SignManager,
+	signAPI sign.Manager,
 	daemonAPI daemonset.DaemonSetCreator,
-	kernelAPI module.KernelMapper,
+	kernelAPI api.ModuleLoaderDataFactory,
 	metricsAPI metrics.Metrics) moduleReconcilerHelperAPI {
 	return &moduleReconcilerHelper{
 		client:     client,
@@ -243,7 +243,7 @@ func (mrh *moduleReconcilerHelper) getRelevantKernelMappingsAndNodes(ctx context
 			continue
 		}
 
-		mld, err := mrh.kernelAPI.GetModuleLoaderDataForKernel(mod, kernelVersion)
+		mld, err := mrh.kernelAPI.FromModule(mod, kernelVersion)
 		if err != nil {
 			nodeLogger.Error(err, "failed to get and process kernel mapping")
 			continue
@@ -301,9 +301,9 @@ func (mrh *moduleReconcilerHelper) handleBuild(ctx context.Context, mld *api.Mod
 
 	completedSuccessfully := false
 	switch buildStatus {
-	case utils.StatusCompleted:
+	case imgbuild.StatusCompleted:
 		completedSuccessfully = true
-	case utils.StatusFailed:
+	case imgbuild.StatusFailed:
 		logger.Info(utils.WarnString("Build job has failed. If the fix is not in Module CR, then delete job after the fix in order to restart the job"))
 	}
 
@@ -320,25 +320,19 @@ func (mrh *moduleReconcilerHelper) handleSigning(ctx context.Context, mld *api.M
 		return true, nil
 	}
 
-	// if we need to sign AND we've built, then we must have built the intermediate image so must figure out its name
-	previousImage := ""
-	if module.ShouldBeBuilt(mld) {
-		previousImage = module.IntermediateImageName(mld.Name, mld.Namespace, mld.ContainerImage)
-	}
-
 	logger := log.FromContext(ctx).WithValues("kernel version", mld.KernelVersion, "image", mld.ContainerImage)
 	signCtx := log.IntoContext(ctx, logger)
 
-	signStatus, err := mrh.signAPI.Sync(signCtx, mld, previousImage, true, mld.Owner)
+	signStatus, err := mrh.signAPI.Sync(signCtx, mld, true, mld.Owner)
 	if err != nil {
 		return false, fmt.Errorf("could not synchronize the signing: %w", err)
 	}
 
 	completedSuccessfully := false
 	switch signStatus {
-	case utils.StatusCompleted:
+	case imgbuild.StatusCompleted:
 		completedSuccessfully = true
-	case utils.StatusFailed:
+	case imgbuild.StatusFailed:
 		logger.Info(utils.WarnString("Sign job has failed. If the fix is not in Module CR, then delete job after the fix in order to restart the job"))
 	}