allow a PodTemplate to be configured by name in declarative pipelines #260

jstrachan · 2017-12-02T09:38:56Z

allows either a nested podTemplate or the use of inheritFrom in declarative pipelines to specify a PodTemplate
includes lots of great feedback from @carlossg
fixes JENKINS-48140

cc @carlossg @shaiazulay

@carlossg

allows either a nested `podTemplate` or the use of `inheritFrom` in declarative pipelines to specify a `PodTemplate` includes lots of great feedback from @carlossg fixes JENKINS-48140

iocanel

For selecting a podTemplate I think that we should stick to using the jenkins label, which is something that works globally regardless of cloud type, agent type etc.

Now, I do see value in being able to define or extend a podTemplate via declerative pipelines. Not sure to what degree it was already possible, so I'll just comment on how I think it should be.

iocanel · 2017-12-02T17:28:00Z

README.md

@@ -266,6 +266,29 @@ Declarative Pipeline support requires Jenkins 2.66+

 Example at [examples/declarative.groovy](examples/declarative.groovy)

+### Referring to PodTemplates
+
+You can refer to a named PodTemplate in the Jenkins configuration via the **inheritFrom** property as follows:


You can already select via declerative pipelines a pod template by label.

iocanel · 2017-12-02T17:30:03Z

README.md

+The meaning of the values is as follows:
+
+* **label** is used to add a kubernetes label to the build pods created by the pipeline jobs
+* **inheritFrom**  is used to lookup a named PodTemplate in the Jenkins configuration for the Kubernetes Cloud and uses that as the basis for the PodTemplate for this pipeline


So, we are practically defining here a new pod template right?

iocanel · 2017-12-02T17:31:25Z

src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloud.java

@@ -71,6 +71,8 @@
 public class KubernetesCloud extends Cloud {
    public static final int DEFAULT_MAX_REQUESTS_PER_HOST = 32;

+    public static final String DEFAULT_CLOUD_NAME = "kubernetes";


I think that this constant is already present somewhere in the code base.

it was @carlossg idea to move it here - I couldn't find any other constant/string in the main codebase I could reuse though

Yeah, it is in: https://github.com/jenkinsci/kubernetes-plugin/blob/master/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/PodTemplateStep.java

and yeah, makes more sense to move it in KubernetesCloud.

iocanel · 2017-12-02T17:34:29Z

src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/KubernetesDeclarativeAgent.java

        return argMap;
    }

+    protected PodTemplate findPodTemplate(String inheritFrom) {


Dublicate code: See https://github.com/jenkinsci/kubernetes-plugin/blob/master/src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateUtils.java#L209

iocanel · 2017-12-02T17:55:01Z

README.md

+
+The meaning of the values is as follows:
+
+* **label** is used to add a kubernetes label to the build pods created by the pipeline jobs


This confuses me. We already have the jenkins label present on the podTemplate.

If we are to be able to specify labels on the pod I'd like to use a proper map (if possible) instead of a single string value.

So I guess I'm now confused why there are label and inheritFrom properties on the KubernetesDeclarativeAgent - I figured label was so that you could override the label used?

I think one thing that confuses me is terminology. What I understand with kubernetes labels is the labels present in the metadata of the kubernetes resource. For the labels used in order to select nodes I usually go with jenkins labels.

The other thing that confuses me, is that label and inheritFrom seem to be usable both in and outside of the podTemplate.

From what I understand there used to be no notion of podTemplate under kubernetes and there were just raw key/value pairs (subset of the actual podTemplate).

I really like the idea of being able to have full control of the podTemplate from declerative pipelines. So +1 on this one. But we need to make it a bit more clear on how things work. To be fair this has to do more with how it already works, and less with what this pr brings in.

iocanel · 2017-12-02T17:59:40Z

...test/resources/org/csanchez/jenkins/plugins/kubernetes/pipeline/declarativeInheritPod.groovy

@@ -0,0 +1,21 @@
+pipeline {
+  agent {


This works already and seems cleaner and less verbose to me.

agent { label 'maven' }

whoah! I had no idea that'd work!!! I just tried it out and it seems to work!!! Many thanks!

iocanel · 2017-12-02T18:01:05Z

src/test/resources/org/csanchez/jenkins/plugins/kubernetes/pipeline/declarativePod.groovy

+  agent {
+    kubernetes {
+      label 'mypod'
+      podTemplate {


This makes sense to me, but I'd like the label to be under podTemplate to reflect the actual structure of the PodTemplate.

iocanel · 2017-12-02T18:01:55Z

...ources/org/csanchez/jenkins/plugins/kubernetes/pipeline/declarativePodWithInheritFrom.groovy

+    kubernetes {
+      label 'mypod'
+      podTemplate {
+        inheritFrom 'maven'


+1: Having inheritFrom like this (under podTemplate) makes much more sense!

So I guess we need to decide should there be a label under agent and kubernetes and if so why and do they have different semantics?

Also should there be inheritFrom under kubernetes and podTemplate? All these permutations are a little confusing

FWIW my 2 main use cases are:

refer to a podTemplate by name/label

inheritFrom a named podTemplate and change something (e.g. the maven containerTemplate's image version)

I guess for (1) we can just use:

agent { label "nameOfPodTemplate" }

then for (2) we maybe go with something like this?

agent { kubernetes { podTemplate { inheritFrom 'maven' containerTemplate { name "maven" image "mycustom-maven-image"

+1 I totally agree on how these two use cases are handled.

Please note that on (1) its labelOfPodTemplate and not the name.

I guess its clear there's some confusion though over the current properties; so might be worth a bit of review and maybe some deprecation?

I guess its clear that the use of label just refers to an existing podTemplate without any modification and inheritFrom always creates a new podTemplate by combining inline customisation with an existing podTemplate. Its just a tad confusing that there are so many places you can use label and inheritFrom.

Maybe we deprecate kubernetes.label and kubernetes.inheritFrom? Then you either use agent.label to use a pod template via label or agent.kubernetes.podTemplate.inheritFrom to create a new PodTemplate (and maybe override some configurations)?

So I can't remember why kubernetes.label exists. It's been a while. =) But agent { label 'foo' } will just try to run on an agent with the label foo, regardless of how that agent is created/provisioned/etc. So if you have the Kubernetes cloud set up on the Jenkins master with a pod template defined with the label foo, an agent will get spun up and used for that Pipeline. But if you have, say, an SSH agent set up with the label foo and it's got a free executor, that'll get used before it tries to provision any new agents.

And yeah, if the current functionality of kubernetes { label ... } is just to define a label on the podTemplate that gets created behind the scenes, I'd deprecate it in favor of the better podTemplate configuration options.

carlossg · 2017-12-04T08:05:31Z

ping @abayer if you could review

more background discussion in this PR jenkinsci#260

timwebster9 · 2017-12-17T15:26:09Z

Any news on this? Eagerly awaiting this PR to be merged... :-)

carlossg · 2017-12-26T13:06:17Z

So @jstrachan @iocanel is this ready to be reviewed or still WiP ? what's the final syntax?

timwebster9 · 2018-01-02T18:02:45Z

Just had a thought - will this allow us to declare a different default JNLP container in our containerTemplate configuration in the Jenkinsfile?

novitoll · 2018-02-07T09:08:04Z

What is the status of this PR?

Currently, only last container template is created, e.g. multiple containers are not working with declarative syntax now

agent {
        kubernetes {
            label 'jenkins-slave'
            cloud 'myK8s'
            containerTemplate(..)
            containerTemplate(..)

abayer · 2018-02-12T16:35:15Z

@jstrachan So I've been thinking on this sort of thing - defining templates at the top level agent and then being able to specify the particular template to use in stage-level agents (with an optional default at the top level as well), and adding support for sidecar containers as well. I don't yet know what the syntax feels like yet, but I'm hoping to have a sense of that later this week. I'm thinking about it more generally than just this plugin - i.e., new agent syntax/types that can be extended for a particular implementation like Kubernetes. If you give me a week or two, I should have a draft replacement incorporating the guts of this PR with the new Declarative syntax.

fredericrous · 2018-02-23T09:20:34Z

Hello, I went full declarative and would very much like the podtemplate configuration to be available.
In the mid time, can I configure it from jenkins system configuration page and call the pod template from declarative pipeline by name?

I configured the kubernetes pod in jenkins configuration page to reflect the below non declarative code

    podTemplate(
        name: 'mypod',
        label: uniqueId, 
        containers: [
            containerTemplate(
                name: 'jnlp', 
                image: 'custom.corp:51021/production/jnlp:3.7-1-alpine',
                args: '${computer.jnlpmac} ${computer.name}'
            ),
            containerTemplate (
                name: 'container-exec', 
                image: 'custom.corp:51021/production/mycontainer',
                command: '/usr/bin/tail -f /dev/null',
            )
        ]
    )

Could I call the pod in jenkins with something like

pipeline {
  agent {
    kubernetes {
            containerTemplate 'mypod'
    }
...
  }
}

The code above doesn't work. I tried different things without success.
If this is not possible. I'm not asking for a new feature, but for a workaround if you have any idea =)
Cheers

colemickens · 2018-03-12T22:30:51Z

What's the status of this? Also, why is the implementation so complicated? Why is it more than just deleting "containerTemplate" and replacing it with "podTemplate" and just setting that single field in the "getArgs" function? Or is it this complexity so that it is backward compatible as well?

jkinred · 2018-03-20T09:14:03Z

There are many of us using declarative that are keen to see this feature land, it looks close!

carlossg · 2018-04-06T19:29:19Z

This is now sort of superseded by #306, it allows defining multicontainer pods in declarative
Although it does not allow picking a predefined template by name I believe the original reason for this PR was using multi-container pods

allow a PodTemplate to be configured by name in declarative pipelines

a8d616b

allows either a nested `podTemplate` or the use of `inheritFrom` in declarative pipelines to specify a `PodTemplate` includes lots of great feedback from @carlossg fixes JENKINS-48140

jstrachan mentioned this pull request Dec 2, 2017

allow a podTemplate to be configured by name in declarative pipelines #253

Closed

iocanel requested changes Dec 2, 2017

View reviewed changes

jstrachan added a commit to jstrachan/kubernetes-plugin that referenced this pull request Dec 4, 2017

add docs on using agent.label for declarative

ffb33d8

more background discussion in this PR jenkinsci#260

carlossg mentioned this pull request Apr 6, 2018

[JENKINS-48135] Support multiple containers in declarative pipeline #306

Merged

carlossg closed this May 23, 2018


		The meaning of the values is as follows:

		* label is used to add a kubernetes label to the build pods created by the pipeline jobs

allow a PodTemplate to be configured by name in declarative pipelines #260

allow a PodTemplate to be configured by name in declarative pipelines #260

Uh oh!

Conversation

jstrachan commented Dec 2, 2017

Uh oh!

iocanel left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jstrachan Dec 4, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jstrachan Dec 4, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

carlossg commented Dec 4, 2017

Uh oh!

timwebster9 commented Dec 17, 2017

Uh oh!

carlossg commented Dec 26, 2017

Uh oh!

timwebster9 commented Jan 2, 2018

Uh oh!

novitoll commented Feb 7, 2018

Uh oh!

abayer commented Feb 12, 2018

Uh oh!

fredericrous commented Feb 23, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

colemickens commented Mar 12, 2018

Uh oh!

jkinred commented Mar 20, 2018

Uh oh!

carlossg commented Apr 6, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

jstrachan Dec 4, 2017 •

edited

Loading

jstrachan Dec 4, 2017 •

edited

Loading

fredericrous commented Feb 23, 2018 •

edited

Loading

carlossg commented Apr 6, 2018 •

edited

Loading