Make Slave 馃牔 Jenkins conn. timeout configurable #141
Conversation
nehaljwani
force-pushed
the
introduce-slave鉃沯enkins-conn-timeout
branch
from
6137ca7
to
5b42e77
Compare
There was a problem hiding this comment.
Thanks! Added some comments, and it is missing a help text like https://github.com/nehaljwani/kubernetes-plugin/blob/5b42e773214cea6bcb91cccda411cc94f5c78c00/src/main/resources/org/csanchez/jenkins/plugins/kubernetes/PodTemplate/help-annotations.html
| @DataBoundSetter | ||
| public void setSlaveConnectTimeoutStr(String slaveConnectTimeoutStr) { | ||
| if (StringUtils.isBlank(slaveConnectTimeoutStr)) { | ||
| setSlaveConnectTimeout(100); // default |
There was a problem hiding this comment.
can we have a constant instead of 100
| @@ -40,6 +40,10 @@ | |||
| <f:textbox/> | |||
| </f:entry> | |||
|
|
|||
| <f:entry field="slaveConnectTimeoutStr" title="${%Timeout in seconds for slave 馃 jenkins conn.}"> | |||
There was a problem hiding this comment.
Can you write Jenkins connection, for a better description
is there a bad char 馃 ?
| @@ -202,6 +204,18 @@ public int getInstanceCap() { | |||
| return instanceCap; | |||
| } | |||
|
|
|||
| public void setSlaveConnectTimeout(int slaveConnectTimeout) { | |||
| if (slaveConnectTimeout < 0) { | |||
| this.slaveConnectTimeout = 0; | |||
There was a problem hiding this comment.
better to set it to the default and log a warning
| } | ||
|
|
||
| public int getSlaveConnectTimeout() { | ||
| return slaveConnectTimeout; |
There was a problem hiding this comment.
when upgrading slaveConnectTimeout won't be in the object, so it will get set to 0 IIRC
You need to handle it in readResolve to set it to the default, or here to return the default if it is 0
For certain slaves, there might be a need to perform some runtime configuration/steps after getting scheduled, but before starting the jenkins job. The default timeout of 100s might not be sufficient for such a scenario. To cater to this use case, this patch introduces the ability to configure the slave to jenkins connection timeout, defaulting to 100s for backwards compatibility.
nehaljwani
force-pushed
the
introduce-slave鉃沯enkins-conn-timeout
branch
from
5b42e77
to
2abc8cf
Compare
|
@carlossg I have updated the PR as per your comments. |
For certain slaves, there might be a need to perform some runtime
configuration/steps after getting scheduled, but before starting the
jenkins job. The default timeout of 100s might not be sufficient for
such a scenario. To cater to this use case, this patch introduces the
ability to configure the slave to jenkins connection timeout, defaulting
to 100s for backwards compatibility.