From 2ca502cc4a3029a76b3d0ded39a5bb407b990c0d Mon Sep 17 00:00:00 2001 From: Stephan Jauernick Date: Sat, 1 Jan 2022 22:23:42 +0100 Subject: [PATCH] WIP/RFC fix windows docker containers - ensureWaiting/injection of the agent --- .../DockerComputerAttachConnector.java | 33 +++++++++++++-- .../connector/DockerComputerConnector.java | 41 +++++++++++++++---- 2 files changed, 63 insertions(+), 11 deletions(-) diff --git a/src/main/java/io/jenkins/docker/connector/DockerComputerAttachConnector.java b/src/main/java/io/jenkins/docker/connector/DockerComputerAttachConnector.java index b9c002b9..a8c1c95c 100644 --- a/src/main/java/io/jenkins/docker/connector/DockerComputerAttachConnector.java +++ b/src/main/java/io/jenkins/docker/connector/DockerComputerAttachConnector.java @@ -45,6 +45,7 @@ import java.util.Arrays; import java.util.Collection; import java.util.Objects; +import java.util.logging.Logger; import javax.annotation.CheckForNull; import javax.annotation.Nonnull; @@ -53,6 +54,7 @@ * @author Nicolas De Loof */ public class DockerComputerAttachConnector extends DockerComputerConnector implements Serializable { + private static final Logger LOGGER = Logger.getLogger(DockerComputerAttachConnector.class.getName()); @CheckForNull private String user; @@ -157,14 +159,37 @@ public String toString() { @Override public void beforeContainerCreated(DockerAPI api, String workdir, CreateContainerCmd cmd) throws IOException, InterruptedException { - ensureWaiting(cmd); + // we pass the workdir into ensureWaiting, so we can run windows detection there + ensureWaiting(workdir, cmd); + } + + @Override + public void beforeContainerStarted(@Nonnull DockerAPI api, @Nonnull String workdir, @Nonnull DockerTransientNode node) throws IOException, InterruptedException { + super.beforeContainerStarted(api, workdir, node); + + // hyper-v virtualised containers can't be access while they are running, so copy the files before starting the container + // TODO: find a better way to detect windows containers than matching on the workdin being c:\bla* + if(workdir.matches("^.:.*")) { + LOGGER.info("This appears to be a windows container, so we will try to inject the remoting jar into the stopped container"); + + // TODO: move this and the similar snippet from afterContainerStarted into a seperate function? + final String containerId = node.getContainerId(); + try(final DockerClient client = api.getClient()) { + injectRemotingJar(containerId, workdir, client); + } + } } @Override public void afterContainerStarted(DockerAPI api, String workdir, DockerTransientNode node) throws IOException, InterruptedException { - final String containerId = node.getContainerId(); - try(final DockerClient client = api.getClient()) { - injectRemotingJar(containerId, workdir, client); + // TODO: find out & decide if theres actual reason for doing the injecting in afterContainerStarted instead of beforeContainerStarted on !windows containers + // TODO: find a better way to detect windows containers than matching on the workdin being c:\bla* + if(!workdir.matches("^.:.*")) { + LOGGER.info("This appears to be a non windows container, so we will try to inject the remoting jar into the running container"); + final String containerId = node.getContainerId(); + try (final DockerClient client = api.getClient()) { + injectRemotingJar(containerId, workdir, client); + } } } diff --git a/src/main/java/io/jenkins/docker/connector/DockerComputerConnector.java b/src/main/java/io/jenkins/docker/connector/DockerComputerConnector.java index aaaf1343..11600269 100644 --- a/src/main/java/io/jenkins/docker/connector/DockerComputerConnector.java +++ b/src/main/java/io/jenkins/docker/connector/DockerComputerConnector.java @@ -27,6 +27,7 @@ import java.io.File; import java.io.IOException; +import java.util.Arrays; import java.util.logging.Level; import java.util.logging.Logger; @@ -108,16 +109,35 @@ public void afterContainerStarted(@Nonnull DockerAPI api, @Nonnull String workdi /** * Ensure container is already set with a command, or set one to make it wait * indefinitely - * + * + * @param workdir * @param cmd The {@link CreateContainerCmd} to be adjusted. */ - protected void ensureWaiting(@Nonnull CreateContainerCmd cmd) { + protected void ensureWaiting(String workdir, @Nonnull CreateContainerCmd cmd) { final String[] cmdAlreadySet = cmd.getCmd(); + if (cmdAlreadySet == null || cmdAlreadySet.length == 0) { - // no command has been set, we need one that will just hang. Typically "sh" waiting for stdin - cmd.withCmd("/bin/sh") - .withTty(true) - .withAttachStdin(false); + // on windows we can't rely on /bin/sh being present, so we use cmd.exe to ensure a process is waiting + // TODO: same here, refined detection mechanism if possible + if(workdir.matches("^.:.*")) { + LOGGER.info("This appears to be a windows container, so we ensure waiting with cmd.exe"); + + cmd.withCmd("cmd.exe", "/K", "echo", "running") + .withTty(true) + .withAttachStdin(false); + } + else { + LOGGER.info("ensure waiting with sh cmd"); + // no command has been set, we need one that will just hang. Typically "sh" waiting for stdin + + cmd.withCmd("/bin/sh") + .withTty(true) + .withAttachStdin(false); + } + } + else + { + LOGGER.info(String.format("ensure waiting cmd: '%s'", Arrays.toString(cmdAlreadySet))); } } @@ -146,11 +166,18 @@ protected void ensureNodeIsKnown(DockerTransientNode node) throws IOException { */ protected String injectRemotingJar(@Nonnull String containerId, @Nonnull String workdir, @Nonnull DockerClient client) { // Copy agent.jar into container + String resultingPath = workdir + '/' + remoting.getName(); + + LOGGER.info(String.format("COPYING REMOTING containerId: '%s' absolutePath: '%s' workdir: '%s' result: '%s'", containerId, remoting.getAbsolutePath(), workdir, resultingPath)); + client.copyArchiveToContainerCmd(containerId) .withHostResource(remoting.getAbsolutePath()) .withRemotePath(workdir) .exec(); - return workdir + '/' + remoting.getName(); + + LOGGER.info("finsihed copying remoting"); + + return resultingPath; } @Restricted(NoExternalUse.class)