[JENKINS-41758] Add a Declarative Agent extension for Kubernetes #127
Conversation
|
fwiw, I can't get any of the tests using |
|
...of course, just after I say that, it works and I get my new test to pass. heh. |
|
can you provide an example of the declarative syntax that this would enable? |
|
Take a look at the test I added? |
|
doh! |
|
So jenkinsci/pipeline-model-definition-plugin#109 will simplify the "should we do checkout?" bit and let us trim down the dependencies again. Also worth noting - I currently don't have a good way of providing a |
|
Moving to Declarative 1.0.2 and simplifying the dependencies and logic a bit. |
|
This pull request originates from a CloudBees employee. At CloudBees, we require that all pull requests be reviewed by other CloudBees employees before we seek to have the change accepted. If you want to learn more about our process please see this explanation. |
|
🐝 |
|
@abayer: this is not work in progress any more, right? Should we try to get it in? |
|
Lemme update it first. =) |
Note that this is just the first work-in-progress commit at this point - I want to make some changes in Declarative and release those before actually merging this (to simplify the dependencies and checkout process) but this is a start.
|
The error looks to be transient noise... |
| private String cloud; | ||
| private String inheritFrom; | ||
|
|
||
| private int instanceCap; |
There was a problem hiding this comment.
These should probably have a centralized config with defaults, same as dockerLabel et.al.
The pipeline dev should not have to know all these things, looks more like admin stuff.
|
I get the same test error locally, changing the pom to helped to fix all but one |
|
Ok, will revisit tomorrow. |
|
Ok, the |
|
Looking at the example in the test it still needs something. At least needs a way to specify that you want to run |
|
Ok, I'll file a followup PR. |
|
Sorry for bumping this closed PR, I compiled the kubernetes plugin from the master branch and tried to run the above pipeline example, but I get a ClassNotFoundException. The plugin seems to work when not using the declarative pipeline syntax, but I prefer the declarative syntax. Did I do something wrong?
|
|
Yeah I tried to demo it yesterday and got the same error. I guess the tests pass because the classpath is built from the maven project but packaging must be wrong. @abayer ? |
|
The question is also, would multiple containers work with declarative pipelines? How would you define multiple container templates? |
|
I'll take a look. That's weird. |
|
Ok, the packaging is fine, I think it's some classloader problem. Experimenting further. |
|
There's something really weird here with the classloader - the |
|
Maybe because the pom has pluginFirstClassLoader=true ? |
|
That could be. But I'm not sure how that would mess with the global classloader... |
|
I think pluginFirstClassLoader=false made it working for me now. |
|
Ah-ha! So yeah, https://github.com/jenkinsci/jenkins/blob/d13b1361e1650494528a7b46f82fe25d7fb5e3c3/core/src/main/java/hudson/PluginManager.java#L1890-L1890 doesn't work for |
|
Ok, I aready wondered if you did test driven development only :) Next error: Could this be because of 400d1ed ? |
|
Yup, |
|
pluginFirstClassLoader was added because some dependencies of |
|
Thanks for the fix! |
|
error: Jenkins doesn’t have label mypod This error still occur in kubernetes 1.0 with jenkins 2.60.2/2.60.3. Could you help a look at this issue? |
|
Read https://github.com/jenkinsci/kubernetes-plugin#declarative-pipeline Declarative Pipeline support requires Jenkins 2.66+ |
|
@carlossg Thanks for your advice, now I try with jenkins 2.78, at first there is no label issue, jenkins-slave aways offline, I expose the port like below: |
| script.checkout script.scm | ||
| } | ||
| } | ||
| script.container(describable.containerTemplate.name) { |
There was a problem hiding this comment.
I am not sure that wrapping every call in container(describable.containerTemplate.name){ } is such a good idea.
All commands are executed inside the one single container template that can be specified, while the pod will at least contain a 'jnlp' container in addition. For users to execute scripts in the jnlp container, they need to specify it:
step {
container('jnlp') {
sh 'echo run on the slave'
}
}
Also, the current example contains an(other) explicit container, see
I would vote for removing the container here, making the default "execution location" the jnlp container (like it is for non-declarative pipeline). That will also enable us to add support for multiple containerTemplates in declarative pipeline.
Or was it a conscious decision to keep the declarative syntax simpler and therefore less powerful?
@carlossg what do you think?
There was a problem hiding this comment.
Just saw #127 (comment), i.e. multiple containerTemplates may not make sense..
There was a problem hiding this comment.
I believe I know the answer (from following the above thread) but does the current declarative support for this plugin support podTemplate inheritance? Such that you can specify X number of containers you want in the POD? (I think the answer is no).
There was a problem hiding this comment.
multiple containers are now possible with the yaml syntax
|
I am getting "Jenkins doesn’t have label mypod" though it run when I did initial setup. Any Idea? |
|
|
|
@marvinthepa @abayer why wouldn't multiple containerTemplates not make sense ? Will you support volumes in the near future ? Did you think about this as you said in #127 (comment) ? |
|
@snackycracky @marvinthepa @abayer I'm running into the same issue of wanting to run multiple containerTemplates in a declarative pipeline. Something like the ability to have podTemplate support in the agent configuration. I'm trying to do something simple like getting both maven and node on the same slave. It seems declarative pipeline is more future-proof so shouldn't we make sure everything is declarative first? |
|
[JENKINS-50533] Support multiple containers in declarative pipeline is implemented in #306 |
JENKINS-41758
Ok, I think this is ready to go.
cc @reviewbybees esp @carlossg