Merge pull request #41 from jenkinsci/Computer.environment-JENKINS-26552

[JENKINS-26552] Provide a full Computer.environment to @StepContextParameter EnvVars
jenkinsci · Mar 28, 2015 · bf05beb · bf05beb
2 parents 22053bd + a2e47d6
commit bf05beb
Show file tree

Hide file tree

Showing 4 changed files with 66 additions and 11 deletions.
diff --git a/aggregator/src/test/java/org/jenkinsci/plugins/workflow/JenkinsRuleExt.java b/aggregator/src/test/java/org/jenkinsci/plugins/workflow/JenkinsRuleExt.java
@@ -24,8 +24,22 @@
 
 package org.jenkinsci.plugins.workflow;
 
+import hudson.EnvVars;
+import hudson.model.Computer;
+import hudson.model.Descriptor;
+import hudson.model.Node.Mode;
 import hudson.model.Run;
+import hudson.model.Slave;
+import hudson.slaves.CommandLauncher;
+import hudson.slaves.NodeProperty;
+import hudson.slaves.RetentionStrategy;
+import hudson.slaves.SlaveComputer;
+import java.io.File;
 import java.io.IOException;
+import java.util.Collections;
+import java.util.Map;
+import javax.annotation.CheckForNull;
+import javax.annotation.Nonnull;
 import org.jvnet.hudson.test.JenkinsRule;
 
 /**
@@ -58,6 +72,41 @@ public static <R extends Run<?,?>> R waitForMessage(String message, R r) throws
         return r;
     }
 
+    /**
+     * Akin to {@link JenkinsRule#createSlave(String, String, EnvVars)} but allows {@link Computer#getEnvironment} to be controlled rather than directly modifying launchers.
+     * @param env variables to override in {@link Computer#getEnvironment}; null values will get unset even if defined in the test environment
+     * @see <a href="https://github.com/jenkinsci/jenkins/pull/1553/files#r23784822">explanation in core PR 1553</a>
+     */
+    public static Slave createSpecialEnvSlave(JenkinsRule rule, String nodeName, @CheckForNull String labels, Map<String,String> env) throws Exception {
+        @SuppressWarnings("deprecation") // keep consistency with original signature rather than force the caller to pass in a TemporaryFolder rule
+        File remoteFS = rule.createTmpDir();
+        SpecialEnvSlave slave = new SpecialEnvSlave(remoteFS, rule.createComputerLauncher(/* yes null */null), nodeName, labels != null ? labels : "", env);
+        rule.jenkins.addNode(slave);
+        return slave;
+    }
+    private static class SpecialEnvSlave extends Slave {
+        private final Map<String,String> env;
+        SpecialEnvSlave(File remoteFS, CommandLauncher launcher, String nodeName, @Nonnull String labels, Map<String,String> env) throws Descriptor.FormException, IOException {
+            super(nodeName, nodeName, remoteFS.getAbsolutePath(), 1, Mode.NORMAL, labels, launcher, RetentionStrategy.NOOP, Collections.<NodeProperty<?>>emptyList());
+            this.env = env;
+        }
+        @Override public Computer createComputer() {
+            return new SpecialEnvComputer(this, env);
+        }
+    }
+    private static class SpecialEnvComputer extends SlaveComputer {
+        private final Map<String,String> env;
+        SpecialEnvComputer(SpecialEnvSlave slave, Map<String,String> env) {
+            super(slave);
+            this.env = env;
+        }
+        @Override public EnvVars getEnvironment() throws IOException, InterruptedException {
+            EnvVars env2 = super.getEnvironment();
+            env2.overrideAll(env);
+            return env2;
+        }
+    }
+
     private JenkinsRuleExt() {}
 
 }
diff --git a/aggregator/src/test/java/org/jenkinsci/plugins/workflow/WorkflowTest.java b/aggregator/src/test/java/org/jenkinsci/plugins/workflow/WorkflowTest.java
@@ -47,8 +47,10 @@
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
+import java.util.Map;
 import java.util.concurrent.atomic.AtomicReference;
 import java.util.logging.ConsoleHandler;
 import java.util.logging.Handler;
@@ -603,9 +605,13 @@ public static void finish(final boolean terminate) {
     @Test public void env() {
         story.addStep(new Statement() {
             @Override public void evaluate() throws Throwable {
+                Map<String,String> slaveEnv = new HashMap<String,String>();
+                slaveEnv.put("BUILD_TAG", null);
+                slaveEnv.put("PERMACHINE", "set");
+                JenkinsRuleExt.createSpecialEnvSlave(story.j, "slave", null, slaveEnv);
                 p = jenkins().createProject(WorkflowJob.class, "demo");
-                p.setDefinition(new CpsFlowDefinition("node {\n"
-                        + "  sh 'echo tag=$BUILD_TAG'\n"
+                p.setDefinition(new CpsFlowDefinition("node('slave') {\n"
+                        + "  sh 'echo tag=$BUILD_TAG PERMACHINE=$PERMACHINE'\n"
                         + "  env.BUILD_TAG='custom'\n"
                         + "  sh 'echo tag2=$BUILD_TAG'\n"
                         + "  env.STUFF='more'\n"
@@ -624,7 +630,7 @@ public static void finish(final boolean terminate) {
                 assertThatWorkflowIsSuspended();
                 SemaphoreStep.success("env/1", null);
                 story.j.assertBuildStatusSuccess(JenkinsRuleExt.waitForCompletion(b));
-                story.j.assertLogContains("tag=jenkins-demo-1", b);
+                story.j.assertLogContains("tag=jenkins-demo-1 PERMACHINE=set", b);
                 story.j.assertLogContains("tag2=custom", b);
                 story.j.assertLogContains("tag3=custom2 stuff=more", b);
                 EnvironmentAction a = b.getAction(EnvironmentAction.class);

diff --git a/cps/src/main/java/org/jenkinsci/plugins/workflow/cps/EnvActionImpl.java b/cps/src/main/java/org/jenkinsci/plugins/workflow/cps/EnvActionImpl.java
@@ -26,7 +26,6 @@
 
 import groovy.lang.GroovyObjectSupport;
 import hudson.EnvVars;
-import hudson.model.Computer;
 import hudson.model.Run;
 import hudson.util.LogTaskListener;
 import java.io.IOException;
@@ -71,14 +70,14 @@ public Map<String,String> getOverriddenEnvironment() {
 
     @Override public Object getProperty(String propertyName) {
         try {
-            String val = getEnvironment().get(propertyName);
-            if (val == null) {
-                Computer computer = CpsThread.current().getContextVariable(Computer.class);
-                if (computer != null) {
-                    return computer.getEnvironment().get(propertyName);
+            EnvVars overridden = CpsThread.current().getContextVariable(EnvVars.class);
+            if (overridden != null) {
+                String val = overridden.get(propertyName);
+                if (val != null) {
+                    return val;
                 }
             }
-            return val;
+            return getEnvironment().get(propertyName);
         } catch (Exception x) {
             LOGGER.log(Level.WARNING, null, x);
             return null;

diff --git a/...ort/src/main/java/org/jenkinsci/plugins/workflow/support/steps/ExecutorStepExecution.java b/...ort/src/main/java/org/jenkinsci/plugins/workflow/support/steps/ExecutorStepExecution.java
@@ -387,7 +387,8 @@ private final class PlaceholderExecutable implements ContinuableExecutable {
                         cookie = UUID.randomUUID().toString();
                         // Switches the label to a self-label, so if the executable is killed and restarted via ExecutorPickle, it will run on the same node:
                         label = computer.getName();
-                        EnvVars env = computer.buildEnvironment(listener);
+                        EnvVars env = computer.getEnvironment();
+                        env.overrideAll(computer.buildEnvironment(listener));
                         env.put(COOKIE_VAR, cookie);
                         synchronized (runningTasks) {
                             runningTasks.put(cookie, context);