[FIXED JENKINS-47106] Only do explicit docker pull if specified

Automatically doing docker pull was breaking usage of local images. It may make sense to re-enable the automatic pull for the top-level agent, though. Also added a way to store arbitrary DeclarativeOptions on the DeclarativeAgent, so that we don't have to hardcode any new options in the future.
jenkinsci · Sep 28, 2017 · 1c275cd · 1c275cd
1 parent 9b10f92
commit 1c275cd
Show file tree

Hide file tree

Showing 7 changed files with 187 additions and 2 deletions.
diff --git a/...inition/src/main/groovy/org/jenkinsci/plugins/pipeline/modeldefinition/model/Agent.groovy b/...inition/src/main/groovy/org/jenkinsci/plugins/pipeline/modeldefinition/model/Agent.groovy
@@ -89,6 +89,9 @@ public class Agent extends MappedClosure<Object,Agent> implements Serializable {
                 }
             }
             a.setDoCheckout(doCheckout)
+            root?.options?.options?.each { k, v ->
+                a.addOption(v)
+            }
 
             return a
         } else {

diff --git a/.../java/org/jenkinsci/plugins/pipeline/modeldefinition/options/impl/AlwaysDoDockerPull.java b/.../java/org/jenkinsci/plugins/pipeline/modeldefinition/options/impl/AlwaysDoDockerPull.java
@@ -0,0 +1,51 @@
+/*
+ * The MIT License
+ *
+ * Copyright (c) 2017, CloudBees, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+package org.jenkinsci.plugins.pipeline.modeldefinition.options.impl;
+
+import hudson.Extension;
+import org.jenkinsci.Symbol;
+import org.jenkinsci.plugins.pipeline.modeldefinition.options.DeclarativeOption;
+import org.jenkinsci.plugins.pipeline.modeldefinition.options.DeclarativeOptionDescriptor;
+import org.kohsuke.stapler.DataBoundConstructor;
+
+import javax.annotation.Nullable;
+
+public class AlwaysDoDockerPull extends DeclarativeOption {
+    private Boolean alwaysDoDockerPull;
+
+    @DataBoundConstructor
+    public AlwaysDoDockerPull(@Nullable Boolean s) {
+        this.alwaysDoDockerPull = s;
+    }
+
+    public boolean getAlwaysDoDockerPull() {
+        return alwaysDoDockerPull == null || alwaysDoDockerPull;
+    }
+
+    @Extension @Symbol("alwaysDoDockerPull")
+    public static class DescriptorImpl extends DeclarativeOptionDescriptor {
+
+    }
+}
diff --git a/...ces/org/jenkinsci/plugins/pipeline/modeldefinition/agent/impl/DockerPipelineScript.groovy b/...ces/org/jenkinsci/plugins/pipeline/modeldefinition/agent/impl/DockerPipelineScript.groovy
@@ -27,6 +27,7 @@ package org.jenkinsci.plugins.pipeline.modeldefinition.agent.impl
 
 import org.jenkinsci.plugins.pipeline.modeldefinition.SyntheticStageNames
 import org.jenkinsci.plugins.pipeline.modeldefinition.Utils
+import org.jenkinsci.plugins.pipeline.modeldefinition.options.impl.AlwaysDoDockerPull
 import org.jenkinsci.plugins.workflow.cps.CpsScript
 
 public class DockerPipelineScript extends AbstractDockerPipelineScript<DockerPipeline> {
@@ -37,8 +38,9 @@ public class DockerPipelineScript extends AbstractDockerPipelineScript<DockerPip
 
     @Override
     public Closure runImage(Closure body) {
+        boolean alwaysDoDockerPull = describable.getOption(AlwaysDoDockerPull)?.alwaysDoDockerPull
         return {
-            if (!Utils.withinAStage()) {
+            if (!Utils.withinAStage() && alwaysDoDockerPull) {
                 script.stage(SyntheticStageNames.agentSetup()) {
                     try {
                         script.getProperty("docker").image(describable.image).pull()
@@ -50,7 +52,7 @@ public class DockerPipelineScript extends AbstractDockerPipelineScript<DockerPip
                 }
             }
             try {
-                if (Utils.withinAStage()) {
+                if (Utils.withinAStage() && alwaysDoDockerPull) {
                     script.getProperty("docker").image(describable.image).pull()
                 }
                 script.getProperty("docker").image(describable.image).inside(describable.args, {

diff --git a/...org/jenkinsci/plugins/pipeline/modeldefinition/options/impl/AlwaysDoDockerPull/help.jelly b/...org/jenkinsci/plugins/pipeline/modeldefinition/options/impl/AlwaysDoDockerPull/help.jelly
@@ -0,0 +1,32 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<?jelly escape-by-default='true'?>
+<!--
+  ~ The MIT License
+  ~
+  ~ Copyright (c) 2017, CloudBees, Inc.
+  ~
+  ~ Permission is hereby granted, free of charge, to any person obtaining a copy
+  ~ of this software and associated documentation files (the "Software"), to deal
+  ~ in the Software without restriction, including without limitation the rights
+  ~ to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+  ~ copies of the Software, and to permit persons to whom the Software is
+  ~ furnished to do so, subject to the following conditions:
+  ~
+  ~ The above copyright notice and this permission notice shall be included in
+  ~ all copies or substantial portions of the Software.
+  ~
+  ~ THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+  ~ IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+  ~ FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+  ~ AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+  ~ LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+  ~ OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+  ~ THE SOFTWARE.
+  -->
+
+<j:jelly xmlns:j="jelly:core">
+    <p>
+        The <code>alwaysDoDockerPull</code> option will ensure that <code>docker pull</code> is run for every
+        <code>docker</code> <code>agent</code>. Do not use this if you are using locally built images.
+    </p>
+</j:jelly>
diff --git a/...el-definition/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/AgentTest.java b/...el-definition/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/AgentTest.java
@@ -317,6 +317,27 @@ public void agentDockerGlobalThenLabel() throws Exception {
             .go();
     }
 
+    @Issue("JENKINS-47106")
+    @Test
+    public void dockerPullLocalImage() throws Exception {
+        assumeDocker();
+        // Bind mounting /var on OS X doesn't work at the moment
+        onAllowedOS(PossibleOS.LINUX);
+
+        sampleRepo.write("Dockerfile", "FROM ubuntu:14.04\n\nRUN echo 'HI THERE' > /hi-there\n\n");
+        sampleRepo.git("init");
+        sampleRepo.git("add", "Dockerfile");
+        sampleRepo.git("commit", "--message=Dockerfile");
+
+        expect("dockerPullLocalImage")
+                .logContains("[Pipeline] { (foo)",
+                        "The answer is 42",
+                        "-v /tmp:/tmp -p 8000:8000",
+                        "HI THERE")
+                .go();
+    }
+
+
     private void agentDocker(final String jenkinsfile, String... additionalLogContains) throws Exception {
         assumeDocker();
         // Bind mounting /var on OS X doesn't work at the moment

diff --git a/pipeline-model-definition/src/test/resources/dockerPullLocalImage.groovy b/pipeline-model-definition/src/test/resources/dockerPullLocalImage.groovy
@@ -0,0 +1,50 @@
+/*
+ * The MIT License
+ *
+ * Copyright (c) 2017, CloudBees, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+pipeline {
+    agent any
+    stages {
+        stage("build image") {
+            steps {
+                sh 'docker build -t test-image:abc .'
+            }
+        }
+        stage("foo") {
+            agent {
+                docker {
+                    image "test-image:abc"
+                    args "-v /tmp:/tmp -p 8000:8000"
+                    reuseNode true
+                }
+            }
+            steps {
+                sh 'cat /hi-there'
+                sh 'echo "The answer is 42"'
+            }
+        }
+    }
+}
+
+
+
diff --git a/.../src/main/java/org/jenkinsci/plugins/pipeline/modeldefinition/agent/DeclarativeAgent.java b/.../src/main/java/org/jenkinsci/plugins/pipeline/modeldefinition/agent/DeclarativeAgent.java
@@ -25,11 +25,17 @@
 package org.jenkinsci.plugins.pipeline.modeldefinition.agent;
 
 import hudson.ExtensionPoint;
+import org.jenkinsci.plugins.pipeline.modeldefinition.options.DeclarativeOption;
 import org.jenkinsci.plugins.pipeline.modeldefinition.withscript.WithScriptDescribable;
 import org.jenkinsci.plugins.pipeline.modeldefinition.withscript.WithScriptScript;
 import org.jenkinsci.plugins.workflow.cps.CpsScript;
 import org.jenkinsci.plugins.workflow.cps.CpsThread;
 
+import javax.annotation.CheckForNull;
+import javax.annotation.Nonnull;
+import java.util.ArrayList;
+import java.util.List;
+
 /**
  * Implementations for {@link DeclarativeAgentDescriptor} - pluggable agent backends for Declarative Pipelines.
  *
@@ -38,6 +44,7 @@
 public abstract class DeclarativeAgent<A extends DeclarativeAgent<A>> extends WithScriptDescribable<A> implements ExtensionPoint {
     protected boolean inStage;
     protected boolean doCheckout;
+    protected List<DeclarativeOption> options = new ArrayList<>();
 
     @Override
     public WithScriptScript getScript(CpsScript cpsScript) throws Exception {
@@ -66,6 +73,25 @@ public boolean isDoCheckout() {
         return doCheckout;
     }
 
+    public void addOption(@Nonnull DeclarativeOption option) {
+        for (DeclarativeOption o : options) {
+            if (o.getClass().equals(option.getClass())) {
+                options.remove(o);
+            }
+        }
+        options.add(option);
+    }
+
+    @CheckForNull
+    public <T extends DeclarativeOption> T getOption(@Nonnull Class<T> optionClass) {
+        for (DeclarativeOption o : options) {
+            if (optionClass.isInstance(o)) {
+                return optionClass.cast(o);
+            }
+        }
+        return null;
+    }
+
     public boolean hasScmContext(CpsScript script) {
         try {
             script.getProperty("scm");