Skip to content

Commit

Permalink
Improve fn tag calculation (#929)
Browse files Browse the repository at this point in the history
  • Loading branch information
pPrecel authored Apr 29, 2024
1 parent 4d4a0cc commit f16934e
Show file tree
Hide file tree
Showing 11 changed files with 109 additions and 77 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,16 @@ func TestFunctionReconciler_buildDeployment(t *testing.T) {
instance: *tt.args.instance,
}

got := s.buildDeployment(buildDeploymentArgs{}, resourceCfg)
got := s.buildDeployment(buildDeploymentArgs{}, cfg{
runtimeBaseImage: "some_image",
fn: FunctionConfig{
ResourceConfig: ResourceConfig{
Function: FunctionResourceConfig{
Resources: resourceCfg,
},
},
},
})

// deployment selector labels are equal with pod labels
for key, value := range got.Spec.Selector.MatchLabels {
Expand Down Expand Up @@ -204,7 +213,16 @@ func TestFunctionReconciler_buildDeploymentWithResources(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
s := systemState{instance: *tt.args.instance}

got := s.buildDeployment(buildDeploymentArgs{}, resourceCfg)
got := s.buildDeployment(buildDeploymentArgs{}, cfg{
runtimeBaseImage: "some_image",
fn: FunctionConfig{
ResourceConfig: ResourceConfig{
Function: FunctionResourceConfig{
Resources: resourceCfg,
},
},
},
})

require.Len(t, got.Spec.Template.Spec.Containers, 1)
require.Equal(t, tt.args.expectedResources, got.Spec.Template.Spec.Containers[0].Resources)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
srcChanged := s.inlineFnSrcChanged(r.cfg.docker.PullAddress, r.cfg.runtimeBaseImage)
if !srcChanged {
cfgStatus := getConditionStatus(s.instance.Status.Conditions, serverlessv1alpha2.ConditionConfigurationReady)
if cfgStatus == corev1.ConditionTrue {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ func stateFnCheckDeployments(ctx context.Context, r *reconciler, s *systemState)
ImagePullAccountName: r.cfg.fn.ImagePullAccountName,
}

expectedDeployment := s.buildDeployment(args, r.cfg.fn.ResourceConfig.Function.Resources)
expectedDeployment := s.buildDeployment(args, r.cfg)
if len(s.deployments.Items) == 0 {
return buildStateFnCreateDeployment(expectedDeployment), nil
}
Expand Down
5 changes: 3 additions & 2 deletions components/serverless/internal/controllers/serverless/fsm.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,9 @@ type k8s struct {
}

type cfg struct {
docker DockerConfig
fn FunctionConfig
runtimeBaseImage string
docker DockerConfig
fn FunctionConfig
}

// nolint
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@ package serverless

import (
"context"
"fmt"
"regexp"
"strings"
"time"

"github.com/pkg/errors"
Expand All @@ -10,6 +13,7 @@ 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"
Expand Down Expand Up @@ -138,6 +142,16 @@ 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,
Expand All @@ -147,8 +161,9 @@ func (r *FunctionReconciler) Reconcile(ctx context.Context, request ctrl.Request
statsCollector: r.statsCollector,
},
cfg: cfg{
fn: r.config,
docker: dockerCfg,
fn: r.config,
docker: dockerCfg,
runtimeBaseImage: latestRuntimeImage,
},
gitClient: r.gitFactory.GetGitClient(contextLogger),
}
Expand Down Expand Up @@ -194,3 +209,20 @@ 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
}
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ func assertSuccessfulFunctionDeployment(t *testing.T, resourceClient resource.Cl
instance: *function,
}

g.Expect(deployment.Spec.Template.Spec.Containers[0].Image).To(gomega.Equal(s.buildImageAddress(regPullAddr)))
g.Expect(deployment.Spec.Template.Spec.Containers[0].Image).To(gomega.Equal(s.buildImageAddress(regPullAddr, "some_image")))
g.Expect(deployment.Spec.Template.Labels).To(gomega.HaveLen(8))
g.Expect(deployment.Spec.Template.Labels[serverlessv1alpha2.FunctionNameLabel]).To(gomega.Equal(function.Name))
g.Expect(deployment.Spec.Template.Labels[serverlessv1alpha2.PodAppNameLabel]).To(gomega.Equal(function.Name))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,7 @@ func TestGitOpsWithContinuousGitCheckout(t *testing.T) {
instance: *function,
}

expectedImage := s.buildImageAddress("localhost:32132")
expectedImage := s.buildImageAddress("localhost:32132", "some_image")
g.Expect(deployment).To(gomega.Not(gomega.BeNil()))
g.Expect(deployment).To(haveSpecificContainer0Image(expectedImage))
g.Expect(deployment).To(haveLabelLen(8))
Expand Down Expand Up @@ -567,7 +567,7 @@ func TestGitOpsWithoutContinuousGitCheckout(t *testing.T) {
instance: *function,
}

expectedImage := s.buildImageAddress("localhost:32132")
expectedImage := s.buildImageAddress("localhost:32132", "some_image")
g.Expect(deployment).To(gomega.Not(gomega.BeNil()))
g.Expect(deployment).To(haveSpecificContainer0Image(expectedImage))
g.Expect(deployment).To(haveLabelLen(8))
Expand Down
35 changes: 3 additions & 32 deletions components/serverless/internal/controllers/serverless/job.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,12 @@ package serverless
import (
"context"
"fmt"
"regexp"
"strings"
"time"

batchv1 "k8s.io/api/batch/v1"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
apilabels "k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/types"
ctrl "sigs.k8s.io/controller-runtime"

serverlessv1alpha2 "github.com/kyma-project/serverless/components/serverless/pkg/apis/serverless/v1alpha2"
Expand Down Expand Up @@ -68,7 +65,7 @@ func buildStateFnCheckImageJob(expectedJob batchv1.Job) stateFn {
return buildStatusUpdateStateFnWithCondition(condition), nil
}

s.fnImage = s.buildImageAddress(r.cfg.docker.PullAddress)
s.fnImage = s.buildImageAddress(r.cfg.docker.PullAddress, r.cfg.runtimeBaseImage)

diffRuntimeImage, err := functionRuntimeChanged(ctx, r, s)
if err != nil {
Expand Down Expand Up @@ -108,11 +105,7 @@ func functionRuntimeChanged(ctx context.Context, r *reconciler, s *systemState)
return !result, nil
}

latestRuntimeImage, err := getRuntimeImageFromConfigMap(ctx, r, s)
if err != nil {
return false, errors.Wrap(err, "while fetching runtime image from config map")
}
result := latestRuntimeImage == functionRuntimeImage
result := r.cfg.runtimeBaseImage == functionRuntimeImage
return !result, nil
}

Expand Down Expand Up @@ -141,11 +134,6 @@ func buildStateFnRunJob(expectedJob batchv1.Job) stateFn {
return nil, errors.Wrap(err, "while creating job")
}

runtimeImage, err := getRuntimeImageFromConfigMap(ctx, r, s)
if err != nil {
return nil, errors.Wrap(err, "while extracting runtime fn-image from config map")
}

condition := serverlessv1alpha2.Condition{
Type: serverlessv1alpha2.ConditionBuildReady,
Status: corev1.ConditionUnknown,
Expand All @@ -154,28 +142,11 @@ func buildStateFnRunJob(expectedJob batchv1.Job) stateFn {
Message: fmt.Sprintf("Job %s created", expectedJob.GetName()),
}

s.instance.Status.RuntimeImage = runtimeImage
s.instance.Status.RuntimeImage = r.cfg.runtimeBaseImage
return buildStatusUpdateStateFnWithCondition(condition), nil
}
}

func getRuntimeImageFromConfigMap(ctx context.Context, r *reconciler, s *systemState) (string, error) {
instance := &corev1.ConfigMap{}
dockerfileConfigMapName := fmt.Sprintf("dockerfile-%s", s.instance.Status.Runtime)
err := r.client.Get(ctx, types.NamespacedName{Namespace: s.instance.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
}

func stateFnInlineDeleteJobs(ctx context.Context, r *reconciler, s *systemState) (stateFn, error) {
r.log.Info("delete Jobs")

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,20 +66,20 @@ func prometheusSvcAnnotations() map[string]string {
"prometheus.io/scrape": "true",
}
}
func (s *systemState) buildImageAddress(registryAddress string) string {
func (s *systemState) buildImageAddress(registryAddress string, runtimeBase string) string {
var imageTag string
isGitType := s.instance.TypeOf(serverlessv1alpha2.FunctionTypeGit)
if isGitType {
imageTag = calculateGitImageTag(&s.instance)
imageTag = calculateGitImageTag(&s.instance, runtimeBase)
} else {
imageTag = calculateInlineImageTag(&s.instance)
imageTag = calculateInlineImageTag(&s.instance, runtimeBase)
}
return fmt.Sprintf("%s/%s-%s:%s", registryAddress, s.instance.Namespace, s.instance.Name, imageTag)
}

// TODO to self - create issue to refactor this
func (s *systemState) inlineFnSrcChanged(dockerPullAddress string) bool {
image := s.buildImageAddress(dockerPullAddress)
func (s *systemState) inlineFnSrcChanged(dockerPullAddress string, runtimeBase string) bool {
image := s.buildImageAddress(dockerPullAddress, runtimeBase)
configurationStatus := getConditionStatus(s.instance.Status.Conditions, serverlessv1alpha2.ConditionConfigurationReady)
rtm := fnRuntime.GetRuntime(s.instance.Spec.Runtime)
fnLabels := s.functionLabels()
Expand Down Expand Up @@ -240,7 +240,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)
imageName := s.buildImageAddress(cfg.docker.PushAddress, cfg.runtimeBaseImage)
args := append(cfg.fn.Build.ExecutorArgs,
fmt.Sprintf("%s=%s", destinationArg, imageName),
fmt.Sprintf("--context=dir://%s", workspaceMountPath))
Expand Down Expand Up @@ -414,8 +414,8 @@ type buildDeploymentArgs struct {
ImagePullAccountName string
}

func (s *systemState) buildDeployment(cfg buildDeploymentArgs, resourceConfig Resources) appsv1.Deployment {
imageName := s.buildImageAddress(cfg.DockerPullAddress)
func (s *systemState) buildDeployment(args buildDeploymentArgs, cfg cfg) appsv1.Deployment {
imageName := s.buildImageAddress(args.DockerPullAddress, cfg.runtimeBaseImage)

const volumeName = "tmp-dir"
emptyDirVolumeSize := resource.MustParse("100Mi")
Expand All @@ -427,8 +427,8 @@ func (s *systemState) buildDeployment(cfg buildDeploymentArgs, resourceConfig Re
deploymentEnvs := buildDeploymentEnvs(
s.instance.GetName(),
s.instance.GetNamespace(),
cfg.TraceCollectorEndpoint,
cfg.PublisherProxyAddress,
args.TraceCollectorEndpoint,
args.PublisherProxyAddress,
)
envs = append(envs, deploymentEnvs...)

Expand Down Expand Up @@ -468,7 +468,7 @@ func (s *systemState) buildDeployment(cfg buildDeploymentArgs, resourceConfig Re
Name: functionContainerName,
Image: imageName,
Env: envs,
Resources: getDeploymentResources(s.instance, resourceConfig),
Resources: getDeploymentResources(s.instance, cfg.fn.ResourceConfig.Function.Resources),
VolumeMounts: volumeMounts,
/*
In order to mark pod as ready we need to ensure the function is actually running and ready to serve traffic.
Expand Down Expand Up @@ -514,7 +514,7 @@ func (s *systemState) buildDeployment(cfg buildDeploymentArgs, resourceConfig Re
SecurityContext: restrictiveContainerSecurityContext(),
},
},
ServiceAccountName: cfg.ImagePullAccountName,
ServiceAccountName: args.ImagePullAccountName,
}
enrichPodSpecWithSecurityContext(&templateSpec, functionUser, functionUserGroup)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -363,22 +363,24 @@ func getCondition(conditions []serverlessv1alpha2.Condition, conditionType serve
return serverlessv1alpha2.Condition{}
}

func calculateInlineImageTag(instance *serverlessv1alpha2.Function) string {
func calculateInlineImageTag(instance *serverlessv1alpha2.Function, runtimeBase string) string {
hash := sha256.Sum256([]byte(strings.Join([]string{
string(instance.GetUID()),
fmt.Sprintf("%v", *instance.Spec.Source.Inline),
instance.EffectiveRuntime(),
runtimeBase,
}, "-")))

return fmt.Sprintf("%x", hash)
}

func calculateGitImageTag(instance *serverlessv1alpha2.Function) string {
func calculateGitImageTag(instance *serverlessv1alpha2.Function, runtimeBase string) string {
data := strings.Join([]string{
string(instance.GetUID()),
instance.Status.Commit,
instance.Status.BaseDir,
instance.EffectiveRuntime(),
runtimeBase,
}, "-")
hash := sha256.Sum256([]byte(data))
return fmt.Sprintf("%x", hash)
Expand Down
Loading

0 comments on commit f16934e

Please sign in to comment.