client-go record: avoid panic when watch creation failed

The previous attempt to fix this in
6aa779f4ed (diff-efa2cd1347df22ace5a516ea794152d00ef2a079db135c81787ed920ecb73658)
didn't address the root cause (or perhaps created it, not sure): the goroutine
must not be started if watch creation failed.

Instead, the error gets logged (as before) and an empty watch gets returned to
the caller (new). This is necessary because the function doesn't have an error
return value and changing that now would be disruptive. The empty watch is
valid and usable, so callers won't crash when they calls Stop.

This showed up recently in failed unit tests, probably because test
cancellation makes this error more likely:

   "Unable start event watcher (will not retry!)" err="broadcaster already
   stopped" logger="TestGarbageCollectorConstruction leaked goroutine"

The logger value and a preceding warning show that this occurs after test
completion.

Kubernetes-commit: 080432c46a7a49c3abf86d7fc5f2a5d7abc92239
This commit is contained in:
Patrick Ohly 2024-05-29 14:10:49 +02:00 committed by Kubernetes Publisher
parent 5d650114a5
commit b4e1cfcfab

View File

@ -395,7 +395,11 @@ func (e *eventBroadcasterImpl) StartStructuredLogging(verbosity klog.Level) watc
func (e *eventBroadcasterImpl) StartEventWatcher(eventHandler func(*v1.Event)) watch.Interface {
watcher, err := e.Watch()
if err != nil {
// This function traditionally returns no error even though it can fail.
// Instead, it logs the error and returns an empty watch. The empty
// watch ensures that callers don't crash when calling Stop.
klog.FromContext(e.cancelationCtx).Error(err, "Unable start event watcher (will not retry!)")
return watch.NewEmptyWatch()
}
go func() {
defer utilruntime.HandleCrash()