From edaef1eed5799298178a544e5e3219127d154ad9 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Wed, 22 Aug 2018 16:30:41 -0400 Subject: [PATCH 01/13] Make it easier to run dockerNode by defaulting everything except the image argument to reasonable values. --- .../docker/pipeline/DockerNodeStep.java | 13 +++++++---- .../pipeline/DockerNodeStepExecution.java | 22 ++++++++++++++----- .../pipeline/DockerNodeStep/config.jelly | 22 +++++++++++-------- .../docker/pipeline/DockerNodeStep/help.html | 7 +++--- .../docker/pipeline/DockerNodeStepTest.java | 15 +++++++++++++ 5 files changed, 57 insertions(+), 22 deletions(-) diff --git a/src/main/java/io/jenkins/docker/pipeline/DockerNodeStep.java b/src/main/java/io/jenkins/docker/pipeline/DockerNodeStep.java index 81ce12cf5..a93c05dc6 100644 --- a/src/main/java/io/jenkins/docker/pipeline/DockerNodeStep.java +++ b/src/main/java/io/jenkins/docker/pipeline/DockerNodeStep.java @@ -4,6 +4,7 @@ import hudson.Extension; import hudson.FilePath; import hudson.Launcher; +import hudson.Util; import hudson.model.Computer; import hudson.model.Item; import hudson.model.Node; @@ -36,13 +37,12 @@ public class DockerNodeStep extends Step { private String image; - private String remoteFs; + private String remoteFs = "/tmp"; private DockerComputerConnector connector; @DataBoundConstructor - public DockerNodeStep(String dockerHost, String image) { - this.dockerHost = dockerHost; + public DockerNodeStep(String image) { this.image = image; } @@ -50,13 +50,18 @@ public String getDockerHost() { return dockerHost; } + @DataBoundSetter + public void setDockerHost(String dockerHost) { + this.dockerHost = Util.fixEmpty(dockerHost); + } + public String getCredentialsId() { return credentialsId; } @DataBoundSetter public void setCredentialsId(String credentialsId) { - this.credentialsId = credentialsId; + this.credentialsId = Util.fixEmpty(credentialsId); } public String getImage() { diff --git a/src/main/java/io/jenkins/docker/pipeline/DockerNodeStepExecution.java b/src/main/java/io/jenkins/docker/pipeline/DockerNodeStepExecution.java index f8211eea5..b31daca74 100644 --- a/src/main/java/io/jenkins/docker/pipeline/DockerNodeStepExecution.java +++ b/src/main/java/io/jenkins/docker/pipeline/DockerNodeStepExecution.java @@ -1,20 +1,20 @@ package io.jenkins.docker.pipeline; +import com.nirima.jenkins.plugins.docker.DockerCloud; import com.nirima.jenkins.plugins.docker.DockerTemplate; import com.nirima.jenkins.plugins.docker.DockerTemplateBase; import hudson.EnvVars; import hudson.FilePath; import hudson.Util; -import hudson.console.PlainTextConsoleOutputStream; import hudson.model.Computer; import hudson.model.Node; import hudson.model.TaskListener; +import hudson.slaves.Cloud; import io.jenkins.docker.DockerTransientNode; import io.jenkins.docker.client.DockerAPI; import io.jenkins.docker.connector.DockerComputerAttachConnector; import io.jenkins.docker.connector.DockerComputerConnector; import jenkins.model.Jenkins; -import jenkins.model.NodeListener; import org.jenkinsci.plugins.docker.commons.credentials.DockerServerEndpoint; import org.jenkinsci.plugins.workflow.graph.FlowNode; import org.jenkinsci.plugins.workflow.steps.BodyExecutionCallback; @@ -23,11 +23,9 @@ import org.jenkinsci.plugins.workflow.support.actions.WorkspaceActionImpl; import javax.annotation.Nonnull; -import java.io.ByteArrayOutputStream; import java.io.IOException; import java.util.UUID; import java.util.concurrent.CompletableFuture; -import java.util.concurrent.Future; /** * @author Nicolas De Loof @@ -83,7 +81,12 @@ private DockerTransientNode createNode(TaskListener listener) { t.setMode(Node.Mode.EXCLUSIVE); - final DockerAPI api = new DockerAPI(new DockerServerEndpoint(dockerHost, credentialsId)); + final DockerAPI api; + if (dockerHost == null && credentialsId == null) { + api = defaultApi(); + } else { + api = new DockerAPI(new DockerServerEndpoint(dockerHost, credentialsId)); + } DockerTransientNode node; Computer computer = null; @@ -114,6 +117,15 @@ private DockerTransientNode createNode(TaskListener listener) { return node; } + private static DockerAPI defaultApi() { + for (Cloud cloud : Jenkins.get().clouds) { + if (cloud instanceof DockerCloud) { + return ((DockerCloud) cloud).getDockerApi(); + } + } + throw new IllegalStateException("Must either specify dockerHost/credentialsId, or define at least one Docker cloud"); + } + private void invokeBody(DockerTransientNode node, TaskListener listener) { diff --git a/src/main/resources/io/jenkins/docker/pipeline/DockerNodeStep/config.jelly b/src/main/resources/io/jenkins/docker/pipeline/DockerNodeStep/config.jelly index 401f166b0..027d36c1f 100644 --- a/src/main/resources/io/jenkins/docker/pipeline/DockerNodeStep/config.jelly +++ b/src/main/resources/io/jenkins/docker/pipeline/DockerNodeStep/config.jelly @@ -1,20 +1,24 @@ - - - - - - - - - + + + + + + + + + + + + + \ No newline at end of file diff --git a/src/main/resources/io/jenkins/docker/pipeline/DockerNodeStep/help.html b/src/main/resources/io/jenkins/docker/pipeline/DockerNodeStep/help.html index 7caa6a31a..cdc60afcb 100644 --- a/src/main/resources/io/jenkins/docker/pipeline/DockerNodeStep/help.html +++ b/src/main/resources/io/jenkins/docker/pipeline/DockerNodeStep/help.html @@ -2,10 +2,9 @@

Allocates a new Jenkins agent using a specified Docker image and runs tasks on it. Example:

-
docker-node(image: 'cloudbees/jnlp-slave-with-java-build-tools') {
+    
dockerNode('cloudbees/jnlp-slave-with-java-build-tools') {
     git 'https://github.com/jglick/simple-maven-project-with-tests'
-    withMaven {
-        sh 'mvn install'
-    }
+    sh 'mvn -B -Dmaven.test.failure.ignore install'
+    junit '**/target/surefire-reports/TEST-*.xml'
 }
\ No newline at end of file diff --git a/src/test/java/io/jenkins/docker/pipeline/DockerNodeStepTest.java b/src/test/java/io/jenkins/docker/pipeline/DockerNodeStepTest.java index 7457e1a6f..88d97db6f 100644 --- a/src/test/java/io/jenkins/docker/pipeline/DockerNodeStepTest.java +++ b/src/test/java/io/jenkins/docker/pipeline/DockerNodeStepTest.java @@ -25,6 +25,7 @@ package io.jenkins.docker.pipeline; import com.google.common.collect.ImmutableSet; +import com.nirima.jenkins.plugins.docker.DockerCloud; import hudson.FilePath; import hudson.model.DownloadService; import hudson.model.Node; @@ -34,6 +35,7 @@ import hudson.tasks.Maven; import hudson.tools.DownloadFromUrlInstaller; import hudson.tools.InstallSourceProperty; +import io.jenkins.docker.client.DockerAPI; import org.apache.commons.lang3.SystemUtils; import org.jenkinsci.plugins.workflow.cps.CpsFlowDefinition; import org.jenkinsci.plugins.workflow.job.WorkflowJob; @@ -60,6 +62,7 @@ import javax.annotation.Nonnull; import java.util.Collections; import java.util.Set; +import org.jenkinsci.plugins.docker.commons.credentials.DockerServerEndpoint; public class DockerNodeStepTest { @@ -214,6 +217,18 @@ public void evaluate() throws Throwable { }); } + @Test + public void defaults() { + story.then(r -> { + r.jenkins.clouds.add(new DockerCloud("whatever", new DockerAPI(new DockerServerEndpoint("unix:///var/run/docker.sock", null)), Collections.emptyList())); + WorkflowJob j = r.createProject(WorkflowJob.class, "p"); + j.setDefinition(new CpsFlowDefinition("dockerNode('openjdk:8') {\n" + + " sh 'java -version && touch stuff && ls -la'\n" + + "}\n", true)); + r.buildAndAssertSuccess(j); + }); + } + @Issue("JENKINS-47805") @Test public void pathModification() throws Exception { From 2e32338701c645797779a35b557ddf097ab4c5c0 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Wed, 22 Aug 2018 17:03:31 -0400 Subject: [PATCH 02/13] Oops, wrong Jenkins core version. --- .../io/jenkins/docker/pipeline/DockerNodeStepExecution.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/io/jenkins/docker/pipeline/DockerNodeStepExecution.java b/src/main/java/io/jenkins/docker/pipeline/DockerNodeStepExecution.java index b31daca74..f6dca3680 100644 --- a/src/main/java/io/jenkins/docker/pipeline/DockerNodeStepExecution.java +++ b/src/main/java/io/jenkins/docker/pipeline/DockerNodeStepExecution.java @@ -118,7 +118,7 @@ private DockerTransientNode createNode(TaskListener listener) { } private static DockerAPI defaultApi() { - for (Cloud cloud : Jenkins.get().clouds) { + for (Cloud cloud : Jenkins.getInstance().clouds) { if (cloud instanceof DockerCloud) { return ((DockerCloud) cloud).getDockerApi(); } From bb990b6a5b30f5bb7c834b6a915691e3dcd79770 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Wed, 22 Aug 2018 17:05:03 -0400 Subject: [PATCH 03/13] Adding a pipeline-model-extensions dep. --- pom.xml | 60 +++++++++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 48 insertions(+), 12 deletions(-) diff --git a/pom.xml b/pom.xml index 9d9e0be18..551ca1edf 100644 --- a/pom.xml +++ b/pom.xml @@ -30,7 +30,7 @@ UTF-8 8 2.4.7 - 2.60.3 + 2.73.3 @@ -83,7 +83,7 @@ org.jenkins-ci.plugins.workflow workflow-api - 2.23.1 + 2.25 true @@ -95,7 +95,7 @@ org.jenkins-ci.plugins.workflow workflow-support - 2.16 + 2.17 true @@ -103,26 +103,32 @@ org.jenkins-ci.plugins.workflow workflow-durable-task-step 2.17 - true + test org.jenkins-ci.plugins.workflow workflow-job - 2.12.2 - true + 2.15 + test org.jenkins-ci.plugins.workflow workflow-cps - 2.41 - true + 2.46 + test - org.jenkins-ci.plugins - script-security - 1.36 + org.jenkinsci.plugins + pipeline-model-extensions + 1.3.1 true + + org.jenkinsci.plugins + pipeline-model-definition + 1.3.1 + test + com.kohlschutter.junixsocket @@ -185,10 +191,40 @@ + + org.jenkins-ci.plugins + script-security + 1.42 + org.jenkins-ci.plugins structs - 1.9 + 1.14 + + + org.jenkins-ci.plugins + credentials + 2.1.16 + + + org.jenkins-ci.plugins + ssh-credentials + 1.13 + + + org.jenkins-ci.plugins + credentials-binding + 1.13 + + + org.jenkins-ci.plugins + scm-api + 2.2.6 + + + org.jenkins-ci.plugins.workflow + workflow-scm-step + 2.6 From 93f2fc2d872e55d5621973160a930fd2a2979801 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Wed, 22 Aug 2018 17:06:45 -0400 Subject: [PATCH 04/13] Fixing Incrementals stuff. --- .mvn/extensions.xml | 2 +- pom.xml | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/.mvn/extensions.xml b/.mvn/extensions.xml index 510f24fbc..1fb5b5c65 100644 --- a/.mvn/extensions.xml +++ b/.mvn/extensions.xml @@ -2,6 +2,6 @@ io.jenkins.tools.incrementals git-changelist-maven-extension - 1.0-beta-3 + 1.0-beta-5 diff --git a/pom.xml b/pom.xml index 9d9e0be18..a57599e19 100644 --- a/pom.xml +++ b/pom.xml @@ -5,12 +5,13 @@ org.jenkins-ci.plugins plugin - 3.12 + 3.19 + io.jenkins.docker docker-plugin - 1.1.6-SNAPSHOT + ${revision}${changelist} hpi Docker plugin @@ -25,7 +26,7 @@ - 1.1.5 + 1.1.6 -SNAPSHOT UTF-8 8 From 4dd11e738b6a22438658f6ed24128d363817887b Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Fri, 24 Aug 2018 10:25:53 -0400 Subject: [PATCH 05/13] [JENKINS-48050] Added dockerContainer agent type for Declarative. --- .../jenkins/docker/pipeline/DockerAgent.java | 32 ++++++++++++ .../pipeline/DockerNodeStepExecution.java | 2 +- .../docker/pipeline/DockerAgent/config.jelly | 7 +++ .../docker/pipeline/DockerAgentScript.groovy | 28 +++++++++++ .../docker/pipeline/DockerAgentTest.java | 50 +++++++++++++++++++ .../docker/pipeline/DockerNodeStepTest.java | 6 +-- 6 files changed, 121 insertions(+), 4 deletions(-) create mode 100644 src/main/java/io/jenkins/docker/pipeline/DockerAgent.java create mode 100644 src/main/resources/io/jenkins/docker/pipeline/DockerAgent/config.jelly create mode 100644 src/main/resources/io/jenkins/docker/pipeline/DockerAgentScript.groovy create mode 100644 src/test/java/io/jenkins/docker/pipeline/DockerAgentTest.java diff --git a/src/main/java/io/jenkins/docker/pipeline/DockerAgent.java b/src/main/java/io/jenkins/docker/pipeline/DockerAgent.java new file mode 100644 index 000000000..f83c2d15e --- /dev/null +++ b/src/main/java/io/jenkins/docker/pipeline/DockerAgent.java @@ -0,0 +1,32 @@ +package io.jenkins.docker.pipeline; + +import hudson.Extension; +import org.jenkinsci.Symbol; +import org.jenkinsci.plugins.pipeline.modeldefinition.agent.DeclarativeAgent; +import org.jenkinsci.plugins.pipeline.modeldefinition.agent.DeclarativeAgentDescriptor; +import org.kohsuke.stapler.DataBoundConstructor; + +@SuppressWarnings("unchecked") // TODO DeclarativeAgent.getDescriptor problem +public class DockerAgent extends DeclarativeAgent { + + public final String image; + + @DataBoundConstructor + public DockerAgent(String image) { + this.image = image; + } + + // TODO other properties accepted by DockerNodeStep + + @Symbol("dockerContainer") + @Extension + public static class DescriptorImpl extends DeclarativeAgentDescriptor { + + @Override + public String getDisplayName() { + return "Start a Docker container with a new agent"; + } + + } + +} diff --git a/src/main/java/io/jenkins/docker/pipeline/DockerNodeStepExecution.java b/src/main/java/io/jenkins/docker/pipeline/DockerNodeStepExecution.java index f6dca3680..f80a54596 100644 --- a/src/main/java/io/jenkins/docker/pipeline/DockerNodeStepExecution.java +++ b/src/main/java/io/jenkins/docker/pipeline/DockerNodeStepExecution.java @@ -75,7 +75,7 @@ private DockerTransientNode createNode(TaskListener listener) { final String uuid = UUID.randomUUID().toString(); final DockerTemplate t = new DockerTemplate( - new DockerTemplateBase(image), + new DockerTemplateBase(image), // TODO call .setPullCredentialsId and also add option to .setRegistryUrl or similar connector, uuid, remoteFs, "1"); diff --git a/src/main/resources/io/jenkins/docker/pipeline/DockerAgent/config.jelly b/src/main/resources/io/jenkins/docker/pipeline/DockerAgent/config.jelly new file mode 100644 index 000000000..96d8c1960 --- /dev/null +++ b/src/main/resources/io/jenkins/docker/pipeline/DockerAgent/config.jelly @@ -0,0 +1,7 @@ + + + + + + + diff --git a/src/main/resources/io/jenkins/docker/pipeline/DockerAgentScript.groovy b/src/main/resources/io/jenkins/docker/pipeline/DockerAgentScript.groovy new file mode 100644 index 000000000..48ace2adb --- /dev/null +++ b/src/main/resources/io/jenkins/docker/pipeline/DockerAgentScript.groovy @@ -0,0 +1,28 @@ +package io.jenkins.docker.pipeline + +import org.jenkinsci.plugins.pipeline.modeldefinition.Utils +import org.jenkinsci.plugins.pipeline.modeldefinition.agent.CheckoutScript +import org.jenkinsci.plugins.pipeline.modeldefinition.agent.DeclarativeAgentScript +import org.jenkinsci.plugins.workflow.cps.CpsScript + +class DockerAgentScript extends DeclarativeAgentScript { + + DockerAgentScript(CpsScript s, DockerAgent a) { + super(s, a) + } + + @Override + Closure run(Closure body) { + return { + try { + script.dockerNode(describable?.image) { + CheckoutScript.doCheckout(script, describable, null, body).call() + } + } catch (Exception e) { + script.getProperty("currentBuild").result = Utils.getResultFromException(e) + throw e + } + } + } + +} diff --git a/src/test/java/io/jenkins/docker/pipeline/DockerAgentTest.java b/src/test/java/io/jenkins/docker/pipeline/DockerAgentTest.java new file mode 100644 index 000000000..84e562256 --- /dev/null +++ b/src/test/java/io/jenkins/docker/pipeline/DockerAgentTest.java @@ -0,0 +1,50 @@ +package io.jenkins.docker.pipeline; + +import com.nirima.jenkins.plugins.docker.DockerCloud; +import io.jenkins.docker.client.DockerAPI; +import java.util.Collections; +import org.jenkinsci.plugins.docker.commons.credentials.DockerServerEndpoint; +import org.jenkinsci.plugins.workflow.cps.CpsFlowDefinition; +import org.jenkinsci.plugins.workflow.job.WorkflowJob; +import org.junit.ClassRule; +import org.junit.Test; +import org.junit.BeforeClass; +import org.junit.Rule; +import org.jvnet.hudson.test.BuildWatcher; +import org.jvnet.hudson.test.JenkinsRule; + +public class DockerAgentTest { + + @BeforeClass + public static void before() { + DockerNodeStepTest.before(); + } + + @ClassRule + public static BuildWatcher buildWatcher = new BuildWatcher(); + + @Rule + public JenkinsRule r = new JenkinsRule(); + + @Test + public void smokes() throws Exception { + // as in DockerNodeStepTest.defaults: + r.jenkins.clouds.add(new DockerCloud("whatever", new DockerAPI(new DockerServerEndpoint("unix:///var/run/docker.sock", null)), Collections.emptyList())); + WorkflowJob j = r.createProject(WorkflowJob.class, "p"); + j.setDefinition(new CpsFlowDefinition( + "pipeline {\n" + + " agent {\n" + + " dockerContainer 'openjdk:8'\n" + + " }\n" + + " stages {\n" + + " stage('whatever') {\n" + + " steps {\n" + + " sh 'java -version'\n" + + " }\n" + + " }\n" + + " }\n" + + "}\n", true)); + r.assertLogContains("openjdk version \"1.8", r.buildAndAssertSuccess(j)); + } + +} diff --git a/src/test/java/io/jenkins/docker/pipeline/DockerNodeStepTest.java b/src/test/java/io/jenkins/docker/pipeline/DockerNodeStepTest.java index 88d97db6f..616446e7c 100644 --- a/src/test/java/io/jenkins/docker/pipeline/DockerNodeStepTest.java +++ b/src/test/java/io/jenkins/docker/pipeline/DockerNodeStepTest.java @@ -48,7 +48,7 @@ import org.jenkinsci.plugins.workflow.steps.StepDescriptor; import org.jenkinsci.plugins.workflow.steps.StepExecution; import org.junit.Assume; -import org.junit.Before; +import org.junit.BeforeClass; import org.junit.ClassRule; import org.junit.Rule; import org.junit.Test; @@ -66,8 +66,8 @@ public class DockerNodeStepTest { - @Before - public void before() { + @BeforeClass + public static void before() { // FIXME on CI windows nodes don't have Docker4Windows Assume.assumeFalse(SystemUtils.IS_OS_WINDOWS); } From c770e0c5397a77c3042ac76fe745f27613a50762 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Fri, 24 Aug 2018 12:47:54 -0400 Subject: [PATCH 06/13] Reverting defaulting of remoteFs; the original default seems to work well enough. --- .../java/io/jenkins/docker/pipeline/DockerNodeStep.java | 2 +- .../jenkins/docker/pipeline/DockerNodeStep/config.jelly | 8 ++++---- .../docker/pipeline/DockerNodeStep/help-remoteFs.html | 5 +++++ .../io/jenkins/docker/pipeline/DockerNodeStepTest.java | 7 ++++--- 4 files changed, 14 insertions(+), 8 deletions(-) create mode 100644 src/main/resources/io/jenkins/docker/pipeline/DockerNodeStep/help-remoteFs.html diff --git a/src/main/java/io/jenkins/docker/pipeline/DockerNodeStep.java b/src/main/java/io/jenkins/docker/pipeline/DockerNodeStep.java index a93c05dc6..d794dc565 100644 --- a/src/main/java/io/jenkins/docker/pipeline/DockerNodeStep.java +++ b/src/main/java/io/jenkins/docker/pipeline/DockerNodeStep.java @@ -37,7 +37,7 @@ public class DockerNodeStep extends Step { private String image; - private String remoteFs = "/tmp"; + private String remoteFs; private DockerComputerConnector connector; diff --git a/src/main/resources/io/jenkins/docker/pipeline/DockerNodeStep/config.jelly b/src/main/resources/io/jenkins/docker/pipeline/DockerNodeStep/config.jelly index 027d36c1f..80722cfc6 100644 --- a/src/main/resources/io/jenkins/docker/pipeline/DockerNodeStep/config.jelly +++ b/src/main/resources/io/jenkins/docker/pipeline/DockerNodeStep/config.jelly @@ -5,12 +5,12 @@ - - - - + + + + diff --git a/src/main/resources/io/jenkins/docker/pipeline/DockerNodeStep/help-remoteFs.html b/src/main/resources/io/jenkins/docker/pipeline/DockerNodeStep/help-remoteFs.html new file mode 100644 index 000000000..af9067a65 --- /dev/null +++ b/src/main/resources/io/jenkins/docker/pipeline/DockerNodeStep/help-remoteFs.html @@ -0,0 +1,5 @@ +
+ Use a specific root directory for the Jenkins agent. + This includes a workspace subdirectory as well as various control files. + If not specified, uses the WORKDIR from the image. +
diff --git a/src/test/java/io/jenkins/docker/pipeline/DockerNodeStepTest.java b/src/test/java/io/jenkins/docker/pipeline/DockerNodeStepTest.java index 88d97db6f..b3023d6e2 100644 --- a/src/test/java/io/jenkins/docker/pipeline/DockerNodeStepTest.java +++ b/src/test/java/io/jenkins/docker/pipeline/DockerNodeStepTest.java @@ -222,9 +222,10 @@ public void defaults() { story.then(r -> { r.jenkins.clouds.add(new DockerCloud("whatever", new DockerAPI(new DockerServerEndpoint("unix:///var/run/docker.sock", null)), Collections.emptyList())); WorkflowJob j = r.createProject(WorkflowJob.class, "p"); - j.setDefinition(new CpsFlowDefinition("dockerNode('openjdk:8') {\n" + - " sh 'java -version && touch stuff && ls -la'\n" + - "}\n", true)); + j.setDefinition(new CpsFlowDefinition( + "dockerNode('openjdk:8') {\n" + + " sh 'java -version && whoami && pwd && touch stuff && ls -lat . ..'\n" + + "}\n", true)); r.buildAndAssertSuccess(j); }); } From 92e62e0a98523328833d94bb34fe44a9212c9cec Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Fri, 24 Aug 2018 12:59:46 -0400 Subject: [PATCH 07/13] Also ensure that Pipeline Syntax does not suggest remoteFs: ''. --- .../java/io/jenkins/docker/pipeline/DockerNodeStep.java | 4 ++-- .../io/jenkins/docker/pipeline/DockerNodeStepTest.java | 9 +++++++++ 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/src/main/java/io/jenkins/docker/pipeline/DockerNodeStep.java b/src/main/java/io/jenkins/docker/pipeline/DockerNodeStep.java index d794dc565..d4799a89c 100644 --- a/src/main/java/io/jenkins/docker/pipeline/DockerNodeStep.java +++ b/src/main/java/io/jenkins/docker/pipeline/DockerNodeStep.java @@ -74,14 +74,14 @@ public String getRemoteFs() { @DataBoundSetter public void setRemoteFs(String remoteFs) { - this.remoteFs = remoteFs; + this.remoteFs = Util.fixEmpty(remoteFs); } public DockerComputerConnector getConnector() { return connector; } - @DataBoundSetter + @DataBoundSetter // TODO this is not mentioned in config.jelly public void setConnector(DockerComputerConnector connector) { this.connector = connector; } diff --git a/src/test/java/io/jenkins/docker/pipeline/DockerNodeStepTest.java b/src/test/java/io/jenkins/docker/pipeline/DockerNodeStepTest.java index b3023d6e2..52acaed25 100644 --- a/src/test/java/io/jenkins/docker/pipeline/DockerNodeStepTest.java +++ b/src/test/java/io/jenkins/docker/pipeline/DockerNodeStepTest.java @@ -47,6 +47,7 @@ import org.jenkinsci.plugins.workflow.steps.StepContext; import org.jenkinsci.plugins.workflow.steps.StepDescriptor; import org.jenkinsci.plugins.workflow.steps.StepExecution; +import static org.junit.Assert.*; import org.junit.Assume; import org.junit.Before; import org.junit.ClassRule; @@ -63,6 +64,8 @@ import java.util.Collections; import java.util.Set; import org.jenkinsci.plugins.docker.commons.credentials.DockerServerEndpoint; +import org.jenkinsci.plugins.structs.describable.DescribableModel; +import org.jenkinsci.plugins.structs.describable.UninstantiatedDescribable; public class DockerNodeStepTest { @@ -220,6 +223,12 @@ public void evaluate() throws Throwable { @Test public void defaults() { story.then(r -> { + DockerNodeStep s = new DockerNodeStep("foo"); + s.setCredentialsId(""); + s.setDockerHost(""); + s.setRemoteFs(""); + UninstantiatedDescribable uninstantiated = new DescribableModel<>(DockerNodeStep.class).uninstantiate2(s); + assertEquals(uninstantiated.toString(), Collections.singleton("image"), uninstantiated.getArguments().keySet()); r.jenkins.clouds.add(new DockerCloud("whatever", new DockerAPI(new DockerServerEndpoint("unix:///var/run/docker.sock", null)), Collections.emptyList())); WorkflowJob j = r.createProject(WorkflowJob.class, "p"); j.setDefinition(new CpsFlowDefinition( From b6a93bf937a577082a8f34092dfbd972a2bbf840 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Mon, 6 Apr 2020 15:10:06 -0400 Subject: [PATCH 08/13] RequireUpperBoundDeps --- pom.xml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/pom.xml b/pom.xml index 5eb43a0b9..a1f73ac1e 100644 --- a/pom.xml +++ b/pom.xml @@ -155,6 +155,12 @@ pipeline-model-definition 1.3.1 test + + + org.apache.commons + commons-lang3 + +
From dd2678d3255d2bc4ad9d2277d24138a3a711a563 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Tue, 23 May 2023 13:19:17 -0400 Subject: [PATCH 09/13] Use a better-supported image https://github.com/jenkinsci/docker-plugin/pull/681#discussion_r1202705847 --- src/test/java/io/jenkins/docker/pipeline/DockerAgentTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/java/io/jenkins/docker/pipeline/DockerAgentTest.java b/src/test/java/io/jenkins/docker/pipeline/DockerAgentTest.java index bba75525b..c4fd1f9bd 100644 --- a/src/test/java/io/jenkins/docker/pipeline/DockerAgentTest.java +++ b/src/test/java/io/jenkins/docker/pipeline/DockerAgentTest.java @@ -36,7 +36,7 @@ public void smokes() throws Exception { WorkflowJob j = r.createProject(WorkflowJob.class, "p"); j.setDefinition(new CpsFlowDefinition( "pipeline {\n" + " agent {\n" - + " dockerContainer 'openjdk:11'\n" + + " dockerContainer 'eclipse-temurin:17'\n" + " }\n" + " stages {\n" + " stage('whatever') {\n" @@ -47,6 +47,6 @@ public void smokes() throws Exception { + " }\n" + "}\n", true)); - r.assertLogContains("openjdk version \"11.", r.buildAndAssertSuccess(j)); + r.assertLogContains("openjdk version \"17.", r.buildAndAssertSuccess(j)); } } From 414e425a49bce95fc5d15ff675297f127575a323 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Tue, 23 May 2023 13:22:15 -0400 Subject: [PATCH 10/13] Add `serialVersionUID` to `DockerAgent` (though the impls in `pipeline-model-definition` lack it) for SpotBugs https://github.com/jenkinsci/docker-plugin/pull/681#discussion_r1202706434 --- src/main/java/io/jenkins/docker/pipeline/DockerAgent.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/main/java/io/jenkins/docker/pipeline/DockerAgent.java b/src/main/java/io/jenkins/docker/pipeline/DockerAgent.java index a3b9a836e..4ed596785 100644 --- a/src/main/java/io/jenkins/docker/pipeline/DockerAgent.java +++ b/src/main/java/io/jenkins/docker/pipeline/DockerAgent.java @@ -9,6 +9,8 @@ @SuppressWarnings("unchecked") // TODO DeclarativeAgent.getDescriptor problem public class DockerAgent extends DeclarativeAgent { + private static final long serialVersionUID = 1; + public final String image; @DataBoundConstructor From 13b6aae1993a830b1c9467ffc9e1700f5f0cfc43 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Tue, 23 May 2023 15:22:17 -0400 Subject: [PATCH 11/13] Let `DockerAgent` use any args supported by `DockerNodeStep` https://github.com/jenkinsci/docker-plugin/pull/681#discussion_r1202754052 --- .../jenkins/docker/pipeline/DockerAgent.java | 99 ++++++++++++++++++- .../docker/pipeline/DockerAgent/config.jelly | 25 ++++- .../pipeline/DockerAgent/help-remoteFs.html | 5 + .../docker/pipeline/DockerAgentScript.groovy | 2 +- .../docker/pipeline/DockerAgentTest.java | 31 +++++- 5 files changed, 154 insertions(+), 8 deletions(-) create mode 100644 src/main/resources/io/jenkins/docker/pipeline/DockerAgent/help-remoteFs.html diff --git a/src/main/java/io/jenkins/docker/pipeline/DockerAgent.java b/src/main/java/io/jenkins/docker/pipeline/DockerAgent.java index 4ed596785..6d11b5153 100644 --- a/src/main/java/io/jenkins/docker/pipeline/DockerAgent.java +++ b/src/main/java/io/jenkins/docker/pipeline/DockerAgent.java @@ -1,24 +1,109 @@ package io.jenkins.docker.pipeline; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import hudson.Extension; +import hudson.ExtensionList; +import hudson.Util; +import hudson.model.Descriptor; +import hudson.model.Item; +import hudson.util.ListBoxModel; +import io.jenkins.docker.connector.DockerComputerConnector; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; import org.jenkinsci.Symbol; +import org.jenkinsci.plugins.docker.commons.credentials.DockerServerEndpoint; import org.jenkinsci.plugins.pipeline.modeldefinition.agent.DeclarativeAgent; import org.jenkinsci.plugins.pipeline.modeldefinition.agent.DeclarativeAgentDescriptor; +import org.kohsuke.stapler.AncestorInPath; import org.kohsuke.stapler.DataBoundConstructor; +import org.kohsuke.stapler.DataBoundSetter; +import org.kohsuke.stapler.QueryParameter; @SuppressWarnings("unchecked") // TODO DeclarativeAgent.getDescriptor problem public class DockerAgent extends DeclarativeAgent { private static final long serialVersionUID = 1; - public final String image; + private final String image; + private String dockerHost; + private String credentialsId; + private String remoteFs; + + @SuppressFBWarnings(value = "SE_BAD_FIELD", justification = "Checked in setter") + private DockerComputerConnector connector; @DataBoundConstructor public DockerAgent(String image) { this.image = image; } - // TODO other properties accepted by DockerNodeStep + public String getImage() { + return image; + } + + public String getDockerHost() { + return dockerHost; + } + + @DataBoundSetter + public void setDockerHost(String dockerHost) { + this.dockerHost = Util.fixEmpty(dockerHost); + } + + public String getCredentialsId() { + return credentialsId; + } + + @DataBoundSetter + public void setCredentialsId(String credentialsId) { + this.credentialsId = Util.fixEmpty(credentialsId); + } + + public String getRemoteFs() { + return remoteFs; + } + + @DataBoundSetter + public void setRemoteFs(String remoteFs) { + this.remoteFs = Util.fixEmpty(remoteFs); + } + + public DockerComputerConnector getConnector() { + if (connector == null) { + return null; + } + DockerNodeStepExecution.assertIsSerializableDockerComputerConnector(connector); + return connector; + } + + @DataBoundSetter + public void setConnector(DockerComputerConnector connector) { + if (connector == null || connector.equals(DockerNodeStepExecution.DEFAULT_CONNECTOR)) { + this.connector = null; + } else { + DockerNodeStepExecution.assertIsSerializableDockerComputerConnector(connector); + this.connector = connector; + } + } + + public Map getAsArgs() { + Map args = new LinkedHashMap<>(); + args.put("image", image); + if (dockerHost != null) { + args.put("dockerHost", dockerHost); + } + if (credentialsId != null) { + args.put("credentialsId", credentialsId); + } + if (remoteFs != null) { + args.put("remoteFs", remoteFs); + } + if (connector != null) { + args.put("connector", connector); + } + return args; + } @Symbol("dockerContainer") @Extension @@ -28,5 +113,15 @@ public static class DescriptorImpl extends DeclarativeAgentDescriptor> getAcceptableConnectorDescriptors() { + return ExtensionList.lookupSingleton(DockerNodeStep.DescriptorImpl.class) + .getAcceptableConnectorDescriptors(); + } } } diff --git a/src/main/resources/io/jenkins/docker/pipeline/DockerAgent/config.jelly b/src/main/resources/io/jenkins/docker/pipeline/DockerAgent/config.jelly index 96d8c1960..b7f358e8a 100644 --- a/src/main/resources/io/jenkins/docker/pipeline/DockerAgent/config.jelly +++ b/src/main/resources/io/jenkins/docker/pipeline/DockerAgent/config.jelly @@ -1,7 +1,26 @@ - - + + + + + + + + + + + + - + + + + + + + + + + diff --git a/src/main/resources/io/jenkins/docker/pipeline/DockerAgent/help-remoteFs.html b/src/main/resources/io/jenkins/docker/pipeline/DockerAgent/help-remoteFs.html new file mode 100644 index 000000000..af9067a65 --- /dev/null +++ b/src/main/resources/io/jenkins/docker/pipeline/DockerAgent/help-remoteFs.html @@ -0,0 +1,5 @@ +
+ Use a specific root directory for the Jenkins agent. + This includes a workspace subdirectory as well as various control files. + If not specified, uses the WORKDIR from the image. +
diff --git a/src/main/resources/io/jenkins/docker/pipeline/DockerAgentScript.groovy b/src/main/resources/io/jenkins/docker/pipeline/DockerAgentScript.groovy index 48ace2adb..008429f68 100644 --- a/src/main/resources/io/jenkins/docker/pipeline/DockerAgentScript.groovy +++ b/src/main/resources/io/jenkins/docker/pipeline/DockerAgentScript.groovy @@ -15,7 +15,7 @@ class DockerAgentScript extends DeclarativeAgentScript { Closure run(Closure body) { return { try { - script.dockerNode(describable?.image) { + script.dockerNode(describable.asArgs) { CheckoutScript.doCheckout(script, describable, null, body).call() } } catch (Exception e) { diff --git a/src/test/java/io/jenkins/docker/pipeline/DockerAgentTest.java b/src/test/java/io/jenkins/docker/pipeline/DockerAgentTest.java index c4fd1f9bd..7da1d010d 100644 --- a/src/test/java/io/jenkins/docker/pipeline/DockerAgentTest.java +++ b/src/test/java/io/jenkins/docker/pipeline/DockerAgentTest.java @@ -6,6 +6,7 @@ import org.jenkinsci.plugins.docker.commons.credentials.DockerServerEndpoint; import org.jenkinsci.plugins.workflow.cps.CpsFlowDefinition; import org.jenkinsci.plugins.workflow.job.WorkflowJob; +import org.junit.Before; import org.junit.BeforeClass; import org.junit.ClassRule; import org.junit.Rule; @@ -26,13 +27,17 @@ public static void before() { @Rule public JenkinsRule r = new JenkinsRule(); - @Test - public void smokes() throws Exception { + @Before + public void setUpCloud() { // as in DockerNodeStepTest.defaults: r.jenkins.clouds.add(new DockerCloud( "whatever", new DockerAPI(new DockerServerEndpoint("unix:///var/run/docker.sock", null)), Collections.emptyList())); + } + + @Test + public void smokes() throws Exception { WorkflowJob j = r.createProject(WorkflowJob.class, "p"); j.setDefinition(new CpsFlowDefinition( "pipeline {\n" + " agent {\n" @@ -49,4 +54,26 @@ public void smokes() throws Exception { true)); r.assertLogContains("openjdk version \"17.", r.buildAndAssertSuccess(j)); } + + @Test + public void withArgs() throws Exception { + WorkflowJob j = r.createProject(WorkflowJob.class, "p"); + j.setDefinition(new CpsFlowDefinition( + "pipeline {\n" + " agent {\n" + + " dockerContainer {\n" + " image 'eclipse-temurin:17'\n" + + " connector attach(jvmArgsString: '-showversion')\n" + + " }\n" + + " }\n" + + " stages {\n" + + " stage('whatever') {\n" + + " steps {\n" + + " sh 'which java'\n" + + " }\n" + + " }\n" + + " }\n" + + "}\n", + true)); + // TODO why does DockerTemplate.doProvisionNode fail to stream container output? + r.assertLogContains("/bin/java", r.buildAndAssertSuccess(j)); + } } From 90617b278d7cc9e267db3300ed38a6846d3532cd Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Tue, 23 May 2023 18:18:30 -0400 Subject: [PATCH 12/13] Placating security scanner https://github.com/jenkinsci/docker-plugin/pull/681#issuecomment-1560067517 --- src/main/java/io/jenkins/docker/pipeline/DockerAgent.java | 3 +++ src/main/java/io/jenkins/docker/pipeline/DockerNodeStep.java | 3 +++ 2 files changed, 6 insertions(+) diff --git a/src/main/java/io/jenkins/docker/pipeline/DockerAgent.java b/src/main/java/io/jenkins/docker/pipeline/DockerAgent.java index 6d11b5153..fc86a3ba9 100644 --- a/src/main/java/io/jenkins/docker/pipeline/DockerAgent.java +++ b/src/main/java/io/jenkins/docker/pipeline/DockerAgent.java @@ -19,6 +19,7 @@ import org.kohsuke.stapler.DataBoundConstructor; import org.kohsuke.stapler.DataBoundSetter; import org.kohsuke.stapler.QueryParameter; +import org.kohsuke.stapler.verb.POST; @SuppressWarnings("unchecked") // TODO DeclarativeAgent.getDescriptor problem public class DockerAgent extends DeclarativeAgent { @@ -114,6 +115,8 @@ public String getDisplayName() { return "Start a Docker container with a new agent"; } + @SuppressWarnings("lgtm[jenkins/no-permission-check]") // done in DockerServerEndpoint + @POST public ListBoxModel doFillCredentialsIdItems(@AncestorInPath Item item, @QueryParameter String uri) { return ExtensionList.lookupSingleton(DockerServerEndpoint.DescriptorImpl.class) .doFillCredentialsIdItems(item, uri); diff --git a/src/main/java/io/jenkins/docker/pipeline/DockerNodeStep.java b/src/main/java/io/jenkins/docker/pipeline/DockerNodeStep.java index 0f14bffa2..dba3da342 100644 --- a/src/main/java/io/jenkins/docker/pipeline/DockerNodeStep.java +++ b/src/main/java/io/jenkins/docker/pipeline/DockerNodeStep.java @@ -28,6 +28,7 @@ import org.kohsuke.stapler.DataBoundConstructor; import org.kohsuke.stapler.DataBoundSetter; import org.kohsuke.stapler.QueryParameter; +import org.kohsuke.stapler.verb.POST; /** * @author Nicolas De Loof @@ -116,6 +117,8 @@ public String getDisplayName() { return "Docker Node (⚠️ Experimental)"; } + @SuppressWarnings("lgtm[jenkins/no-permission-check]") // done in DockerServerEndpoint + @POST public ListBoxModel doFillCredentialsIdItems(@AncestorInPath Item item, @QueryParameter String uri) { DockerServerEndpoint.DescriptorImpl descriptor = (DockerServerEndpoint.DescriptorImpl) Jenkins.get().getDescriptorOrDie(DockerServerEndpoint.class); From 817669cff0091d7c4c8d9fcfac0e506abd2216f4 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Tue, 23 May 2023 18:20:42 -0400 Subject: [PATCH 13/13] =?UTF-8?q?If=20`DockerNodeStep`=20is=20marked=20?= =?UTF-8?q?=E2=80=9C=E2=9A=A0=EF=B8=8F=20Experimental=E2=80=9D=20then=20`D?= =?UTF-8?q?ockerAgent`=20should=20be=20too?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/main/java/io/jenkins/docker/pipeline/DockerAgent.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/io/jenkins/docker/pipeline/DockerAgent.java b/src/main/java/io/jenkins/docker/pipeline/DockerAgent.java index fc86a3ba9..3ade83012 100644 --- a/src/main/java/io/jenkins/docker/pipeline/DockerAgent.java +++ b/src/main/java/io/jenkins/docker/pipeline/DockerAgent.java @@ -112,7 +112,7 @@ public static class DescriptorImpl extends DeclarativeAgentDescriptor