-
Notifications
You must be signed in to change notification settings - Fork 99
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
allow listen address to be configured alongside port #429
base: master
Are you sure you want to change the base?
Conversation
This is useful if cases where collins runs on a node with multiple interfaces, or behind a proxy.
LGTM! Could you add this to the |
I don't think the Dockerfile has a precedent for configuration like this. For example the port is just hard coded as |
@cburroughs it would be nice to have listen.address explicitly set to |
@cburroughs maybe this is because I'm running docker 1.10.0, but I was able to add the following to the dockerfile and it respected the environment variable.
And then when I built and inspected the container, I got port 9000 exposed:
So I think we could set the port using an environment variable in the Dockerfile, and make all of the scripts used in the container respect that variable to change the port that gets used. Not sure if it's worth doing or not though, since at runtime it could be changed. Maybe this is a job for docker's build-args, but I haven't used them before so I don't have a good idea of how they work yet. |
I'd not use the script with the Dockerfile. I personally never used those scripts and in general try to avoid using 'vendor scripts' to bring up my services. I think it's fine to provide them here for convenience, but wouldn't change the Dockerfile to use it. So IMO, this can get merged. Is there something else blocking it? |
This is useful if cases where collins runs on a node with multiple
interfaces, or behind a proxy.