Add allowEmptyServices for Docker provider #8690
Conversation
rtribotte
added
kind/enhancement
jvasseur
force-pushed
the
don-t-ignore-docker-unhealthy
branch
from
d145c40
to
1d0f9ff
Compare
jvasseur
force-pushed
the
don-t-ignore-docker-unhealthy
branch
from
1d0f9ff
to
6ebf9ea
Compare
|
I have rebased the PR on the last version of master. Can I do something to help move this PR forward ? |
|
Hello @jvasseur, Thanks for your interest in Traefik, This PR is labeled as We will come back to you once the design review iteration is done. |
|
Ok no problems, it's just that since the PR was opened quite some time ago I wanted to make sure it wasn't stuck waiting for something on my side but I understand that reviewing PRs takes time 🙂 |
|
Hello @jvasseur , just to give you a quick update on this, it should definitely be an enhancement and not a bug since the behavior was intended and it works like that across different providers. Also it is a breaking change because it modifies existing behavior without backward compatibility. Having said that I believe now is a good time to discuss that change since work for a new major is ramping up, although the team is still focused on other priorities right now so we still don't have an ETA to give. |
tomMoulard
force-pushed
the
don-t-ignore-docker-unhealthy
branch
from
6ebf9ea
to
68d3caf
Compare
tomMoulard
changed the title
tomMoulard
force-pushed
the
don-t-ignore-docker-unhealthy
branch
from
68d3caf
to
fe37e0f
Compare
|
Hello @jvasseur, As you probably noticed, we started to review the changes, and we are wondering about the specific use case this PR is addressing, can you please elaborate on this? Thanks |
I don't know if this is the original intent, but for me the use-case is for things like GitLab, which can take several minutes to start up after doing an update. Currently, Traefik returns a 404 page while GitLab is booting, Ave several times I've had users contact me to complain that "GitLab has disappeared". These users know enough that if the error message was a 503 they would understand that it was temporarily down and would be back in a few minutes. |
Basically the use case is when the container is unhealthy because it is starting like the example @kohenkatz proposed or because it crashed and isn't restarted yet we want to return a 503 instead of the current 404 that is surprising for users and could have consequences for things like SEO. My idea was to mirror how most kubernetes ingress controllers work, since the ingress config is always present it doesn't matter if the pods are up or not, the service is always register and thus will generate a 503 if pods are not up. But since the docker doesn't have a differentiation between some kind of ingress config and the containers themselves I proposed this PR to allow keeping the service configuration even if no containers are healthy. |
kevinpollet
force-pushed
the
don-t-ignore-docker-unhealthy
branch
from
fe37e0f
to
221d8c0
Compare
Don't ignore configuration from labels on healthy containers and instead generate a service without servers. This will make such case generate 503 for unhealthy services instead of 404.
tomMoulard
force-pushed
the
don-t-ignore-docker-unhealthy
branch
from
221d8c0
to
8205b0a
Compare
What does this PR do?
This PR will stops the docker provider from ignoring unhealthy containers but instead will still pull configuration from them but without adding them as servers to the generated services.
Motivation
This will make Traefik return a 503 status code for unhealthy services instead of the current 404.
This should fix the docker provider part of #1689
More
Additional Notes
I've made the PR for master because I wasn't sure if this should be considered an enhancements or a bugfix. I will be happy to rebase it on another branch if needed.