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

Issue 613 tcp dynamic #626

Merged
merged 8 commits into from
Dec 2, 2019
Merged

Conversation

murphymj25
Copy link
Contributor

No description provided.

@CLAassistant
Copy link

CLAassistant commented Mar 21, 2019

CLA assistant check
All committers have signed the CLA.

@murphymj25
Copy link
Contributor Author

@craighazen and I have put together this PR to address #613. This allows you to start a "tcp-dynamic" proxy that will start TCP listeners based on the consul urlprefix. It will also match the service based on ip:port. Please review and let us know if there is any feedback.

config/load.go Outdated Show resolved Hide resolved
@murphymj25
Copy link
Contributor Author

murphymj25 commented May 16, 2019

I need to do some more updates to this. If there are manual overrides that don't match the expected syntax, we get a panic.

@scalp42
Copy link

scalp42 commented Jun 3, 2019

@murphymj25 thanks a lot for the contribution. It fixed the issue of having to define the TCP in advance and allow the tags to be the authoritative source.

@murphymj25
Copy link
Contributor Author

@scalp42 Let me know how your experience goes. We're just starting to get a few services deployed using this, so probably a few things we'll need to update yet.

@aaronhurt
Copy link
Member

@murphymj25, @scalp42 Yes, absolutely. Let us know how the testing goes and we'll look at getting this merged.

@scalp42
Copy link

scalp42 commented Jun 4, 2019

I think we're at the crossroad of leveraging consul-template see #613, running custom Fabio binary (which we really don't want to) or just leaving the dynamic nature behind regarding Fabio config file and TCP listeners (probably where we're headed).

@murphymj25
Copy link
Contributor Author

I found one error where we had the error catch for the dialer after the defer to close the connection. This resulted in a panic if a connection could not be established. I updated the PR with the correct order.

@murphymj25
Copy link
Contributor Author

Other than this one error, we have several apps using this, everything has been working well.

@madsholden
Copy link

Any chance of this being merged and released soon? It would be really nice to have.

@aaronhurt
Copy link
Member

@madsholden I'll take a review today. Thank you all for your patience. @murphymj25 Thank you for all the testing and updates!

@murphymj25
Copy link
Contributor Author

No problem! We haven't encountered any other issues apart from the one update I made. We probably have around 20-30 different services running via this manner.

@aaronhurt
Copy link
Member

No problem! We haven't encountered any other issues apart from the one update I made. We probably have around 20-30 different services running via this manner.

Awesome, thanks again.

@aaronhurt aaronhurt merged commit 3584cbb into fabiolb:master Dec 2, 2019
@sunliusi
Copy link

When will the new version be released? This is a great feature

@renesnezic
Copy link

You mentioned this will match based on ip:port.. Does it match based on host:port?

I've got a scenario where I'm running multiple mail servers inside of separate docker containers..
Meaning I'd like to route traffic from example.com on ports 587 & 143 to one container and traffic for acme.com on ports 587 & 143 to a different container. And I obviously don't want to hard-code the container private IPs. Would this work?

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.

8 participants