fix(store-client): non-blocking certificate watcher

[miguelvr]

Summary

certwatcher.Start() is a blocking call, which I did not realize after refactoring my original PR.

This makes the certificate watcher start on a separate goroutine to avoid blocking progress of the main goroutine

Type of Change

• 🐛 Bug fix
• ✨ New feature
• 💥 Breaking change
• 📚 Documentation
• 🔧 Refactoring
• 🔨 Build/CI

Component(s) Affected

• Core Services
• Documentation/CI
• Fault Management
• Health Monitors
• Janitor
• Other: ____________

Testing

• Tests pass locally
• Manual testing completed
• No breaking changes (or documented)

Checklist

• Self-review completed
• Documentation updated (if needed)
• Ready for review

Summary by CodeRabbit

• Improvements
  • MongoDB client initialization now operates asynchronously, enabling faster startup times and preventing initialization from blocking on certificate validation operations.
  • Enhanced logging integration provides improved visibility into certificate monitoring operations and errors.
  • Improved reliability with graceful error handling that no longer interrupts client creation, ensuring services remain available during certificate operation issues.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[copy-pr-bot]

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

[coderabbitai]

📝 Walkthrough

Walkthrough

The MongoDB client initialization was refactored to start the certificate watcher in a non-blocking goroutine instead of synchronously. Controller-runtime logging integration via slog was added to the client setup, with errors from the background watcher logged without failing client creation.

Changes

Cohort / File(s)	Summary
MongoDB Client Initialization store-client/pkg/client/mongodb_client.go	Added logr and ctrllog imports for controller-runtime logging. Configured slog integration in NewMongoDBClient. Refactored certificate watcher startup to execute in a background goroutine with error logging instead of blocking initialization.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 A watcher now hops in the background, so free,
No blocking, no waiting—just logging with glee!
With slog by our side and controller-runtime's care,
The client springs up without delay anywhere! ✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'fix(store-client): non-blocking certificate watcher' directly and clearly describes the main change—making the certificate watcher non-blocking by running it in a separate goroutine.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5c07522 and bfc693c.

📒 Files selected for processing (1)

• store-client/pkg/client/mongodb_client.go

🧰 Additional context used

📓 Path-based instructions (1)

**/*.go

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.go: Follow standard Go conventions with gofmt and golint
Use structured logging via log/slog in Go code
Wrap errors with context using fmt.Errorf("context: %w", err) in Go code
Within retry.RetryOnConflict blocks, return errors without wrapping to preserve retry behavior
Use meaningful variable names such as synced over ok for cache sync checks
Use client-go for Kubernetes API interactions in Go code
Prefer informers over direct API calls for watching Kubernetes resources
Implement proper shutdown handling with context cancellation in Go code
Package-level godoc required for all Go packages
Function comments required for all exported Go functions
Use inline comments for complex logic only in Go code
TODO comments should reference issues in Go code
Extract informer event handler setup into helper methods
Use separate informers for different Kubernetes resource types

Files:

• store-client/pkg/client/mongodb_client.go

🧠 Learnings (1)

📚 Learning: 2025-12-23T05:02:22.108Z

Learnt from: tanishagoyal2
Repo: NVIDIA/NVSentinel PR: 609
File: store-client/pkg/client/postgresql_pipeline_builder.go:119-132
Timestamp: 2025-12-23T05:02:22.108Z
Learning: In the NVSentinel codebase, protobuf fields stored in MongoDB should use lowercase field names (e.g., processingstrategy, componentclass, checkname). Ensure pipeline filters and queries that access protobuf fields in the database consistently use lowercase field names in the store-client package, avoiding camelCase mappings for MongoDB reads/writes.

Applied to files:

• store-client/pkg/client/mongodb_client.go

🔇 Additional comments (3)

store-client/pkg/client/mongodb_client.go (3)

24-24: LGTM: Logging integration imports

The imports support controller-runtime logging integration via slog, which is used later in the function.

Also applies to: 30-30

---

359-360: LGTM: Controller-runtime logging integration

Correctly configures controller-runtime to use slog for structured logging, following the coding guidelines.

---

376-383: Fix misleading success log message for certificate watcher

The log message "Certificate watcher started successfully" (line 387) is logged before the async goroutine actually completes, creating a false impression of success. Consider one of these approaches:

1. Move the success log inside the goroutine after cw.Start() succeeds
2. Change the log message to "Certificate watcher startup initiated" to clarify it's async
3. Log errors at a level that's more visible (the current error log may be missed in normal operation)

Additionally, consider adding panic recovery in the goroutine for robustness:

 go func() {
+	defer func() {
+		if r := recover(); r != nil {
+			slog.Error("Certificate watcher panicked", "panic", r)
+		}
+	}()
 	if err := cw.Start(cwCtx); err != nil {
 		slog.Error("Certificate watcher stopped with error", "error", err)
 	}
 }()

Likely an incorrect or invalid review comment.

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.5.0)

level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies"

---

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.}