From 74f793c9c8e0458a9c8343833e7fea649539303d Mon Sep 17 00:00:00 2001 From: Tomasz Nowak Date: Wed, 13 Sep 2017 12:26:53 +0200 Subject: [PATCH 1/4] Fix overriding HOME env var. Remove duplicated env vars --- .../kubernetes/KubernetesLauncher.java | 23 ++++++++------- .../pipeline/KubernetesPipelineTest.java | 13 +++++++++ .../pipeline/runWithOverriddenEnvVars.groovy | 29 +++++++++++++++++++ 3 files changed, 54 insertions(+), 11 deletions(-) create mode 100644 src/test/resources/org/csanchez/jenkins/plugins/kubernetes/pipeline/runWithOverriddenEnvVars.groovy diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesLauncher.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesLauncher.java index 703c7c53ce..9775318157 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesLauncher.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesLauncher.java @@ -310,24 +310,25 @@ private Container createContainer(KubernetesSlave slave, ContainerTemplate conta // and `?` for java build tools. So we force HOME to a safe location. env.put("HOME", containerTemplate.getWorkingDir()); - List envVarsList = new ArrayList<>(); + Map envVarsMap = new HashMap<>(); - if (globalEnvVars != null) { - envVarsList.addAll(globalEnvVars.stream() - .map(TemplateEnvVar::buildEnvVar) - .collect(Collectors.toList())); - } if (containerTemplate.getEnvVars() != null) { - envVarsList.addAll(containerTemplate.getEnvVars().stream() - .map(TemplateEnvVar::buildEnvVar) - .collect(Collectors.toList())); + containerTemplate.getEnvVars().forEach(item -> + envVarsMap.computeIfAbsent(item.getKey(), k -> item.buildEnvVar()) + ); + } + if (globalEnvVars != null) { + globalEnvVars.forEach( item -> + envVarsMap.computeIfAbsent(item.getKey(), k -> item.buildEnvVar()) + ); } List defaultEnvVars = env.entrySet().stream() .map(entry -> new EnvVar(entry.getKey(), entry.getValue(), null)) .collect(Collectors.toList()); - envVarsList.addAll(defaultEnvVars); - EnvVar[] envVars = envVarsList.stream().toArray(EnvVar[]::new); + defaultEnvVars.forEach( item -> envVarsMap.putIfAbsent(item.getName(), item)); + + EnvVar[] envVars = envVarsMap.values().stream().toArray(EnvVar[]::new); List arguments = Strings.isNullOrEmpty(containerTemplate.getArgs()) ? Collections.emptyList() : parseDockerCommand(containerTemplate.getArgs() // diff --git a/src/test/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/KubernetesPipelineTest.java b/src/test/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/KubernetesPipelineTest.java index 7aebe93011..54f8767036 100644 --- a/src/test/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/KubernetesPipelineTest.java +++ b/src/test/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/KubernetesPipelineTest.java @@ -156,6 +156,19 @@ private void assertEnvVars(JenkinsRuleNonLocalhost r2, WorkflowRun b) throws Exc r.assertLogContains("OUTSIDE_POD_ENV_VAR_FROM_SECRET = " + POD_ENV_VAR_FROM_SECRET_VALUE + "\n", b); } + @Test + public void runWithOverriddenEnvVariables() throws Exception { + WorkflowJob p = r.jenkins.createProject(WorkflowJob.class, "p"); + p.setDefinition(new CpsFlowDefinition(loadPipelineScript("runWithOverriddenEnvVars.groovy"), true)); + WorkflowRun b = p.scheduleBuild2(0).waitForStart(); + assertNotNull(b); + r.assertBuildStatusSuccess(r.waitForCompletion(b)); + r.assertLogContains("OUTSIDE_CONTAINER_HOME_ENV_VAR = /home/jenkins\n", b); + r.assertLogContains("INSIDE_CONTAINER_HOME_ENV_VAR = /root\n",b); + r.assertLogContains("OUTSIDE_CONTAINER_POD_ENV_VAR = " + POD_ENV_VAR_VALUE + "\n", b); + r.assertLogContains("INSIDE_CONTAINER_POD_ENV_VAR = " + CONTAINER_ENV_VAR_VALUE + "\n",b); + } + @Test public void runJobWithSpaces() throws Exception { WorkflowJob p = r.jenkins.createProject(WorkflowJob.class, "p with spaces"); diff --git a/src/test/resources/org/csanchez/jenkins/plugins/kubernetes/pipeline/runWithOverriddenEnvVars.groovy b/src/test/resources/org/csanchez/jenkins/plugins/kubernetes/pipeline/runWithOverriddenEnvVars.groovy new file mode 100644 index 0000000000..5d88356f79 --- /dev/null +++ b/src/test/resources/org/csanchez/jenkins/plugins/kubernetes/pipeline/runWithOverriddenEnvVars.groovy @@ -0,0 +1,29 @@ +podTemplate(label: 'mypod', + envVars: [ + envVar(key: 'POD_ENV_VAR', value: 'pod-env-var-value') + ], + containers: [ + containerTemplate(name: 'busybox', image: 'busybox', ttyEnabled: true, command: '/bin/cat', + envVars: [ + envVar(key: 'HOME', value: '/root'), + envVar(key: 'POD_ENV_VAR', value: 'container-env-var-value') + ], + ), + ]) { + + node ('mypod') { + sh """ + echo OUTSIDE_CONTAINER_HOME_ENV_VAR = \$HOME + echo OUTSIDE_CONTAINER_POD_ENV_VAR = \$POD_ENV_VAR + """ + stage('Run busybox') { + container('busybox') { + sh 'echo inside container' + sh """ + echo INSIDE_CONTAINER_HOME_ENV_VAR = \$HOME + echo INSIDE_CONTAINER_POD_ENV_VAR = \$POD_ENV_VAR + """ + } + } + } +} From f7381bc798ad0c96b0850a9b305d028c4f19db6f Mon Sep 17 00:00:00 2001 From: Tomasz Nowak Date: Thu, 14 Sep 2017 14:19:05 +0200 Subject: [PATCH 2/4] Remove unnecessary variable --- .../jenkins/plugins/kubernetes/KubernetesLauncher.java | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesLauncher.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesLauncher.java index 9775318157..3e0508fda5 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesLauncher.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesLauncher.java @@ -318,15 +318,14 @@ private Container createContainer(KubernetesSlave slave, ContainerTemplate conta ); } if (globalEnvVars != null) { - globalEnvVars.forEach( item -> + globalEnvVars.forEach(item -> envVarsMap.computeIfAbsent(item.getKey(), k -> item.buildEnvVar()) ); } - List defaultEnvVars = env.entrySet().stream() - .map(entry -> new EnvVar(entry.getKey(), entry.getValue(), null)) - .collect(Collectors.toList()); - defaultEnvVars.forEach( item -> envVarsMap.putIfAbsent(item.getName(), item)); + env.entrySet().forEach(item -> + envVarsMap.computeIfAbsent(item.getKey(), k -> new EnvVar(item.getKey(), item.getValue(), null)) + ); EnvVar[] envVars = envVarsMap.values().stream().toArray(EnvVar[]::new); From 5d879e24ceb3878831e88594beb6ba9d8bbe3452 Mon Sep 17 00:00:00 2001 From: Tomasz Nowak Date: Thu, 14 Sep 2017 14:32:26 +0200 Subject: [PATCH 3/4] Use single quotes --- .../pipeline/runWithOverriddenEnvVars.groovy | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/test/resources/org/csanchez/jenkins/plugins/kubernetes/pipeline/runWithOverriddenEnvVars.groovy b/src/test/resources/org/csanchez/jenkins/plugins/kubernetes/pipeline/runWithOverriddenEnvVars.groovy index 5d88356f79..26984a4860 100644 --- a/src/test/resources/org/csanchez/jenkins/plugins/kubernetes/pipeline/runWithOverriddenEnvVars.groovy +++ b/src/test/resources/org/csanchez/jenkins/plugins/kubernetes/pipeline/runWithOverriddenEnvVars.groovy @@ -12,17 +12,17 @@ podTemplate(label: 'mypod', ]) { node ('mypod') { - sh """ - echo OUTSIDE_CONTAINER_HOME_ENV_VAR = \$HOME - echo OUTSIDE_CONTAINER_POD_ENV_VAR = \$POD_ENV_VAR - """ + sh ''' + echo OUTSIDE_CONTAINER_HOME_ENV_VAR = $HOME + echo OUTSIDE_CONTAINER_POD_ENV_VAR = $POD_ENV_VAR + ''' stage('Run busybox') { container('busybox') { - sh 'echo inside container' - sh """ - echo INSIDE_CONTAINER_HOME_ENV_VAR = \$HOME - echo INSIDE_CONTAINER_POD_ENV_VAR = \$POD_ENV_VAR - """ + sh ''' + echo inside container + echo INSIDE_CONTAINER_HOME_ENV_VAR = $HOME + echo INSIDE_CONTAINER_POD_ENV_VAR = $POD_ENV_VAR + ''' } } } From 4fdd36fad4152053c8df0eae3b91907d953312b7 Mon Sep 17 00:00:00 2001 From: Tomasz Nowak Date: Thu, 14 Sep 2017 15:24:28 +0200 Subject: [PATCH 4/4] Take last one env var --- .../kubernetes/KubernetesLauncher.java | 19 ++++++++++--------- .../pipeline/runWithOverriddenEnvVars.groovy | 2 ++ 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesLauncher.java b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesLauncher.java index 3e0508fda5..40d9b874ef 100644 --- a/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesLauncher.java +++ b/src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesLauncher.java @@ -312,20 +312,21 @@ private Container createContainer(KubernetesSlave slave, ContainerTemplate conta Map envVarsMap = new HashMap<>(); - if (containerTemplate.getEnvVars() != null) { - containerTemplate.getEnvVars().forEach(item -> - envVarsMap.computeIfAbsent(item.getKey(), k -> item.buildEnvVar()) - ); - } + env.entrySet().forEach(item -> + envVarsMap.put(item.getKey(), new EnvVar(item.getKey(), item.getValue(), null)) + ); + if (globalEnvVars != null) { globalEnvVars.forEach(item -> - envVarsMap.computeIfAbsent(item.getKey(), k -> item.buildEnvVar()) + envVarsMap.put(item.getKey(), item.buildEnvVar()) ); } - env.entrySet().forEach(item -> - envVarsMap.computeIfAbsent(item.getKey(), k -> new EnvVar(item.getKey(), item.getValue(), null)) - ); + if (containerTemplate.getEnvVars() != null) { + containerTemplate.getEnvVars().forEach(item -> + envVarsMap.put(item.getKey(), item.buildEnvVar()) + ); + } EnvVar[] envVars = envVarsMap.values().stream().toArray(EnvVar[]::new); diff --git a/src/test/resources/org/csanchez/jenkins/plugins/kubernetes/pipeline/runWithOverriddenEnvVars.groovy b/src/test/resources/org/csanchez/jenkins/plugins/kubernetes/pipeline/runWithOverriddenEnvVars.groovy index 26984a4860..95aa718a45 100644 --- a/src/test/resources/org/csanchez/jenkins/plugins/kubernetes/pipeline/runWithOverriddenEnvVars.groovy +++ b/src/test/resources/org/csanchez/jenkins/plugins/kubernetes/pipeline/runWithOverriddenEnvVars.groovy @@ -1,11 +1,13 @@ podTemplate(label: 'mypod', envVars: [ + envVar(key: 'POD_ENV_VAR', value: 'pod-env-var-value-first'), envVar(key: 'POD_ENV_VAR', value: 'pod-env-var-value') ], containers: [ containerTemplate(name: 'busybox', image: 'busybox', ttyEnabled: true, command: '/bin/cat', envVars: [ envVar(key: 'HOME', value: '/root'), + envVar(key: 'POD_ENV_VAR', value: 'container-env-var-value-first'), envVar(key: 'POD_ENV_VAR', value: 'container-env-var-value') ], ),