[shell-operator] fix: stop informer correctly

[timmilesdw]

Overview

Implement graceful shutdown for client-go informers and add structured shutdown logs across shell-operator components.

What this PR does / why we need it

• Introduces proper lifecycle tracking and waiting for informer goroutines to exit:
  • Adds a done channel to Factory and starts informer.Run(...) exactly once per factory; closes done when the goroutine exits.
  • FactoryStore.Stop(...) now, upon removing the last handler, cancels the shared context and waits for <-done before deleting the factory entry.
  • Adds FactoryStore.WaitStopped(index) to block until a factory for a given FactoryIndex fully stops.
• Adds completion signaling to informers:
  • resourceInformer.wait() waits for shared informer termination via FactoryStore.WaitStopped.
  • namespaceInformer wraps Run() with a done channel and exposes wait().
• Adds upper-level waits:
  • Monitor now has Wait() to wait for all resource/namespace informers to stop.
  • KubeEventsManager now has Wait() to aggregate Monitor.Wait() calls without holding locks while blocking.
• Makes operator shutdown truly graceful with logs:
  • Shutdown() now logs each stage, cancels KubeEventsManager, then waits for completion (Wait()), then stops and waits for queues.

Special notes for your reviewer

• Concurrency:
  • KubeEventsManager.Wait() snapshots monitors under RLock and releases the lock before waiting, avoiding deadlocks and map iteration races.
  • FactoryStore.Stop(...) waits outside the lock for the done channel, then reacquires the lock to delete the factory and broadcast to waiters.
• Logging:
  • Added clear shutdown logs to operator.Shutdown(); feel free to request additional log details or levels.
• Risk/mitigation:
  • If an informer fails to exit after cancel(), waits will block. Current behavior intentionally favors correctness. If desired, we can add timeouts to Wait() paths in a follow-up.
• How to validate:
  • Start shell-operator; send SIGTERM/SIGINT; observe logs:
    • “shutdown: begin” → “schedule manager stopped” → “kube events manager canceled, waiting for informers” → “kube events manager done” → “task queues stop signaled, waiting” → “task queues stopped”.

[Copilot]

Pull request overview

This PR implements graceful shutdown for client-go informers with proper lifecycle tracking and completion signaling. The changes ensure that all informer goroutines fully exit before resources are cleaned up, preventing potential race conditions and panics during shutdown.

Changes:

• Replaced PauseHandleEvents() method with Stop() and Wait() methods across the kube events manager stack
• Added proper lifecycle tracking to Factory with a done channel that signals when informer goroutines exit
• Implemented WaitStopped() in FactoryStore using condition variables to allow blocking until factory cleanup completes
• Enhanced operator shutdown with structured logging at each phase of the shutdown sequence

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
pkg/kube_events_manager/factory.go	Added done channel to Factory, implemented WaitStopped() with condition variable, ensured single informer.Run() per factory
pkg/kube_events_manager/kube_events_manager.go	Replaced PauseHandleEvents() with Stop() and Wait(), snapshots monitors before waiting to avoid deadlocks
pkg/kube_events_manager/monitor.go	Converted PauseHandleEvents() to Wait() that blocks until all informers stop
pkg/kube_events_manager/resource_informer.go	Replaced pauseHandleEvents() with wait() that blocks on FactoryStore.WaitStopped()
pkg/kube_events_manager/namespace_informer.go	Added done channel and wait() method, wrapped Run() to signal completion
pkg/shell-operator/operator.go	Added structured shutdown logs and proper Wait() call after Stop() for informers
test/integration/kube_event_manager/kube_event_manager_test.go	Updated test cleanup to use Stop() instead of PauseHandleEvents()

Comments suppressed due to low confidence (1)

pkg/kube_events_manager/resource_informer.go:64

• The stopped field is still checked in handleWatchEvent() (line 281), but it's never set to true in the new implementation since pauseHandleEvents() was removed. This appears to be dead code that should either be removed or properly set when the context is canceled.

	stopped bool

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.