Skip to content

projectquota: return errors from StartWatcher instead of klog.Fatalln#426

Open
tamalsaha wants to merge 1 commit into
masterfrom
projectquota-no-fatal
Open

projectquota: return errors from StartWatcher instead of klog.Fatalln#426
tamalsaha wants to merge 1 commit into
masterfrom
projectquota-no-fatal

Conversation

@tamalsaha

Copy link
Copy Markdown
Contributor

Problem

ProjectQuotaReconciler.StartWatcher called klog.Fatalln in three places — a nil reconciler, an unknown resource Kind, and a failed Watch:

if r.ctrl == nil {
    klog.Fatalln("ProjectQuota reconciler is not setup yet!")
}
...
klog.Fatalln(err)

StartWatcher is invoked at runtime from the resource-discovery poller (graph.PollNewResourceTypes). A klog.Fatalln there calls os.Exit, so any one of these conditions on any single discovered resource type takes down the entire kube-ui-server process instead of failing just that watch.

Fix

  • StartWatcher now returns an error for each of the three conditions (with the offending resource/GVK included in the message).
  • The poller logs the error via klog.ErrorS and continues, consistent with the rest of the resilient discovery loop (cf. Isolate per-connection failures in graph reconciler #422 "Isolate per-connection failures").

No behavior in this path should be able to crash the aggregated apiserver.

Testing

  • go build ./... passes.
  • go vet and gofmt clean on the changed files.
  • grep klog.Fatal over pkg/ returns nothing.

ProjectQuotaReconciler.StartWatcher called klog.Fatalln on a nil reconciler,
an unknown resource Kind, or a failed Watch. Because StartWatcher runs from
the resource-discovery poller (graph.PollNewResourceTypes), any of these
conditions terminated the whole kube-ui-server process rather than failing
the single watch.

Make StartWatcher return an error and have the poller log it and continue,
consistent with the rest of the resilient discovery loop. No condition in
this path should bring down the aggregated apiserver.

Signed-off-by: Tamal Saha <tamal@appscode.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant