Prospective fix for jenkinsci#757 and JENKINS-59790.

Refactor DockerTransientNode constructor so the launcher is provided
post-construction.
Refactor DockerComputerConnector.beforeContainerStarted(...) so it takes
a node (that holds a containerId) instead of a containerId.
Refactor DockerTemplate.doProvision(...) so that the DockerTransientNode
is created without a launcher before we start the container.
Enhance DockerComputerJNLPConnector so that it adds the node to Jenkins
immediately before the container is created, ensuring that it'll be
known to Jenkins (just) before the JNLP process starts.

src/main/java/com/nirima/jenkins/plugins/docker/DockerTemplate.java

@@ -604,12 +604,7 @@ private DockerTransientNode doProvisionNode(DockerAPI api, DockerClient client,
         LOGGER.info("Started container ID {} for node {} from image: {}", containerId, nodeName, getImage());
 
         try {
-            connector.beforeContainerStarted(api, remoteFs, containerId);
-            client.startContainerCmd(containerId).exec();
-            connector.afterContainerStarted(api, remoteFs, containerId);
-
-            final ComputerLauncher launcher = connector.createLauncher(api, containerId, remoteFs, listener);
-            final DockerTransientNode node = new DockerTransientNode(nodeName, containerId, remoteFs, launcher);
+            final DockerTransientNode node = new DockerTransientNode(nodeName, containerId, remoteFs);
             node.setNodeDescription("Docker Agent [" + getImage() + " on "+ api.getDockerHost().getUri() + " ID " + containerId + "]");
             node.setMode(mode);
             node.setLabelString(labelString);
@@ -618,6 +613,11 @@ private DockerTransientNode doProvisionNode(DockerAPI api, DockerClient client,
             node.setRemoveVolumes(removeVolumes);
             node.setStopTimeout(stopTimeout);
             node.setDockerAPI(api);
+            connector.beforeContainerStarted(api, remoteFs, node);
+            client.startContainerCmd(containerId).exec();
+            connector.afterContainerStarted(api, remoteFs, containerId);
+            final ComputerLauncher launcher = connector.createLauncher(api, containerId, remoteFs, listener);
+            node.setLauncher(launcher);
             finallyRemoveTheContainer = false;
             return node;
         } finally {

src/main/java/io/jenkins/docker/DockerTransientNode.java

@@ -47,8 +47,42 @@ public class DockerTransientNode extends Slave {
 
     private AtomicBoolean acceptingTasks = new AtomicBoolean(true);
 
-    public DockerTransientNode(@Nonnull String nodeName, String containerId, String workdir, ComputerLauncher launcher) throws Descriptor.FormException, IOException {
-        super(nodeName, workdir, launcher);
+    /**
+     * @deprecated Use {@link #DockerTransientNode(String, String, String)} then
+     *             {@link #setLauncher(ComputerLauncher)}.
+     * @param nodeName    Passed to
+     *                    {@link #DockerTransientNode(String, String, String)}.
+     * @param containerId Passed to
+     *                    {@link #DockerTransientNode(String, String, String)}.
+     * @param workdir     Passed to
+     *                    {@link #DockerTransientNode(String, String, String)}.
+     * @param launcher    Passed to {@link #setLauncher(ComputerLauncher)}
+     * @throws             Descriptor.FormException See
+     *                     {@link #DockerTransientNode(String, String, String)}.
+     * @throws IOException See {@link #DockerTransientNode(String, String, String)}.
+     */
+    @Deprecated
+    public DockerTransientNode(@Nonnull String nodeName, @Nonnull String containerId, String workdir, ComputerLauncher launcher) throws Descriptor.FormException, IOException {
+        this(nodeName, containerId, workdir);
+        setLauncher(launcher);
+    }
+
+    /**
+     * Preferred constructor. Note that, unless this is a JNLP node, callers will
+     * later have to call {@link #setLauncher(ComputerLauncher)}.
+     * 
+     * @param nodeName    Name of the node; passed to
+     *                    {@link Slave#Slave(String, String, ComputerLauncher)}.
+     * @param containerId Docker container id.
+     * @param workdir     remoteFs home dir; passed to
+     *                    {@link Slave#Slave(String, String, ComputerLauncher)}.
+     * @throws             Descriptor.FormException See
+     *                     {@link Slave#Slave(String, String, ComputerLauncher)}.
+     * @throws IOException See
+     *                     {@link Slave#Slave(String, String, ComputerLauncher)}.
+     */
+    public DockerTransientNode(@Nonnull String nodeName, @Nonnull String containerId, String workdir) throws Descriptor.FormException, IOException {
+        super(nodeName, workdir, null);
         this.containerId = containerId;
         setNumExecutors(1);
         setMode(Mode.EXCLUSIVE);

src/main/java/io/jenkins/docker/connector/DockerComputerConnector.java

@@ -10,9 +10,9 @@
 import hudson.remoting.Channel;
 import hudson.remoting.Which;
 import hudson.slaves.ComputerLauncher;
+import io.jenkins.docker.DockerTransientNode;
 import io.jenkins.docker.client.DockerAPI;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
+import jenkins.model.Jenkins;
 
 import javax.annotation.Nonnull;
 import java.io.File;
@@ -27,8 +27,6 @@
  */
 public abstract class DockerComputerConnector extends AbstractDescribableImpl<DockerComputerConnector> {
 
-    private static final Logger LOGGER = LoggerFactory.getLogger(DockerComputerConnector.class);
-
     protected static final File remoting;
 
     static {
@@ -48,7 +46,7 @@ public void beforeContainerCreated(DockerAPI api, String workdir, CreateContaine
      * Container has been created but not started yet, that's a good opportunity to inject <code>remoting.jar</code>
      * using {@link #injectRemotingJar(String, String, DockerClient)}
      */
-    public void beforeContainerStarted(DockerAPI api, String workdir, String containerId) throws IOException, InterruptedException {}
+    public void beforeContainerStarted(DockerAPI api, String workdir, DockerTransientNode node) throws IOException, InterruptedException {}
 
     /**
      * Container has started. Good place to check it's healthy before considering agent is ready to accept connexions
@@ -70,16 +68,22 @@ protected void ensureWaiting(CreateContainerCmd cmd) {
     }
 
     /**
-     * Utility method to copy remoting runtime into container on specified working directory
+     * Ensure that a DockerNode is known to Jenkins so that Jenkins will accept an incoming JNLP connection etc.
+     * @throws IOException if Jenkins is unable to persist the details.
      */
-    protected String injectRemotingJar(String containerId, String workdir, DockerClient client) throws IOException {
+    protected void ensureNodeIsKnown(DockerTransientNode node) throws IOException {
+        Jenkins.getInstance().addNode(node);
+    }
 
+    /**
+     * Utility method to copy remoting runtime into container on specified working directory
+     */
+    protected String injectRemotingJar(String containerId, String workdir, DockerClient client) {
         // Copy slave.jar into container
         client.copyArchiveToContainerCmd(containerId)
                 .withHostResource(remoting.getAbsolutePath())
                 .withRemotePath(workdir)
                 .exec();
-
         return workdir + '/' + remoting.getName();
     }
 
@@ -104,5 +108,4 @@ public final ComputerLauncher createLauncher(final DockerAPI api, @Nonnull final
      * DockerAgentConnector so adequate setup did take place.
      */
     protected abstract ComputerLauncher createLauncher(DockerAPI api, String workdir, InspectContainerResponse inspect, TaskListener listener) throws IOException, InterruptedException;
-
 }

src/main/java/io/jenkins/docker/connector/DockerComputerJNLPConnector.java

@@ -16,6 +16,7 @@
 import hudson.slaves.JNLPLauncher;
 import hudson.slaves.NodeProperty;
 import hudson.util.LogTaskListener;
+import io.jenkins.docker.DockerTransientNode;
 import io.jenkins.docker.client.DockerAPI;
 import io.jenkins.docker.client.DockerEnvUtils;
 import jenkins.model.Jenkins;
@@ -169,7 +170,14 @@ public void beforeContainerCreated(DockerAPI api, String workdir, CreateContaine
     }
 
     @Override
-    public void afterContainerStarted(DockerAPI api, String workdir, String containerId) throws IOException, InterruptedException {
+    public void beforeContainerStarted(DockerAPI api, String workdir, DockerTransientNode node)
+            throws IOException, InterruptedException {
+        // For JNLP, we need to have the Jenkins Node known to Jenkins as a valid JNLP
+        // node before the container starts, otherwise it might get started before
+        // Jenkins is ready for it.
+        // That's why we explicitly add the node here instead of allowing the cloud
+        // provisioning process to add it later.
+        ensureNodeIsKnown(node);
     }
 
     private EnvVars calculateVariablesForVariableSubstitution(final String nodeName, final String secret,

src/main/java/io/jenkins/docker/connector/DockerComputerSSHConnector.java

@@ -26,6 +26,7 @@
 import hudson.security.ACL;
 import hudson.slaves.ComputerLauncher;
 import hudson.util.ListBoxModel;
+import io.jenkins.docker.DockerTransientNode;
 import io.jenkins.docker.client.DockerAPI;
 import jenkins.bouncycastle.api.PEMEncodable;
 import jenkins.model.Jenkins;
@@ -202,9 +203,10 @@ public void beforeContainerCreated(DockerAPI api, String workdir, CreateContaine
     }
 
     @Override
-    public void beforeContainerStarted(DockerAPI api, String workdir, String containerId) throws IOException, InterruptedException {
+    public void beforeContainerStarted(DockerAPI api, String workdir, DockerTransientNode node) throws IOException, InterruptedException {
         final String key = sshKeyStrategy.getInjectedKey();
         if (key != null) {
+            final String containerId = node.getContainerId();
             String AuthorizedKeysCommand = "#!/bin/sh\n"
                     + "[ \"$1\" = \"" + sshKeyStrategy.getUser() + "\" ] "
                     + "&& echo '" + key + "'"