Skip to content

Add Marathon guide. #1578

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.

Sign up for GitHub

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

Jump to bottom
Merged
emilevauge merged 1 commit into from
May 17, 2017
Merged

Add Marathon guide. #1578

emilevauge merged 1 commit into from
May 17, 2017

Conversation

@gsemet
Copy link
Contributor

@gsemet gsemet commented May 10, 2017 •
edited by timoreimann
Loading

@timoreimann timoreimann mentioned this pull request May 10, 2017
Closed
@timoreimann
Copy link
Contributor

timoreimann commented May 10, 2017

Refs #1250.

@timoreimann timoreimann self-requested a review May 10, 2017 12:56
@timoreimann timoreimann changed the title [DOC] Marathon service rotation explanation Add Marathon guide. May 10, 2017
@timoreimann
Copy link
Contributor

timoreimann commented May 10, 2017

hey @stibbons, I went crazy and expanded what you have started to a full guide. Would you mind taking a look and tell me if it makes sense?

@timoreimann
Copy link
Contributor

timoreimann commented May 10, 2017

@argeiger @gnur @tcolgate if you're still using Marathon along with Traefik and would be willing to prove-read the Marathon guide put together here, I'd be grateful. :-)

@timoreimann timoreimann removed their request for review May 10, 2017 14:27
@ldez
Copy link
Contributor

ldez commented May 10, 2017

ping @juliens

@gsemet
Copy link
Contributor Author

gsemet commented May 10, 2017

Great! I don't know what I did but my review does not appear on Github "File changed". It is here: 2f7d104
Just a few questions ! This doc is great!

@timoreimann
Copy link
Contributor

timoreimann commented May 10, 2017

@stibbons

I don't know what I did but my review does not appear on Github "File changed".

Looks like you commented on a single commit instead of the entire change set. But don't worry, I see them. :-)

Thanks for the feedback -- will address it soon!

@timoreimann
Copy link
Contributor

timoreimann commented May 10, 2017

@stibbons Feedback incorporated. :-)

@argeiger
Copy link
Contributor

argeiger commented May 10, 2017

LGTM. You might just want to point out the version of marathon that implements readiness checks

Thanks for putting this together.

@ldez
Copy link
Contributor

ldez commented May 10, 2017

@stibbons could you fix spelling? https://travis-ci.org/containous/traefik/jobs/230798308#L829

@timoreimann
Copy link
Contributor

timoreimann commented May 10, 2017

@argeiger

You might just want to point out the version of marathon that implements readiness checks.

Good point -- I added the info for both the readiness checks and graceful termination handler.

@gsemet
Copy link
Contributor Author

gsemet commented May 10, 2017

+1

juliens
juliens reviewed May 10, 2017
View reviewed changes
Copy link
Member

@juliens juliens left a comment

Choose a reason for hiding this comment

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

Great work.
If only I could have had these documents 2 months earlier ;)

docs/user-guide/marathon.md Outdated
Copy link
Member

@juliens juliens May 10, 2017

Choose a reason for hiding this comment

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

We may need an example. I'm not sure all users can see what to do with template.

Copy link
Contributor

@timoreimann timoreimann May 10, 2017

Choose a reason for hiding this comment

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

I agree but would rather see the example in a more generic spot of the Traefik documentation since it applies to all providers. WDYT?

Copy link
Member

@juliens juliens May 11, 2017

Choose a reason for hiding this comment

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

I agree, and I realize yesterday that I can't find documentation on this.
One more todo ! 😉

Copy link
Contributor

@timoreimann timoreimann May 11, 2017

Choose a reason for hiding this comment

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

I have created #1590 to track the effort.

@timoreimann
Copy link
Contributor

timoreimann commented May 11, 2017

@juliens anything else you think is missing? Otherwise, I'd kindly ask for your LGTM. :-)

@timoreimann
Copy link
Contributor

timoreimann commented May 11, 2017

/cc @containous/traefik for potential other reviewers.

@juliens
Copy link
Member

juliens commented May 11, 2017

LGTM

emilevauge
emilevauge approved these changes May 17, 2017
View reviewed changes
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 a lot @stibbons !
LGTM
I suggest we change the base branch to v1.3 to get this right into the next release :)

@emilevauge emilevauge changed the base branch from master to v1.3 May 17, 2017 12:32
@emilevauge emilevauge changed the base branch from v1.3 to master May 17, 2017 12:37
@emilevauge emilevauge changed the base branch from master to v1.3 May 17, 2017 12:46
@emilevauge emilevauge changed the base branch from v1.3 to master May 17, 2017 12:47
@gsemet @emilevauge
Add Marathon guide
0b414ed 
Copy/pasted from very comprehensive slack response from @ttr
https://traefik.slack.com/archives/C0CDT22PJ/p1494347929571784?thread_ts=1494339388.375916&cid=C0CDT22PJ

Signed-off-by: Gaetan Semet <gaetan@xeberon.net>
@emilevauge emilevauge changed the base branch from master to v1.3 May 17, 2017 13:03
@emilevauge
Copy link
Member

emilevauge commented May 17, 2017

Changed base branch to containous:v1.3 and rebased.

@emilevauge emilevauge merged commit bc6f764 into traefik:v1.3 May 17, 2017
@ldez ldez added this to the 1.3 milestone May 19, 2017
@timoreimann timoreimann mentioned this pull request Jun 9, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@juliens juliens juliens left review comments

@emilevauge emilevauge emilevauge approved these changes

@vdemeester vdemeester Awaiting requested review from vdemeester

+2 more reviewers

@timoreimann timoreimann timoreimann left review comments

@ldez ldez ldez approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

area/documentation area/provider/marathon size/S

Projects

None yet

Milestone

1.3

Development

Successfully merging this pull request may close these issues.

6 participants

@gsemet @timoreimann @ldez @argeiger @juliens @emilevauge