3.3.0-rc breaks /health API compatibility

[liggitt]

The /health response in the 3.3.0 release candidates breaks compatibility with the response from previous releases

Prior to 3.3:

{"health":"true"}

3.3.0-rc:

{"health":true}

while it is true that the boolean is a better match for the data presented, it's probably not worth breaking compatibility with existing monitoring tools over it. it would be good to switch the type back prior to releasing 3.3.0.

see example impact of compatibility break at kubernetes/kubernetes#58240

[xiang90]

@liggitt why does k8s rely on parsing the http body rather than checking http status code?

changing string to type is more a bug fix than an intentional breaking change. we never intend to make it a string at the first place.

[liggitt]

@liggitt why does k8s rely on parsing the http body rather than http status code?

Because that was what was documented:

If it returns {"health": "true"}, then the cluster is healthy

changing string to type is more a bug fix than an intentional breaking change. we never intend to make it a string at the first place.

Nevertheless, that's the API you have. Unless there's a good reason, it shouldn't change in breaking ways.

[xiang90]

Nevertheless, that's the API you have. Unless there's a good reason, it shouldn't change in breaking ways.

The reason is to fix the unintended behavior. It seems to me that k8s does not need to parse http body. Also it can easily parse the boolean type when the string one fails as a fallback.

We could choose to not change the current health endpoint. And we could have added another healthy endpoint to fix the problem, and add more healthy related information. But I do not feel this "breaking" change is hard for application to adopt. Can you provide some reasons for why k8s cannot adopt the change, besides that some people try to use a currently unsupported version of etcd for k8s.

[liggitt]

Also it can easily parse the boolean type when the string one fails as a fallback.

Pushing workarounds for API breaks into all clients doesn't make sense to me.

And we could have added another healthy endpoint to fix the problem, and add more healthy related information.

Adding more health related info is fine... you can do that in a backwards compatible way. Changing the type of the existing field is the break. If this were a new API, a boolean would definitely make sense, but since it's not, the downside of breaking clients outweighs the cleanliness of a better matching field type.

[xiang90]

Pushing workarounds for API breaks into all clients doesn't make sense to me.

Can you tell me which client is affected? go/jave clients do not query this API. We searched around while changing this API, few client exposes health API directly.