From 0fe3ea8ae72ea0be692e0a093fb4473507c53532 Mon Sep 17 00:00:00 2001 From: Andrew Bayer Date: Fri, 9 Sep 2016 09:48:55 -0700 Subject: [PATCH] [FIXED JENKINS-37932] Add "agent any" Better approach for "run anywhere". I think this gets rid of any need to make the agent section optional, at least for now. --- .../modeldefinition/ast/ModelASTAgent.groovy | 5 +-- .../modeldefinition/model/Agent.groovy | 7 ++-- .../modeldefinition/parser/ModelParser.groovy | 5 +-- .../validator/ModelValidator.groovy | 6 ++-- .../ClosureModelTranslator.groovy | 5 +++ .../modeldefinition/AbstractModelDefTest.java | 1 + .../pipeline/modeldefinition/AgentTest.java | 12 +++++++ src/test/resources/agentAny.groovy | 35 +++++++++++++++++++ src/test/resources/json/agentAny.json | 19 ++++++++++ 9 files changed, 86 insertions(+), 9 deletions(-) create mode 100644 src/test/resources/agentAny.groovy create mode 100644 src/test/resources/json/agentAny.json diff --git a/src/main/groovy/org/jenkinsci/plugins/pipeline/modeldefinition/ast/ModelASTAgent.groovy b/src/main/groovy/org/jenkinsci/plugins/pipeline/modeldefinition/ast/ModelASTAgent.groovy index 4d2a46b6f..5a8a98dd7 100644 --- a/src/main/groovy/org/jenkinsci/plugins/pipeline/modeldefinition/ast/ModelASTAgent.groovy +++ b/src/main/groovy/org/jenkinsci/plugins/pipeline/modeldefinition/ast/ModelASTAgent.groovy @@ -59,8 +59,9 @@ public final class ModelASTAgent extends ModelASTElement { public String toGroovy() { String argStr // TODO: Stop special-casing agent none. - if (args instanceof ModelASTSingleArgument && args.value.value.equals("none")) { - argStr = "none" + if (args instanceof ModelASTSingleArgument && + (args.value.value.equals("none") || args.value.value.equals("any"))) { + argStr = args.value.value } else { argStr = args.toGroovy() } diff --git a/src/main/groovy/org/jenkinsci/plugins/pipeline/modeldefinition/model/Agent.groovy b/src/main/groovy/org/jenkinsci/plugins/pipeline/modeldefinition/model/Agent.groovy index 78fb1490b..3921c7504 100644 --- a/src/main/groovy/org/jenkinsci/plugins/pipeline/modeldefinition/model/Agent.groovy +++ b/src/main/groovy/org/jenkinsci/plugins/pipeline/modeldefinition/model/Agent.groovy @@ -45,6 +45,9 @@ public class Agent implements Serializable { @Whitelisted String label + @Whitelisted + Boolean any + @Whitelisted public Agent(Map args) { if (args.containsKey("docker")) { @@ -57,12 +60,12 @@ public class Agent implements Serializable { @Whitelisted public Agent(boolean b) { - + this.any = b } @Whitelisted public boolean hasAgent() { - return docker != null || label != null + return any || docker != null || label != null } // TODO: Rewrite as an extension point and get this by extension discovery, but we knew that already. diff --git a/src/main/groovy/org/jenkinsci/plugins/pipeline/modeldefinition/parser/ModelParser.groovy b/src/main/groovy/org/jenkinsci/plugins/pipeline/modeldefinition/parser/ModelParser.groovy index cf0612acf..8a5293b5a 100644 --- a/src/main/groovy/org/jenkinsci/plugins/pipeline/modeldefinition/parser/ModelParser.groovy +++ b/src/main/groovy/org/jenkinsci/plugins/pipeline/modeldefinition/parser/ModelParser.groovy @@ -460,9 +460,10 @@ class ModelParser { if (e instanceof VariableExpression) { if (e.name.equals("none")) { return ModelASTValue.fromConstant("none", e) // Special casing for agent none. + } else if (e.name.equals("any")) { + return ModelASTValue.fromConstant("any", e) // Special casing for agent any. } } - // for other composite expressions, treat it as in-place GString return ModelASTValue.fromGString("\${"+getSourceText(e)+"}", e) } @@ -479,7 +480,7 @@ class ModelParser { if (exp instanceof ConstantExpression) { return castOrNull(String,exp.value); } - // TODO: This may be too broad a way to catch 'agent none'. + // TODO: This may be too broad a way to catch 'agent none' and 'agent any'. else if (exp instanceof VariableExpression) { return castOrNull(String,exp.name); } diff --git a/src/main/groovy/org/jenkinsci/plugins/pipeline/modeldefinition/validator/ModelValidator.groovy b/src/main/groovy/org/jenkinsci/plugins/pipeline/modeldefinition/validator/ModelValidator.groovy index eaee8bc6a..be0e1f727 100644 --- a/src/main/groovy/org/jenkinsci/plugins/pipeline/modeldefinition/validator/ModelValidator.groovy +++ b/src/main/groovy/org/jenkinsci/plugins/pipeline/modeldefinition/validator/ModelValidator.groovy @@ -363,14 +363,14 @@ class ModelValidator { if (agent.args instanceof ModelASTSingleArgument) { ModelASTSingleArgument singleArg = (ModelASTSingleArgument) agent.args - if (singleArg.value.getValue() != 'none') { - errorCollector.error(agent.args, "Invalid argument for agent - '${singleArg.value.getValue()}' - must be map of config options or bare none.") + if (singleArg.value.getValue() != 'none' && singleArg.value.getValue() != 'any') { + errorCollector.error(agent.args, "Invalid argument for agent - '${singleArg.value.getValue()}' - must be map of config options or bare none or any.") valid = false } else if (agent.args instanceof ModelASTNamedArgumentList) { ModelASTNamedArgumentList namedArgs = (ModelASTNamedArgumentList)agent.args namedArgs.arguments.each { k, v -> if (!(Agent.agentConfigKeys().contains(k.key))) { - errorCollector.error(k, "Invalid config option '${k.key}'. Valid config optiosn are ${Agent.agentConfigKeys()}.") + errorCollector.error(k, "Invalid config option '${k.key}'. Valid config options are ${Agent.agentConfigKeys()}.") valid = false } } diff --git a/src/main/resources/org/jenkinsci/plugins/pipeline/modeldefinition/ClosureModelTranslator.groovy b/src/main/resources/org/jenkinsci/plugins/pipeline/modeldefinition/ClosureModelTranslator.groovy index 9d95e9a48..9a53a4591 100644 --- a/src/main/resources/org/jenkinsci/plugins/pipeline/modeldefinition/ClosureModelTranslator.groovy +++ b/src/main/resources/org/jenkinsci/plugins/pipeline/modeldefinition/ClosureModelTranslator.groovy @@ -45,6 +45,11 @@ public class ClosureModelTranslator implements MethodMissingWrapper, Serializabl */ boolean none = false + /** + * Placeholder to make sure 'agent any' works. + */ + boolean any = true + ClosureModelTranslator(Class clazz) { actualClass = clazz } diff --git a/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/AbstractModelDefTest.java b/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/AbstractModelDefTest.java index 6d203c4cc..849ddeaaa 100644 --- a/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/AbstractModelDefTest.java +++ b/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/AbstractModelDefTest.java @@ -86,6 +86,7 @@ public void setUp() throws Exception { public static final List SHOULD_PASS_CONFIGS = ImmutableList.of( "simplePipeline", + "agentAny", "agentDocker", "agentLabel", "agentNoneWithNode", diff --git a/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/AgentTest.java b/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/AgentTest.java index 56dacb377..055224b9a 100644 --- a/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/AgentTest.java +++ b/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/AgentTest.java @@ -28,6 +28,7 @@ import hudson.slaves.EnvironmentVariablesNodeProperty; import org.jenkinsci.plugins.workflow.job.WorkflowRun; import org.junit.Test; +import org.jvnet.hudson.test.Issue; /** * @author Andrew Bayer @@ -48,6 +49,17 @@ public void agentLabel() throws Exception { j.assertLogContains("ONSLAVE=true", b); } + @Issue("JENKINS-37932") + @Test + public void agentAny() throws Exception { + prepRepoWithJenkinsfile("agentAny"); + + WorkflowRun b = getAndStartBuild(); + j.assertBuildStatusSuccess(j.waitForCompletion(b)); + j.assertLogContains("[Pipeline] { (foo)", b); + j.assertLogContains("THIS WORKS", b); + } + @Test public void noCheckoutScmInWrongContext() throws Exception { DumbSlave s = j.createOnlineSlave(); diff --git a/src/test/resources/agentAny.groovy b/src/test/resources/agentAny.groovy new file mode 100644 index 000000000..ba33ea6ef --- /dev/null +++ b/src/test/resources/agentAny.groovy @@ -0,0 +1,35 @@ +/* + * The MIT License + * + * Copyright (c) 2016, 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("foo") { + sh('echo "THIS WORKS"') + } + } +} + + + diff --git a/src/test/resources/json/agentAny.json b/src/test/resources/json/agentAny.json new file mode 100644 index 000000000..e437a22a1 --- /dev/null +++ b/src/test/resources/json/agentAny.json @@ -0,0 +1,19 @@ +{"pipeline": { + "stages": [ { + "name": "foo", + "branches": [ { + "name": "default", + "steps": [ { + "name": "sh", + "arguments": { + "isConstant": true, + "value": "echo \"THIS WORKS\"" + } + }] + }] + }], + "agent": { + "isConstant": true, + "value": "any" + } +}} \ No newline at end of file