[stable/jenkins] Add support for custom pod templates

[lgg42]

Signed-off-by: Luis Garnica Guilarte luisgarnica42@gmail.com

Is this a new chart

No.

NOTE: We're experiencing a high volume of PRs to this repo and reviews will be delayed. Please host your own chart repository and submit your repository to the Helm Hub instead of this repo to make them discoverable to the community. Here is how to submit new chart repositories to the Helm Hub.

What this PR does / why we need it:

This PR allows to configure additional pod templates in the default kubernetes cloud.
We want to configure from scratch custom pod templates the same way we do with all the settings using JCasC.

Which issue this PR fixes

(optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged)

• fixes #

Special notes for your reviewer:

Checklist

[Place an '[x]' (no spaces) in all applicable fields. Please remove unrelated fields.]

• DCO signed
• Chart Version bumped
• Variables are documented in the README.md
• Title of the PR starts with chart name (e.g. [stable/mychartname])

[k8s-ci-robot]

Hi @lgg42. Thanks for your PR.

I'm waiting for a helm member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[wmcdona89]

What about putting this under master.JCasC.podTemplates so all the JCasC config values would be grouped together. Another option could be to put it in the agent section as podTemplates.

[lgg42]

I also think is a nice change. I left a comment in your PR. I think maybe your PR is the one to go. Let's see how we manage to get this feature working the best possible way. I'm on hold (on committing) until we find common ground 😄

[lgg42]

Change done to agent.podTemplates as we talked in your PR.

[lgg42]

/assign @mogaal

[wmcdona89]

@lgg42 Turns out I'll be able to leverage agent.podTemplates to inject "agents" into the default casc. Here's an example with the new podTemplate template in my PR.

# A list of agents. Each entry corresponds to a chart `agent` in terms of the values that can be set.
additionalAgents:
  - podName: maven
    customJenkinsLabels: maven
    image: jenkins/jnlp-agent-maven
  - podName: node
    customJenkinsLabels: node
    image: jenkins/jnlp-agent-node

agent:
  podTemplates:
    agents: |-
      {{- /* save chart agent as `set` modifies the input dictionary */}}
      {{- $rootAgent := .Values.agent }}

      {{- /* each additional agent inherits from $rootAgent */}}
      {{- range .Values.additionalAgents }}
      {{- include "jenkins.casc.podTemplate" (set $ "Values" (set $.Values "agent" (merge . $rootAgent))) }}
      {{- end }}

      {{- /* restore chart agent */}}
      {{- $_ := set .Values "agent" $rootAgent }}

[lgg42]

@lgg42 Turns out I'll be able to leverage agent.podTemplates to inject "agents" into the default casc. Here's an example with the new podTemplate template in my PR.

# A list of agents. Each entry corresponds to a chart `agent` in terms of the values that can be set.
additionalAgents:
  - podName: maven
    customJenkinsLabels: maven
    image: jenkins/jnlp-agent-maven
  - podName: node
    customJenkinsLabels: node
    image: jenkins/jnlp-agent-node

agent:
  podTemplates:
    agents: |-
      {{- /* save chart agent as `set` modifies the input dictionary */}}
      {{- $rootAgent := .Values.agent }}

      {{- /* each additional agent inherits from $rootAgent */}}
      {{- range .Values.additionalAgents }}
      {{- include "jenkins.casc.podTemplate" (set $ "Values" (set $.Values "agent" (merge . $rootAgent))) }}
      {{- end }}

      {{- /* restore chart agent */}}
      {{- $_ := set .Values "agent" $agent }}

Amazing!!! 💪

[arencibiafrancisco]

Can't wait for this to be merged! Have been looking for this! Another JenkinsX user here. agent.podTemplates is perfect!

[wmcdona89]

FYI. I noticed the JenkinsX values file doesn't use multi-line strings. As it turns out, having this be a multi-line string (and not calling toYaml in the template) actually provides additional flexibility and allows for passing in a block of templating.

agent:
  podTemplates:
    agents: |-
      {{- /* save chart agent as `set` modifies the input dictionary */}}
      {{- $rootAgent := .Values.agent }}

      {{- /* each additional agent inherits from $rootAgent */}}
      {{- range .Values.additionalAgents }}
      {{- include "jenkins.casc.podTemplate" (set $ "Values" (set $.Values "agent" (merge . $rootAgent))) }}
      {{- end }}

      {{- /* restore chart agent */}}
      {{- $_ := set .Values "agent" $rootAgent }}

[lgg42]

I'm glad you pointed me this out and resulted to be a good thing. I just followed examples from other parts of this chart.
So, does it looks already ok to test to you?
It's my first contribution here, how long does this process tend to take?

[torstenwalter]

This looks good to me. I have just some minor remarks - see inline comments.
Could you do me a favor and add defaultConfig: true in ci/casc-values.yaml. I think it"s time to check these setting as part of the build.

[lgg42]

@torstenwalter thanks for your change comments! all resolved. I also added the line you requested in ci/casc-values.yaml.

[torstenwalter]

/ok-to-test

[torstenwalter]

/retest

[lgg42]

/retest

[lgg42]

CI's failing with

I0404 04:49:59.725] Error: incompatible versions client[v2.16.5] server[v2.15.2]

Is this something I could fix from my part?

[unguiculus]

/retest

[torstenwalter]

/lgtm

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: lgg42, torstenwalter

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• stable/jenkins/OWNERS [torstenwalter]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[torstenwalter]

@lgg42 Thanks a lot for the PR.

[lgg42]

Thank you all for allowing this PR go through! ♥️