Fix runtimeBaseImage retrieval (#1478)

Co-authored-by: Andrzej Pankowski <[email protected]>

components/serverless/internal/controllers/serverless/build_resources_test.go

@@ -101,7 +101,6 @@ func TestFunctionReconciler_buildDeployment(t *testing.T) {
 			}
 
 			got := s.buildDeployment(buildDeploymentArgs{}, cfg{
-				runtimeBaseImage: "some_image",
 				fn: FunctionConfig{
 					ResourceConfig: ResourceConfig{
 						Function: FunctionResourceConfig{
@@ -212,7 +211,6 @@ func TestFunctionReconciler_buildDeploymentWithResources(t *testing.T) {
 			s := systemState{instance: *tt.args.instance}
 
 			got := s.buildDeployment(buildDeploymentArgs{}, cfg{
-				runtimeBaseImage: "some_image",
 				fn: FunctionConfig{
 					ResourceConfig: ResourceConfig{
 						Function: FunctionResourceConfig{

components/serverless/internal/controllers/serverless/configmap.go

@@ -26,7 +26,7 @@ func stateFnInlineCheckSources(ctx context.Context, r *reconciler, s *systemStat
 		return nil, errors.Wrap(err, "while listing deployments")
 	}
 
-	srcChanged := s.inlineFnSrcChanged(r.cfg.docker.PullAddress, r.cfg.runtimeBaseImage)
+	srcChanged := s.inlineFnSrcChanged(r.cfg.docker.PullAddress, s.runtimeBaseImage)
 	if !srcChanged {
 		cfgStatus := getConditionStatus(s.instance.Status.Conditions, serverlessv1alpha2.ConditionConfigurationReady)
 		if cfgStatus == corev1.ConditionTrue {

components/serverless/internal/controllers/serverless/fsm.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"
 	"reflect"
+	"regexp"
 	"runtime"
 	"strings"
 	"time"
@@ -31,9 +32,8 @@ type k8s struct {
 }
 
 type cfg struct {
-	runtimeBaseImage string
-	docker           DockerConfig
-	fn               FunctionConfig
+	docker DockerConfig
+	fn     FunctionConfig
 }
 
 // nolint
@@ -287,6 +287,13 @@ func stateFnInitialize(ctx context.Context, r *reconciler, s *systemState) (stat
 		return nil, errors.Wrap(err, "context error")
 	}
 
+	latestRuntimeImage, err := r.getRuntimeImageFromConfigMap(ctx, s.instance.GetNamespace(), s.instance.Spec.Runtime)
+	if err != nil {
+		return nil, errors.Wrap(err, "failed to fetch runtime image from config map")
+	}
+
+	s.runtimeBaseImage = latestRuntimeImage
+
 	isGitType := s.instance.TypeOf(serverlessv1alpha2.FunctionTypeGit)
 	if isGitType {
 		return stateFnGitCheckSources, nil
@@ -309,3 +316,21 @@ func skipGitSourceCheck(f serverlessv1alpha2.Function, cfg cfg) bool {
 
 	return time.Since(configured.LastTransitionTime.Time) < cfg.fn.FunctionReadyRequeueDuration
 }
+
+func (r *reconciler) getRuntimeImageFromConfigMap(ctx context.Context, namespace string, runtime serverlessv1alpha2.Runtime) (string, error) {
+	instance := &corev1.ConfigMap{}
+	dockerfileConfigMapName := fmt.Sprintf("dockerfile-%s", runtime)
+	err := r.client.Get(ctx, types.NamespacedName{Namespace: namespace, Name: dockerfileConfigMapName}, instance)
+	if err != nil {
+		return "", errors.Wrap(err, "while extracting correct config map for given runtime")
+	}
+	baseImage := instance.Data["Dockerfile"]
+	re := regexp.MustCompile(`base_image=.*`)
+	matchedLines := re.FindStringSubmatch(baseImage)
+	if len(matchedLines) == 0 {
+		return "", errors.Errorf("could not find the base image from %s", dockerfileConfigMapName)
+	}
+
+	runtimeImage := strings.TrimPrefix(matchedLines[0], "base_image=")
+	return runtimeImage, err
+}

components/serverless/internal/controllers/serverless/function_reconcile.go

@@ -2,9 +2,6 @@ package serverless
 
 import (
 	"context"
-	"fmt"
-	"regexp"
-	"strings"
 	"time"
 
 	"github.com/pkg/errors"
@@ -13,7 +10,6 @@ import (
 	appsv1 "k8s.io/api/apps/v1"
 	autoscalingv1 "k8s.io/api/autoscaling/v1"
 	corev1 "k8s.io/api/core/v1"
-	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/tools/record"
 	"k8s.io/client-go/util/workqueue"
 	ctrl "sigs.k8s.io/controller-runtime"
@@ -142,16 +138,6 @@ func (r *FunctionReconciler) Reconcile(ctx context.Context, request ctrl.Request
 
 	contextLogger.Debug("starting state machine")
 
-	runtime := instance.Status.Runtime
-	if runtime == "" {
-		runtime = instance.Spec.Runtime
-	}
-
-	latestRuntimeImage, err := r.getRuntimeImageFromConfigMap(ctx, instance.GetNamespace(), runtime)
-	if err != nil {
-		return ctrl.Result{}, errors.Wrap(err, "failed to fetch runtime image from config map")
-	}
-
 	stateReconciler := reconciler{
 		fn:  r.initStateFunction,
 		log: contextLogger,
@@ -161,9 +147,8 @@ func (r *FunctionReconciler) Reconcile(ctx context.Context, request ctrl.Request
 			statsCollector: r.statsCollector,
 		},
 		cfg: cfg{
-			fn:               r.config,
-			docker:           dockerCfg,
-			runtimeBaseImage: latestRuntimeImage,
+			fn:     r.config,
+			docker: dockerCfg,
 		},
 		gitClient: r.gitFactory.GetGitClient(contextLogger),
 	}
@@ -209,20 +194,3 @@ func (r *FunctionReconciler) readDockerConfig(ctx context.Context, instance *ser
 	}
 
 }
-
-func (r *FunctionReconciler) getRuntimeImageFromConfigMap(ctx context.Context, namespace string, runtime serverlessv1alpha2.Runtime) (string, error) {
-	instance := &corev1.ConfigMap{}
-	dockerfileConfigMapName := fmt.Sprintf("dockerfile-%s", runtime)
-	err := r.client.Get(ctx, types.NamespacedName{Namespace: namespace, Name: dockerfileConfigMapName}, instance)
-	if err != nil {
-		return "", errors.Wrap(err, "while extracting correct config map for given runtime")
-	}
-	baseImage := instance.Data["Dockerfile"]
-	re := regexp.MustCompile(`base_image=.*`)
-	matchedLines := re.FindStringSubmatch(baseImage)
-	if len(matchedLines) == 0 {
-		return "", errors.Errorf("could not find the base image from %s", dockerfileConfigMapName)
-	}
-	runtimeImage := strings.TrimPrefix(matchedLines[0], "base_image=")
-	return runtimeImage, err
-}

components/serverless/internal/controllers/serverless/function_reconcile_gitops_test.go

@@ -152,7 +152,7 @@ func TestGitOpsWithContinuousGitCheckout(t *testing.T) {
 				config:            testCfg,
 				gitFactory:        factory,
 				statsCollector:    statsCollector,
-				initStateFunction: stateFnGitCheckSources,
+				initStateFunction: stateFnInitialize,
 			}
 
 			fnLabels := reconciler.internalFunctionLabels(inFunction)
@@ -477,7 +477,7 @@ func TestGitOpsWithoutContinuousGitCheckout(t *testing.T) {
 				config:            testCfg,
 				gitFactory:        factory,
 				statsCollector:    statsCollector,
-				initStateFunction: stateFnGitCheckSources,
+				initStateFunction: stateFnInitialize,
 			}
 
 			fnLabels := reconciler.internalFunctionLabels(inFunction)

components/serverless/internal/controllers/serverless/job.go

@@ -65,9 +65,9 @@ func buildStateFnCheckImageJob(expectedJob batchv1.Job) stateFn {
 			return buildStatusUpdateStateFnWithCondition(condition), nil
 		}
 
-		s.fnImage = s.buildImageAddress(r.cfg.docker.PullAddress, r.cfg.runtimeBaseImage)
+		s.fnImage = s.buildImageAddress(r.cfg.docker.PullAddress, s.runtimeBaseImage)
 
-		diffRuntimeImage := functionRuntimeChanged(ctx, r, s)
+		diffRuntimeImage := functionRuntimeChanged(ctx, s)
 
 		if diffRuntimeImage {
 			return stateFnInlineDeleteJobs, nil
@@ -92,7 +92,7 @@ func buildStateFnCheckImageJob(expectedJob batchv1.Job) stateFn {
 	}
 }
 
-func functionRuntimeChanged(_ context.Context, r *reconciler, s *systemState) bool {
+func functionRuntimeChanged(_ context.Context, s *systemState) bool {
 	functionRuntimeImage := s.instance.Status.RuntimeImage
 	if functionRuntimeImage == "" {
 		return false
@@ -102,7 +102,7 @@ func functionRuntimeChanged(_ context.Context, r *reconciler, s *systemState) bo
 		return !result
 	}
 
-	result := r.cfg.runtimeBaseImage == functionRuntimeImage
+	result := s.runtimeBaseImage == functionRuntimeImage
 	return !result
 }
 
@@ -139,7 +139,7 @@ func buildStateFnRunJob(expectedJob batchv1.Job) stateFn {
 			Message:            fmt.Sprintf("Job %s created", expectedJob.GetName()),
 		}
 
-		s.instance.Status.RuntimeImage = r.cfg.runtimeBaseImage
+		s.instance.Status.RuntimeImage = s.runtimeBaseImage
 		return buildStatusUpdateStateFnWithCondition(condition), nil
 	}
 }

components/serverless/internal/controllers/serverless/system_state.go

@@ -27,13 +27,14 @@ type SystemState interface{}
 
 // TODO extract interface
 type systemState struct {
-	instance    serverlessv1alpha2.Function
-	fnImage     string               // TODO make sure this is needed
-	configMaps  corev1.ConfigMapList // TODO create issue to refactor this (only 1 config map should be here)
-	deployments appsv1.DeploymentList
-	jobs        batchv1.JobList
-	services    corev1.ServiceList
-	hpas        autoscalingv1.HorizontalPodAutoscalerList
+	instance         serverlessv1alpha2.Function
+	fnImage          string               // TODO make sure this is needed
+	configMaps       corev1.ConfigMapList // TODO create issue to refactor this (only 1 config map should be here)
+	deployments      appsv1.DeploymentList
+	jobs             batchv1.JobList
+	services         corev1.ServiceList
+	hpas             autoscalingv1.HorizontalPodAutoscalerList
+	runtimeBaseImage string
 }
 
 var _ SystemState = systemState{}
@@ -240,7 +241,7 @@ func (s *systemState) buildGitJobRepoFetcherContainer(gitOptions git.Options, cf
 }
 
 func (s *systemState) buildJobExecutorContainer(cfg cfg, volumeMounts []corev1.VolumeMount) corev1.Container {
-	imageName := s.buildImageAddress(cfg.docker.PushAddress, cfg.runtimeBaseImage)
+	imageName := s.buildImageAddress(cfg.docker.PushAddress, s.runtimeBaseImage)
 	args := append(cfg.fn.Build.ExecutorArgs,
 		fmt.Sprintf("%s=%s", destinationArg, imageName),
 		fmt.Sprintf("--context=dir://%s", workspaceMountPath))
@@ -410,7 +411,7 @@ type buildDeploymentArgs struct {
 }
 
 func (s *systemState) buildDeployment(args buildDeploymentArgs, cfg cfg) appsv1.Deployment {
-	imageName := s.buildImageAddress(args.DockerPullAddress, cfg.runtimeBaseImage)
+	imageName := s.buildImageAddress(args.DockerPullAddress, s.runtimeBaseImage)
 
 	const volumeName = "tmp-dir"
 	emptyDirVolumeSize := resource.MustParse("100Mi")

components/serverless/internal/controllers/serverless/validation_test.go

@@ -41,8 +41,8 @@ func TestValidation_Invalid(t *testing.T) {
 	//GIVEN
 	ctx := context.TODO()
 
-	k8sClient := fake.NewClientBuilder().WithStatusSubresource(&serverlessv1alpha2.Function{}).Build()
 	require.NoError(t, serverlessv1alpha2.AddToScheme(scheme.Scheme))
+	k8sClient := fake.NewClientBuilder().WithScheme(scheme.Scheme).WithStatusSubresource(&serverlessv1alpha2.Function{}).Build()
 	resourceClient := serverlessResource.New(k8sClient, scheme.Scheme)
 
 	statsCollector := &automock.StatsCollector{}
@@ -602,9 +602,7 @@ func TestValidation_Valid(t *testing.T) {
 				cfg: cfg{fn: FunctionConfig{ResourceConfig: minResourcesCfg}}}
 
 			//WHEN
-			nextFn, err := stateFnValidateFunction(ctx, r, s)
-			require.NoError(t, err)
-			_, err = nextFn(context.TODO(), r, s)
+			_, err := stateFnValidateFunction(ctx, r, s)
 
 			//THEN
 			require.NoError(t, err)