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

Make Traefik health checks label-configurable with Marathon. #1320

Merged
merged 4 commits into from
Apr 28, 2017

Conversation

timoreimann
Copy link
Contributor

@timoreimann timoreimann commented Mar 20, 2017

For the two existing health check parameters (path [formerly known as URL] and interval), we add support for Marathon labels.

Changes in detail:

  • Extend the Marathon provider and template.
  • Refactor Server.loadConfig to reduce duplication.
  • Refactor the healthcheck package slightly to accommodate the changes and allow extending by future parameters.
  • Update documentation.

Fixes #1232.

@timoreimann
Copy link
Contributor Author

This PR builds upon the changes introduced by #1319. Thus, it is currently based on that branch and will be rebased onto master as soon as the dependent PR merges.

@timoreimann timoreimann added this to the 1.3 milestone Mar 20, 2017
@timoreimann timoreimann force-pushed the configure-healthcheck-via-marathon-label branch 3 times, most recently from 4ce1619 to 279570e Compare March 23, 2017 11:06
@timoreimann timoreimann force-pushed the configure-healthcheck-via-marathon-label branch from 279570e to 73f02f6 Compare March 24, 2017 10:46
@timoreimann timoreimann added the kind/enhancement a new or improved feature. label Mar 24, 2017
@timoreimann timoreimann force-pushed the start-healthcheck-early branch 2 times, most recently from cb54814 to 60ea919 Compare April 7, 2017 17:12
@timoreimann timoreimann force-pushed the configure-healthcheck-via-marathon-label branch from 73f02f6 to 031c82c Compare April 10, 2017 19:17
@timoreimann timoreimann changed the base branch from start-healthcheck-early to master April 10, 2017 19:17
@timoreimann timoreimann force-pushed the configure-healthcheck-via-marathon-label branch 3 times, most recently from 8e68623 to 9c62808 Compare April 13, 2017 00:45
@ldez
Copy link
Contributor

ldez commented Apr 19, 2017

@timoreimann #1319 is merged, I think you can rebase on master now.

@timoreimann
Copy link
Contributor Author

timoreimann commented Apr 19, 2017

@ldez already did a few days ago (forgot to comment).

Latest merge conflicts are due to more recent PRs being merged. Will update the PR later.

This shouldn't stop reviewing though.

@timoreimann timoreimann force-pushed the configure-healthcheck-via-marathon-label branch 4 times, most recently from 4d7f3d5 to 8a99789 Compare April 19, 2017 22:40
Copy link
Contributor

@ldez ldez left a comment

Choose a reason for hiding this comment

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

@timoreimann timoreimann force-pushed the configure-healthcheck-via-marathon-label branch 5 times, most recently from ed5a0d7 to 7f62c39 Compare April 26, 2017 10:27
@@ -17,15 +17,15 @@ import (
)

// HealchCheck test suites (using libcompose)
Copy link
Contributor

Choose a reason for hiding this comment

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

missing typo.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the note, done.

@timoreimann timoreimann force-pushed the configure-healthcheck-via-marathon-label branch from 06d365d to db3e461 Compare April 27, 2017 11:36
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 @timoreimann
LGTM :)

You added 2 new labels in the Marathon provider that we should report in others providers later: https://github.com/containous/traefik/pull/1320/files#diff-ed1959fa6ceca7431deaf68202208463R29. Would you open a proposal in an issue to refactor and centralize labels/tags/annotations for all providers? This would simplify a lot the providers mechanism :)

For the two existing health check parameters (path and interval), we add
support for Marathon labels.

Changes in detail:

- Extend the Marathon provider and template.
- Refactor Server.loadConfig to reduce duplication.
- Refactor the healthcheck package slightly to accommodate the changes
  and allow extending by future parameters.
- Update documentation.
@timoreimann timoreimann force-pushed the configure-healthcheck-via-marathon-label branch from 8023297 to 5d43b9e Compare April 28, 2017 16:17
@timoreimann timoreimann merged commit ce49289 into master Apr 28, 2017
@ldez ldez deleted the configure-healthcheck-via-marathon-label branch April 28, 2017 20:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants