Improve ports parsing

docker-compose-rule-core/src/main/java/com/palantir/docker/compose/DockerComposeManager.java

@@ -126,7 +126,7 @@ public ShutdownStrategy shutdownStrategy() {
     @Value.Default
     public com.palantir.docker.compose.execution.DockerCompose dockerCompose() {
         com.palantir.docker.compose.execution.DockerCompose
-                dockerCompose = new DefaultDockerCompose(dockerComposeExecutable(), machine());
+                dockerCompose = new DefaultDockerCompose(dockerComposeExecutable(), dockerExecutable(), machine());
         return new RetryingDockerCompose(retryAttempts(), dockerCompose);
     }
 

docker-compose-rule-core/src/main/java/com/palantir/docker/compose/connection/Ports.java

@@ -15,8 +15,6 @@
  */
 package com.palantir.docker.compose.connection;
 
-import com.google.common.base.Preconditions;
-import com.google.common.base.Strings;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
@@ -48,9 +46,8 @@ public Stream<DockerPort> stream() {
         return ports.stream();
     }
 
-    public static Ports parseFromDockerComposePs(String psOutput, String dockerMachineIp) {
-        Preconditions.checkArgument(!Strings.isNullOrEmpty(psOutput), "No container found");
-        Matcher matcher = PORT_PATTERN.matcher(psOutput);
+    public static Ports parseFromPortInformation(String portInformation, String dockerMachineIp) {
+        Matcher matcher = PORT_PATTERN.matcher(portInformation);
         List<DockerPort> ports = new ArrayList<>();
         while (matcher.find()) {
             String matchedIpAddress = matcher.group(IP_ADDRESS);

docker-compose-rule-core/src/main/java/com/palantir/docker/compose/execution/DefaultDockerCompose.java

@@ -19,8 +19,10 @@
 import static org.joda.time.Duration.standardMinutes;
 
 import com.github.zafarkhaja.semver.Version;
+import com.google.common.base.Joiner;
 import com.google.common.base.Strings;
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Lists;
 import com.palantir.docker.compose.configuration.DockerComposeFiles;
 import com.palantir.docker.compose.configuration.ProjectName;
 import com.palantir.docker.compose.connection.Container;
@@ -45,102 +47,111 @@ public class DefaultDockerCompose implements DockerCompose {
     private static final Duration LOG_TIMEOUT = standardMinutes(1);
     private static final Logger log = LoggerFactory.getLogger(DefaultDockerCompose.class);
 
-    private final Command command;
+    private final Command dockerComposeCommand;
+    private final DockerComposeExecutable dockerComposeRawExecutable;
+    private final Command dockerCommand;
     private final DockerMachine dockerMachine;
-    private final DockerComposeExecutable rawExecutable;
 
 
     public DefaultDockerCompose(DockerComposeFiles dockerComposeFiles, DockerMachine dockerMachine, ProjectName projectName) {
         this(DockerComposeExecutable.builder()
                 .dockerComposeFiles(dockerComposeFiles)
                 .dockerConfiguration(dockerMachine)
                 .projectName(projectName)
-                .build(), dockerMachine);
+                .build(),
+                DockerExecutable.builder()
+                        .dockerConfiguration(dockerMachine)
+                .build(),
+                dockerMachine);
     }
 
-    public DefaultDockerCompose(DockerComposeExecutable rawExecutable, DockerMachine dockerMachine) {
-        this.rawExecutable = rawExecutable;
-        this.command = new Command(rawExecutable, log::trace);
+    public DefaultDockerCompose(
+            DockerComposeExecutable dockerComposeRawExecutable,
+            DockerExecutable dockerRawExecutable,
+            DockerMachine dockerMachine) {
+        this.dockerComposeRawExecutable = dockerComposeRawExecutable;
+        this.dockerComposeCommand = new Command(dockerComposeRawExecutable, log::trace);
+        this.dockerCommand = new Command(dockerRawExecutable, log::trace);
         this.dockerMachine = dockerMachine;
     }
 
     @Override
     public void pull() throws IOException, InterruptedException {
-        command.execute(Command.throwingOnError(), "pull");
+        dockerComposeCommand.execute(Command.throwingOnError(), "pull");
     }
 
     @Override
     public void build() throws IOException, InterruptedException {
-        command.execute(Command.throwingOnError(), "build");
+        dockerComposeCommand.execute(Command.throwingOnError(), "build");
     }
 
     @Override
     public void up() throws IOException, InterruptedException {
-        command.execute(Command.throwingOnError(), "up", "-d");
+        dockerComposeCommand.execute(Command.throwingOnError(), "up", "-d");
     }
 
     @Override
     public void down() throws IOException, InterruptedException {
-        command.execute(swallowingDownCommandDoesNotExist(), "down", "--volumes");
+        dockerComposeCommand.execute(swallowingDownCommandDoesNotExist(), "down", "--volumes");
     }
 
     @Override
     public void stop() throws IOException, InterruptedException {
-        command.execute(Command.throwingOnError(), "stop");
+        dockerComposeCommand.execute(Command.throwingOnError(), "stop");
 
     }
 
     @Override
     public void kill() throws IOException, InterruptedException {
-        command.execute(Command.throwingOnError(), "kill");
+        dockerComposeCommand.execute(Command.throwingOnError(), "kill");
     }
 
     @Override
     public void rm() throws IOException, InterruptedException {
-        command.execute(Command.throwingOnError(), "rm", "--force", "-v");
+        dockerComposeCommand.execute(Command.throwingOnError(), "rm", "--force", "-v");
     }
 
     @Override
     public void up(Container container) throws IOException, InterruptedException {
-        command.execute(Command.throwingOnError(), "up", "-d",  container.getContainerName());
+        dockerComposeCommand.execute(Command.throwingOnError(), "up", "-d",  container.getContainerName());
     }
 
     @Override
     public void start(Container container) throws IOException, InterruptedException {
-        command.execute(Command.throwingOnError(), "start", container.getContainerName());
+        dockerComposeCommand.execute(Command.throwingOnError(), "start", container.getContainerName());
     }
 
     @Override
     public void stop(Container container) throws IOException, InterruptedException {
-        command.execute(Command.throwingOnError(), "stop", container.getContainerName());
+        dockerComposeCommand.execute(Command.throwingOnError(), "stop", container.getContainerName());
     }
 
     @Override
     public void kill(Container container) throws IOException, InterruptedException {
-        command.execute(Command.throwingOnError(), "kill", container.getContainerName());
+        dockerComposeCommand.execute(Command.throwingOnError(), "kill", container.getContainerName());
     }
 
     @Override
     public String exec(DockerComposeExecOption dockerComposeExecOption, String containerName,
             DockerComposeExecArgument dockerComposeExecArgument) throws IOException, InterruptedException {
         verifyDockerComposeVersionAtLeast(VERSION_1_7_0, "You need at least docker-compose 1.7 to run docker-compose exec");
         String[] fullArgs = constructFullDockerComposeExecArguments(dockerComposeExecOption, containerName, dockerComposeExecArgument);
-        return command.execute(Command.throwingOnError(), fullArgs);
+        return dockerComposeCommand.execute(Command.throwingOnError(), fullArgs);
     }
 
     @Override
     public String run(DockerComposeRunOption dockerComposeRunOption, String containerName,
             DockerComposeRunArgument dockerComposeRunArgument) throws IOException, InterruptedException {
         String[] fullArgs = constructFullDockerComposeRunArguments(dockerComposeRunOption, containerName, dockerComposeRunArgument);
-        return command.execute(Command.throwingOnError(), fullArgs);
+        return dockerComposeCommand.execute(Command.throwingOnError(), fullArgs);
     }
 
     private void verifyDockerComposeVersionAtLeast(Version targetVersion, String message) throws IOException, InterruptedException {
         validState(version().greaterThanOrEqualTo(targetVersion), message);
     }
 
     private Version version() throws IOException, InterruptedException {
-        String versionOutput = command.execute(Command.throwingOnError(), "-v");
+        String versionOutput = dockerComposeCommand.execute(Command.throwingOnError(), "-v");
         return DockerComposeVersion.parseFromDockerComposeVersion(versionOutput);
     }
 
@@ -170,7 +181,7 @@ private static String[] constructFullDockerComposeRunArguments(DockerComposeRunO
 
     @Override
     public List<ContainerName> ps() throws IOException, InterruptedException {
-        String psOutput = command.execute(Command.throwingOnError(), "ps");
+        String psOutput = dockerComposeCommand.execute(Command.throwingOnError(), "ps");
         return ContainerNames.parseFromDockerComposePs(psOutput);
     }
 
@@ -181,12 +192,12 @@ public Optional<String> id(Container container) throws IOException, InterruptedE
 
     @Override
     public String config() throws IOException, InterruptedException {
-        return command.execute(Command.throwingOnError(), "config");
+        return dockerComposeCommand.execute(Command.throwingOnError(), "config");
     }
 
     @Override
     public List<String> services() throws IOException, InterruptedException {
-        String servicesOutput = command.execute(Command.throwingOnError(), "config", "--services");
+        String servicesOutput = dockerComposeCommand.execute(Command.throwingOnError(), "config", "--services");
         return Arrays.asList(servicesOutput.split("(\r|\n)+"));
     }
 
@@ -210,7 +221,7 @@ public boolean writeLogs(String container, OutputStream output) throws IOExcepti
     }
 
     private Optional<String> id(String containerName) throws IOException, InterruptedException {
-        String id = command.execute(Command.throwingOnError(), "ps", "-q", containerName);
+        String id = dockerComposeCommand.execute(Command.throwingOnError(), "ps", "-q", containerName);
         if (id.isEmpty()) {
             return Optional.empty();
         }
@@ -220,12 +231,20 @@ private Optional<String> id(String containerName) throws IOException, Interrupte
     private Process logs(String container) throws IOException, InterruptedException {
         verifyDockerComposeVersionAtLeast(VERSION_1_7_0,
                 "You need at least docker-compose 1.7 to run docker-compose logs");
-        return rawExecutable.execute("logs", "--no-color", container);
+        return dockerComposeRawExecutable.execute("logs", "--no-color", container);
     }
 
     @Override
     public Ports ports(String service) throws IOException, InterruptedException {
-        return Ports.parseFromDockerComposePs(psOutput(service), dockerMachine.getIp());
+        validState(!Strings.isNullOrEmpty(service), String.format("Service cannot be empty"));
+
+        String containerId = dockerComposeCommand.execute(swallowingServiceDoesNotExist(), "ps", "-q", service);
+        validState(!Strings.isNullOrEmpty(containerId), String.format("No container ID found for service with name '%s'.", service));
+
+        String portInformation = dockerCommand.execute(Command.throwingOnError(),
+                "ps", "--no-trunc", "--format", "{{.Ports}}", "--filter", String.format("id=%s", containerId));
+
+        return Ports.parseFromPortInformation(portInformation, dockerMachine.getIp());
     }
 
     private static ErrorHandler swallowingDownCommandDoesNotExist() {
@@ -244,9 +263,20 @@ private static boolean downCommandWasPresent(String output) {
         return !output.contains("No such command");
     }
 
-    private String psOutput(String service) throws IOException, InterruptedException {
-        String psOutput = command.execute(Command.throwingOnError(), "ps", service);
-        validState(!Strings.isNullOrEmpty(psOutput), "No container with name '" + service + "' found");
-        return psOutput;
+    private static ErrorHandler swallowingServiceDoesNotExist() {
+        return (exitCode, output, commandName, commands) -> {
+            if (exitCode == 1 && output.isEmpty()) {
+                // Note: This (badly) checks if the command most likely failed due to
+                // the service not existing. If the ErrorHandler had access to
+                // error output, the proper way to check this would be to inspect
+                // the error output for "ERROR: No such service: <service_name>"
+
+                String fullCommand = Joiner.on(" ").join(Lists.asList(commandName, commands));
+                String serviceName = commands[commands.length - 1];
+
+                log.warn("It looks like `{}` failed.", fullCommand);
+                log.warn("This probably happened because no `{}` service exists.", serviceName);
+            }
+        };
     }
 }

docker-compose-rule-core/src/test/java/com/palantir/docker/compose/connection/PortsShould.java

@@ -31,63 +31,52 @@ public class PortsShould {
     public ExpectedException exception = ExpectedException.none();
 
     @Test
-    public void result_in_no_ports_when_there_are_no_ports_in_ps_output() {
-        String psOutput = "------";
-        Ports ports = Ports.parseFromDockerComposePs(psOutput, null);
+    public void result_in_no_ports_when_there_are_no_ports_in_port_information() {
+        String portInformation = "";
+        Ports ports = Ports.parseFromPortInformation(portInformation, LOCALHOST_IP);
         Ports expected = new Ports(emptyList());
         assertThat(ports, is(expected));
     }
 
     @Test
     public void result_in_single_port_when_there_is_single_tcp_port_mapping() {
-        String psOutput = "0.0.0.0:5432->5432/tcp";
-        Ports ports = Ports.parseFromDockerComposePs(psOutput, LOCALHOST_IP);
+        String portInformation = "0.0.0.0:5432->5432/tcp";
+        Ports ports = Ports.parseFromPortInformation(portInformation, LOCALHOST_IP);
         Ports expected = new Ports(newArrayList(new DockerPort(LOCALHOST_IP, 5432, 5432)));
         assertThat(ports, is(expected));
     }
 
     @Test
-    public void
-            result_in_single_port_with_ip_other_than_localhost_when_there_is_single_tcp_port_mapping() {
-        String psOutput = "10.0.1.2:1234->2345/tcp";
-        Ports ports = Ports.parseFromDockerComposePs(psOutput, LOCALHOST_IP);
+    public void result_in_single_port_with_ip_other_than_localhost_when_there_is_single_tcp_port_mapping() {
+        String portInformation = "10.0.1.2:1234->2345/tcp";
+        Ports ports = Ports.parseFromPortInformation(portInformation, LOCALHOST_IP);
         Ports expected = new Ports(newArrayList(new DockerPort("10.0.1.2", 1234, 2345)));
         assertThat(ports, is(expected));
     }
 
     @Test
     public void result_in_two_ports_when_there_are_two_tcp_port_mappings() {
-        String psOutput = "0.0.0.0:5432->5432/tcp, 0.0.0.0:5433->5432/tcp";
-        Ports ports = Ports.parseFromDockerComposePs(psOutput, LOCALHOST_IP);
+        String portInformation = "0.0.0.0:5432->5432/tcp, 0.0.0.0:5433->5432/tcp";
+        Ports ports = Ports.parseFromPortInformation(portInformation, LOCALHOST_IP);
         Ports expected = new Ports(newArrayList(new DockerPort(LOCALHOST_IP, 5432, 5432),
                                                 new DockerPort(LOCALHOST_IP, 5433, 5432)));
         assertThat(ports, is(expected));
     }
 
     @Test
     public void result_in_no_ports_when_there_is_a_non_mapped_exposed_port() {
-        String psOutput = "5432/tcp";
-        Ports ports = Ports.parseFromDockerComposePs(psOutput, LOCALHOST_IP);
+        String portInformation = "5432/tcp";
+        Ports ports = Ports.parseFromPortInformation(portInformation, LOCALHOST_IP);
         Ports expected = new Ports(emptyList());
         assertThat(ports, is(expected));
     }
 
     @Test
-    public void parse_actual_docker_compose_output() {
-        String psOutput =
-                  "       Name                      Command               State                                         Ports                                        \n"
-                + "-------------------------------------------------------------------------------------------------------------------------------------------------\n"
-                + "postgres_postgres_1   /bin/sh -c /usr/local/bin/ ...   Up      0.0.0.0:8880->8880/tcp, 8881/tcp, 8882/tcp, 8883/tcp, 8884/tcp, 8885/tcp, 8886/tcp \n"
-                + "";
-        Ports ports = Ports.parseFromDockerComposePs(psOutput, LOCALHOST_IP);
-        Ports expected = new Ports(newArrayList(new DockerPort(LOCALHOST_IP, 8880, 8880)));
+    public void result_in_a_variety_of_ports_when_there_are_many_ports_in_port_information() {
+        String portInformation = "0.0.0.0:8880->8880/tcp, 0.0.0.0:8881->8881/tcp, 8882/tcp, 8883/tcp, 8884/tcp";
+        Ports ports = Ports.parseFromPortInformation(portInformation, LOCALHOST_IP);
+        Ports expected = new Ports(newArrayList(new DockerPort(LOCALHOST_IP, 8880, 8880),
+                                                new DockerPort(LOCALHOST_IP, 8881, 8881)));
         assertThat(ports, is(expected));
     }
-
-    @Test
-    public void throw_illegal_state_exception_when_no_running_container_found_for_service() {
-        exception.expect(IllegalArgumentException.class);
-        exception.expectMessage("No container found");
-        Ports.parseFromDockerComposePs("", "");
-    }
 }