fix: node drainer enqueuing duplicate elements [WIP]

[XRFXLP]

Summary

Not working

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

• Bug Fixes
  • Fixed an issue that could cause duplicate operations during change stream replays and cold starts, improving system efficiency and preventing redundant processing.

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

[coderabbitai]

📝 Walkthrough

Walkthrough

The setInitialStatusAndEnqueue function in the reconciler now conditionally enqueues to the queue manager only when ModifiedCount > 0. Previously, enqueuing occurred unconditionally. This change prevents duplicate enqueues during change stream replays or cold starts.

Changes

Cohort / File(s)	Summary
Reconciler Queue Logic node-drainer/pkg/reconciler/reconciler.go	Modified setInitialStatusAndEnqueue to skip enqueueing when ModifiedCount ≤ 0, with debug logging, preventing duplicate queue operations during replays.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main change: preventing duplicate enqueueing in the node drainer by making enqueue conditional on ModifiedCount > 0.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

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

---

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

🧹 Nitpick comments (1)

node-drainer/pkg/reconciler/reconciler.go (1)

231-242: Excellent fix for duplicate enqueues!

The conditional enqueue based on ModifiedCount > 0 correctly prevents duplicate enqueues during change stream replays or cold starts. The atomic MongoDB update with the $ne: StatusInProgress filter ensures only one thread can claim an event, and only that thread will enqueue it.

Optional: Improve log message accuracy

The debug log message at line 238 says "Event already in progress" but this case also occurs when MatchedCount = 0 (document not found). Consider making the message more precise:

-	slog.Debug("Event already in progress, skipping enqueue",
+	slog.Debug("Skipping enqueue - event not claimed",
 		"node", nodeName,
-		"documentID", fmt.Sprintf("%v", documentID))
+		"documentID", fmt.Sprintf("%v", documentID),
+		"modifiedCount", result.ModifiedCount,
+		"matchedCount", result.MatchedCount)

This provides clearer context when debugging, distinguishing between "already in progress" vs. "document not found" cases.

📜 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 82e7180 and 50732b5.

📒 Files selected for processing (1)

• node-drainer/pkg/reconciler/reconciler.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:

• node-drainer/pkg/reconciler/reconciler.go

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)

• GitHub Check: modules-lint-test (labeler)
• GitHub Check: modules-lint-test (janitor)
• GitHub Check: modules-lint-test (node-drainer)
• GitHub Check: modules-lint-test (fault-quarantine)
• GitHub Check: modules-lint-test (platform-connectors)
• GitHub Check: E2E Tests (AMD64 + PostgreSQL)
• GitHub Check: E2E Tests (AMD64 + MongoDB)
• GitHub Check: E2E Tests (ARM64 + MongoDB)
• GitHub Check: E2E Tests (ARM64 + PostgreSQL)
• GitHub Check: CodeQL PR Analysis