-
-
Notifications
You must be signed in to change notification settings - Fork 8.8k
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
Simplifying JNLPLauncher
#8762
Simplifying JNLPLauncher
#8762
Changes from all commits
0d101a6
3be5ae0
601722a
457557d
44ebca2
ed5818a
8f57907
c6d02b4
d24f3fb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -34,7 +34,6 @@ | |
import hudson.model.Computer; | ||
import hudson.model.Descriptor; | ||
import hudson.model.TaskListener; | ||
import hudson.util.FormValidation; | ||
import jenkins.model.Jenkins; | ||
import jenkins.model.identity.InstanceIdentityProvider; | ||
import jenkins.slaves.RemotingWorkDirSettings; | ||
|
@@ -43,10 +42,10 @@ | |
import org.apache.commons.lang.StringUtils; | ||
import org.jenkinsci.Symbol; | ||
import org.kohsuke.accmod.Restricted; | ||
import org.kohsuke.accmod.restrictions.DoNotUse; | ||
import org.kohsuke.accmod.restrictions.NoExternalUse; | ||
import org.kohsuke.stapler.DataBoundConstructor; | ||
import org.kohsuke.stapler.DataBoundSetter; | ||
import org.kohsuke.stapler.QueryParameter; | ||
|
||
/** | ||
* {@link ComputerLauncher} via inbound connections. | ||
|
@@ -56,28 +55,23 @@ | |
*/ | ||
public class JNLPLauncher extends ComputerLauncher { | ||
/** | ||
* If the agent needs to tunnel the connection to the controller, | ||
* specify the "host:port" here. This can include the special | ||
* syntax "host:" and ":port" to indicate the default host/port | ||
* shall be used. | ||
* | ||
* <p> | ||
* Null if no tunneling is necessary. | ||
* | ||
* @since 1.250 | ||
* Deprecated (only used with deprecated {@code -jnlpUrl} mode), but cannot mark it as such without breaking CasC. | ||
*/ | ||
@DataBoundSetter | ||
@CheckForNull | ||
public final String tunnel; | ||
public String tunnel; | ||
|
||
/** | ||
* @deprecated No longer used. | ||
*/ | ||
@Deprecated | ||
public final transient String vmargs = null; | ||
|
||
@Deprecated | ||
@NonNull | ||
private RemotingWorkDirSettings workDirSettings = RemotingWorkDirSettings.getEnabledDefaults(); | ||
|
||
@Deprecated | ||
private boolean webSocket; | ||
|
||
/** | ||
|
@@ -88,14 +82,7 @@ | |
public static final String CUSTOM_INBOUND_URL_PROPERTY = "jenkins.agent.inboundUrl"; | ||
|
||
/** | ||
* Constructor. | ||
* @param tunnel Tunnel settings | ||
* @param vmargs JVM arguments | ||
* @param workDirSettings Settings for Work Directory management in Remoting. | ||
* If {@code null}, {@link RemotingWorkDirSettings#getEnabledDefaults()} | ||
* will be used to enable work directories by default in new agents. | ||
* @since 2.68 | ||
* @deprecated use {@link #JNLPLauncher(String, String)} and {@link #setWorkDirSettings(RemotingWorkDirSettings)} | ||
* @deprecated no useful properties, use {@link #JNLPLauncher()} | ||
*/ | ||
@Deprecated | ||
public JNLPLauncher(@CheckForNull String tunnel, @CheckForNull String vmargs, @CheckForNull RemotingWorkDirSettings workDirSettings) { | ||
|
@@ -105,36 +92,30 @@ | |
} | ||
} | ||
|
||
// TODO cannot easily make tunnel into a @DataBoundSetter because then the @DataBoundConstructor would be on a no-arg constructor | ||
// which is already defined and deprecated. Could retroactively let no-arg constructor use default for workDirSettings, | ||
// which would be a behavioral change only for callers of the Java constructor (unlikely). | ||
Comment on lines
-109
to
-110
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would only affect
|
||
@DataBoundConstructor | ||
/** | ||
* @deprecated no useful properties, use {@link #JNLPLauncher()} | ||
*/ | ||
@Deprecated | ||
public JNLPLauncher(@CheckForNull String tunnel) { | ||
this.tunnel = Util.fixEmptyAndTrim(tunnel); | ||
} | ||
|
||
/** | ||
* @deprecated use {@link JNLPLauncher#JNLPLauncher(String)} | ||
* @deprecated no useful properties, use {@link #JNLPLauncher()} | ||
*/ | ||
@Deprecated | ||
public JNLPLauncher(@CheckForNull String tunnel, @CheckForNull String vmargs) { | ||
this.tunnel = Util.fixEmptyAndTrim(tunnel); | ||
} | ||
|
||
/** | ||
* @deprecated This Launcher does not enable the work directory. | ||
* It is recommended to use {@link #JNLPLauncher(boolean)} | ||
*/ | ||
@Deprecated | ||
@DataBoundConstructor | ||
public JNLPLauncher() { | ||
this(false); | ||
} | ||
|
||
/** | ||
* Constructor with default options. | ||
* | ||
* @param enableWorkDir If {@code true}, the work directory will be enabled with default settings. | ||
* @deprecated no useful properties, use {@link #JNLPLauncher()} | ||
*/ | ||
@Deprecated | ||
jglick marked this conversation as resolved.
Show resolved
Hide resolved
|
||
public JNLPLauncher(boolean enableWorkDir) { | ||
this(null, null, enableWorkDir | ||
? RemotingWorkDirSettings.getEnabledDefaults() | ||
|
@@ -150,16 +131,14 @@ | |
return this; | ||
} | ||
|
||
/** | ||
* Returns work directory settings. | ||
* | ||
* @since 2.72 | ||
*/ | ||
@NonNull | ||
@Deprecated | ||
public RemotingWorkDirSettings getWorkDirSettings() { | ||
return workDirSettings; | ||
} | ||
|
||
/** | ||
* Deprecated (only used with deprecated {@code -jnlpUrl} mode), but cannot mark it as such without breaking CasC. | ||
*/ | ||
@DataBoundSetter | ||
public final void setWorkDirSettings(@NonNull RemotingWorkDirSettings workDirSettings) { | ||
this.workDirSettings = workDirSettings; | ||
|
@@ -170,15 +149,13 @@ | |
return false; | ||
} | ||
|
||
/** | ||
* @since 2.216 | ||
*/ | ||
@Deprecated | ||
public boolean isWebSocket() { | ||
return webSocket; | ||
} | ||
|
||
/** | ||
* @since 2.216 | ||
* Deprecated (only used with deprecated {@code -jnlpUrl} mode), but cannot mark it as such without breaking CasC. | ||
*/ | ||
@DataBoundSetter | ||
public void setWebSocket(boolean webSocket) { | ||
|
@@ -210,6 +187,11 @@ | |
return getRemotingOptions(escapeWindows(computer.getName())); | ||
} | ||
|
||
@Restricted(DoNotUse.class) | ||
public boolean isConfigured() { | ||
return webSocket || tunnel != null || workDirSettings.isConfigured(); | ||
} | ||
|
||
private String getRemotingOptions(String computerName) { | ||
StringBuilder sb = new StringBuilder(); | ||
sb.append("-name "); | ||
|
@@ -296,23 +278,19 @@ | |
return DescriptorImpl.class.equals(getClass()); | ||
} | ||
|
||
public FormValidation doCheckWebSocket(@QueryParameter boolean webSocket, @QueryParameter String tunnel) { | ||
jglick marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if (webSocket) { | ||
if (!WebSockets.isSupported()) { | ||
return FormValidation.error(Messages.JNLPLauncher_WebsocketNotEnabled()); | ||
} | ||
if (Util.fixEmptyAndTrim(tunnel) != null) { | ||
return FormValidation.error(Messages.JNLPLauncher_TunnelingNotSupported()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This and other improper combinations now handled by args4j in the launcher itself. |
||
} | ||
} else { | ||
if (Jenkins.get().getTcpSlaveAgentListener() == null) { | ||
return FormValidation.error(Messages.JNLPLauncher_TCPPortDisabled()); | ||
} | ||
if (InstanceIdentityProvider.RSA.getCertificate() == null || InstanceIdentityProvider.RSA.getPrivateKey() == null) { | ||
return FormValidation.error(Messages.JNLPLauncher_InstanceIdentityRequired()); | ||
} | ||
} | ||
return FormValidation.ok(); | ||
@Restricted(DoNotUse.class) | ||
public boolean isTcpSupported() { | ||
return Jenkins.get().getTcpSlaveAgentListener() != null; | ||
} | ||
|
||
@Restricted(DoNotUse.class) | ||
public boolean isInstanceIdentityInstalled() { | ||
return InstanceIdentityProvider.RSA.getCertificate() != null && InstanceIdentityProvider.RSA.getPrivateKey() != null; | ||
} | ||
|
||
@Restricted(DoNotUse.class) | ||
public boolean isWebSocketSupported() { | ||
return WebSockets.isSupported(); | ||
} | ||
|
||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
configured=\ | ||
Configuring any properties on the inbound agent launcher is deprecated \ | ||
as it would only have an effect in deprecated <code>-jnlpUrl</code> mode. \ | ||
Instead pass any desired options to the agent yourself. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -25,41 +25,13 @@ THE SOFTWARE. | |
<?jelly escape-by-default='true'?> | ||
<j:jelly xmlns:j="jelly:core" xmlns:st="jelly:stapler" xmlns:d="jelly:define" xmlns:l="/lib/layout" xmlns:t="/lib/hudson" xmlns:f="/lib/form" xmlns:i="jelly:fmt"> | ||
<j:choose> | ||
<j:when test="${!it.launcher.webSocket and app.slaveAgentPort == -1}"> | ||
<div class="error"> | ||
${%slaveAgentPort.disabled} | ||
<l:isAdmin><a href="${rootURL}/configureSecurity">${%configure.link.text}</a>.</l:isAdmin> | ||
</div> | ||
</j:when> | ||
<j:when test="${it.offline and !it.temporarilyOffline}"> | ||
<j:set var="jenkinsURL" value="${h.inferHudsonURL(request)}"/> | ||
<j:set var="copy_agent_jar_unix" value="curl -sO ${jenkinsURL}jnlpJars/agent.jar" /> | ||
<j:set var="copy_agent_jar_windows" value="curl.exe -sO ${jenkinsURL}jnlpJars/agent.jar" /> | ||
<j:set var="copy_java_cmd_unix" value="java -jar agent.jar -url ${jenkinsURL} ${it.launcher.getRemotingOptionsUnix(it)}${it.launcher.getWorkDirOptions(it)}" /> | ||
<j:set var="copy_java_cmd_windows" value="java -jar agent.jar -url ${jenkinsURL} ${it.launcher.getRemotingOptionsWindows(it)}${it.launcher.getWorkDirOptions(it)}" /> | ||
<j:if test="${h.hasPermission(it, it.CONNECT)}"> | ||
<j:choose> | ||
<j:when test="${it.ACL.hasPermission(app.ANONYMOUS, it.CONNECT)}"> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This clause tried to omit the |
||
<h3> | ||
${%slaveAgent.cli.run} (Unix) | ||
<l:copyButton text="${copy_agent_jar_unix};${copy_java_cmd_unix}"/> | ||
</h3> | ||
<pre> | ||
${copy_agent_jar_unix} | ||
${copy_java_cmd_unix} | ||
</pre> | ||
|
||
<h3> | ||
${%slaveAgent.cli.run} (Windows) | ||
<l:copyButton text="${copy_agent_jar_windows} & ${copy_java_cmd_windows}"/> | ||
</h3> | ||
<pre> | ||
${copy_agent_jar_windows} | ||
${copy_java_cmd_windows} | ||
</pre> | ||
|
||
</j:when> | ||
<j:otherwise> | ||
<j:set var="copy_java_cmd_secret_unix" value="java -jar agent.jar -url ${jenkinsURL} -secret ${it.jnlpMac} ${it.launcher.getRemotingOptionsUnix(it)}${it.launcher.getWorkDirOptions(it)}" /> | ||
<j:set var="copy_java_cmd_secret_windows" value="java -jar agent.jar -url ${jenkinsURL} -secret ${it.jnlpMac} ${it.launcher.getRemotingOptionsWindows(it)}${it.launcher.getWorkDirOptions(it)}" /> | ||
<h3> | ||
|
@@ -104,8 +76,28 @@ ${copy_secret_to_file} | |
${copy_agent_jar_windows} | ||
${copy_java_cmd_secret2_windows} | ||
</pre> | ||
</j:otherwise> | ||
</j:choose> | ||
<j:if test="${!it.launcher.configured}"> | ||
<p> | ||
${%commonOptions} | ||
</p> | ||
</j:if> | ||
<j:if test="${!it.launcher.descriptor.tcpSupported}"> | ||
<p> | ||
${%tcp-port-disabled} | ||
<l:isAdmin><a href="${rootURL}/manage/configureSecurity/">${%configure.link.text}</a>.</l:isAdmin> | ||
</p> | ||
Comment on lines
+85
to
+88
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similar to older message, but not marked as an error since it may be perfectly normal in a WebSockets-only installation. |
||
</j:if> | ||
<j:if test="${!it.launcher.descriptor.instanceIdentityInstalled}"> | ||
<p> | ||
${%instance-identity-missing} | ||
<l:isAdmin><a href="${rootURL}/manage/pluginManager/available">${%install-instance-identity}</a>.</l:isAdmin> | ||
</p> | ||
Comment on lines
+91
to
+94
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
</j:if> | ||
<j:if test="${!it.launcher.descriptor.webSocketSupported}"> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (untested) |
||
<p> | ||
${%web-socket-unsupported} | ||
</p> | ||
</j:if> | ||
<p> | ||
${%powerShell.cli.curl} | ||
</p> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,9 +20,22 @@ | |
# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN | ||
# THE SOFTWARE. | ||
|
||
configure.link.text=Go to security configuration screen and change it | ||
slaveAgentPort.disabled=JNLP agent port is disabled and agents cannot connect this way. | ||
configure.link.text=Go to the security configuration screen and change it | ||
slaveAgent.connected=Agent is connected. | ||
slaveAgent.cli.run=Run from agent command line: | ||
slaveAgent.cli.run.secret=Or run from agent command line, with the secret stored in a file: | ||
powerShell.cli.curl=Note: PowerShell users must use curl.exe instead of curl because curl is a default PowerShell cmdlet alias for Invoke-WebRequest. | ||
powerShell.cli.curl=Note: PowerShell users must use curl.exe instead of curl because curl is a default PowerShell cmdlet alias for Invoke-WebRequest. | ||
commonOptions=\ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I doubt |
||
The most commonly used option is <code>-webSocket</code>. \ | ||
Run <code>java -jar agent.jar -help</code> for more. | ||
tcp-port-disabled=\ | ||
The TCP port is disabled so TCP agents may not be connected. \ | ||
You may still use WebSocket agents. | ||
instance-identity-missing=\ | ||
The <code>instance-identity</code> plugin appears to be missing. \ | ||
This would prevent you from connecting a TCP agent. \ | ||
You may still use WebSocket agents. | ||
install-instance-identity=Install it now | ||
web-socket-unsupported=\ | ||
WebSockets are not supported in this Jenkins installation. \ | ||
You may still use TCP agents. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,7 +20,6 @@ | |
# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN | ||
# THE SOFTWARE. | ||
|
||
slaveAgentPort.disabled=El puerto TCP para los agentes via JNLP está deshabilitado. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Renaming key and deleting old translations since there is now a significant extra clause about WebSockets. |
||
launch\ agent=Lanzar agente | ||
Or\ if\ the\ agent\ is\ headless\:=O si el agente no tiene pantalla | ||
Run\ from\ agent\ command\ line\:=Ejecutar agente desde la línea de comandos del nodo | ||
|
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.
Should only affect loading of ancient node definitions from disk, or I guess someone defining nodes via XML produced from template who for some reason declined to specify a launcher.