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

Report correct port even when dynamically allocated #297

Open
davidjsherman opened this issue Feb 14, 2021 · 5 comments
Open

Report correct port even when dynamically allocated #297

davidjsherman opened this issue Feb 14, 2021 · 5 comments

Comments

@davidjsherman
Copy link

The services launcher reports the requested port regardless of which port was actually allocated by the operating system.

When launching several service instances on the same machine, it is better to allow the OS to dynamically choose a port from the IANA ephemeral range, than to guess at what port numbers haven't been used. Tornado will happily bind an ephemeral port if port 0 is requested.

To verify that this works with poppy-services, run

$ poppy-services --poppy-simu --http --http-port 0 poppy-ergo-jr &
$ sleep 10 ; ss -tlp
State  Recv-Q Send-Q Local Address:Port   Peer Address:Port                                         
LISTEN 0      128          0.0.0.0:42961       0.0.0.0:*     users:(("poppy-services",pid=70,fd=7)) 
LISTEN 0      128             [::]:42961          [::]:*     users:(("poppy-services",pid=70,fd=8)) 

The services launcher should retrieve the list of bound sockets from Tornado, instead of reporting the port that was requested

if args.http or args.poppy_simu: msg+= ' http_port={},'.format(args.http_port)

@davidjsherman
Copy link
Author

I'm not sure where to put the regression test for this. I suppose ci/run_tests.sh?

So, as a shell fragment:

bash -c 'stdbuf -o0 poppy-services --poppy-simu --http --http-port 0 poppy-ergo-jr & pid=$! ; \
    sleep 20 ; \
    echo real=$(ss -tlp | grep pid=$pid | perl -ne '\''print $p if (($p)=($_=~m/0\.0\.0\.0:(\d+)/g))'\'') ; \
    kill $pid' | 
perl -ne 'END{exit($X{port}!=$X{real})} $X{$k}=$v if (($k,$v)=($_=~m/(port|real)=(\d+)/g))' && 
echo PORTS AGREE || echo PORTS MISMATCH

🙄

@davidjsherman
Copy link
Author

This would be easier if you had a clirunner to capture the program output!

@ymollard
Copy link
Member

Is it a recommendation or have you met a situation in which this port display was problematic? When using 0 to open many robot instances, maybe?

In order to implement a test, I would go for a dedicated test framework instead of custom bash code in the CI script. Then the CI script would invoke that framework. Maybe a good occasion to migrate to the clirunner and then implement the tests with the unittests framework from Python? 0:)

@davidjsherman
Copy link
Author

There are two cases I have run into.

  1. When running independent tests in parallel on the same machine, each with a simulator as a test fixture. It would be easier to rely on the fixture for the port number, than to discover the allocated port through an OS-specific mechanism.
  2. When running a pool of simulated robots on a computer that has other services running in the 8080 range. Allowing dynamic port allocation makes it unnecessary to globally negotiate a port range with the other users.

Dynamic allocation does work, so reporting should be correct. If I start ten simulators with port 0, each reports the requested port as “0”

for i in $(yes|head -10); do poppy-services --poppy-simu --http --http-port 0 poppy-ergo-jr & sleep 2; done
[1] 75783
Attempt 1 to start the robot...
Simulated robot created! He is running on: ip=10.0.0.109
Server started on: http_port=0.
[2] 75785
Attempt 1 to start the robot...
Simulated robot created! He is running on: ip=10.0.0.109
Server started on: http_port=0.
[3] 75788
Attempt 1 to start the robot...
Simulated robot created! He is running on: ip=10.0.0.109
Server started on: http_port=0.
[...]

but each process is listening on a real port allocated from the ephemeral range

$ lsof -i4 -sTCP:LISTEN -n -P -R | grep ^Python | cat -n
     1	Python    75783 75505 sherman   10u  IPv4 0x8087a0bd505de48b      0t0  TCP *:52978 (LISTEN)
     2	Python    75785 75505 sherman    8u  IPv4 0x8087a0bd50fef123      0t0  TCP *:52979 (LISTEN)
     3	Python    75788 75505 sherman    8u  IPv4 0x8087a0bd510e5aab      0t0  TCP *:52981 (LISTEN)
     4	Python    75790 75505 sherman   10u  IPv4 0x8087a0bd3d1c8aab      0t0  TCP *:52982 (LISTEN)
     5	Python    75792 75505 sherman    8u  IPv4 0x8087a0bd50f7548b      0t0  TCP *:52984 (LISTEN)
     6	Python    75794 75505 sherman    8u  IPv4 0x8087a0bd5056a79b      0t0  TCP *:52986 (LISTEN)
     7	Python    75796 75505 sherman    8u  IPv4 0x8087a0bd510ea79b      0t0  TCP *:52987 (LISTEN)
     8	Python    75798 75505 sherman   11u  IPv4 0x8087a0bd510eb123      0t0  TCP *:52988 (LISTEN)
     9	Python    75801 75505 sherman   10u  IPv4 0x8087a0bd4ba7d79b      0t0  TCP *:52989 (LISTEN)
    10	Python    75803 75505 sherman    8u  IPv4 0x8087a0bd504d8aab      0t0  TCP *:52990 (LISTEN)

so it is wrong to report the requested port rather than the allocated port.

@davidjsherman
Copy link
Author

We have cookiecutter templates that set up tox environments for testing new Python projects. Out of the box our configurations use pytest / pytest-bdd rather than unittest and discover.

I can see whether our environment could be adapted. Theoretically, tox can use unittest2 and discover, but I have no experience with that.

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

No branches or pull requests

2 participants