warning should not be a fail status

[saez0pub]

I would like to have a behavior like the DNS provider of consul.
Even if the service is warning, It is considered as UP.

Moreover, another difference is that if any service not served by traefik is in warning state, traefik remove it from consul catalog.

What does this PR do?

Change failure detection for consul catalog

Motivation

A warning on a service on a node disable all frontends of this node on consul

More

• Not Added/updated tests
• Not Added/updated documentation

Additional Notes

I have tested it manualy, I don't know what test I could write.
related to #4536

[ldez]

Hello @trondhindenes what's your opinion about this PR ?

[trondhindenes]

If I understand this PR correctly, it makes the following change:
Previously, Traefik would treat a backend node as "unhealthy" if it was in any of the following states: critical, maintenance
This PR changes the behavior so that the following states disables the backend node in Traefik: critical, warning, maintenance

Assuming I understand it correctly, I don't have an opinion - we don't use the warning status for anything internally.
It is interesting tho. For example if a backend contains 10 nodes and 2 of them are in warning while 8 are healthy I would probably prefer to only send traffic to the 8 healthy ones. However, if all 10 nodes were in a warning state, I would prefer traffic to be sent to all of them.

Traefik doesn't currently have any logic to decide routing based on the health of individual nodes in the backend (I'm pretty sure HaProxy has something like this), so for now I guess it comes down to whether or not to treat "warning" as a failure state or not. I don't really have a strong opinion, but since "warning" is a degraded state, I guess backwards compatibility should be most important (meaning this setting shouldn't be changed just like that). I guess the natural thing to do is to introduce a config setting to control the behavior of the "warning" state but keep the default as-is.

That's just my two cents :-)

[pgrange]

Hi,

This is the way consul manages healthyness: while a node has all its checks either passing or warning, the node is part of the service. Only when a check becomes critical or the node is in maintenance, is the node unable to serve.

Since it is the way consul handles consul checks, I would say this is the way they have to be handled. So I believe this is the way traefik has to handle consul checks.

Regarding backward compatibility, I would be very surprised that anyone using consul, with the way it handles checks, would be badly impacted by this pr since it reflects the expected behavior.

See also #4536

Regards,

[ldez]

Thank you for this PR.

Sorry for the late answer.

Could you rebase this pull request on v1.7 branch?

[saez0pub]

Hello, I have rebased on 1.7.

Thank you.

[saez0pub]

Hello,

Any status update ?

[mmatur]

Hello @saez0pub,

Sorry for the late reply. Could you please, like suggested by @trondhindenes, add a parameter to keep the default as-is.

[saez0pub]

Hello,

I've added an option, and tested it manually.

with the option not set (by default WarningIsCritical = true)
Start :

• passing : in backend
• warning : not in backend
• critical : not in backend

Node Check state transition :

• passing > warning : removed from backend
• passing > critical : removed from backend
• warning > passing : added in backend
• warning > critical : stil not in backend
• critical > passing : added in backend
• critical > warning : stil not in backend

with the consulcatalog option WarningIsCritical = false
Start :

• passing : in backend
• warning : in backend
• critical : not in backend

Node Check state transition :

• passing > warning : still in backend
• passing > critical : removed from backend
• warning > passing : still in backend
• warning > critical : removed from backend
• critical > passing : added in backend
• critical > warning : added in backend

This option is by default on the old behaviour.

I suggest you to match consul's logic in the next major release and invert the default value of this parameter. Using a consul catalog provider and changing the healthyness detection is something hard for users comming from consul.

[ldez]

Thanks 👍

[mmatur]

Thanks for this PR @saez0pub 👏
LGTM

[dtomcej]

LGTM