-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Make jenkins agent more flexible #1602
Conversation
* The name of the agent container can now be customized (the `jnlp` deprecated name is no longer required) * It is no longer required to extend the jenkins inbound-agent image anymore, or provide the agent.jar in the image. Custom images only need to provide a JRE, and required build tools.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks promising! Now we just need to find a statically-linked JRE we can copy in as well…
src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateBuilder.java
Outdated
Show resolved
Hide resolved
src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplate.java
Outdated
Show resolved
Hide resolved
src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateBuilder.java
Outdated
Show resolved
Hide resolved
src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/KubernetesDeclarativeAgent.java
Outdated
Show resolved
Hide resolved
src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/PodTemplateStep.java
Outdated
Show resolved
Hide resolved
src/main/resources/org/csanchez/jenkins/plugins/kubernetes/PodTemplate/help-agentContainer.html
Outdated
Show resolved
Hide resolved
src/main/resources/org/csanchez/jenkins/plugins/kubernetes/PodTemplate/help-agentInjection.html
Outdated
Show resolved
Hide resolved
src/main/resources/org/csanchez/jenkins/plugins/kubernetes/PodTemplate/help-agentInjection.html
Outdated
Show resolved
Hide resolved
...ces/org/csanchez/jenkins/plugins/kubernetes/pipeline/KubernetesDeclarativeAgent/config.jelly
Show resolved
Hide resolved
...main/resources/org/csanchez/jenkins/plugins/kubernetes/pipeline/PodTemplateStep/config.jelly
Show resolved
Hide resolved
Not sure it exists yet |
src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplate.java
Outdated
Show resolved
Hide resolved
src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateUtils.java
Outdated
Show resolved
Hide resolved
examples/multi-container.groovy
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like the change in this case is more confusing than helpful? It destroys the parallelism between the containers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What parallelism ? You still have a maven
and a golang
container, the only difference is that the jnlp
container is gone.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right but you are using both of these containers for similar purposes—running selected build steps—yet one is being used as the agent container merely because its toolchain happens to include a JRE.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, as long as you have a JRE in the toolchain and you can avoid container
step (and associated scalability issues), I think it is good.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done |
I don't think this should be called done https://github.com/jenkinsci/kubernetes-plugin/blob/master/README.md
Not true
Not true
Not true
Not true |
@balakine Feel free to provide a pull request to fix these |
If only the author of the change that rendered the documentation obsolete was still in the room... |
I'm sorry, but your comment, even if true, was rude. If you are unsatisfied, please help yourself. |
I apologize for hurting your feelings. You are spending your personal time for a project I benefit from, so I should be choosing kinder words and comment in a more constructive manner. |
Following jenkinsci#1602, there are several sentences in the README that can be made more generic, avoiding references to `jnlp`. As it is still the default value, I couldn't remove all references, but I think it is slightly clearer now.
* Update README with agent injection in mind Following #1602, there are several sentences in the README that can be made more generic, avoiding references to `jnlp`. As it is still the default value, I couldn't remove all references, but I think it is slightly clearer now. * Fix review and refresh pod template screenshot * Refresh
jnlp
deprecated name is no longer required)inbound-agent
image anymore, or provide theagent.jar
in the image. Custom images only need to provide a JRE, and required build tools.One example of usage
Testing done
Submitter checklist