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

feat: add support for excluding or setting the default agent serviceA… #1166

Merged
merged 2 commits into from
Aug 13, 2024

Conversation

h0tw1r3
Copy link
Contributor

@h0tw1r3 h0tw1r3 commented Aug 8, 2024

What does this PR do?

Decouples the default template serviceAccount from serviceAccountAgent.name.

Basically I found myself wanting to set the agent.defaultsProviderTemplate to default, and also the ability to exclude the serviceAccount from the autogenerated default template.

Submitter checklist

  • I bumped the "version" key in ./charts/jenkins/Chart.yaml.
  • I added a new changelog entry to ./charts/jenkins/CHANGELOG.md.
  • I followed the technical requirements.
  • I ran .github/helm-docs.sh from the project root.

@h0tw1r3
Copy link
Contributor Author

h0tw1r3 commented Aug 8, 2024

Does this seem worthwhile, or am I going in completely the wrong direction (or both)? :)

@timja
Copy link
Member

timja commented Aug 9, 2024

hmm I'm a little confused, won't it use the default one by default if you don't specify create https://github.com/jenkinsci/helm-charts/blob/main/charts/jenkins/templates/_helpers.tpl#L565-L568

@h0tw1r3
Copy link
Contributor Author

h0tw1r3 commented Aug 9, 2024

hmm I'm a little confused, won't it use the default one by default if you don't specify create https://github.com/jenkinsci/helm-charts/blob/main/charts/jenkins/templates/_helpers.tpl#L565-L568

It does, but if you also set agent.defaultsProviderTemplate: "default", it's impossible for jobs to override the serviceAccountName.

For example, if you set the agent.defaultsProviderTemplate: "default", and need a pod with a different serviceAccountName, something like this:

Jenkinsfile

pipeline {
    agent {
        kubernetes {
            yaml '''
                apiVersion: v1
                kind: Pod
                spec:
                    serviceAccountName: other-account
                    containers:
                    - name: dind
                      image: docker:dind
                      securityContext:
                        privileged: true
                '''
        }
    }
}

When the job runs and the yaml get's merged with the "default" template, serviceAccountName is never overridden no matter what I set inheritYamlMergeStrategy or yamlMergeStrategy to in the "default" pod template.

Perhaps this is intended behavior? This PR just gives the ability to not add the servicecAccountName to the "default" pod template.

@timja
Copy link
Member

timja commented Aug 9, 2024

sure sounds fine then if you can make the build pass

@h0tw1r3 h0tw1r3 force-pushed the conditional-template-serviceaccount branch from 1769e51 to 968ba75 Compare August 12, 2024 17:30
@h0tw1r3 h0tw1r3 force-pushed the conditional-template-serviceaccount branch from 968ba75 to 0a97e78 Compare August 12, 2024 17:33
@h0tw1r3 h0tw1r3 marked this pull request as ready for review August 12, 2024 17:39
@h0tw1r3 h0tw1r3 requested a review from a team as a code owner August 12, 2024 17:39
@timja timja merged commit f8f6e72 into jenkinsci:main Aug 13, 2024
6 checks passed
@h0tw1r3 h0tw1r3 deleted the conditional-template-serviceaccount branch August 13, 2024 19: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.

2 participants