palantir/docker-compose-rule

Port matching is breaks when docker-compose ps output changes

hirochri opened this issue · 6 comments

hirochri commented

What happened?

I was using the docker compose rule and found that after adding health checks to my docker-compose.yml the docker compose rule fails because it cannot it can't find the internal port for a random container and then triggers this exception:

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

Line 91 in a2adeae

.orElseThrow(() -> new IllegalArgumentException("No internal port '" + internalPort + "' for container '" + containerName + "': " + portMappings));

Did some digging, and came to the conclusion that this breaks because there is some flawed logic in the parsing of the ports from docker-compose output.

  1. We grab the process output from running docker-compose ps here and pass it into the parseFromDockerComposePs method:

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

    Line 228 in 446e363

    return Ports.parseFromDockerComposePs(psOutput(service), dockerMachine.getIp());

  2. Within parseFromDockerComposePs, a regex is used to match the ports within the output

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

    Line 30 in f07795c

    private static final Pattern PORT_PATTERN = Pattern.compile("((\\d+).(\\d+).(\\d+).(\\d+)):(\\d+)->(\\d+)/tcp");

When there are no health checks within the docker-compose.yml, the output often looks like this:

        Name                    Command           State           Ports         
--------------------------------------------------------------------------------
container.name.here   service/bin/linux-        Up      0.0.0.0:1234->1234/tcp
                        amd64/go ...

Here the regex matches successfully and pulls port 1234 out of 0.0.0.0:1234->1234/tcp

When there are health checks, the output for docker-compose ps gets shifted and truncated at a random spot, causing the port information to be split across multiple lines:

        Name                 Command             State              Ports       
--------------------------------------------------------------------------------
container.name.here   service/bin/linux-     Up (healthy)   0.0.0.0:1234->1234/
                         amd64/go ...                       tcp

This makes it so that the regex is unable to be matched, so no ports are returned from parseFromDockerComposePs, and then the entire docker compose rule fails (as mentioned above).

Here is a test case that confirms this is the issue:

 @Test
    public void testcase() {
        Pattern PORT_PATTERN = Pattern.compile("((\\d+).(\\d+).(\\d+).(\\d+)):(\\d+)->(\\d+)/tcp");

        String psOutput = "       Name                 Command             State              Ports        \n"
                + "--------------------------------------------------------------------------------\n"
                + "container.name.here   service/bin/linux-     Up (healthy)   0.0.0.0:1234->1234/t\n"
                + "                      amd64/go ...                          cp                  ";

        Matcher matcher = PORT_PATTERN.matcher(psOutput);
        System.out.println(matcher.find()); // False

        String psOutput2 = "       Name                 Command             State              Ports        \n"
                + "--------------------------------------------------------------------------------\n"
                + "container.name.here   service/bin/linux-     Up (healthy)   0.0.0.0:1234->1234/tcp\n" // Added -cp
                + "                      amd64/go ...                          cp                  ";

        Matcher matcher2 = PORT_PATTERN.matcher(psOutput2);
        System.out.println(matcher2.find()); // True
    }

What did you want to happen?

The parsing of the ports from the command line output should not fail because the whitespacing changed, it should be clever enough to pull out the port information from the output (since all the information is there). Either we do some manipulation of the output coming from docker-compose ps, or make the regex that parses it do a little more

hirochri commented

A good way to solve this problem would be to use the output of

docker ps --no-trunc --filter id=$(docker-compose ps -q <container_name>) --format "{{ .Ports }}"

Which will return 0.0.0.0:1234->1234/tcp, 0.0.0.0:4567->4567/tcp

This lets the regex keep working as expected

hirochri commented

Am working on a fix, will PR when its ready

CRogers commented

@hirochri Nice problem breakdown, I'm actually amazed the regex approach has lasted this long. Please tag me in the PR/message me internally when you need a review.

  • 👍1
tzyl commented

Unfortunately I am still seeing this issue on my local machine using docker-compose-rule 1.4.2 which has the fix from #424.

macOS Mojave 10.14.6
docker-compose version 1.25.1-rc1, build d92e9bee

Example docker ps output when debugging docker-compose-rule:

         Name                   Command           State           Ports
--------------------------------------------------------------------------------
container.name.here      /abc/something/hello/a   Up      11/tcp, 0.0.0.0:1234->
                         /b/c ...                         1234/tcp, 5678/tcp,
                                                          9999/tcp
CRogers commented

Hopefully this should be fixed in docker-compose 1.25.2, which has the release note:

Assume infinite terminal width when not running in a terminal

Docker for mac 2.2.0.0 is scheduled for release will include this fixed version of docker-compose. I think this only affects people on the edge channel of Docker-for-mac.

tzyl commented

Switched back from edge (docker-compose 1.25.1-rc1) to stable (docker-compose 1.25.2) and don't see the issue anymore!

  • 👍1