[JENKINS-40524] Rework agent syntax to be more extensible

Andrew Bayer added a comment - 2016-12-17 21:47

PR up at https://github.com/jenkinsci/pipeline-model-definition-plugin/pull/73

Andrew Bayer added a comment - 2016-12-17 21:47 PR up at https://github.com/jenkinsci/pipeline-model-definition-plugin/pull/73

Andrew Bayer added a comment - 2016-12-20 17:12

So I'd really like to get some feedback on two possible syntax approaches - I'm fine with either one, so I need input. =)

First, which is what's currently in the PR:

agent {
  label "foo"
}

agent {
  docker "httpd:2.4.12"
  dockerArgs "-v /tmp:/tmp -p 80:80"
  label "foo"
  // the below would be for things like private registry, kubernetes pod config, whatever
  someComplicatedOption {
    optA "banana"
    optB "monkey"
  }
}

agent {
  dockerfile "true"
  dockerArgs "-v /tmp:/tmp -p 80:80"
  label "foo"
}

Second:

agent {
  // i.e., DeclarativeAgent types with only one required argument and only one specified argument don't need the nested block
  label  "foo"
}

agent {
  // Also would work with 'docker "httpd:2.4.12"' if the image is the only desired argument
  docker {
    image "httpd:2.4.12"
    args "-v /tmp:/tmp -p 80:80"
    label "foo"
    // the below would be for things like private registry, kubernetes pod config, whatever
    someComplicatedOption {
      optA "banana"
      optB "monkey"
  }
}

agent {
  // Also would work with 'dockerfile true' as above
  dockerfile {
    args "-v /tmp:/tmp -p 80:80"
    label "foo"
  }
}

And of course, agent any and agent none will still be working.

Thoughts, hrmpw, michaelneale, jamesdumay, rsandell, herau, imod, lsglick, pleibiger?

Andrew Bayer added a comment - 2016-12-20 17:12 So I'd really like to get some feedback on two possible syntax approaches - I'm fine with either one, so I need input. =) First, which is what's currently in the PR: agent { label "foo" } agent { docker "httpd:2.4.12" dockerArgs "-v /tmp:/tmp -p 80:80" label "foo" // the below would be for things like private registry, kubernetes pod config, whatever someComplicatedOption { optA "banana" optB "monkey" } } agent { dockerfile " true " dockerArgs "-v /tmp:/tmp -p 80:80" label "foo" } Second: agent { // i.e., DeclarativeAgent types with only one required argument and only one specified argument don't need the nested block label "foo" } agent { // Also would work with 'docker "httpd:2.4.12" ' if the image is the only desired argument docker { image "httpd:2.4.12" args "-v /tmp:/tmp -p 80:80" label "foo" // the below would be for things like private registry, kubernetes pod config, whatever someComplicatedOption { optA "banana" optB "monkey" } } agent { // Also would work with 'dockerfile true ' as above dockerfile { args "-v /tmp:/tmp -p 80:80" label "foo" } } And of course, agent any and agent none will still be working. Thoughts, hrmpw , michaelneale , jamesdumay , rsandell , herau , imod , lsglick , pleibiger ?

Logan Glickfield added a comment - 2016-12-20 17:21

I'm a fan of option 2.

The first option leaves some confusion as to what exactly label means. It's unclear to me whether it will prefer an existing labeled node and fall back to docker, or whether it will always create a new docker node and just apply that label. The second provides some clarity that the label is just an attribute of the docker/dockerfile nodes.

Logan Glickfield added a comment - 2016-12-20 17:21 I'm a fan of option 2. The first option leaves some confusion as to what exactly label means. It's unclear to me whether it will prefer an existing labeled node and fall back to docker, or whether it will always create a new docker node and just apply that label. The second provides some clarity that the label is just an attribute of the docker/dockerfile nodes.

Patrick Wolf added a comment - 2016-12-20 17:26

question abayer

the second example in the second option

agent {
  // Also would work with 'docker "httpd:2.4.12"' if the image is the only desired argument
  docker {
    image "httpd:2.4.12"
    args "-v /tmp:/tmp -p 80:80"
    label "foo"
    // the below would be for things like private registry, kubernetes pod config, whatever
    someComplicatedOption {
      optA "banana"
      optB "monkey"
  }
}

What does label refer to? Is that the agent label or the container label? Would this be supported?

agent {
  label "nodename"
  docker {
    image "httpd:2.4.12"
    args "-v /tmp:/tmp -p 80:80"
    label "foo"
    // the below would be for things like private registry, kubernetes pod config, whatever
    someComplicatedOption {
      optA "banana"
      optB "monkey"
  }
}

Patrick Wolf added a comment - 2016-12-20 17:26 question abayer the second example in the second option agent { // Also would work with 'docker "httpd:2.4.12" ' if the image is the only desired argument docker { image "httpd:2.4.12" args "-v /tmp:/tmp -p 80:80" label "foo" // the below would be for things like private registry, kubernetes pod config, whatever someComplicatedOption { optA "banana" optB "monkey" } } What does label refer to? Is that the agent label or the container label? Would this be supported? agent { label "nodename" docker { image "httpd:2.4.12" args "-v /tmp:/tmp -p 80:80" label "foo" // the below would be for things like private registry, kubernetes pod config, whatever someComplicatedOption { optA "banana" optB "monkey" } }

Andrew Bayer added a comment - 2016-12-20 17:35

hrmpw - no, the second would not be possible. label inside docker or dockerfile would be equivalent to label 'foo' - i.e., which agent to run on in the first place.

Andrew Bayer added a comment - 2016-12-20 17:35 hrmpw - no, the second would not be possible. label inside docker or dockerfile would be equivalent to label 'foo' - i.e., which agent to run on in the first place.

Patrick Wolf added a comment - 2016-12-20 17:37

Then the second option is confusing and misleading.

Patrick Wolf added a comment - 2016-12-20 17:37 Then the second option is confusing and misleading.

Peter Leibiger added a comment - 2016-12-21 12:31 - edited

I like the second option, to make the label more clear maybe something like this?

agent {
  any {
    label  "foo"
  }
}

or

agent {
  node {
    label  "foo"
  }
}

Peter Leibiger added a comment - 2016-12-21 12:31 - edited I like the second option, to make the label more clear maybe something like this? agent { any { label "foo" } } or agent { node { label "foo" } }

Peter Leibiger added a comment - 2016-12-21 12:39

That could lead to the following options:

agent none

agent {
  node any
}

agent {
  node {
    label "foo"
  }
}

Where 1st and 2nd actually are equivalent.

Peter Leibiger added a comment - 2016-12-21 12:39 That could lead to the following options: agent none agent { node any } agent { node { label "foo" } } Where 1st and 2nd actually are equivalent.

Andrew Bayer added a comment - 2016-12-27 15:48

pleibiger So I'm not a big fan of that - I definitely understand the thinking, but given that label always just has the one value, it feels like nesting it inside a closure is a bit overly verbose. We're committed to continuing to support agent any, which means "run on any agent" (while agent none means "don't run in a node block at all and leave that up to individual stages"), so I don't think we'd get anything from nesting label further.

I've been pondering this for the last week and think I'm going to go with the current PR's approach - i.e.,

agent {
  label "foo"
}

agent {
  docker "httpd:2.4.12"
  dockerArgs "-v /tmp:/tmp -p 80:80"
  label "foo"
  // the below would be for things like private registry, kubernetes pod config, whatever
  someComplicatedOption {
    optA "banana"
    optB "monkey"
  }
}

agent {
  dockerfile "true"
  dockerArgs "-v /tmp:/tmp -p 80:80"
  label "foo"
}

It's not ideal, but it's the easiest to do by far. I completely get the confusion of label nested inside a docker block, but to do a docker block with a separate label definition would actually require a fairly drastic change to how the DeclarativeAgent classes work, since we'd go from having just one DeclarativeAgent (i.e., one of https://github.com/jenkinsci/pipeline-model-definition-plugin/tree/master/pipeline-model-definition/src/main/java/org/jenkinsci/plugins/pipeline/modeldefinition/agent/impl, identified by a key) to having to combine multiple, which is...messy. I'm gonna sleep on it one more night and see if an idea comes to me for a good implementation of combining them, but I don't think that's likely to happen. =)

Andrew Bayer added a comment - 2016-12-27 15:48 pleibiger So I'm not a big fan of that - I definitely understand the thinking, but given that label always just has the one value, it feels like nesting it inside a closure is a bit overly verbose. We're committed to continuing to support agent any , which means "run on any agent" (while agent none means "don't run in a node block at all and leave that up to individual stages"), so I don't think we'd get anything from nesting label further. I've been pondering this for the last week and think I'm going to go with the current PR's approach - i.e., agent { label "foo" } agent { docker "httpd:2.4.12" dockerArgs "-v /tmp:/tmp -p 80:80" label "foo" // the below would be for things like private registry, kubernetes pod config, whatever someComplicatedOption { optA "banana" optB "monkey" } } agent { dockerfile " true " dockerArgs "-v /tmp:/tmp -p 80:80" label "foo" } It's not ideal, but it's the easiest to do by far. I completely get the confusion of label nested inside a docker block, but to do a docker block with a separate label definition would actually require a fairly drastic change to how the DeclarativeAgent classes work, since we'd go from having just one DeclarativeAgent (i.e., one of https://github.com/jenkinsci/pipeline-model-definition-plugin/tree/master/pipeline-model-definition/src/main/java/org/jenkinsci/plugins/pipeline/modeldefinition/agent/impl , identified by a key) to having to combine multiple, which is...messy. I'm gonna sleep on it one more night and see if an idea comes to me for a good implementation of combining them, but I don't think that's likely to happen. =)

Patrick Wolf added a comment - 2016-12-27 22:17

abayer What happens if someone does this:

agent {
  dockerArgs "-v /tmp:/tmp -p 80:80"
  label "foo"
}

Would this be caught in syntax check?

Patrick Wolf added a comment - 2016-12-27 22:17 abayer What happens if someone does this: agent { dockerArgs "-v /tmp:/tmp -p 80:80" label "foo" } Would this be caught in syntax check?

Michael Neale added a comment - 2016-12-27 22:47

I think the final "flattened" form suggested above looks quite good. The common cases will be quite clear, and it appears to be extensible. You often use label alongside docker so this makes sense to me as a user. Will validation catch things like a user putting in dockerArgs but not docker? if they put both docker and dockerFile etc? (can be a future enhancement, if not)

Michael Neale added a comment - 2016-12-27 22:47 I think the final "flattened" form suggested above looks quite good. The common cases will be quite clear, and it appears to be extensible. You often use label alongside docker so this makes sense to me as a user. Will validation catch things like a user putting in dockerArgs but not docker? if they put both docker and dockerFile etc? (can be a future enhancement, if not)

Andrew Bayer added a comment - 2016-12-27 23:19

Yes, validation will catch those cases.

Andrew Bayer added a comment - 2016-12-27 23:19 Yes, validation will catch those cases.

Peter Leibiger added a comment - 2016-12-29 12:39

Fine by me. I am just worried that this section will become a hazzle to document if the agent specific options don't get split up.
Another thing i was thinking about was this.

agent {
  name docker
  options {
    args "-v /tmp:/tmp -p 80:80"
    label "foo"
  }
}

Peter Leibiger added a comment - 2016-12-29 12:39 Fine by me. I am just worried that this section will become a hazzle to document if the agent specific options don't get split up. Another thing i was thinking about was this. agent { name docker options { args "-v /tmp:/tmp -p 80:80" label "foo" } }

Michael Neale added a comment - 2016-12-30 00:27

pleibiger yes I can see your point, however I think validation and good samples and tooling will help. Most of the time the config is quite flat and simple... so I think this will be ok.

Michael Neale added a comment - 2016-12-30 00:27 pleibiger yes I can see your point, however I think validation and good samples and tooling will help. Most of the time the config is quite flat and simple... so I think this will be ok.

Peter Leibiger added a comment - 2016-12-30 13:07

michaelneale Sounds good.

Peter Leibiger added a comment - 2016-12-30 13:07 michaelneale Sounds good.

Andrew Bayer added a comment - 2016-12-30 17:06

Yeah, I'm gonna go with the current implementation but reserve the right to revisit this in the future, 'cos it's an area that I think we'll need to have more agent implementations to really know what's best. But for now, yeah. We're good. Just gotta make a few tweaks to the PR to properly support nested maps and we should be good to go.

Andrew Bayer added a comment - 2016-12-30 17:06 Yeah, I'm gonna go with the current implementation but reserve the right to revisit this in the future, 'cos it's an area that I think we'll need to have more agent implementations to really know what's best. But for now, yeah. We're good. Just gotta make a few tweaks to the PR to properly support nested maps and we should be good to go.

Andrew Bayer added a comment - 2016-12-30 22:42

Ouch, nested maps is gonna take some time, so I'm pulling that for now and going with the current state. We don't have a need for nested maps support right now anyway.

Andrew Bayer added a comment - 2016-12-30 22:42 Ouch, nested maps is gonna take some time, so I'm pulling that for now and going with the current state. We don't have a need for nested maps support right now anyway.

Andrew Bayer added a comment - 2017-01-06 02:32

Ok, new PR (https://github.com/jenkinsci/pipeline-model-definition-plugin/pull/85) up with a new syntax! =)

agent none

agent {
    label "foo"
}

agent {
    docker {
       nodeLabel "foo"
       image "some-image"
       args "--hi-there"
    }
 }

agent {
    dockerfile true
}
// or if you've got multiple arguments...
agent {
    dockerfile {
        dockerfile true
        args "--hi-there"
    }
}

I know it's not ideal, but I think it's better than what's there in the original PR, and it's dramatically easier than allowing specifying label, docker (block) etc together at the same level in agent. I think it solves the label confusion by clarifying it as nodeLabel, but let me know if I'm wrong. =)

Andrew Bayer added a comment - 2017-01-06 02:32 Ok, new PR ( https://github.com/jenkinsci/pipeline-model-definition-plugin/pull/85 ) up with a new syntax! =) agent none agent { label "foo" } agent { docker { nodeLabel "foo" image "some-image" args "--hi-there" } } agent { dockerfile true } // or if you've got multiple arguments... agent { dockerfile { dockerfile true args "--hi-there" } } I know it's not ideal, but I think it's better than what's there in the original PR, and it's dramatically easier than allowing specifying label , docker (block) etc together at the same level in agent . I think it solves the label confusion by clarifying it as nodeLabel , but let me know if I'm wrong. =)

Patrick Wolf added a comment - 2017-01-06 16:49

Crap. We've changed it again. I already documented the previous way and it is going to print. Let me know the final syntax so I can stop the cards being printed, abayer

Patrick Wolf added a comment - 2017-01-06 16:49 Crap. We've changed it again. I already documented the previous way and it is going to print. Let me know the final syntax so I can stop the cards being printed, abayer

SCM/JIRA link daemon added a comment - 2017-01-09 19:41

Code changed in jenkins
User: Andrew Bayer
Path:
SYNTAX.md
pipeline-model-api/src/main/java/org/jenkinsci/plugins/pipeline/modeldefinition/ast/ModelASTAgent.java
pipeline-model-api/src/main/java/org/jenkinsci/plugins/pipeline/modeldefinition/ast/ModelASTClosureMap.java
pipeline-model-api/src/main/java/org/jenkinsci/plugins/pipeline/modeldefinition/validator/ModelValidator.java
pipeline-model-api/src/main/resources/ast-schema.json
pipeline-model-definition/src/main/groovy/org/jenkinsci/plugins/pipeline/modeldefinition/model/Agent.groovy
pipeline-model-definition/src/main/groovy/org/jenkinsci/plugins/pipeline/modeldefinition/model/Root.groovy
pipeline-model-definition/src/main/groovy/org/jenkinsci/plugins/pipeline/modeldefinition/model/Stage.groovy
pipeline-model-definition/src/main/groovy/org/jenkinsci/plugins/pipeline/modeldefinition/parser/JSONParser.groovy
pipeline-model-definition/src/main/groovy/org/jenkinsci/plugins/pipeline/modeldefinition/parser/ModelParser.groovy
pipeline-model-definition/src/main/groovy/org/jenkinsci/plugins/pipeline/modeldefinition/validator/ModelValidatorImpl.groovy
pipeline-model-definition/src/main/java/org/jenkinsci/plugins/pipeline/modeldefinition/agent/impl/DockerPipeline.java
pipeline-model-definition/src/main/java/org/jenkinsci/plugins/pipeline/modeldefinition/agent/impl/DockerPipelineFromDockerfile.java
pipeline-model-definition/src/main/resources/org/jenkinsci/plugins/pipeline/modeldefinition/ClosureModelTranslator.groovy
pipeline-model-definition/src/main/resources/org/jenkinsci/plugins/pipeline/modeldefinition/Messages.properties
pipeline-model-definition/src/main/resources/org/jenkinsci/plugins/pipeline/modeldefinition/ModelInterpreter.groovy
pipeline-model-definition/src/main/resources/org/jenkinsci/plugins/pipeline/modeldefinition/agent/impl/DockerPipelineFromDockerfileScript.groovy
pipeline-model-definition/src/main/resources/org/jenkinsci/plugins/pipeline/modeldefinition/agent/impl/DockerPipelineScript.groovy
pipeline-model-definition/src/main/resources/org/jenkinsci/plugins/pipeline/modeldefinition/config/FolderConfig/config.groovy
pipeline-model-definition/src/main/resources/org/jenkinsci/plugins/pipeline/modeldefinition/config/FolderConfig/help-dockerLabel.html
pipeline-model-definition/src/main/resources/org/jenkinsci/plugins/pipeline/modeldefinition/config/GlobalConfig/config.groovy
pipeline-model-definition/src/main/resources/org/jenkinsci/plugins/pipeline/modeldefinition/config/GlobalConfig/help-dockerLabel.html
pipeline-model-definition/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/AbstractModelDefTest.java
pipeline-model-definition/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/AgentTest.java
pipeline-model-definition/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/ValidatorTest.java
pipeline-model-definition/src/test/resources/agentDocker.groovy
pipeline-model-definition/src/test/resources/agentDockerEnvSpecLabel.groovy
pipeline-model-definition/src/test/resources/agentDockerEnvTest.groovy
pipeline-model-definition/src/test/resources/agentDockerWithEmptyDockerArgs.groovy
pipeline-model-definition/src/test/resources/agentDockerWithNullDockerArgs.groovy
pipeline-model-definition/src/test/resources/agentLabel.groovy
pipeline-model-definition/src/test/resources/agentTypeOrdering.groovy
pipeline-model-definition/src/test/resources/buildPluginParentPOM.groovy
pipeline-model-definition/src/test/resources/dockerGlobalVariable.groovy
pipeline-model-definition/src/test/resources/dockerGlobalVariableInScript.groovy
pipeline-model-definition/src/test/resources/environmentInStage.groovy
pipeline-model-definition/src/test/resources/errors/agentMissingRequiredParam.groovy
pipeline-model-definition/src/test/resources/errors/agentUnknownParamForType.groovy
pipeline-model-definition/src/test/resources/errors/duplicateEnvironment.groovy
pipeline-model-definition/src/test/resources/errors/emptyEnvironment.groovy
pipeline-model-definition/src/test/resources/errors/globalLibraryNonStepBody.groovy
pipeline-model-definition/src/test/resources/errors/globalLibraryObjectMethodCall.groovy
pipeline-model-definition/src/test/resources/errors/invalidMetaStepSyntax.groovy
pipeline-model-definition/src/test/resources/errors/notInstalledToolVersion.groovy
pipeline-model-definition/src/test/resources/errors/perStageConfigEmptySteps.groovy
pipeline-model-definition/src/test/resources/errors/perStageConfigMissingSteps.groovy
pipeline-model-definition/src/test/resources/errors/perStageConfigUnknownSection.groovy
pipeline-model-definition/src/test/resources/errors/unknownAgentType.groovy
pipeline-model-definition/src/test/resources/errors/unlistedToolType.groovy
pipeline-model-definition/src/test/resources/failureBeforeStages.groovy
pipeline-model-definition/src/test/resources/fromAlternateDockerfile.groovy
pipeline-model-definition/src/test/resources/fromDockerfile.groovy
pipeline-model-definition/src/test/resources/globalLibrarySuccess.groovy
pipeline-model-definition/src/test/resources/globalLibrarySuccessInScript.groovy
pipeline-model-definition/src/test/resources/json/agentAny.json
pipeline-model-definition/src/test/resources/json/agentDocker.json
pipeline-model-definition/src/test/resources/json/agentLabel.json
pipeline-model-definition/src/test/resources/json/agentNoneWithNode.json
pipeline-model-definition/src/test/resources/json/agentTypeOrdering.json
pipeline-model-definition/src/test/resources/json/basicWhen.json
pipeline-model-definition/src/test/resources/json/environmentInStage.json
pipeline-model-definition/src/test/resources/json/errors/agentMissingRequiredParam.json
pipeline-model-definition/src/test/resources/json/errors/agentUnknownParamForType.json
pipeline-model-definition/src/test/resources/json/errors/emptyEnvironment.json
pipeline-model-definition/src/test/resources/json/errors/emptyJobProperties.json
pipeline-model-definition/src/test/resources/json/errors/emptyParallel.json
pipeline-model-definition/src/test/resources/json/errors/emptyParameters.json
pipeline-model-definition/src/test/resources/json/errors/emptyPostBuild.json
pipeline-model-definition/src/test/resources/json/errors/emptyStages.json
pipeline-model-definition/src/test/resources/json/errors/emptyTriggers.json
pipeline-model-definition/src/test/resources/json/errors/invalidBuildCondition.json
pipeline-model-definition/src/test/resources/json/errors/invalidParameterTypeMethodCall.json
pipeline-model-definition/src/test/resources/json/errors/invalidWrapperType.json
pipeline-model-definition/src/test/resources/json/errors/malformed.json
pipeline-model-definition/src/test/resources/json/errors/missingAgent.json
pipeline-model-definition/src/test/resources/json/errors/missingRequiredMethodCallArg.json
pipeline-model-definition/src/test/resources/json/errors/missingRequiredStepParameters.json
pipeline-model-definition/src/test/resources/json/errors/missingStages.json
pipeline-model-definition/src/test/resources/json/errors/mixedMethodArgs.json
pipeline-model-definition/src/test/resources/json/errors/notInstalledToolType.json
pipeline-model-definition/src/test/resources/json/errors/notInstalledToolVersion.json
pipeline-model-definition/src/test/resources/json/errors/notificationsSectionRemoved.json
pipeline-model-definition/src/test/resources/json/errors/perStageConfigEmptySteps.json
pipeline-model-definition/src/test/resources/json/errors/perStageConfigMissingSteps.json
pipeline-model-definition/src/test/resources/json/errors/perStageConfigUnknownSection.json
pipeline-model-definition/src/test/resources/json/errors/rejectParallelMixedInSteps.json
pipeline-model-definition/src/test/resources/json/errors/rejectPropertiesStepInMethodCall.json
pipeline-model-definition/src/test/resources/json/errors/rejectStageInSteps.json
pipeline-model-definition/src/test/resources/json/errors/stageWithoutName.json
pipeline-model-definition/src/test/resources/json/errors/unknownAgentType.json
pipeline-model-definition/src/test/resources/json/errors/unknownBareAgentType.json
pipeline-model-definition/src/test/resources/json/errors/unknownStepParameter.json
pipeline-model-definition/src/test/resources/json/errors/unlistedToolType.json
pipeline-model-definition/src/test/resources/json/errors/wrongParameterNameMethodCall.json
pipeline-model-definition/src/test/resources/json/globalLibrarySuccess.json
pipeline-model-definition/src/test/resources/json/legacyMetaStepSyntax.json
pipeline-model-definition/src/test/resources/json/metaStepSyntax.json
pipeline-model-definition/src/test/resources/json/multipleVariablesForAgent.json
pipeline-model-definition/src/test/resources/json/multipleWrappers.json
pipeline-model-definition/src/test/resources/json/parallelPipeline.json
pipeline-model-definition/src/test/resources/json/parallelPipelineQuoteEscaping.json
pipeline-model-definition/src/test/resources/json/parallelPipelineWithFailFast.json
pipeline-model-definition/src/test/resources/json/parallelPipelineWithSpaceInBranch.json
pipeline-model-definition/src/test/resources/json/perStageConfigAgent.json
pipeline-model-definition/src/test/resources/json/simpleEnvironment.json
pipeline-model-definition/src/test/resources/json/simpleJobProperties.json
pipeline-model-definition/src/test/resources/json/simpleParameters.json
pipeline-model-definition/src/test/resources/json/simplePipeline.json
pipeline-model-definition/src/test/resources/json/simplePostBuild.json
pipeline-model-definition/src/test/resources/json/simpleScript.json
pipeline-model-definition/src/test/resources/json/simpleTools.json
pipeline-model-definition/src/test/resources/json/simpleTriggers.json
pipeline-model-definition/src/test/resources/json/simpleWrapper.json
pipeline-model-definition/src/test/resources/json/skippedWhen.json
pipeline-model-definition/src/test/resources/json/steps/simpleScript.json
pipeline-model-definition/src/test/resources/json/stringsNeedingEscapeLogic.json
pipeline-model-definition/src/test/resources/json/toolsInStage.json
pipeline-model-definition/src/test/resources/json/twoStagePipeline.json
pipeline-model-definition/src/test/resources/json/validStepParameters.json
pipeline-model-definition/src/test/resources/legacyMetaStepSyntax.groovy
pipeline-model-definition/src/test/resources/metaStepSyntax.groovy
pipeline-model-definition/src/test/resources/multipleVariablesForAgent.groovy
pipeline-model-definition/src/test/resources/noCheckoutScmInWrongContext.groovy
pipeline-model-definition/src/test/resources/nonLiteralEnvironment.groovy
pipeline-model-definition/src/test/resources/perStageConfigAgent.groovy
pipeline-model-definition/src/test/resources/postStage/localAll.groovy
pipeline-model-definition/src/test/resources/simpleEnvironment.groovy
pipeline-model-definition/src/test/resources/simpleTools.groovy
pipeline-model-definition/src/test/resources/toolsInStage.groovy
pipeline-model-definition/src/test/resources/when/simpleWhen.groovy
pipeline-model-definition/src/test/resources/when/whenEmpty.groovy
pipeline-model-definition/src/test/resources/when/whenException.groovy
http://jenkins-ci.org/commit/pipeline-model-definition-plugin/68f711c650b518296b154a9590d933ab251875fd
Log:
[FIXED JENKINS-40524] Reworked agent syntax to be more extensible.

Moves agent to a block (except for any and none) and moves any agent
type with more than one argument into a nested block as well. For example:

```groovy
agent {
label "foo"
}

agent {
docker

{ nodeLabel "foo" image "some-image" args "--hi-there" }

}
```

Also renamed arguments for docker and dockerfile - label to nodeLabel,
dockerArgs to args, docker to image. Still thinking about how to
handle dockerfile more elegantly.

This also simplifies the JSON model considerably.

SCM/JIRA link daemon added a comment - 2017-01-09 19:41 Code changed in jenkins User: Andrew Bayer Path: SYNTAX.md pipeline-model-api/src/main/java/org/jenkinsci/plugins/pipeline/modeldefinition/ast/ModelASTAgent.java pipeline-model-api/src/main/java/org/jenkinsci/plugins/pipeline/modeldefinition/ast/ModelASTClosureMap.java pipeline-model-api/src/main/java/org/jenkinsci/plugins/pipeline/modeldefinition/validator/ModelValidator.java pipeline-model-api/src/main/resources/ast-schema.json pipeline-model-definition/src/main/groovy/org/jenkinsci/plugins/pipeline/modeldefinition/model/Agent.groovy pipeline-model-definition/src/main/groovy/org/jenkinsci/plugins/pipeline/modeldefinition/model/Root.groovy pipeline-model-definition/src/main/groovy/org/jenkinsci/plugins/pipeline/modeldefinition/model/Stage.groovy pipeline-model-definition/src/main/groovy/org/jenkinsci/plugins/pipeline/modeldefinition/parser/JSONParser.groovy pipeline-model-definition/src/main/groovy/org/jenkinsci/plugins/pipeline/modeldefinition/parser/ModelParser.groovy pipeline-model-definition/src/main/groovy/org/jenkinsci/plugins/pipeline/modeldefinition/validator/ModelValidatorImpl.groovy pipeline-model-definition/src/main/java/org/jenkinsci/plugins/pipeline/modeldefinition/agent/impl/DockerPipeline.java pipeline-model-definition/src/main/java/org/jenkinsci/plugins/pipeline/modeldefinition/agent/impl/DockerPipelineFromDockerfile.java pipeline-model-definition/src/main/resources/org/jenkinsci/plugins/pipeline/modeldefinition/ClosureModelTranslator.groovy pipeline-model-definition/src/main/resources/org/jenkinsci/plugins/pipeline/modeldefinition/Messages.properties pipeline-model-definition/src/main/resources/org/jenkinsci/plugins/pipeline/modeldefinition/ModelInterpreter.groovy pipeline-model-definition/src/main/resources/org/jenkinsci/plugins/pipeline/modeldefinition/agent/impl/DockerPipelineFromDockerfileScript.groovy pipeline-model-definition/src/main/resources/org/jenkinsci/plugins/pipeline/modeldefinition/agent/impl/DockerPipelineScript.groovy pipeline-model-definition/src/main/resources/org/jenkinsci/plugins/pipeline/modeldefinition/config/FolderConfig/config.groovy pipeline-model-definition/src/main/resources/org/jenkinsci/plugins/pipeline/modeldefinition/config/FolderConfig/help-dockerLabel.html pipeline-model-definition/src/main/resources/org/jenkinsci/plugins/pipeline/modeldefinition/config/GlobalConfig/config.groovy pipeline-model-definition/src/main/resources/org/jenkinsci/plugins/pipeline/modeldefinition/config/GlobalConfig/help-dockerLabel.html pipeline-model-definition/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/AbstractModelDefTest.java pipeline-model-definition/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/AgentTest.java pipeline-model-definition/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/ValidatorTest.java pipeline-model-definition/src/test/resources/agentDocker.groovy pipeline-model-definition/src/test/resources/agentDockerEnvSpecLabel.groovy pipeline-model-definition/src/test/resources/agentDockerEnvTest.groovy pipeline-model-definition/src/test/resources/agentDockerWithEmptyDockerArgs.groovy pipeline-model-definition/src/test/resources/agentDockerWithNullDockerArgs.groovy pipeline-model-definition/src/test/resources/agentLabel.groovy pipeline-model-definition/src/test/resources/agentTypeOrdering.groovy pipeline-model-definition/src/test/resources/buildPluginParentPOM.groovy pipeline-model-definition/src/test/resources/dockerGlobalVariable.groovy pipeline-model-definition/src/test/resources/dockerGlobalVariableInScript.groovy pipeline-model-definition/src/test/resources/environmentInStage.groovy pipeline-model-definition/src/test/resources/errors/agentMissingRequiredParam.groovy pipeline-model-definition/src/test/resources/errors/agentUnknownParamForType.groovy pipeline-model-definition/src/test/resources/errors/duplicateEnvironment.groovy pipeline-model-definition/src/test/resources/errors/emptyEnvironment.groovy pipeline-model-definition/src/test/resources/errors/globalLibraryNonStepBody.groovy pipeline-model-definition/src/test/resources/errors/globalLibraryObjectMethodCall.groovy pipeline-model-definition/src/test/resources/errors/invalidMetaStepSyntax.groovy pipeline-model-definition/src/test/resources/errors/notInstalledToolVersion.groovy pipeline-model-definition/src/test/resources/errors/perStageConfigEmptySteps.groovy pipeline-model-definition/src/test/resources/errors/perStageConfigMissingSteps.groovy pipeline-model-definition/src/test/resources/errors/perStageConfigUnknownSection.groovy pipeline-model-definition/src/test/resources/errors/unknownAgentType.groovy pipeline-model-definition/src/test/resources/errors/unlistedToolType.groovy pipeline-model-definition/src/test/resources/failureBeforeStages.groovy pipeline-model-definition/src/test/resources/fromAlternateDockerfile.groovy pipeline-model-definition/src/test/resources/fromDockerfile.groovy pipeline-model-definition/src/test/resources/globalLibrarySuccess.groovy pipeline-model-definition/src/test/resources/globalLibrarySuccessInScript.groovy pipeline-model-definition/src/test/resources/json/agentAny.json pipeline-model-definition/src/test/resources/json/agentDocker.json pipeline-model-definition/src/test/resources/json/agentLabel.json pipeline-model-definition/src/test/resources/json/agentNoneWithNode.json pipeline-model-definition/src/test/resources/json/agentTypeOrdering.json pipeline-model-definition/src/test/resources/json/basicWhen.json pipeline-model-definition/src/test/resources/json/environmentInStage.json pipeline-model-definition/src/test/resources/json/errors/agentMissingRequiredParam.json pipeline-model-definition/src/test/resources/json/errors/agentUnknownParamForType.json pipeline-model-definition/src/test/resources/json/errors/emptyEnvironment.json pipeline-model-definition/src/test/resources/json/errors/emptyJobProperties.json pipeline-model-definition/src/test/resources/json/errors/emptyParallel.json pipeline-model-definition/src/test/resources/json/errors/emptyParameters.json pipeline-model-definition/src/test/resources/json/errors/emptyPostBuild.json pipeline-model-definition/src/test/resources/json/errors/emptyStages.json pipeline-model-definition/src/test/resources/json/errors/emptyTriggers.json pipeline-model-definition/src/test/resources/json/errors/invalidBuildCondition.json pipeline-model-definition/src/test/resources/json/errors/invalidParameterTypeMethodCall.json pipeline-model-definition/src/test/resources/json/errors/invalidWrapperType.json pipeline-model-definition/src/test/resources/json/errors/malformed.json pipeline-model-definition/src/test/resources/json/errors/missingAgent.json pipeline-model-definition/src/test/resources/json/errors/missingRequiredMethodCallArg.json pipeline-model-definition/src/test/resources/json/errors/missingRequiredStepParameters.json pipeline-model-definition/src/test/resources/json/errors/missingStages.json pipeline-model-definition/src/test/resources/json/errors/mixedMethodArgs.json pipeline-model-definition/src/test/resources/json/errors/notInstalledToolType.json pipeline-model-definition/src/test/resources/json/errors/notInstalledToolVersion.json pipeline-model-definition/src/test/resources/json/errors/notificationsSectionRemoved.json pipeline-model-definition/src/test/resources/json/errors/perStageConfigEmptySteps.json pipeline-model-definition/src/test/resources/json/errors/perStageConfigMissingSteps.json pipeline-model-definition/src/test/resources/json/errors/perStageConfigUnknownSection.json pipeline-model-definition/src/test/resources/json/errors/rejectParallelMixedInSteps.json pipeline-model-definition/src/test/resources/json/errors/rejectPropertiesStepInMethodCall.json pipeline-model-definition/src/test/resources/json/errors/rejectStageInSteps.json pipeline-model-definition/src/test/resources/json/errors/stageWithoutName.json pipeline-model-definition/src/test/resources/json/errors/unknownAgentType.json pipeline-model-definition/src/test/resources/json/errors/unknownBareAgentType.json pipeline-model-definition/src/test/resources/json/errors/unknownStepParameter.json pipeline-model-definition/src/test/resources/json/errors/unlistedToolType.json pipeline-model-definition/src/test/resources/json/errors/wrongParameterNameMethodCall.json pipeline-model-definition/src/test/resources/json/globalLibrarySuccess.json pipeline-model-definition/src/test/resources/json/legacyMetaStepSyntax.json pipeline-model-definition/src/test/resources/json/metaStepSyntax.json pipeline-model-definition/src/test/resources/json/multipleVariablesForAgent.json pipeline-model-definition/src/test/resources/json/multipleWrappers.json pipeline-model-definition/src/test/resources/json/parallelPipeline.json pipeline-model-definition/src/test/resources/json/parallelPipelineQuoteEscaping.json pipeline-model-definition/src/test/resources/json/parallelPipelineWithFailFast.json pipeline-model-definition/src/test/resources/json/parallelPipelineWithSpaceInBranch.json pipeline-model-definition/src/test/resources/json/perStageConfigAgent.json pipeline-model-definition/src/test/resources/json/simpleEnvironment.json pipeline-model-definition/src/test/resources/json/simpleJobProperties.json pipeline-model-definition/src/test/resources/json/simpleParameters.json pipeline-model-definition/src/test/resources/json/simplePipeline.json pipeline-model-definition/src/test/resources/json/simplePostBuild.json pipeline-model-definition/src/test/resources/json/simpleScript.json pipeline-model-definition/src/test/resources/json/simpleTools.json pipeline-model-definition/src/test/resources/json/simpleTriggers.json pipeline-model-definition/src/test/resources/json/simpleWrapper.json pipeline-model-definition/src/test/resources/json/skippedWhen.json pipeline-model-definition/src/test/resources/json/steps/simpleScript.json pipeline-model-definition/src/test/resources/json/stringsNeedingEscapeLogic.json pipeline-model-definition/src/test/resources/json/toolsInStage.json pipeline-model-definition/src/test/resources/json/twoStagePipeline.json pipeline-model-definition/src/test/resources/json/validStepParameters.json pipeline-model-definition/src/test/resources/legacyMetaStepSyntax.groovy pipeline-model-definition/src/test/resources/metaStepSyntax.groovy pipeline-model-definition/src/test/resources/multipleVariablesForAgent.groovy pipeline-model-definition/src/test/resources/noCheckoutScmInWrongContext.groovy pipeline-model-definition/src/test/resources/nonLiteralEnvironment.groovy pipeline-model-definition/src/test/resources/perStageConfigAgent.groovy pipeline-model-definition/src/test/resources/postStage/localAll.groovy pipeline-model-definition/src/test/resources/simpleEnvironment.groovy pipeline-model-definition/src/test/resources/simpleTools.groovy pipeline-model-definition/src/test/resources/toolsInStage.groovy pipeline-model-definition/src/test/resources/when/simpleWhen.groovy pipeline-model-definition/src/test/resources/when/whenEmpty.groovy pipeline-model-definition/src/test/resources/when/whenException.groovy http://jenkins-ci.org/commit/pipeline-model-definition-plugin/68f711c650b518296b154a9590d933ab251875fd Log: [FIXED JENKINS-40524] Reworked agent syntax to be more extensible. Moves agent to a block (except for any and none) and moves any agent type with more than one argument into a nested block as well. For example: ```groovy agent { label "foo" } agent { docker { nodeLabel "foo" image "some-image" args "--hi-there" } } ``` Also renamed arguments for docker and dockerfile - label to nodeLabel, dockerArgs to args, docker to image. Still thinking about how to handle dockerfile more elegantly. This also simplifies the JSON model considerably.

Liam Newman added a comment - 2019-10-23 03:21

Bulk closing resolved issues.

Liam Newman added a comment - 2019-10-23 03:21 Bulk closing resolved issues.

Jenkins

Details

Description

Attachments

Issue Links

Activity

Collapse comment: Andrew Bayer added a comment - 2016-12-17 21:47

Expand comment: Andrew Bayer added a comment - 2016-12-17 21:47

Collapse comment: Andrew Bayer added a comment - 2016-12-20 17:12

Expand comment: Andrew Bayer added a comment - 2016-12-20 17:12

Collapse comment: Logan Glickfield added a comment - 2016-12-20 17:21

Expand comment: Logan Glickfield added a comment - 2016-12-20 17:21

Collapse comment: Patrick Wolf added a comment - 2016-12-20 17:26

Expand comment: Patrick Wolf added a comment - 2016-12-20 17:26

Collapse comment: Andrew Bayer added a comment - 2016-12-20 17:35

Expand comment: Andrew Bayer added a comment - 2016-12-20 17:35

Collapse comment: Patrick Wolf added a comment - 2016-12-20 17:37

Expand comment: Patrick Wolf added a comment - 2016-12-20 17:37

Collapse comment: Peter Leibiger added a comment - 2016-12-21 12:31, Edited by Peter Leibiger - 2016-12-21 12:40

Expand comment: Peter Leibiger added a comment - 2016-12-21 12:31, Edited by Peter Leibiger - 2016-12-21 12:40

Collapse comment: Peter Leibiger added a comment - 2016-12-21 12:39

Expand comment: Peter Leibiger added a comment - 2016-12-21 12:39

Collapse comment: Andrew Bayer added a comment - 2016-12-27 15:48

Expand comment: Andrew Bayer added a comment - 2016-12-27 15:48

Collapse comment: Patrick Wolf added a comment - 2016-12-27 22:17

Expand comment: Patrick Wolf added a comment - 2016-12-27 22:17

Collapse comment: Michael Neale added a comment - 2016-12-27 22:47

Expand comment: Michael Neale added a comment - 2016-12-27 22:47

Collapse comment: Andrew Bayer added a comment - 2016-12-27 23:19

Expand comment: Andrew Bayer added a comment - 2016-12-27 23:19

Collapse comment: Peter Leibiger added a comment - 2016-12-29 12:39

Expand comment: Peter Leibiger added a comment - 2016-12-29 12:39

Collapse comment: Michael Neale added a comment - 2016-12-30 00:27

Expand comment: Michael Neale added a comment - 2016-12-30 00:27

Collapse comment: Peter Leibiger added a comment - 2016-12-30 13:07

Expand comment: Peter Leibiger added a comment - 2016-12-30 13:07

Collapse comment: Andrew Bayer added a comment - 2016-12-30 17:06

Expand comment: Andrew Bayer added a comment - 2016-12-30 17:06

Collapse comment: Andrew Bayer added a comment - 2016-12-30 22:42

Expand comment: Andrew Bayer added a comment - 2016-12-30 22:42

Collapse comment: Andrew Bayer added a comment - 2017-01-06 02:32

Expand comment: Andrew Bayer added a comment - 2017-01-06 02:32

Collapse comment: Patrick Wolf added a comment - 2017-01-06 16:49

Expand comment: Patrick Wolf added a comment - 2017-01-06 16:49

Collapse comment: SCM/JIRA link daemon added a comment - 2017-01-09 19:41

Expand comment: SCM/JIRA link daemon added a comment - 2017-01-09 19:41

Collapse comment: Liam Newman added a comment - 2019-10-23 03:21

Expand comment: Liam Newman added a comment - 2019-10-23 03:21

People

Dates