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

GCE: ingress only shows the first backend's healthiness in backends annotation #1395

Closed
MrHohn opened this issue Sep 20, 2017 · 7 comments
Closed

Comments

@MrHohn
Copy link
Member

MrHohn commented Sep 20, 2017

From kubernetes/enhancements#27 (comment).

We attach backends annotation to ingress object after LB creation:

  ...
  backends:		{"k8s-be-30910--7b4223ab4c1af15d":"UNHEALTHY"}

And from the implementation:
https://github.com/kubernetes/ingress/blob/937cde666e533e4f70087207910d6135c672340a/controllers/gce/backends/backends.go#L437-L452

Using only the first backend's healthiness to represent the healthiness for all backends seems incorrect.

cc @freehan

@yastij
Copy link
Member

yastij commented Sep 20, 2017

@MrHohn - anyone working on this one ? if not I can send a PR

@MrHohn
Copy link
Member Author

MrHohn commented Sep 20, 2017

@yastij Nope, though I'm not quite sure how should we present the backends healthiness in annotation --- for huge cluster, we might have too many backends (nodes). It also seems unwise to append all of them via annotation...

cc @nicksardo

@yastij
Copy link
Member

yastij commented Sep 20, 2017

Maybe state healthy when all the backends are (checking all backends) , and unhealthy when some are (specifying which ones aren't healthy)

@MrHohn
Copy link
Member Author

MrHohn commented Sep 20, 2017

Maybe state healthy when all the backends are (checking all backends) , and unhealthy when some are (specifying which ones aren't healthy)

Yeah that sort of makes sense, though for the externlTrafficPolicy=Local case, some of the backends (nodes) may intentionally fail LB healthcheck so that traffic will only go to nodes that contains backend pods. Showing this as unhealthy may scare users, and it isn't actually unhealthy :(

@yastij
Copy link
Member

yastij commented Sep 20, 2017

@MrHohn - we can detect this case nope ? if it is set to Local we can ignore the unhealthy status ?

@nicksardo
Copy link
Contributor

I do not know if people use externalTrafficPolicy=Local with ingress (I've never tried it) and it's not something we document with ingress. It may technically work, but I don't know how well it works in production with rolling updates and other edge cases. Although if we wanted to support that case, another option is to correlate the instance status with the pods location(node).

I agree that this annotation is not accurate. Even if it shows a correct status, the annotation is only refreshed on every sync (which may be 10 minutes or longer if there are a lot of ingress objects). My question is whether this annotation is worth keeping. Wouldn't users be better off looking at the GCP Console for backend status? Do users have daemons which poll this annotation and perform alerts? If the only case we're concerned about is bad healthcheck configuration breaking all backends, couldn't we create an alert saying "All backends are unhealthy - please investigate"?

cc @csbell @nikhiljindal

@bowei
Copy link
Member

bowei commented Oct 11, 2017

This issue was moved to kubernetes/ingress-gce#35

@bowei bowei closed this as completed Oct 11, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants