Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve fn tag calculation #929

Merged
merged 7 commits into from
Apr 29, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
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 All @@ -20,6 +24,7 @@ import (

"github.com/kyma-project/serverless/components/serverless/internal/git"
"github.com/kyma-project/serverless/components/serverless/internal/resource"
"github.com/kyma-project/serverless/components/serverless/pkg/apis/serverless/v1alpha2"
serverlessv1alpha2 "github.com/kyma-project/serverless/components/serverless/pkg/apis/serverless/v1alpha2"
)

Expand Down Expand Up @@ -138,6 +143,11 @@ func (r *FunctionReconciler) Reconcile(ctx context.Context, request ctrl.Request

contextLogger.Debug("starting state machine")

latestRuntimeImage, err := r.getRuntimeImageFromConfigMap(ctx, &instance)
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 +157,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 +205,20 @@ func (r *FunctionReconciler) readDockerConfig(ctx context.Context, instance *ser
}

}

func (r *FunctionReconciler) getRuntimeImageFromConfigMap(ctx context.Context, function *v1alpha2.Function) (string, error) {
dbadura marked this conversation as resolved.
Show resolved Hide resolved
instance := &corev1.ConfigMap{}
dockerfileConfigMapName := fmt.Sprintf("dockerfile-%s", function.Spec.Runtime)
err := r.client.Get(ctx, types.NamespacedName{Namespace: function.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
Loading