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

Add basic compatibility with marathon-lb #720

Merged
merged 2 commits into from Oct 12, 2016
Merged

Add basic compatibility with marathon-lb #720

merged 2 commits into from Oct 12, 2016

Conversation

guilhem
Copy link
Contributor

@guilhem guilhem commented Oct 6, 2016

Add compatibility with labels: HAPROXY_GROUP and HAPROXY_0_VHOST.

  • HAPROXY_GROUP become a new tag
  • HAPROXY_0_VHOST become a rule Host:

https://github.com/mesosphere/marathon-lb

@vdemeester
Copy link
Contributor

/cc @containous/traefik

@guilhem
Copy link
Contributor Author

guilhem commented Oct 7, 2016

I hope this to be merged in 1.1 👼

@vdemeester vdemeester added this to the 1.1 milestone Oct 7, 2016
@vdemeester vdemeester changed the base branch from master to v1.1 October 7, 2016 12:45
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.

@guilhem thanks for your contribution!
Could you add a unit test for this use case?

Add compatibility with labels: `HAPROXY_GROUP` and `HAPROXY_0_VHOST`.
* `HAPROXY_GROUP` become a new tag
* `HAPROXY_0_VHOST` become a rule `Host:`

https://github.com/mesosphere/marathon-lb
@guilhem
Copy link
Contributor Author

guilhem commented Oct 7, 2016

@emilevauge done :)
don't hesitate to tell me if this isn't enough

},
},
marathonLBCompatibility: true,
expected: "Host:foo.bar",
Copy link
Member

Choose a reason for hiding this comment

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

Not sure to understand here 🤔
I was expecting expected: "Host:notvalid" instead, reading the code in getFrontendRule. Am I missing something?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

if traefik.frontend.rule is present there is a return before trying HAPROXY label.

I prefer this behavior with traefik labels superseding other one.

Copy link
Member

Choose a reason for hiding this comment

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

OK thanks :)

@emilevauge
Copy link
Member

@guilhem needs rebase on v1.1 branch :)

@guilhem
Copy link
Contributor Author

guilhem commented Oct 12, 2016

@emilevauge done :)

@guilhem
Copy link
Contributor Author

guilhem commented Oct 12, 2016

I think you can squash with 1 button (to remove "merge" commit)

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.

Thanks @guilhem
LGTM!
/cc @containous/traefik

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 🐮

@vdemeester vdemeester merged commit 3b2410d into traefik:v1.1 Oct 12, 2016
@guilhem guilhem deleted the marathon-lb branch October 14, 2016 13:05
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.

None yet

4 participants