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

Describe how to run UDP and TCP with port-share in parallel #373

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

usr42
Copy link

@usr42 usr42 commented Apr 16, 2018

I've added a brief explanation how to run a server with UDP and a secondary with TCP and port-share in parallel.

@usr42
Copy link
Author

usr42 commented May 9, 2018

@kylemanna Do you need anything else for this PR?

docs/tcp.md Outdated
@@ -21,7 +21,7 @@ specified protocol, adjust the mapping appropriately:
## Running a Second Fallback TCP Container
Instead of choosing between UDP and TCP, you can use both. A single instance of OpenVPN can only listen for a single protocol on a single port, but this image makes it easy to run two instances simultaneously. After building, configuring, and starting a standard container listening for UDP traffic on 1194, you can start a second container listening for tcp traffic on port 443:

docker run -v $OVPN_DATA:/etc/openvpn --rm -p 443:1194/tcp --privileged kylemanna/openvpn ovpn_run --proto tcp
docker run -v $OVPN_DATA:/etc/openvpn -d -p 443:1194/tcp --privileged kylemanna/openvpn ovpn_run --proto tcp
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change needs to go away. Why did this change?

Copy link
Author

@usr42 usr42 May 10, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without the -d parameter you start the container in the foreground. I would expect the use case is to run the OpenVPN server as a daemon in the background.
Is there a reason, why you start the TCP container in the foreground, but the UDP one as a daemon in the background?

If you want to delete the container after closing (--rm) is a matter of taste. For a randomly named container like here (without --name) I would probably also use --rm and start a new randomly named container when needed.

So, what about:

docker run -v $OVPN_DATA:/etc/openvpn --rm -d -p 443:1194/tcp --privileged \
kylemanna/openvpn ovpn_run --proto tcp

(with --rm and -d)?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're changing a line that has nothing to do with your pull request. There are other pages dealing with properly running Docker containers with init systems.

Please review the CONTRIBUTING document.

Commits should be short and concise, squashed if appropriate. Shouldn't change extraneous things.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right. This change is unrelated to the rest of the PR. So, I've reverted it.
I still think it is a valid change, so maybe you will fix it by your self.

For me the overhead of opening a second PR for a minimal documentation change (just 3 characters), is not sensible and not worth the effort.

Frankly, I don't get, why you are so fussy about splitting into separate PRs. Unfortunately, you did not tell my, if you think the change is a good idea or why not. Probably you have good reasons for that.
From my point of view, the change is completely unrelated to running Docker containers with init systems. I'm using a docker-compose approach.

Still, thank you very much for the awesome docker image! Great work! I'm really happy to benefit from this image.
But sadly, for opening another (minimal documentation) PR I will think about twice :(.

To still make it happen, don't add the `port-share` option to the configuration (or remove it, if already done) and run the TCP server with following command:

docker run -v $OVPN_DATA:/etc/openvpn -d -p 443:1194/tcp --cap-add=NET_ADMIN kylemanna/openvpn \
ovpn_run --proto tcp --port-share FORWARD-SERVER-IP FORWARD-SERVER-IP

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should be:

Suggested change
ovpn_run --proto tcp --port-share FORWARD-SERVER-IP FORWARD-SERVER-IP
ovpn_run --proto tcp --port-share FORWARD-SERVER-IP FORWARD-SERVER-PORT

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

3 participants