Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Port matching is breaks when docker-compose ps output changes #411

Open
hirochri opened this issue Dec 4, 2019 · 6 comments
Open

Port matching is breaks when docker-compose ps output changes #411

hirochri opened this issue Dec 4, 2019 · 6 comments

Comments

@hirochri
Copy link

hirochri commented Dec 4, 2019

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:

.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:

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

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

    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
Copy link
Author

hirochri commented Dec 4, 2019

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
Copy link
Author

hirochri commented Dec 4, 2019

Am working on a fix, will PR when its ready

@CRogers
Copy link
Contributor

CRogers commented Dec 5, 2019

@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.

@tzyl
Copy link

tzyl commented Jan 22, 2020

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
Copy link
Contributor

CRogers commented Jan 22, 2020

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
Copy link

tzyl commented Jan 23, 2020

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants