From a05367eb83d5fad9e93749bbaaad91031cd59da8 Mon Sep 17 00:00:00 2001 From: Andrew Bayer Date: Thu, 18 Apr 2019 13:42:06 -0400 Subject: [PATCH] fix: Simplify merging and fix override of top level env vars Derived from what I've done over at https://github.com/tektoncd/pipeline/pull/767 - this is just cleaner, simpler, better. --- pkg/jx/cmd/step_create_task.go | 4 +- .../step_create_task/from_yaml/jenkins-x.yml | 3 + .../step_create_task/from_yaml/tasks.yml | 24 +-- .../step_create_task/js_build_pack/tasks.yml | 20 +- .../maven_build_pack/jenkins-x.yml | 2 + .../maven_build_pack/tasks.yml | 72 +++---- .../per_step_container_build_pack/tasks.yml | 20 +- pkg/tekton/syntax/pipeline.go | 176 ++++++++++++++---- pkg/tekton/syntax/pipeline_test.go | 22 ++- 9 files changed, 242 insertions(+), 101 deletions(-) diff --git a/pkg/jx/cmd/step_create_task.go b/pkg/jx/cmd/step_create_task.go index f8f5a0561a..0be5b83aab 100644 --- a/pkg/jx/cmd/step_create_task.go +++ b/pkg/jx/cmd/step_create_task.go @@ -546,6 +546,8 @@ func (o *StepCreateTaskOptions) GenerateTektonCRDs(packsDir string, projectConfi } } + parsed.AddContainerEnvVarsToPipeline(pipelineConfig.Env) + // TODO: Seeing weird behavior seemingly related to https://golang.org/doc/faq#nil_error // if err is reused, maybe we need to switch return types (perhaps upstream in build-pipeline)? if validateErr := parsed.Validate(ctx); validateErr != nil { @@ -1213,7 +1215,7 @@ func (o *StepCreateTaskOptions) modifyEnvVars(container *corev1.Container, globa } for _, e := range globalEnv { - if kube.GetSliceEnvVar(envVars, e.Name) == nil { + if kube.GetSliceEnvVar(envVars, e.Name) == nil && e.ValueFrom != nil { envVars = append(envVars, e) } } diff --git a/pkg/jx/cmd/test_data/step_create_task/from_yaml/jenkins-x.yml b/pkg/jx/cmd/test_data/step_create_task/from_yaml/jenkins-x.yml index 1b8f36672d..267f8b8c46 100644 --- a/pkg/jx/cmd/test_data/step_create_task/from_yaml/jenkins-x.yml +++ b/pkg/jx/cmd/test_data/step_create_task/from_yaml/jenkins-x.yml @@ -5,6 +5,9 @@ pipelineConfig: pipelines: release: pipeline: + environment: + - name: GIT_AUTHOR_NAME + value: somebodyelse options: containerOptions: resources: diff --git a/pkg/jx/cmd/test_data/step_create_task/from_yaml/tasks.yml b/pkg/jx/cmd/test_data/step_create_task/from_yaml/tasks.yml index 99acc662f9..2dae4d9f17 100755 --- a/pkg/jx/cmd/test_data/step_create_task/from_yaml/tasks.yml +++ b/pkg/jx/cmd/test_data/step_create_task/from_yaml/tasks.yml @@ -27,6 +27,10 @@ items: command: - jx env: + - name: FRUIT + value: BANANA + - name: GIT_AUTHOR_NAME + value: somebodyelse - name: DOCKER_REGISTRY - name: BUILD_NUMBER value: "1" @@ -44,8 +48,6 @@ items: value: really-long - name: JX_BATCH_MODE value: "true" - - name: FRUIT - value: BANANA image: rawlingsj/builder-jx:wip34 name: git-merge volumeMounts: @@ -66,10 +68,12 @@ items: configMapKeyRef: key: docker.registry name: jenkins-x-docker-registry + - name: FRUIT + value: BANANA - name: GIT_AUTHOR_EMAIL value: jenkins-x@googlegroups.com - name: GIT_AUTHOR_NAME - value: jenkins-x-bot + value: somebodyelse - name: GIT_COMMITTER_EMAIL value: jenkins-x@googlegroups.com - name: GIT_COMMITTER_NAME @@ -94,8 +98,6 @@ items: value: really-long - name: JX_BATCH_MODE value: "true" - - name: FRUIT - value: BANANA image: jenkinsxio/builder-nodejs:0.1.235 name: step2 resources: @@ -128,10 +130,12 @@ items: configMapKeyRef: key: docker.registry name: jenkins-x-docker-registry + - name: FRUIT + value: BANANA - name: GIT_AUTHOR_EMAIL value: jenkins-x@googlegroups.com - name: GIT_AUTHOR_NAME - value: jenkins-x-bot + value: somebodyelse - name: GIT_COMMITTER_EMAIL value: jenkins-x@googlegroups.com - name: GIT_COMMITTER_NAME @@ -156,8 +160,6 @@ items: value: really-long - name: JX_BATCH_MODE value: "true" - - name: FRUIT - value: BANANA image: jenkinsxio/builder-nodejs:0.1.235 name: step3 resources: @@ -220,10 +222,12 @@ items: configMapKeyRef: key: docker.registry name: jenkins-x-docker-registry + - name: FRUIT + value: BANANA - name: GIT_AUTHOR_EMAIL value: jenkins-x@googlegroups.com - name: GIT_AUTHOR_NAME - value: jenkins-x-bot + value: somebodyelse - name: GIT_COMMITTER_EMAIL value: jenkins-x@googlegroups.com - name: GIT_COMMITTER_NAME @@ -248,8 +252,6 @@ items: value: really-long - name: JX_BATCH_MODE value: "true" - - name: FRUIT - value: BANANA image: jenkinsxio/builder-nodejs:0.1.235 name: step2 resources: diff --git a/pkg/jx/cmd/test_data/step_create_task/js_build_pack/tasks.yml b/pkg/jx/cmd/test_data/step_create_task/js_build_pack/tasks.yml index 38d83cd756..67516af40c 100755 --- a/pkg/jx/cmd/test_data/step_create_task/js_build_pack/tasks.yml +++ b/pkg/jx/cmd/test_data/step_create_task/js_build_pack/tasks.yml @@ -24,13 +24,15 @@ items: image: rawlingsj/builder-jx:wip34 name: git-merge env: - - name: DOCKER_CONFIG - value: /home/jenkins/.docker/ - name: DOCKER_REGISTRY valueFrom: configMapKeyRef: key: docker.registry name: jenkins-x-docker-registry + - name: TILLER_NAMESPACE + value: kube-system + - name: DOCKER_CONFIG + value: /home/jenkins/.docker/ - name: GIT_AUTHOR_EMAIL value: jenkins-x@googlegroups.com - name: GIT_AUTHOR_NAME @@ -39,8 +41,6 @@ items: value: jenkins-x@googlegroups.com - name: GIT_COMMITTER_NAME value: jenkins-x-bot - - name: TILLER_NAMESPACE - value: kube-system - name: XDG_CONFIG_HOME value: /workspace/xdg_config - name: BUILD_NUMBER @@ -59,7 +59,19 @@ items: value: build-pack - name: JX_BATCH_MODE value: "true" + resources: + requests: + cpu: 400m + memory: 512Mi + securityContext: + privileged: true volumeMounts: + - mountPath: /home/jenkins + name: workspace-volume + - mountPath: /var/run/docker.sock + name: docker-daemon + - mountPath: /home/jenkins/.docker + name: volume-0 - mountPath: /etc/podinfo name: podinfo readOnly: true diff --git a/pkg/jx/cmd/test_data/step_create_task/maven_build_pack/jenkins-x.yml b/pkg/jx/cmd/test_data/step_create_task/maven_build_pack/jenkins-x.yml index ae7557667c..726fb769ce 100644 --- a/pkg/jx/cmd/test_data/step_create_task/maven_build_pack/jenkins-x.yml +++ b/pkg/jx/cmd/test_data/step_create_task/maven_build_pack/jenkins-x.yml @@ -2,3 +2,5 @@ pipelineConfig: env: - name: FRUIT value: BANANA + - name: GIT_AUTHOR_NAME + value: somebodyelse diff --git a/pkg/jx/cmd/test_data/step_create_task/maven_build_pack/tasks.yml b/pkg/jx/cmd/test_data/step_create_task/maven_build_pack/tasks.yml index 5bfaaabded..dc633e40f1 100755 --- a/pkg/jx/cmd/test_data/step_create_task/maven_build_pack/tasks.yml +++ b/pkg/jx/cmd/test_data/step_create_task/maven_build_pack/tasks.yml @@ -24,6 +24,10 @@ items: image: rawlingsj/builder-jx:wip34 name: git-merge env: + - name: FRUIT + value: BANANA + - name: GIT_AUTHOR_NAME + value: somebodyelse - name: DOCKER_REGISTRY - name: BUILD_NUMBER value: "1" @@ -41,8 +45,6 @@ items: value: master - name: JX_BATCH_MODE value: "true" - - name: FRUIT - value: BANANA volumeMounts: - mountPath: /etc/podinfo name: podinfo @@ -61,10 +63,12 @@ items: configMapKeyRef: key: docker.registry name: jenkins-x-docker-registry + - name: FRUIT + value: BANANA - name: GIT_AUTHOR_EMAIL value: jenkins-x@googlegroups.com - name: GIT_AUTHOR_NAME - value: jenkins-x-bot + value: somebodyelse - name: GIT_COMMITTER_EMAIL value: jenkins-x@googlegroups.com - name: GIT_COMMITTER_NAME @@ -93,8 +97,6 @@ items: value: master - name: JX_BATCH_MODE value: "true" - - name: FRUIT - value: BANANA image: jenkinsxio/builder-maven-java11:0.1.235 name: setup-jx-git-credentials resources: @@ -131,10 +133,12 @@ items: configMapKeyRef: key: docker.registry name: jenkins-x-docker-registry + - name: FRUIT + value: BANANA - name: GIT_AUTHOR_EMAIL value: jenkins-x@googlegroups.com - name: GIT_AUTHOR_NAME - value: jenkins-x-bot + value: somebodyelse - name: GIT_COMMITTER_EMAIL value: jenkins-x@googlegroups.com - name: GIT_COMMITTER_NAME @@ -163,8 +167,6 @@ items: value: master - name: JX_BATCH_MODE value: "true" - - name: FRUIT - value: BANANA image: jenkinsxio/builder-maven-java11:0.1.235 name: setversion-next-version resources: @@ -201,10 +203,12 @@ items: configMapKeyRef: key: docker.registry name: jenkins-x-docker-registry + - name: FRUIT + value: BANANA - name: GIT_AUTHOR_EMAIL value: jenkins-x@googlegroups.com - name: GIT_AUTHOR_NAME - value: jenkins-x-bot + value: somebodyelse - name: GIT_COMMITTER_EMAIL value: jenkins-x@googlegroups.com - name: GIT_COMMITTER_NAME @@ -233,8 +237,6 @@ items: value: master - name: JX_BATCH_MODE value: "true" - - name: FRUIT - value: BANANA image: jenkinsxio/builder-maven-java11:0.1.235 name: setversion-set-version resources: @@ -271,10 +273,12 @@ items: configMapKeyRef: key: docker.registry name: jenkins-x-docker-registry + - name: FRUIT + value: BANANA - name: GIT_AUTHOR_EMAIL value: jenkins-x@googlegroups.com - name: GIT_AUTHOR_NAME - value: jenkins-x-bot + value: somebodyelse - name: GIT_COMMITTER_EMAIL value: jenkins-x@googlegroups.com - name: GIT_COMMITTER_NAME @@ -303,8 +307,6 @@ items: value: master - name: JX_BATCH_MODE value: "true" - - name: FRUIT - value: BANANA image: jenkinsxio/builder-maven-java11:0.1.235 name: setversion-tag-version resources: @@ -341,10 +343,12 @@ items: configMapKeyRef: key: docker.registry name: jenkins-x-docker-registry + - name: FRUIT + value: BANANA - name: GIT_AUTHOR_EMAIL value: jenkins-x@googlegroups.com - name: GIT_AUTHOR_NAME - value: jenkins-x-bot + value: somebodyelse - name: GIT_COMMITTER_EMAIL value: jenkins-x@googlegroups.com - name: GIT_COMMITTER_NAME @@ -373,8 +377,6 @@ items: value: master - name: JX_BATCH_MODE value: "true" - - name: FRUIT - value: BANANA image: jenkinsxio/builder-maven-java11:0.1.235 name: build-mvn-deploy resources: @@ -411,10 +413,12 @@ items: configMapKeyRef: key: docker.registry name: jenkins-x-docker-registry + - name: FRUIT + value: BANANA - name: GIT_AUTHOR_EMAIL value: jenkins-x@googlegroups.com - name: GIT_AUTHOR_NAME - value: jenkins-x-bot + value: somebodyelse - name: GIT_COMMITTER_EMAIL value: jenkins-x@googlegroups.com - name: GIT_COMMITTER_NAME @@ -443,8 +447,6 @@ items: value: master - name: JX_BATCH_MODE value: "true" - - name: FRUIT - value: BANANA image: jenkinsxio/builder-maven-java11:0.1.235 name: build-skaffold-version resources: @@ -481,10 +483,12 @@ items: configMapKeyRef: key: docker.registry name: jenkins-x-docker-registry + - name: FRUIT + value: BANANA - name: GIT_AUTHOR_EMAIL value: jenkins-x@googlegroups.com - name: GIT_AUTHOR_NAME - value: jenkins-x-bot + value: somebodyelse - name: GIT_COMMITTER_EMAIL value: jenkins-x@googlegroups.com - name: GIT_COMMITTER_NAME @@ -513,8 +517,6 @@ items: value: master - name: JX_BATCH_MODE value: "true" - - name: FRUIT - value: BANANA image: jenkinsxio/builder-maven-java11:0.1.235 name: build-container-build resources: @@ -551,10 +553,12 @@ items: configMapKeyRef: key: docker.registry name: jenkins-x-docker-registry + - name: FRUIT + value: BANANA - name: GIT_AUTHOR_EMAIL value: jenkins-x@googlegroups.com - name: GIT_AUTHOR_NAME - value: jenkins-x-bot + value: somebodyelse - name: GIT_COMMITTER_EMAIL value: jenkins-x@googlegroups.com - name: GIT_COMMITTER_NAME @@ -583,8 +587,6 @@ items: value: master - name: JX_BATCH_MODE value: "true" - - name: FRUIT - value: BANANA image: jenkinsxio/builder-maven-java11:0.1.235 name: build-post-build resources: @@ -621,10 +623,12 @@ items: configMapKeyRef: key: docker.registry name: jenkins-x-docker-registry + - name: FRUIT + value: BANANA - name: GIT_AUTHOR_EMAIL value: jenkins-x@googlegroups.com - name: GIT_AUTHOR_NAME - value: jenkins-x-bot + value: somebodyelse - name: GIT_COMMITTER_EMAIL value: jenkins-x@googlegroups.com - name: GIT_COMMITTER_NAME @@ -653,8 +657,6 @@ items: value: master - name: JX_BATCH_MODE value: "true" - - name: FRUIT - value: BANANA image: jenkinsxio/builder-maven-java11:0.1.235 name: promote-changelog resources: @@ -691,10 +693,12 @@ items: configMapKeyRef: key: docker.registry name: jenkins-x-docker-registry + - name: FRUIT + value: BANANA - name: GIT_AUTHOR_EMAIL value: jenkins-x@googlegroups.com - name: GIT_AUTHOR_NAME - value: jenkins-x-bot + value: somebodyelse - name: GIT_COMMITTER_EMAIL value: jenkins-x@googlegroups.com - name: GIT_COMMITTER_NAME @@ -723,8 +727,6 @@ items: value: master - name: JX_BATCH_MODE value: "true" - - name: FRUIT - value: BANANA image: jenkinsxio/builder-maven-java11:0.1.235 name: promote-helm-release resources: @@ -761,10 +763,12 @@ items: configMapKeyRef: key: docker.registry name: jenkins-x-docker-registry + - name: FRUIT + value: BANANA - name: GIT_AUTHOR_EMAIL value: jenkins-x@googlegroups.com - name: GIT_AUTHOR_NAME - value: jenkins-x-bot + value: somebodyelse - name: GIT_COMMITTER_EMAIL value: jenkins-x@googlegroups.com - name: GIT_COMMITTER_NAME @@ -793,8 +797,6 @@ items: value: master - name: JX_BATCH_MODE value: "true" - - name: FRUIT - value: BANANA image: jenkinsxio/builder-maven-java11:0.1.235 name: promote-jx-promote resources: diff --git a/pkg/jx/cmd/test_data/step_create_task/per_step_container_build_pack/tasks.yml b/pkg/jx/cmd/test_data/step_create_task/per_step_container_build_pack/tasks.yml index 7235b2ea7c..0ecc1d88cc 100755 --- a/pkg/jx/cmd/test_data/step_create_task/per_step_container_build_pack/tasks.yml +++ b/pkg/jx/cmd/test_data/step_create_task/per_step_container_build_pack/tasks.yml @@ -24,13 +24,15 @@ items: image: rawlingsj/builder-jx:wip34 name: git-merge env: - - name: DOCKER_CONFIG - value: /home/jenkins/.docker/ - name: DOCKER_REGISTRY valueFrom: configMapKeyRef: key: docker.registry name: jenkins-x-docker-registry + - name: TILLER_NAMESPACE + value: kube-system + - name: DOCKER_CONFIG + value: /home/jenkins/.docker/ - name: GIT_AUTHOR_EMAIL value: jenkins-x@googlegroups.com - name: GIT_AUTHOR_NAME @@ -39,8 +41,6 @@ items: value: jenkins-x@googlegroups.com - name: GIT_COMMITTER_NAME value: jenkins-x-bot - - name: TILLER_NAMESPACE - value: kube-system - name: XDG_CONFIG_HOME value: /workspace/xdg_config - name: BUILD_NUMBER @@ -60,7 +60,19 @@ items: - name: JX_BATCH_MODE value: "true" workingDir: /workspace/source + resources: + requests: + cpu: 400m + memory: 600Mi + securityContext: + privileged: true volumeMounts: + - mountPath: /home/jenkins + name: workspace-volume + - mountPath: /var/run/docker.sock + name: docker-daemon + - mountPath: /home/jenkins/.docker + name: volume-0 - mountPath: /etc/podinfo name: podinfo readOnly: true diff --git a/pkg/tekton/syntax/pipeline.go b/pkg/tekton/syntax/pipeline.go index 2367f5ae1d..1cb6cc79cf 100644 --- a/pkg/tekton/syntax/pipeline.go +++ b/pkg/tekton/syntax/pipeline.go @@ -2,6 +2,7 @@ package syntax import ( "context" + "encoding/json" "fmt" "os" "regexp" @@ -10,7 +11,6 @@ import ( "strings" "time" - "github.com/imdario/mergo" "github.com/jenkins-x/jx/pkg/apis/jenkins.io/v1" "github.com/jenkins-x/jx/pkg/util" "github.com/knative/pkg/apis" @@ -19,6 +19,7 @@ import ( corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/equality" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/strategicpatch" ) // GitMergeImage is the default image name that is used in the git merge step of a pipeline @@ -646,11 +647,49 @@ func toContainerEnvVars(origEnv []EnvVar) []corev1.EnvVar { return env } -func scopedEnv(newEnv []corev1.EnvVar, parentEnv []corev1.EnvVar, o *RootOptions) []corev1.EnvVar { +// AddContainerEnvVarsToPipeline allows for adding a slice of container environment variables directly to the +// pipeline, if they're not already defined. +func (p *ParsedPipeline) AddContainerEnvVarsToPipeline(origEnv []corev1.EnvVar) { + if len(origEnv) > 0 { + envMap := make(map[string]EnvVar) + + // Add the container env vars first. + for _, e := range origEnv { + if e.ValueFrom == nil { + envMap[e.Name] = EnvVar{ + Name: e.Name, + Value: e.Value, + } + } + } + + // Overwrite with the existing pipeline environment, if it exists + for _, e := range p.Environment { + envMap[e.Name] = e + } + + env := make([]EnvVar, 0, len(envMap)) + + // Avoid nondeterministic results by sorting the keys and appending vars in that order. + var envVars []string + for k := range envMap { + envVars = append(envVars, k) + } + sort.Strings(envVars) + + for _, envVar := range envVars { + env = append(env, envMap[envVar]) + } + + p.Environment = env + } +} + +func scopedEnv(newEnv []corev1.EnvVar, parentEnv []corev1.EnvVar) []corev1.EnvVar { if len(parentEnv) == 0 && len(newEnv) == 0 { return nil } - envMap := getContainerOptionsEnvVars(o) + envMap := make(map[string]corev1.EnvVar) for _, e := range parentEnv { envMap[e.Name] = e @@ -664,7 +703,7 @@ func scopedEnv(newEnv []corev1.EnvVar, parentEnv []corev1.EnvVar, o *RootOptions } func (j *ParsedPipeline) toStepEnvVars() []corev1.EnvVar { - envMap := getContainerOptionsEnvVars(&j.Options) + envMap := make(map[string]corev1.EnvVar) for _, e := range j.Environment { envMap[e.Name] = corev1.EnvVar{Name: e.Name, Value: e.Value} @@ -673,18 +712,6 @@ func (j *ParsedPipeline) toStepEnvVars() []corev1.EnvVar { return EnvMapToSlice(envMap) } -func getContainerOptionsEnvVars(o *RootOptions) map[string]corev1.EnvVar { - envMap := make(map[string]corev1.EnvVar) - - if o != nil && o.ContainerOptions != nil { - for _, e := range o.ContainerOptions.Env { - envMap[e.Name] = e - } - } - - return envMap -} - type transformedStage struct { Stage Stage // Only one of Sequential, Parallel, and Task is non-empty @@ -797,8 +824,6 @@ func stageToTask(s Stage, pipelineIdentifier string, buildIdentifier string, nam stageContainer := &corev1.Container{} - var rootOptions *RootOptions - if !equality.Semantic.DeepEqual(s.Options, StageOptions{}) { o := s.Options if !equality.Semantic.DeepEqual(o.Timeout, Timeout{}) { @@ -815,17 +840,17 @@ func stageToTask(s Stage, pipelineIdentifier string, buildIdentifier string, nam } stageContainer = o.ContainerOptions - - rootOptions = &o.RootOptions } if parentContainer != nil { - if err := mergo.Merge(stageContainer, parentContainer); err != nil { + merged, err := mergeContainers(parentContainer, stageContainer) + if err != nil { return nil, errors.Wrapf(err, "Error merging stage and parent container overrides: %s", err) } + stageContainer = merged } - env := scopedEnv(toContainerEnvVars(s.Environment), parentEnv, rootOptions) + env := scopedEnv(toContainerEnvVars(s.Environment), parentEnv) agent := s.Agent @@ -834,7 +859,11 @@ func stageToTask(s Stage, pipelineIdentifier string, buildIdentifier string, nam } stepCounter := 0 - defaultTaskSpec := getDefaultTaskSpec(env) + defaultTaskSpec, err := getDefaultTaskSpec(env, stageContainer) + if err != nil { + return nil, err + } + if len(s.Steps) > 0 { t := &tektonv1alpha1.Task{ TypeMeta: metav1.TypeMeta{ @@ -956,6 +985,61 @@ func stageToTask(s Stage, pipelineIdentifier string, buildIdentifier string, nam return nil, errors.New("no steps, sequential stages, or parallel stages") } +func mergeContainers(parentContainer, childContainer *corev1.Container) (*corev1.Container, error) { + if parentContainer == nil { + return childContainer, nil + } else if childContainer == nil { + return parentContainer, nil + } + + // We need JSON bytes to generate a patch to merge the child containers onto the parent container, so marshal the parent. + parentAsJSON, err := json.Marshal(parentContainer) + if err != nil { + return nil, err + } + // We need to do a three-way merge to actually combine the parent and child containers, so we need an empty container + // as the "original" + emptyAsJSON, err := json.Marshal(&corev1.Container{}) + if err != nil { + return nil, err + } + // Marshal the child to JSON + childAsJSON, err := json.Marshal(childContainer) + if err != nil { + return nil, err + } + + // Get the patch meta for Container, which is needed for generating and applying the merge patch. + patchSchema, err := strategicpatch.NewPatchMetaFromStruct(parentContainer) + + if err != nil { + return nil, err + } + + // Create a merge patch, with the empty JSON as the original, the child JSON as the modified, and the parent + // JSON as the current - this lets us do a deep merge of the parent and child containers, with awareness of + // the "patchMerge" tags. + patch, err := strategicpatch.CreateThreeWayMergePatch(emptyAsJSON, childAsJSON, parentAsJSON, patchSchema, true) + if err != nil { + return nil, err + } + + // Actually apply the merge patch to the parent JSON. + mergedAsJSON, err := strategicpatch.StrategicMergePatchUsingLookupPatchMeta(parentAsJSON, patch, patchSchema) + if err != nil { + return nil, err + } + + // Unmarshal the merged JSON to a Container pointer, and return it. + merged := &corev1.Container{} + err = json.Unmarshal(mergedAsJSON, merged) + if err != nil { + return nil, err + } + + return merged, nil +} + func isNestedFirstStepsStage(enclosingStage *transformedStage) bool { if enclosingStage != nil { if enclosingStage.PreviousSiblingStage != nil { @@ -993,10 +1077,12 @@ func generateSteps(step Step, inheritedAgent string, env []corev1.EnvVar, parent for _, volume := range podTemplate.Spec.Volumes { volumes[volume.Name] = volume } - if !equality.Semantic.DeepEqual(c, corev1.Container{}) { - if err := mergo.Merge(c, containers[0]); err != nil { + if !equality.Semantic.DeepEqual(c, &corev1.Container{}) { + merged, err := mergeContainers(&containers[0], c) + if err != nil { return nil, nil, stepCounter, errors.Wrapf(err, "Error merging pod template and parent container: %s", err) } + c = merged } else { c = &containers[0] } @@ -1026,13 +1112,12 @@ func generateSteps(step Step, inheritedAgent string, env []corev1.EnvVar, parent c.Stdin = false c.TTY = false - - c.Env = scopedEnv(env, c.Env, nil) + c.Env = scopedEnv(env, c.Env) steps = append(steps, *c) } else if !equality.Semantic.DeepEqual(step.Loop, Loop{}) { for _, v := range step.Loop.Values { - loopEnv := scopedEnv([]corev1.EnvVar{{Name: step.Loop.Variable, Value: v}}, env, nil) + loopEnv := scopedEnv([]corev1.EnvVar{{Name: step.Loop.Variable, Value: v}}, env) for _, s := range step.Loop.Steps { loopSteps, loopVolumes, loopCounter, loopErr := generateSteps(s, stepImage, loopEnv, parentContainer, podTemplates, stepCounter) @@ -1363,22 +1448,31 @@ func validateStageNames(j *ParsedPipeline) (err *apis.FieldError) { } // todo JR lets remove this when we switch tekton to using git merge type pipelineresources -func getDefaultTaskSpec(envs []corev1.EnvVar) tektonv1alpha1.TaskSpec { +func getDefaultTaskSpec(envs []corev1.EnvVar, parentContainer *corev1.Container) (tektonv1alpha1.TaskSpec, error) { v := os.Getenv("BUILDER_JX_IMAGE") if v == "" { v = GitMergeImage } - return tektonv1alpha1.TaskSpec{ - Steps: []corev1.Container{ - { - Name: "git-merge", - //Image: "gcr.io/jenkinsxio/builder-jx:0.1.297", - Image: v, - Command: []string{"jx"}, - Args: []string{"step", "git", "merge", "--verbose"}, - WorkingDir: "/workspace/source", - Env: envs, - }, - }, + + childContainer := &corev1.Container{ + Name: "git-merge", + //Image: "gcr.io/jenkinsxio/builder-jx:0.1.297", + Image: v, + Command: []string{"jx"}, + Args: []string{"step", "git", "merge", "--verbose"}, + WorkingDir: "/workspace/source", + Env: envs, } + + if parentContainer != nil { + merged, err := mergeContainers(parentContainer, childContainer) + if err != nil { + return tektonv1alpha1.TaskSpec{}, err + } + childContainer = merged + } + + return tektonv1alpha1.TaskSpec{ + Steps: []corev1.Container{*childContainer}, + }, nil } diff --git a/pkg/tekton/syntax/pipeline_test.go b/pkg/tekton/syntax/pipeline_test.go index 6205110707..cca151a932 100644 --- a/pkg/tekton/syntax/pipeline_test.go +++ b/pkg/tekton/syntax/pipeline_test.go @@ -831,7 +831,10 @@ func TestParseJenkinsfileYaml(t *testing.T) { tb.TaskInputs( tb.InputsResource("workspace", tektonv1alpha1.PipelineResourceTypeGit, tb.ResourceTargetPath("workspace"))), - tb.Step("git-merge", syntax.GitMergeImage, tb.Command("jx"), tb.Args("step", "git", "merge", "--verbose"), workingDir("/workspace/source")), + tb.Step("git-merge", syntax.GitMergeImage, tb.Command("jx"), tb.Args("step", "git", "merge", "--verbose"), workingDir("/workspace/source"), + ContainerResourceLimits("0.2", "128Mi"), + ContainerResourceRequests("0.1", "64Mi"), + ), tb.Step("step2", "some-image", tb.Command("/bin/sh", "-c"), tb.Args("echo hello world"), workingDir("/workspace/source"), ContainerResourceLimits("0.2", "128Mi"), ContainerResourceRequests("0.1", "64Mi"), @@ -876,7 +879,10 @@ func TestParseJenkinsfileYaml(t *testing.T) { tb.TaskInputs( tb.InputsResource("workspace", tektonv1alpha1.PipelineResourceTypeGit, tb.ResourceTargetPath("workspace"))), - tb.Step("git-merge", syntax.GitMergeImage, tb.Command("jx"), tb.Args("step", "git", "merge", "--verbose"), workingDir("/workspace/source")), + tb.Step("git-merge", syntax.GitMergeImage, tb.Command("jx"), tb.Args("step", "git", "merge", "--verbose"), workingDir("/workspace/source"), + ContainerResourceLimits("0.4", "256Mi"), + ContainerResourceRequests("0.2", "128Mi"), + ), tb.Step("step2", "some-image", tb.Command("/bin/sh", "-c"), tb.Args("echo hello world"), workingDir("/workspace/source"), ContainerResourceLimits("0.4", "256Mi"), ContainerResourceRequests("0.2", "128Mi"), @@ -919,7 +925,10 @@ func TestParseJenkinsfileYaml(t *testing.T) { tb.TaskInputs( tb.InputsResource("workspace", tektonv1alpha1.PipelineResourceTypeGit, tb.ResourceTargetPath("workspace"))), - tb.Step("git-merge", syntax.GitMergeImage, tb.Command("jx"), tb.Args("step", "git", "merge", "--verbose"), workingDir("/workspace/source")), + tb.Step("git-merge", syntax.GitMergeImage, tb.Command("jx"), tb.Args("step", "git", "merge", "--verbose"), workingDir("/workspace/source"), + ContainerResourceLimits("0.4", "256Mi"), + ContainerResourceRequests("0.1", "64Mi"), + ), tb.Step("step2", "some-image", tb.Command("/bin/sh", "-c"), tb.Args("echo hello world"), workingDir("/workspace/source"), ContainerResourceLimits("0.4", "256Mi"), ContainerResourceRequests("0.1", "64Mi"), @@ -958,7 +967,10 @@ func TestParseJenkinsfileYaml(t *testing.T) { tb.TaskInputs( tb.InputsResource("workspace", tektonv1alpha1.PipelineResourceTypeGit, tb.ResourceTargetPath("workspace"))), - tb.Step("git-merge", syntax.GitMergeImage, tb.Command("jx"), tb.Args("step", "git", "merge", "--verbose"), workingDir("/workspace/source")), + tb.Step("git-merge", syntax.GitMergeImage, tb.Command("jx"), tb.Args("step", "git", "merge", "--verbose"), workingDir("/workspace/source"), + ContainerResourceLimits("0.2", "128Mi"), + ContainerResourceRequests("0.1", "64Mi"), + ), tb.Step("step2", "some-image", tb.Command("/bin/sh", "-c"), tb.Args("echo hello world"), workingDir("/workspace/source"), ContainerResourceLimits("0.2", "128Mi"), ContainerResourceRequests("0.1", "64Mi"), @@ -1007,10 +1019,10 @@ func TestParseJenkinsfileYaml(t *testing.T) { tb.ResourceTargetPath("workspace"))), tb.Step("git-merge", syntax.GitMergeImage, tb.Command("jx"), tb.Args("step", "git", "merge", "--verbose"), workingDir("/workspace/source"), tb.EnvVar("ANOTHER_OVERRIDE_STAGE_ENV", "New value"), + tb.EnvVar("SOME_VAR", "A value for the env var"), tb.EnvVar("OVERRIDE_ENV", "New value"), tb.EnvVar("OVERRIDE_STAGE_ENV", "New value"), tb.EnvVar("SOME_OTHER_VAR", "A value for the other env var"), - tb.EnvVar("SOME_VAR", "A value for the env var"), ), tb.Step("step2", "some-image", tb.Command("/bin/sh", "-c"), tb.Args("echo hello world"), workingDir("/workspace/source"), tb.EnvVar("ANOTHER_OVERRIDE_STAGE_ENV", "New value"),