Skip to content
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

chore(Jenkinsfile) inherit pod definition from existing pod template #465

Merged
merged 3 commits into from
Jun 3, 2024

Conversation

dduportal
Copy link
Collaborator

@dduportal dduportal commented May 27, 2024

Related to jenkins-infra/helpdesk#4106, this PR is a try to have an inherited pod template definition for the build agent on ci.jenkins.io.

The goal is to ensure that administrator-defined settings such as tolerations can be reused without having to maintain a full-fledged pod YAML.

I had difficulties to have the inheritance working with a YAML inline (of from file) so I chose t override the image in pure Groovy: the merge never happened which looks weird to me: either I got the inherited image or the local yaml but never the merge (tried implicit merge, or setting up all yaml merge strategies). I most probably missed something: another pair of eye would help.

Partial success as we have the expected image and environment variables, along with inherited pod. The agent starts but the test phase of the pipeline fails which might be related to the not-transplanted elements:

  • SecurityContext (I can't find a way to specify it using Groovy function, only with rawYaml)
  • Resources limits (it uses the 4vCPUs and 12 Gb from parent).

@dduportal dduportal marked this pull request as ready for review May 27, 2024 17:04
@dduportal dduportal requested a review from a team as a code owner May 27, 2024 17:04
@timja
Copy link
Member

timja commented May 27, 2024

I've given you access, not sure if you'll have to create a new PR or not (or just replay)

@dduportal dduportal marked this pull request as draft May 31, 2024 08:43
@dduportal
Copy link
Collaborator Author

OK, I'm definitievely missing something with the Kubernetes plugin:

  • I can merge from groovy but not from yaml, even if I use the merge() method

=> Let's assume we want packaging to have it own pod template provided by jenkins-infra team

@dduportal
Copy link
Collaborator Author

I got the following problem: if I use the inheritFrom directive for the podTemplate() step along with either yaml (inline YAML) or read(.*)Yaml (pod template read from a file, tried both trusted and untrusted), the merge does not seems to be performed deeply:

  • Adding a new container template (e.g. NOT named jnlp) reuse the inherited pod definition and successfully adds the 2nd container
  • But if the container template is named jnlp, either I got the child container only, losing parent Pod directives such as tolerations (with the override() default yaml merge method) or I only got the parent directive (if I switch to the merge() YAML merge method).

However, if I use Groovy form, then the merge is performed as expected, which makes me say it's not because of the parent pod template setup (default to override() for all and allows inheriting the yaml merge method).

cc @Vlatombe @jglick @timja if it rings a bell (I bet it's something I don't understand in the inheritance system).

=> As such, I'll stick to the groovy method for now, as a tradeoff between developer autonomy (this repository needs advanced tooling such as ansible which might require tuning the pod template) and "admin defined setup).

@dduportal dduportal marked this pull request as ready for review June 3, 2024 17:00
@dduportal
Copy link
Collaborator Author

@timja @basil @MarkEWaite , this PR is ready to roll: it spins up a pod with succes from the docker-packaging image and the "Build" stage works as expected.

Please note that the tests errors are during the "Test" stage, and happens in a VM (Docker) agent.
The error is due to docker/docker-py#3256 which is well described in ansible-collections/community.docker#868. Which should be fixed with either Ansible 9.7.0 and/or Ansible 10 (and/or adding a custom collection for docker-community).

I leave you to it if you want to merge this PR despite the checks errors, or if you want to fix the Tests first?

@timja timja enabled auto-merge (squash) June 3, 2024 18:01
Copy link
Member

@timja timja left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@timja timja merged commit c2cf8d0 into jenkinsci:master Jun 3, 2024
3 checks passed
@dduportal dduportal deleted the patch-1 branch June 4, 2024 05:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants