From 703613a4759af304de4c61dc8cab111e4cfa142d Mon Sep 17 00:00:00 2001 From: mr-chenguang <37072324+lcgash@users.noreply.github.com> Date: Mon, 20 Jan 2025 19:11:18 +0800 Subject: [PATCH] fix: custom runtime container name is invalid (#3065) * fix: custom runtime container name is invalid * lint: pkg/executor/executortype/newdeploy/newdeploy.go * Custom name for runtime container Signed-off-by: Md Soharab Ansari --------- Signed-off-by: Md Soharab Ansari Co-authored-by: Md Soharab Ansari --- pkg/executor/executortype/newdeploy/newdeploy.go | 12 +++++++++++- pkg/executor/executortype/poolmgr/gp_deployment.go | 12 +++++++++++- pkg/executor/util/util.go | 10 ++++++++++ pkg/fission-cli/cmd/spec/spec.go | 6 ++++++ 4 files changed, 38 insertions(+), 2 deletions(-) diff --git a/pkg/executor/executortype/newdeploy/newdeploy.go b/pkg/executor/executortype/newdeploy/newdeploy.go index 06abdd4d..6223a526 100644 --- a/pkg/executor/executortype/newdeploy/newdeploy.go +++ b/pkg/executor/executortype/newdeploy/newdeploy.go @@ -281,10 +281,20 @@ func (deploy *NewDeploy) getDeploymentSpec(ctx context.Context, fn *fv1.Function }, } + // If custom runtime container name - default env name + mainContainerName := env.ObjectMeta.Name + if env.Spec.Runtime.Container != nil && env.Spec.Runtime.Container.Name != "" && env.Spec.Runtime.PodSpec != nil { + if util.DoesContainerExistInPodSpec(env.Spec.Runtime.Container.Name, env.Spec.Runtime.PodSpec) { + mainContainerName = env.Spec.Runtime.Container.Name + } else { + return nil, fmt.Errorf("runtime container %s not found in pod spec", env.Spec.Runtime.Container.Name) + } + } + // Order of merging is important here - first fetcher, then containers and lastly pod spec err = deploy.fetcherConfig.AddSpecializingFetcherToPodSpec( &deployment.Spec.Template.Spec, - env.ObjectMeta.Name, + mainContainerName, fn, env, ) diff --git a/pkg/executor/executortype/poolmgr/gp_deployment.go b/pkg/executor/executortype/poolmgr/gp_deployment.go index a2706b2d..84fbb266 100644 --- a/pkg/executor/executortype/poolmgr/gp_deployment.go +++ b/pkg/executor/executortype/poolmgr/gp_deployment.go @@ -189,8 +189,18 @@ func (gp *GenericPool) genDeploymentSpec(env *fv1.Environment) (*appsv1.Deployme Template: pod, } + // If custom runtime container name - default env name + mainContainerName := env.ObjectMeta.Name + if env.Spec.Runtime.Container != nil && env.Spec.Runtime.Container.Name != "" && env.Spec.Runtime.PodSpec != nil { + if util.DoesContainerExistInPodSpec(env.Spec.Runtime.Container.Name, env.Spec.Runtime.PodSpec) { + mainContainerName = env.Spec.Runtime.Container.Name + } else { + return nil, fmt.Errorf("runtime container %s not found in pod spec", env.Spec.Runtime.Container.Name) + } + } + // Order of merging is important here - first fetcher, then containers and lastly pod spec - err = gp.fetcherConfig.AddFetcherToPodSpec(&deploymentSpec.Template.Spec, env.ObjectMeta.Name) + err = gp.fetcherConfig.AddFetcherToPodSpec(&deploymentSpec.Template.Spec, mainContainerName) if err != nil { return nil, err } diff --git a/pkg/executor/util/util.go b/pkg/executor/util/util.go index 65454bc5..6368d59c 100644 --- a/pkg/executor/util/util.go +++ b/pkg/executor/util/util.go @@ -168,3 +168,13 @@ func CreateDumpFile(logger *zap.Logger) (*os.File, error) { return os.Create(fmt.Sprintf("%s/%s-%d.txt", dumpPath, dumpFileName, time.Now().Unix())) } + +// DoesContainerExistInPodSpec checks if the container with the given name exists in the pod spec +func DoesContainerExistInPodSpec(containerName string, podSpec *apiv1.PodSpec) bool { + for _, container := range podSpec.Containers { + if container.Name == containerName { + return true + } + } + return false +} diff --git a/pkg/fission-cli/cmd/spec/spec.go b/pkg/fission-cli/cmd/spec/spec.go index cfe83c13..ff6b76d3 100644 --- a/pkg/fission-cli/cmd/spec/spec.go +++ b/pkg/fission-cli/cmd/spec/spec.go @@ -30,6 +30,7 @@ import ( "sigs.k8s.io/yaml" fv1 "github.com/fission/fission/pkg/apis/core/v1" + executorUtil "github.com/fission/fission/pkg/executor/util" "github.com/fission/fission/pkg/fission-cli/cliwrapper/cli" "github.com/fission/fission/pkg/fission-cli/cmd" "github.com/fission/fission/pkg/fission-cli/cmd/spec/types" @@ -489,6 +490,11 @@ func (fr *FissionResources) Validate(input cli.Input, client cmd.Client) ([]stri environments[fmt.Sprintf("%s:%s", e.ObjectMeta.Name, e.ObjectMeta.Namespace)] = struct{}{} if ((e.Spec.Runtime.Container != nil) && (e.Spec.Runtime.PodSpec != nil)) || ((e.Spec.Builder.Container != nil) && (e.Spec.Builder.PodSpec != nil)) { warnings = append(warnings, "You have provided both - container spec and pod spec and while merging the pod spec will take precedence.") + if e.Spec.Runtime.Container.Name != "" && e.Spec.Runtime.PodSpec != nil { + if !executorUtil.DoesContainerExistInPodSpec(e.Spec.Runtime.Container.Name, e.Spec.Runtime.PodSpec) { + result = multierror.Append(result, fmt.Errorf("runtime container %s does not exist in the pod spec", e.Spec.Runtime.Container.Name)) + } + } } // Unlike CLI can change the environment version silently, // we have to warn the user to modify spec file when this takes place.