[JENKINS-72592] Fix inbound agent connector NPE on Jenkins 2.437+

[jglick]

Fixes #1047. Or so I suppose. I could not get an actual agent to connect but that is I guess an artifact of trying to run this plugin from hpi:run, since I do not have access to a system whereby the Jenkins controller would be accessible from containers:

java.io.IOException: Failed to connect to http://host.docker.internal:8080/jenkins/tcpSlaveAgentListener/: host.docker.internal
	at org.jenkinsci.remoting.engine.JnlpAgentEndpointResolver.resolve(JnlpAgentEndpointResolver.java:217)
	at hudson.remoting.Engine.innerRun(Engine.java:809)
	at hudson.remoting.Engine.run(Engine.java:563)
Caused by: java.net.UnknownHostException: host.docker.internal
	at java.base/sun.nio.ch.NioSocketImpl.connect(Unknown Source)
	at java.base/java.net.Socket.connect(Unknown Source)
	at java.base/sun.net.NetworkClient.doConnect(Unknown Source)
	at java.base/sun.net.www.http.HttpClient.openServer(Unknown Source)
	at java.base/sun.net.www.http.HttpClient.openServer(Unknown Source)
	at java.base/sun.net.www.http.HttpClient.<init>(Unknown Source)
	at java.base/sun.net.www.http.HttpClient.New(Unknown Source)
	at java.base/sun.net.www.http.HttpClient.New(Unknown Source)
	at java.base/sun.net.www.protocol.http.HttpURLConnection.getNewHttpClient(Unknown Source)
	at java.base/sun.net.www.protocol.http.HttpURLConnection.plainConnect0(Unknown Source)
	at java.base/sun.net.www.protocol.http.HttpURLConnection.plainConnect(Unknown Source)
	at java.base/sun.net.www.protocol.http.HttpURLConnection.connect(Unknown Source)
	at org.jenkinsci.remoting.engine.JnlpAgentEndpointResolver.resolve(JnlpAgentEndpointResolver.java:214)
	... 2 more

Not sure if moby/moby#40007 was supposed to fix this or what.

[jglick]

Note that removing this (and the usage in beforeContainerCreated etc.) would be incompatible for anyone who was actually using the tunnel field using GUI or otherwise XML config. I suspect this option is rarely used (specialized network configuration). Could be restored for compatibility if it seems important.

Other much more obvious fields of JNLPLauncher—webSocket, workDir—were not interpreted at all, so anyone requiring them would I guess have had to put the corresponding options directly into entryPointArguments (as they would now need to to with -tunnel).

[jglick]

Incompatible for JCasC users configuring this connector using something like

connector:
  jnlp:
    jnlpLauncher: inbound

which should now be (I guess) just

connector: jnlp

Not sure exactly, without test coverage. Possibly though awkward to restore compatibility by adding a getter/setter that are not @Deprecated, as in the core PR. This is assuming again that tunnel was not actually configured this way.

[akomakom]

I'll try to test this today

[jglick]

Assuming you do not care to deal with a local Maven setup, the simplest way is to just use Plugin Manager » Advanced and install from the URL https://repo.jenkins-ci.org/incrementals/io/jenkins/docker/docker-plugin/1.6-rc1184.419d1ddd0e68/docker-plugin-1.6-rc1184.419d1ddd0e68.hpi (see the Incrementals check here).

[akomakom]

Assuming you do not care to deal with a local Maven setup, the simplest way is to just use Plugin Manager » Advanced and install from the URL https://repo.jenkins-ci.org/incrementals/io/jenkins/docker/docker-plugin/1.6-rc1184.419d1ddd0e68/docker-plugin-1.6-rc1184.419d1ddd0e68.hpi (see the Incrementals check here).

Thanks, way faster than waiting an hour for maven to download the internet.
Your branch is working fine in my quick test (Jenkins 2.444). 🎉

[jglick]

BTW I confirmed that there is some effective test coverage insofar as DockerComputerJNLPConnectorTest does fail if you e.g.

diff --git src/main/java/io/jenkins/docker/connector/DockerComputerJNLPConnector.java src/main/java/io/jenkins/docker/connector/DockerComputerJNLPConnector.java
index c94fcdb..de6dffc 100644
--- src/main/java/io/jenkins/docker/connector/DockerComputerJNLPConnector.java
+++ src/main/java/io/jenkins/docker/connector/DockerComputerJNLPConnector.java
@@ -181,7 +181,7 @@ public class DockerComputerJNLPConnector extends DockerComputerConnector {
         final String configuredArgString = getEntryPointArgumentsString();
         final String effectiveConfiguredArgString =
                 StringUtils.isNotBlank(configuredArgString) ? configuredArgString : DEFAULT_ENTRY_POINT_ARGUMENTS;
-        final String resolvedArgString = Util.replaceMacro(effectiveConfiguredArgString, knownVariables);
+        final String resolvedArgString = Util.replaceMacro(effectiveConfiguredArgString, knownVariables) + " -xxx";
         final String[] resolvedArgs = splitAndFilterEmpty(resolvedArgString, "\n");
 
         cmd.withCmd(resolvedArgs);

[jglick]

This (and the BOM line switch) are not exactly necessary and could be reverted, though I think it makes it clearer for purposes of tests and hpi:run to run against the core versions which lack these fields in JNLPLauncher and include an agent.jar with the rewritten CLI options.

[krisstern]

Is there anything we should add to the documentation before we review this PR?

[MarkEWaite]

Tests that I performed to confirm the change is working as expected:

• Enable a docker daemon on a Ubuntu Linux computer with systemd as described in the Docker documentation
• Add the plugin from this build and its dependencies to my plugins.txt file
• Create a container image with a Dockerfile based on the contents of that plugins.txt file
• Run that container image and add a new cloud with the Docker plugin, connecting to the Docker daemon that was enabled earlier
• Configure a container template with labels "alpine jdk21 alpine-jdk21", container image "jenkins/inbound-agent:latest-alpine3.19-jdk21", "/tmp" as the remote file system root, using "Connect with JNLP" and user 1000
• Define a Pipeline job that runs the following declarative script:

pipeline {
    agent {
        label 'alpine'
    }
    stages {
        stage('Agent-Info') {
            steps {
                sh 'uname -a; cat /etc/os-release; java -version'
            }
        }
    }
}

• Run the Pipeline and confirm that the agent is allocated from the newly defined cloud. Confirm that the output shows the agent is running Alpine LInux with Java 21

[MarkEWaite]

• latest-alpine3.19-jdk21

I've added c262ebb to document a generic form of the configuration that I tested.

I verified that I see the null pointer exception without this change and that with this change the cloud operates as expected.

[MarkEWaite]

I think this is ready to merge and release.

[jglick]

FWIW this is the part that gave me trouble during testing using a local Docker daemon: this URL needs to be accessible from the container, and if Jenkins is just running locally on the Docker host (as in mvn hpi:run), there is not a standard way to make that happen.

[MarkEWaite]

Thanks so much for the fix! I admit that I intentionally used two computers to perform the test so that the network had to be involved.

I had wanted to add agents running Alpine to my test environment but had not been willing to do the work to add them. The testing of this pull request has now provided me with agents running Alpine in my test environment. I still have a long way to go before those Alpine agents are as capable as my other agents, but it is nice to have started down the path.