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

Fix Docker 'run' command #104

Closed
wants to merge 2 commits into from
Closed

Fix Docker 'run' command #104

wants to merge 2 commits into from

Conversation

sbach
Copy link
Contributor

@sbach sbach commented Dec 9, 2017

The official README.rst uses --network host thus giving the container full access to the host's networking. The following -p 5000:5000 thus becomes unnecessary.

After quick investigation, I would like to offer a different solution (note: the use of --network comes from #90).

Here are the ports used by the container:

/ # netstat -tupln
Active Internet connections (only servers)
Proto Recv-Q Send-Q Local Address           Foreign Address         State       PID/Program name    
tcp        0      0 127.0.0.1:5000          0.0.0.0:*               LISTEN      1/python2
tcp        0      0 0.0.0.0:34543           0.0.0.0:*               LISTEN      -
tcp        0      0 0.0.0.0:111             0.0.0.0:*               LISTEN      -
tcp        0      0 0.0.0.0:22              0.0.0.0:*               LISTEN      -
tcp        0      0 127.0.0.1:631           0.0.0.0:*               LISTEN      -
tcp        0      0 0.0.0.0:34359           0.0.0.0:*               LISTEN      -
tcp        0      0 0.0.0.0:45021           0.0.0.0:*               LISTEN      -
tcp        0      0 127.0.0.1:8001          0.0.0.0:*               LISTEN      -
tcp        0      0 0.0.0.0:2049            0.0.0.0:*               LISTEN      -
tcp        0      0 0.0.0.0:38691           0.0.0.0:*               LISTEN      -
tcp        0      0 :::43143                :::*                    LISTEN      -
tcp        0      0 :::111                  :::*                    LISTEN      -
tcp        0      0 :::51155                :::*                    LISTEN      -
tcp        0      0 :::22                   :::*                    LISTEN      -
tcp        0      0 ::1:631                 :::*                    LISTEN      -
tcp        0      0 :::34263                :::*                    LISTEN      -
tcp        0      0 :::2049                 :::*                    LISTEN      -
tcp        0      0 :::44259                :::*                    LISTEN      -

After isolating the required ports to be exposed (by changing the bind IP address) it looks like we only need 5000 and 631. However, I have yet to determine which piece of code listen on 631.

The first commit of this PR only cleanup the Docker command so it can be pasted in a shell without error(s).

@rfk
Copy link
Contributor

rfk commented Dec 10, 2017

Thanks for digging into this @sbach! I just want to note that I'm travelling for the next few weeks so it may take me a while before I can look at this in any detail, but I'm definitely interested in following up.

@sbach
Copy link
Contributor Author

sbach commented Dec 11, 2017

Bad state of my networking stack made that work. 631 isn't the required port.
@rfk I will investigate more and come with a real fix. Will re-open at that time.

@sbach sbach closed this Dec 11, 2017
@sbach
Copy link
Contributor Author

sbach commented Dec 11, 2017

Using tcpdump and strace, I validated this is not an issue of unpublished port and discovered that requests aren't received by gunicorn.

As gunicorn is listening on localhost:5000 (see README.rst) it is not binding to the container's IP address. That said, it will not receive requests from the outside. Using gunicorn --bind 0.0.0.0:5000 should be a safe fix.

I have updated my branch at: https://github.com/sbach/syncserver/tree/fix/docker

@rfk Can you validate the updated Docker command works on you side? I could then open a proper PR.

@rfk
Copy link
Contributor

rfk commented Dec 20, 2017

@sbach thanks, yes, it looks like this change should work. We recently found a similar issue with a trial docker deployment in production.

@sbach sbach mentioned this pull request Dec 29, 2017
Mic92 pushed a commit to Mic92/syncserver that referenced this pull request Feb 23, 2022
…sue-97

feat: fix extractors, middleware for e2e tests
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 this pull request may close these issues.

None yet

2 participants