source.Channel gets shutdown almost immediately and never processes any events

[charith-elastic]

After upgrading to controller-runtime 0.8.0, I discovered that our external event source implemented using source.Channel is not generating any events. Looking at the code, one odd thing that stood out to me was the useless lock here:

controller-runtime/pkg/source/source.go

Lines 241 to 244 in 818a2ce

	cs.destLock.Lock()
	defer cs.destLock.Unlock()
	return nil

It used to protect the append to the dest array, but now the append has moved further up as a fix to #942. After fixing the lock in the code, the event source started working again so I think there must be a race somewhere. The odd thing is that this unprotected append was present in 0.7.0 as well and that didn't affect us. So I am not sure whether my hypothesis is entirely correct.

Either way, I'll be happy to submit a patch to fix the lock unless there are any objections.

[charith-elastic]

I was too quick to celebrate with the above comment. It was just a lucky coincidence with timing. I think I found the root cause now:

controller-runtime/pkg/internal/controller/controller.go

Lines 163 to 171 in 3c5b358

	for _, watch := range c.startWatches {
	c.Log.Info("Starting EventSource", "source", watch.src)
	watchStartCtx, cancel := context.WithTimeout(ctx, c.CacheSyncTimeout)
	defer cancel()
	if err := watch.src.Start(watchStartCtx, watch.handler, c.Queue, watch.predicates...); err != nil {
	return err
	}
	}

The context created in line 166 is cancelled almost immediately by the defer statement. That context is used by the source.Channel implementation to control the goroutine that distributes the events:

controller-runtime/pkg/source/source.go

Lines 270 to 281 in 3c5b358

	func (cs *Channel) syncLoop(ctx context.Context) {
	for {
	select {
	case <-ctx.Done():
	// Close destination channels
	cs.doStop()
	return
	case evt := <-cs.Source:
	cs.distribute(evt)
	}
	}
	}

If I understand the issue that resulted in this change (#1219) correctly, I think that the timeout context should only be used with SyncingSource implementations when the WaitForSync method is called.