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

Allow for configuration of which environment variable to search for #12

Merged
merged 1 commit into from
Jun 5, 2017

Conversation

masterful
Copy link
Contributor

Apologies in advance - I'm entirely unfamiliar with Go, this is my first foray, so if I've done something dumb feel free to point it out as a learning opportunity 😉.

If you hate this or just don't see the use case for it I won't be offended if you just close this PR, but I thought I should at least offer back the changes I made in case you or someone else finds it helpful.

I found myself with the need to run two load balancers pointing to the same ECS cluster, but servicing different services through separate reverse proxies (internal vs. external). While another way to get around this would be to create more than one cluster we found it more efficient to utilize the resources of one cluster ... What that meant was we needed a way to tell the reverse proxy which containers to pay attention to, without them both proxying the same containers. I've created a fork of ecs-nginx-proxy as well which uses my version of ecs-gen and pushed to docker hub here in case you wanted to test it out.

This PR gives the ability to specify (via the command line or by environment variable) which environment variable to search container definitions for to use as the hostname. It defaults to virtual_host (the current behaviour).

While this doesn't quite address the question in #10 - it does allow the use of a differently named environment variable.

@codesuki
Copy link
Owner

Sorry for the late reply. Super busy here. I will test it and merge!
When I first looked at the code I didn't see the use case but after reading your description it makes sense. Thanks for contributing back your changes!

@masterful
Copy link
Contributor Author

Happy to contribute back! And as I mentioned before - I haven't really used Go before, so if you have suggestions on how to better structure the code/changes please let me know 😄

@codesuki
Copy link
Owner

Finally I can see this pull request. Apparently you were tagged by GitHubs spam system and I couldn't even open this pull request (getting 404) but it seems they fixed it. I'm super busy but will try to check back beginning of next week.

@masterful
Copy link
Contributor Author

Yeah - sorry about that. I asked them about it but they can't give me any details as to why it happened since they'd like to keep their process secret...

@codesuki codesuki merged commit ba2b96d into codesuki:master Jun 5, 2017
@masterful masterful deleted the configurable-env-host-variable branch June 14, 2017 12:47
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