fix container watcher error propagation

[YakirOren]

Propagate container watcher errors to main instead of swallowing them, add FIM-only mode fallback

Summary by CodeRabbit

• Bug Fixes
  • Improved startup failure handling for container monitoring: when monitoring fails, systems configured for FIM now continue in FIM-only mode with warnings and without container monitoring; non-FIM failures preserve existing exit behavior.
  • Adjusted shutdown deferral so cleanup runs only when startup succeeded or FIM is active.
  • Cleanup and startup sequencing refined to maintain existing startup steps while handling failure paths more gracefully.

[coderabbitai]

📝 Walkthrough

Walkthrough

Container watcher startup and shutdown handling were changed: failures from container watcher Start now return wrapped errors instead of fatal logging; cmd/main.go treats container watcher Start failures differently depending on FIM mode (continue with warnings when FIM enabled, otherwise preserve original exit behavior). Deferred Stop() is only scheduled when appropriate.

Changes

Cohort / File(s)	Summary
Startup / container watcher control flow cmd/main.go, pkg/containerwatcher/v2/container_watcher.go	Adjusted container watcher Start error handling to return wrapped errors instead of fatal logs; cmd/main.go now conditionally continues startup in FIM-only mode when watcher Start fails and FIM is enabled, and defers Stop() only when Start succeeded or FIM is active.

Sequence Diagram(s)

sequenceDiagram
    participant Main
    participant ProfileMgr
    participant Network
    participant ContainerWatcher
    participant FIM
    participant Shutdown

    Main->>ProfileMgr: initialize
    Main->>Network: start stream
    Main->>ContainerWatcher: Start()
    alt ContainerWatcher Start succeeds
        ContainerWatcher-->>Main: success
        Main->>ContainerWatcher: defer Stop()
    else ContainerWatcher Start fails
        ContainerWatcher-->>Main: wrapped error
        Main->>FIM: is FIM enabled?
        alt FIM enabled
            Main-->>Main: log warning, continue (FIM-only)
            Main->>Shutdown: skip container watcher monitoring
        else FIM disabled
            Main-->>Shutdown: perform exit per error
        end
    end

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 I hopped through code where watchers might trip,
I wrapped up the errors, gave startup a skip,
If FIM keeps me steady, I quietly stay,
Else I tidy the exits and end the play,
A hopeful little rabbit, bounding light on each tip.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'fix container watcher error propagation' directly and clearly summarizes the main change: improving error handling in the container watcher to ensure errors are properly propagated instead of being silently suppressed.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix/container-watcher-startup

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

pkg/containerwatcher/v2/container_watcher.go (1)

304-315: ⚠️ Potential issue | 🟠 Major

Avoid leaking background loops when Start() fails and FIM-only mode continues.

Now that Start() returns errors (instead of fatal exit), failures after the EOL handler / worker loops are started can leave goroutines running indefinitely because ctx isn’t canceled and Stop() exits early when running == false. In FIM-only mode this contradicts “container monitoring disabled” and can waste resources. Consider deferring these goroutines and EOL registration until after successful init (or add explicit cleanup on error).

🔧 Suggested reordering to start background loops only after successful init

-	go func() {
-		// Read from the container EOL notification channel and call unregisterContainer
-		for {
-			select {
-			case container := <-cw.containerEolNotificationChannel:
-				if container != nil {
-					logger.L().Debug("Container EOL notification received", helpers.String("containerID", container.Runtime.ContainerID))
-					cw.unregisterContainer(container)
-				}
-			case <-ctx.Done():
-				logger.L().Ctx(ctx).Info("Stopping container EOL notification handler")
-				return
-			}
-		}
-	}()
-
-	cw.containerProfileManager.RegisterForContainerEndOfLife(cw.containerEolNotificationChannel)
-
-	// Start event processing loop
-	go cw.eventProcessingLoop()
-
-	// Start worker pool goroutine
-	go cw.workerPoolLoop()
-
 	cw.gadgetRuntime = local.New()
 	if err := cw.gadgetRuntime.Init(nil); err != nil {
 		return fmt.Errorf("initializing gadget runtime: %w", err)
 	}
@@
 	tracerManagerV2 := NewTracerManager(cw.cfg, tracerFactory)
 	if err := tracerManagerV2.StartAllTracers(ctx); err != nil {
 		return fmt.Errorf("starting tracer manager: %w", err)
 	}
 	cw.tracerManagerV2 = tracerManagerV2
+
+	// Start background handlers only after successful init
+	go func() {
+		// Read from the container EOL notification channel and call unregisterContainer
+		for {
+			select {
+			case container := <-cw.containerEolNotificationChannel:
+				if container != nil {
+					logger.L().Debug("Container EOL notification received", helpers.String("containerID", container.Runtime.ContainerID))
+					cw.unregisterContainer(container)
+				}
+			case <-ctx.Done():
+				logger.L().Ctx(ctx).Info("Stopping container EOL notification handler")
+				return
+			}
+		}
+	}()
+	cw.containerProfileManager.RegisterForContainerEndOfLife(cw.containerEolNotificationChannel)
+
+	// Start event processing loop
+	go cw.eventProcessingLoop()
+
+	// Start worker pool goroutine
+	go cw.workerPoolLoop()