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

Ensure that we don't add balancees with no health check runs if there is a health check defined on it #1080

Conversation

jangie
Copy link
Contributor

@jangie jangie commented Jan 28, 2017

Have been running into Traefik balancing against marathon balancees on container startup where the container is not yet ready to serve traffic.

As of the current behavior, the marathon provider will allow balancing to:

  • Items which do not have health checks
  • Items which have health checks where all results are passing
  • Items which have health checks which have not been run yet

This PR aims to ensure that items which have health checks have at least one passing health check result prior to allowing them to be balanced against, removing the last category.

@jangie jangie force-pushed the only-add-marathon-balancees-if-any-healthcheck-results branch 2 times, most recently from a86e667 to b5b89ee Compare January 31, 2017 20:37
… is a health check defined on it

Change test behavior
@jangie jangie force-pushed the only-add-marathon-balancees-if-any-healthcheck-results branch from b5b89ee to ad12a72 Compare February 3, 2017 13: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.

LGTM
Thanks @jangie
/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 b22dc21 into traefik:master Feb 3, 2017
@timoreimann
Copy link
Contributor

This PR reverted a change introduced by @gomes in #653 and "re-breaks" Marathon apps in Traefik during leader elections.

I will create a new issue to discuss resolutions.

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.

5 participants