Add support for readiness checks. #1883
Conversation
timoreimann
added
area/provider/marathon
kind/enhancement
timoreimann
force-pushed
the
marathon-readiness-checks
branch
from
78a2c7f to
da238f9
Compare
provider/marathon/readiness.go
Outdated
There was a problem hiding this comment.
Note to reviewers: Ideally, I'd like to support logging in "trace level" more generally (i.e., without having to pass the trace flag into a particular function that needs this information).
I'm open to suggestions.
There was a problem hiding this comment.
Indeed, much better. Done.
provider/marathon/readiness.go
Outdated
There was a problem hiding this comment.
Note to reviewers: The readiness checker logic should be fairly clear except for this corner case (hence the lengthy comment). Internally, we went through several iterations to improve the code and iron out corner cases (though with Bamboo instead of Traefik, the logic is identical -- I basically copy & pasted the implementation). It's been running in our production system for several weeks now, so it should have gained a fair level of robustness.
Nevertheless, I'd happy for suggestions to make it simpler/better.
timoreimann
force-pushed
the
marathon-readiness-checks
branch
from
da238f9 to
18e7e69
Compare
ldez
force-pushed
the
marathon-test-builder
branch
from
6dcc278 to
4546613
Compare
timoreimann
force-pushed
the
marathon-readiness-checks
branch
from
18e7e69 to
50489fd
Compare
|
@timoreimann wow, impressive job 👏 |
|
The Javaness in me is less of a love and more of a bad, hard-to-shake-off habit. 😬 I'm very open to shortening the option name. While an integration test would be nice, I can say from experience with this feature that covering all cases will be difficult: It depends a lot on how applications/tasks behave and the timing involved. It took us two or three internal patches to get this right with Bamboo. On the plus side, this means the algorithm is battle-tested to a certain degree already. And users can turn on/off readiness check filtering on demand. That said, if someone can help out on the upstream libcompose blocker, I'd be more than happy to contribute an integration test. 👼 |
timoreimann
force-pushed
the
marathon-readiness-checks
branch
from
50489fd to
beafbc7
Compare
provider/marathon/readiness.go
Outdated
provider/marathon/readiness_test.go
Outdated
| }, | ||
| } | ||
|
|
||
| for _, c := range cases { |
There was a problem hiding this comment.
I prefer test or case instead of c
There was a problem hiding this comment.
I can't use case because that's a reserved keyword in Go, so I renamed everything to test.
Last time I checked I haven't seen a bug report on the matter. I'll double-check, file a report if I find nothing, and post it here. |
This change adds support for Marathon readiness checks: If the option is set on Traefik and applications are found to come with configured readiness checks, the check results (as conducted by Marathon and exposed through its API) will be taken into account at deployment times in order to decide if a task should be taken into load-balancing rotation or not. Note that I had to extend the event filter that controls how often the Marathon provider polls the API so that the readiness checker is able to pick up all necessary readiness state transitions.
traefiker
force-pushed
the
marathon-readiness-checks
branch
from
eb7c72f to
7fb888a
Compare
This change adds support for Marathon readiness checks: If the option is set on Traefik and applications are found to come with configured readiness checks, the check results (as conducted by Marathon and exposed through its API) will be taken into account at deployment times
in order to decide if a task should be taken into load-balancing rotation or not.
Note that I had to extend the event filter that controls how often the Marathon provider polls the API so that the readiness checker is able to pick up all necessary readiness state transitions.
Fixes #1185, #1559
Based on #1871 for now; will be rebased onto master once the dependent branch gets merged.