New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve the inheritFrom functionality to better cover containers and volumes. #84

Merged
merged 5 commits into from Nov 29, 2016

Conversation

Projects
None yet
2 participants
@iocanel

iocanel commented Oct 31, 2016

This pull request allows a PodTemplate to define a parent template that will be used as a base.

This allows the user to remove repeating/boilerplate code from pipeline and move it in the plugin configuration.

The main benefits are reusability (e.g. don't add the jnlp container everywhere), readability, and more importantly decoupling the pipelines from specific configurations, that might change between environments.

For example: There are environments that may or may not allow the use of hostPath mounts, privileged containers etc. So instead of maintaining two different sets of Jenkinsfile inside a project one could just move those environment sensitive details in the parent pod, that could even be defined inside the jenkins docker image.

@iocanel

This comment has been minimized.

Show comment
Hide comment
@iocanel

iocanel Oct 31, 2016

Apparently, this is dublicating existing code....

iocanel commented Oct 31, 2016

Apparently, this is dublicating existing code....

@iocanel iocanel changed the title from Allow a pod template to "extend" an existing one. to Improve the inheritFrom functionality to better cover containers and volumes. Nov 1, 2016

@carlossg

Added some comments and needs rebase

Show outdated Hide outdated src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloud.java
@@ -85,6 +88,12 @@
private static final String DEFAULT_ID = "jenkins/slave-default";
private static final String WORKSPACE_VOLUME_NAME = "workspace-volume";
private static final String JNLP_NAME = "jnlp";
private static final String DEFAULT_JNLP_ARGUMETS = "${computer.jnlpmac} ${computer.name}";

This comment has been minimized.

@carlossg

carlossg Nov 8, 2016

typo ARGUMETS

@carlossg

carlossg Nov 8, 2016

typo ARGUMETS

@@ -13,6 +13,10 @@
<f:textbox/>
</f:entry>
<f:entry field="inheritFrom" title="The name of the pod template to inherit from">

This comment has been minimized.

@carlossg

carlossg Nov 8, 2016

I think this needs some help text in help-inheritFrom.html, and some docs in the README

@carlossg

carlossg Nov 8, 2016

I think this needs some help text in help-inheritFrom.html, and some docs in the README

@iocanel

This comment has been minimized.

Show comment
Hide comment
@iocanel

iocanel Nov 8, 2016

@carlossg: I will apply your comments, rebase and squash.

Since, I added this pull request I've been playing a lot with this feature and I am starting to think that it would make sense to be able to inheritFrom multiple parents. So that I can easily compose a pod out of prexisting templates (I think of it as maven profiles). So one could define in the config.xml a template for 'maven', 'golang' etc and then just use that. So the example we have at the readme could then look like:

podTemplate(label: 'mypod', inheritFrom: ['maven', 'golang'],
volumes: [secretVolume(secretName: 'shared-secrets', mountPath: '/etc/shared-secrets')]) {
node ('mypod') {
    stage 'Get a Maven project'
    git 'https://github.com/jenkinsci/kubernetes-plugin.git'
    container('maven') {
        stage 'Build a Maven project'
        sh 'mvn clean install'
    }

    stage 'Get a Golang project'
    git url: 'https://github.com/hashicorp/terraform.git'
    container('golang') {
        stage 'Build a Go project'
        sh """
        mkdir -p /go/src/github.com/hashicorp
        ln -s `pwd` /go/src/github.com/hashicorp/terraform
        cd /go/src/github.com/hashicorp/terraform && make core-dev
        """
    }
  }
}

Would you like to add that feature too?

iocanel commented Nov 8, 2016

@carlossg: I will apply your comments, rebase and squash.

Since, I added this pull request I've been playing a lot with this feature and I am starting to think that it would make sense to be able to inheritFrom multiple parents. So that I can easily compose a pod out of prexisting templates (I think of it as maven profiles). So one could define in the config.xml a template for 'maven', 'golang' etc and then just use that. So the example we have at the readme could then look like:

podTemplate(label: 'mypod', inheritFrom: ['maven', 'golang'],
volumes: [secretVolume(secretName: 'shared-secrets', mountPath: '/etc/shared-secrets')]) {
node ('mypod') {
    stage 'Get a Maven project'
    git 'https://github.com/jenkinsci/kubernetes-plugin.git'
    container('maven') {
        stage 'Build a Maven project'
        sh 'mvn clean install'
    }

    stage 'Get a Golang project'
    git url: 'https://github.com/hashicorp/terraform.git'
    container('golang') {
        stage 'Build a Go project'
        sh """
        mkdir -p /go/src/github.com/hashicorp
        ln -s `pwd` /go/src/github.com/hashicorp/terraform
        cd /go/src/github.com/hashicorp/terraform && make core-dev
        """
    }
  }
}

Would you like to add that feature too?

@carlossg

This comment has been minimized.

Show comment
Hide comment
@carlossg

carlossg Nov 8, 2016

wouldn't that be more like

containers: [ 'maven', 'golang' ] 

or something like that?
because is the pod inheriting another pod or each container template?

carlossg commented Nov 8, 2016

wouldn't that be more like

containers: [ 'maven', 'golang' ] 

or something like that?
because is the pod inheriting another pod or each container template?

@iocanel

This comment has been minimized.

Show comment
Hide comment
@iocanel

iocanel Nov 8, 2016

The problem with using something like: containers: [ 'maven', 'golang' ] is that you can just reference a containerTemplate by name, since its not an independent entity (they are always nested under podTemplates).

Of course, this is something we could change if we wanted, but by inheriting podTemplates also allows us to inherit volumes and I can see that being useful too.

iocanel commented Nov 8, 2016

The problem with using something like: containers: [ 'maven', 'golang' ] is that you can just reference a containerTemplate by name, since its not an independent entity (they are always nested under podTemplates).

Of course, this is something we could change if we wanted, but by inheriting podTemplates also allows us to inherit volumes and I can see that being useful too.

@carlossg

This comment has been minimized.

Show comment
Hide comment
@carlossg

carlossg Nov 9, 2016

right, so we'd need a description somewhere about what gets inherited from podTemplate inheritFrom and what's the precedence (volumes, containers,...)

carlossg commented Nov 9, 2016

right, so we'd need a description somewhere about what gets inherited from podTemplate inheritFrom and what's the precedence (volumes, containers,...)

@iocanel

This comment has been minimized.

Show comment
Hide comment
@iocanel

iocanel Nov 10, 2016

Fixed typo, added docs, rebased, squashed and added support for multiple parent pod template via inheritFrom.

iocanel commented Nov 10, 2016

Fixed typo, added docs, rebased, squashed and added support for multiple parent pod template via inheritFrom.

@carlossg carlossg merged commit 5d3a949 into jenkinsci:master Nov 29, 2016

1 check passed

Jenkins This pull request looks good
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment