Informers do not surface API server request failures to callers

[seh]

When I call cache.NewInformer to create an Informer and then call cache.Controller.Run on the returned value, the controller periodically contacts the API server to list the objects of the given type. At some point the API server rejects these requests, causing the controller to log messages like the following:

Failed to list *v1beta1.Ingress: the server has asked for the client to provide credentials (get ingresses.extensions)\n","stream":"stderr","time":"2017-03-21T14:14:37.397681364Z"}

Failed to list *v1.Service: the server has asked for the client to provide credentials (get services)\n","stream":"stderr","time":"2017-03-21T14:14:37.398311081Z"}

Failed to list *v1.Endpoints: the server has asked for the client to provide credentials (get endpoints)\n","stream":"stderr","time":"2017-03-21T14:14:37.399217817Z"}

These failures repeat periodically, swelling the log file, and it may take many days before we notice that our controller is ostensibly still running, but only inertly; it can't get its job done without talking to the API server. Sometimes the server changes its mind and starts fulfilling the requests again, but these failure periods can persist for days.

A caller of cache.Controller.Run should have some way of detecting that these failures are occurring in order to declare the process unhealthy. Retrying automatically to smooth over intermittent network trouble is a nice feature, but with neither the ability to control it nor detect its ongoing failure makes it dangerous.

I would be happy with either of the following two improvements:

• Accept a callback (perhaps via a new sibling method for Controller.Run) that tells a caller when these request failures arrive.
  It could also accept a caller-provided channel, and push errors into the channel when they arise, dropping errors that can't be delivered synchronously.
• Provide a way to integrate a controller into a "healthz" handler.
  That leaves the health criteria opaque to callers—and probably begs for some way to configure the thresholds—but still allows a calling process to indicate that it's in dire shape.

We discussed this gap in the "kubernetes-dev" channel in the "Kubernetes" Slack team.

[caesarxuchao]

cc @lavalamp he's the original author

[seh]

It occurred to me after writing the second suggestion about the channel that if the caller provides it, the caller can choose a buffer for the channel, but the caller can also close the channel prematurely, causing Controller.Run* to panic later. Alternately, if Controller.Run* returned the channel, then the caller can't control the buffer, but at least the caller can't close the channel either, assuming the returned channel only allows receive operations.

With a callback, we then have to worry about the caller panicking, or taking so long time to react that we can't attend to the next scheduled retrying of the failed operation. The main goal of hearing about these errors is counting them—best done over a rolling window. It's not so important which errors are arising; what matters is that lack of events firing on the ResourceEventHandler is not just for lack of observable activity in the cluster, but rather because we're deaf to any such activity.

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

Prevent issues from auto-closing with an /lifecycle frozen comment.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or @fejta.
/lifecycle stale

[seh]

/remove-lifecycle stale

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

[seh]

/remove-lifecycle stale

[lavalamp]

I agree with: `Provide a way to integrate a controller into a "healthz" handler` cc @cheftako

…

On Thu, Mar 22, 2018 at 11:20 AM Steven E. Harris ***@***.***> wrote: /remove-lifecycle stale — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#155 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAnglkECit83qyuQIndPXbniOy4-5ROzks5tg-twgaJpZM4Mj_gW> .

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

[seh]

/remove-lifecycle stale

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

[fejta-bot]

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten

[seh]

/remove-lifecycle rotten

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

[george-angel]

/remove-lifecycle stale

[discordianfish]

This issues makes it impossible to handle errors like this: kubernetes/ingress-nginx#2837

[discordianfish]

Unless I'm mistaken that means that every client using the informer has no way to handle connection issues with the apiserver and cluster operators have no way to monitor this either.

[discordianfish]

It should be possible to fix this now with kubernetes/kubernetes#73937 merged.

[discordianfish]

I think it's best to integrate these with healthz. I am not a huge fan of the callback idea as there's not much useful to be done if the informer stops working.

Not exactly sure what 'integrate with healthz' means, but a callback/channel would be what I (naively?) would want, to simply fail/exit with error or reconnect the informer. An error channel seems the idiomatic way to me.

I also think that we need to surface a metric that counts requests made by a client-- if a controller stops making more requests for long enough, it needs attention. We should also, in sibling metrics, track observed latency and observed errors.

While such metric would be useful either way, I think it's more robust to handle failing connections more explicitly.

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

[george-angel]

/remove-lifecycle stale

[nicks]

This problem has been causing a lot of problems for us. Informers get into an access denied error loop and spew out an overwhelming amount of error logs. For details, see tilt-dev/tilt#2702

I proposed a PR upstream that will address our problems, but I don't know if it addresses some of the other ideas on this thread around healthz: kubernetes/kubernetes#87329

[seh]

What a day! Thank you, @nicks, and all the careful reviewers.