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 marathon provider #1090

Merged
merged 1 commit into from Feb 4, 2017
Merged

Conversation

diegooliveira
Copy link
Contributor

@diegooliveira diegooliveira commented Jan 31, 2017

The IP-Per-Task PR introduced a bug using the marathon application
port mapping. This port should be used only in the proxy server, the
downstream connection should always be made with the task port.

This commit fix the regression and adds an unit test to prevent new
problems in this setup.

Fixes #1072

Copy link
Contributor

@timoreimann timoreimann left a comment

Choose a reason for hiding this comment

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

Thanks for the fix. LGTM. 👍

@timoreimann
Copy link
Contributor

timoreimann commented Feb 1, 2017

CI is failing for Docker 1.13, might just be a flake. @diegooliveira, do you want to try to retrigger Travis (git commit --amend --no-edit && git push --force-with-lease)?

@emilevauge
Copy link
Member

@timoreimann CI is always failing with Docker 1.13...
I will remove this version in #1067

@timoreimann
Copy link
Contributor

@emilevauge gotcha, thanks for the info.

@timoreimann
Copy link
Contributor

Needs another rebase. :-)

@emilevauge emilevauge force-pushed the IP-Per-Task-Fix branch 4 times, most recently from af9e32c to 247dcb4 Compare February 3, 2017 14:57
Copy link
Member

@emilevauge emilevauge left a comment

Choose a reason for hiding this comment

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

LGTM
Thanks @diegooliveira

Copy link
Contributor

@vdemeester vdemeester left a comment

Choose a reason for hiding this comment

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

LGTM 🐸

    The IP-Per-Task PR introduced a bug using the marathon application
port mapping. This port should be used only in the proxy server, the
downstream connection should be always made with the task port.

    This commit fix the regression and adds a unit test to prevent new
problems in this setup.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Marathon integration changed default backend server port from task-level to application-level
5 participants