add mesos provider inspired by mesos-dns & marathon provider #353

skydjol · 2016-05-04T12:31:40Z

Backend Implementation Proposal

We use Marathon, Chronos and homemade Framework on Mesos, and we want to have a universal backend to manage both.

The Marathon backend works well, and we, first, want to implement a Chronos Backend. But Chronos don't manage callback as Marathon. And what about our handmade Mesos Framework ?

So we decide to lookup in the Mesos DNS code (https://github.com/mesosphere/mesos-dns) to see how callbacks are managed in Mesos.
Mesos DNS uses a pull mecanism (https://github.com/mesosphere/mesos-dns/blob/master/records%2Fgenerator.go#L190) to check every changes.

We think that having a unified backend for Mesos is a very good point (because we don't know all Mesos frameworks we'll using in the future) using same behavior as Mesos DNS.

It's our first time we use Go (and we don't have yet a correct IDE integration for now ... ), so we hope this pull request is good and we don't commit huge mistakes ...
We add this project in our travis instance (https://travis-ci.org/saagie/traefik) and the build passes (of course we generate a binary and test it on our machines).
The only things we don't do is ... tests. Shame on us ... But we follow the Commit Strip principle : http://www.commitstrip.com/shop/fr/affiches/11-affiche-tester-c-est-douter.html .
But it's not easy when your dev env is not fully OK.

Don't hesitate if you have any remarks or questions.

BTW : Your TiA at DevoxxFR was very good and it's after this we decide to test traefik

emilevauge · 2016-05-05T11:56:24Z

Hi @skydjol, thanks a lot !
There is currently an issue with travis on go cross compilations. I need to investigate on it before reviewing your PR.

emilevauge · 2016-05-05T17:52:04Z

You can now rebase to master to get travis working again ;) The issue was in https://github.com/mitchellh/gox.

skydjol · 2016-05-05T19:09:41Z

Hi,
I rebase and now travis built correctly.
;-)

emilevauge · 2016-05-07T10:56:28Z

provider/mesos.go

+		if sj, err = provider.loadWrap(ip, port); err == nil && sj.Leader != "" {
+			return sj, nil
+		}
+		log.Debugf("Warning: Zookeeper is wrong about leader")


Maybe log.Warn here?

emilevauge · 2016-05-07T19:08:54Z

Hey @skydjol, what a great PR :)
Here are few comments:

I find a bit confusing the naming of loadWrap.
the code located in provider/mesos/ has been directly copied from https://github.com/mesosphere/mesos-dns. This is not a good idea ;) Could you reuse it importing the project as a dependency instead?
It would be really great to have unit-tests (like those in integration/), and if possible, integration tests (in integration/) on this. The test environment is entirely in Docker, you should get it working easily. If not, of course, we would be glad to help on this :)
a bit of documentation (in docs/)?
Again, thanks a lot for your great contribution :)

@vdemeester, any comments?

emilevauge · 2016-05-17T19:18:15Z

@skydjol WDYT?

PierreLeresteux · 2016-05-19T17:28:32Z

Hi Emile,
I work with @skydjol and we talk a lot about your answer. We don't have enough time now to do one of all ... but we don't forget this PR ... Hope the next week, we'll work on it.

We'll try to focus on mesos-dns import as depencency , documentation then tests ... First, we'll retry to have a fully work env for Go ... If we fail, we'll ask you :)

PierreLeresteux · 2016-06-14T16:00:42Z

Hi Emile,

I've added some Unit and Integration tests (based on the work done for Marathon).

I, also, renamed the "loadWrap" method to explain a little bit more.

To integrate the "mesos-dns" dependency, we have some errors because some of visibility of some functions in mesos-dns. Moreover we integrate the healthy feature (and maybe other).

One more thing to do is documentation (and test in "real situation" after the master branch merge).

emilevauge · 2016-06-15T16:28:32Z

Hi @PierreLeresteux, thanks for your work :)
As I said, I will not merge this one while taking all that code from mesos-dns project.
May I suggest to make a PR on this mesos-dns fork instead: https://github.com/containous/mesos-dns ?
WDYT?

PierreLeresteux · 2016-06-16T07:55:13Z

Hi Emile ... We'll try to do this today.

It sounds very interresting...

emilevauge · 2016-06-18T12:09:48Z

Thanks a lot @PierreLeresteux :)
If you don't mind, this PR will be merged after the 1.0.0 release as we are already in RC2 :)

emilevauge · 2016-07-08T13:10:29Z

glide.yaml

+- package: github.com/jarcoal/httpmock
+- package: github.com/mesosphere/mesos-dns
+  vcs: git
+  repo: https://github.com/saagie/mesos-dns.git


Could you change it to github.com/containous/mesos-dns ?

emilevauge · 2016-07-08T13:30:10Z

Hi @skydjol & @PierreLeresteux, I made few comments :)
@containous/traefik could you also review this one?

PierreLeresteux · 2016-07-11T07:30:06Z

Hi @emilevauge . Last friday, I've commited some modifications regarding your comments. My travis said it's ok, but on this PR, strange behavior, travis seems to be unhappy ... (mark "In progress", but when I jump to see more details, the status is marked as broken ... )

Let me know if you or someone else in the team have some remarks on this PR.

PierreLeresteux · 2016-07-12T12:48:53Z

Regarding your comments @errm , I make some changes (My bad for "marathon")

errm · 2016-07-12T14:14:40Z

Awesome LGTM

emilevauge · 2016-07-12T14:16:32Z

Thanks @errm for the review :) Thanks @PierreLeresteux for the fixes.
As this PR is quite big, I will wait for another review from @vdemeester before merging ;)

vdemeester · 2016-07-12T20:45:04Z

configuration.go

@@ -272,9 +273,16 @@ func NewTraefikDefaultPointersConfiguration() *TraefikConfiguration {
 	//default Kubernetes
 	var defaultKubernetes provider.Kubernetes
 	defaultKubernetes.Watch = true
-	defaultKubernetes.Endpoint = "http://127.0.0.1:8080"
+	defaultKubernetes.Endpoint = "127.0.0.1:8080"


Hum, this change doesn't seem realetd…

vdemeester · 2016-07-12T21:34:08Z

Few nits and I feel there is room for improvement (in few places)
But overall LGTM 🐸

This is really cool 👍

emilevauge · 2016-07-19T12:49:54Z

Ping ?

PierreLeresteux · 2016-07-19T15:23:14Z

Hi, we've pushed some modifications regarding the comment from @vdemeester ...

Is it ok for you now ?

emilevauge · 2016-07-19T15:41:08Z

On last thing, could you squash your commits ? 19 commits is a bit too much and we want to avoid Merge branch 'master' ... in history ;)
Thanks a lot for this great contribution 🎉 !

PierreLeresteux · 2016-07-20T10:39:38Z

Hi, sure ... here is a one commit for this feature ...
Hope it'll be a part of the "camembert" 1.1 version of Traefik ('cause we're Normands)

emilevauge · 2016-07-20T10:46:37Z

Thanks a lot @PierreLeresteux @skydjol for you great contribution 👏
LGTM :)

Hope it'll be a part of the "camembert" 1.1 version of Traefik ('cause we're Normands)

Let the cheese lobbying start ^^

`golang/glog` is used by: - `github.com/mesos/mesos-go` (no version) - `k8s.io/client-go` (`44145f04b68cf362d9c4df2182967c2275eaefed`) In traefik#353 (add Mesos provider, 20 Jul 2016), the `golang/glog` hash is `fca8c8854093a154ff1eb580aae10276ad6b1b5f`. The problem appear in traefik#836 (use k8s client, 1 Dec 2016). Refs: - Traefik: - traefik#836 - traefik@131f581 - Glog - https://github.com/golang/glog/commits/master - golang/glog#13 - golang/glog@44145f0 - golang/glog@fca8c88 - k8s - https://github.com/kubernetes/client-go/blob/e121606b0d09b2e1c467183ee46217fa85a6b672/Godeps/Godeps.json - https://github.com/kubernetes/client-go/blob/master/Godeps/Godeps.json

`golang/glog` is used by: - `github.com/mesos/mesos-go` (no version) - `k8s.io/client-go` (`44145f04b68cf362d9c4df2182967c2275eaefed`) In #353 (add Mesos provider, 20 Jul 2016), the `golang/glog` hash is `fca8c8854093a154ff1eb580aae10276ad6b1b5f`. The problem appear in #836 (use k8s client, 1 Dec 2016). Refs: - Traefik: - #836 - 131f581 - Glog - https://github.com/golang/glog/commits/master - golang/glog#13 - golang/glog@44145f0 - golang/glog@fca8c88 - k8s - https://github.com/kubernetes/client-go/blob/e121606b0d09b2e1c467183ee46217fa85a6b672/Godeps/Godeps.json - https://github.com/kubernetes/client-go/blob/master/Godeps/Godeps.json

skydjol force-pushed the feature/mesos branch from e421559 to f03c91f Compare May 5, 2016 18:47

emilevauge reviewed May 7, 2016
View reviewed changes

skydjol force-pushed the feature/mesos branch from f03c91f to 2f09c9e Compare May 23, 2016 19:59

emilevauge added the contributor/waiting-for-corrections label May 26, 2016

skydjol force-pushed the feature/mesos branch 3 times, most recently from 1c36552 to 2ac68bf Compare June 17, 2016 11:53

emilevauge added this to the 1.1 milestone Jun 18, 2016

emilevauge added status/2-needs-review and removed contributor/waiting-for-corrections labels Jun 18, 2016

skydjol force-pushed the feature/mesos branch from 2ac68bf to 7ce5bc4 Compare June 23, 2016 09:20

emilevauge reviewed Jul 8, 2016
View reviewed changes

PierreLeresteux force-pushed the feature/mesos branch from f10e540 to 53b8c12 Compare July 8, 2016 14:55

vdemeester reviewed Jul 12, 2016
View reviewed changes

emilevauge added the contributor/waiting-for-corrections label Jul 13, 2016

emilevauge removed the contributor/waiting-for-corrections label Jul 19, 2016

emilevauge added the contributor/waiting-for-corrections label Jul 19, 2016

add mesos provider inspired by mesos-dns & marathon provider

5afcf17

skydjol force-pushed the feature/mesos branch from a5fa0cf to 5afcf17 Compare July 20, 2016 10:07

emilevauge added status/3-needs-merge and removed contributor/waiting-for-corrections status/2-needs-review labels Jul 20, 2016

emilevauge merged commit 8e333d0 into traefik:master Jul 20, 2016

emilevauge mentioned this pull request Jul 21, 2016

Update docs with new Mesos provider #548

Closed

ldez added area/provider/marathon area/provider/mesos kind/enhancement a new or improved feature. and removed status/3-needs-merge labels Jun 11, 2017

ldez mentioned this pull request Jun 11, 2017

Clean glide #1737

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add mesos provider inspired by mesos-dns & marathon provider #353

add mesos provider inspired by mesos-dns & marathon provider #353

skydjol commented May 4, 2016

emilevauge commented May 5, 2016

emilevauge commented May 5, 2016

skydjol commented May 5, 2016

emilevauge May 7, 2016

emilevauge commented May 7, 2016 •

edited

Loading

emilevauge commented May 17, 2016

PierreLeresteux commented May 19, 2016

PierreLeresteux commented Jun 14, 2016

emilevauge commented Jun 15, 2016

PierreLeresteux commented Jun 16, 2016

emilevauge commented Jun 18, 2016

emilevauge Jul 8, 2016

emilevauge commented Jul 8, 2016

PierreLeresteux commented Jul 11, 2016

PierreLeresteux commented Jul 12, 2016

errm commented Jul 12, 2016

emilevauge commented Jul 12, 2016

vdemeester Jul 12, 2016

vdemeester commented Jul 12, 2016

emilevauge commented Jul 19, 2016

PierreLeresteux commented Jul 19, 2016

emilevauge commented Jul 19, 2016

PierreLeresteux commented Jul 20, 2016

emilevauge commented Jul 20, 2016

add mesos provider inspired by mesos-dns & marathon provider #353

add mesos provider inspired by mesos-dns & marathon provider #353

Conversation

skydjol commented May 4, 2016

emilevauge commented May 5, 2016

emilevauge commented May 5, 2016

skydjol commented May 5, 2016

emilevauge May 7, 2016

Choose a reason for hiding this comment

emilevauge commented May 7, 2016 • edited Loading

emilevauge commented May 17, 2016

PierreLeresteux commented May 19, 2016

PierreLeresteux commented Jun 14, 2016

emilevauge commented Jun 15, 2016

PierreLeresteux commented Jun 16, 2016

emilevauge commented Jun 18, 2016

emilevauge Jul 8, 2016

Choose a reason for hiding this comment

emilevauge commented Jul 8, 2016

PierreLeresteux commented Jul 11, 2016

PierreLeresteux commented Jul 12, 2016

errm commented Jul 12, 2016

emilevauge commented Jul 12, 2016

vdemeester Jul 12, 2016

Choose a reason for hiding this comment

vdemeester commented Jul 12, 2016

emilevauge commented Jul 19, 2016

PierreLeresteux commented Jul 19, 2016

emilevauge commented Jul 19, 2016

PierreLeresteux commented Jul 20, 2016

emilevauge commented Jul 20, 2016

emilevauge commented May 7, 2016 •

edited

Loading