feat(notification): skip waitFor notifications from stale generations

[adityathebe]

Pending waitFor notifications can remain pending while a Kubernetes resource moves on to a newer rollout generation.

Store the config metadata.generation in the notification payload when the pending notification is created. During pending processing, compare it with the current resource generation.

If the generation changed, treat the pending notification as stale and mark it skipped instead of sending it for the newer rollout.

Summary by CodeRabbit

• New Features
  • Notifications are now skipped when resource generation changes before the notification waiting period elapses, with a clear message indicating the generation change.

[coderabbitai]

Walkthrough

The PR introduces generation-based staleness detection for notifications. It adds logic to extract metadata.generation from config items, compares it against the payload's stored generation, and skips notification dispatch if the generation has changed since the notification was initially queued, marking the record as skipped.

Changes

Cohort / File(s)	Summary
Generation Detection Logic notification/generation.go	New module providing functions to extract metadata.generation from config items, normalize values across Go types, and determine whether to skip notifications based on generation changes.
Notification Pipeline Integration notification/job.go	Integrates generation-based staleness check as an early exit in processPendingNotification, marking history records as skipped with generation-specific error message when generation has changed.
Payload Enhancement notification/send.go	Adds new ResourceGeneration field to NotificationEventPayload struct and populates it during payload creation from config generation data.
Test Coverage notification/notification_test.go	Adds test case verifying that pending notifications transition to skipped status when config generation changes between initial notification creation and processing.

Possibly related PRs

• fix(notification): skip stale notification targets #2968: Implements similar early-exit skip logic in processPendingNotification but for when the resource no longer exists.
• fix: restore original event properties for pending notifications #3023: Modifies the same notification pipeline and payload/CEL environment construction in processPendingNotification.

Suggested reviewers

• moshloop

🚥 Pre-merge checks | ✅ 4 | ❌ 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 (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: skipping waitFor notifications when the resource generation has changed, which is the core functionality added across all modified files.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/notification-waitfor-generation

✨ Simplify code

• Create PR with simplified code
• Commit simplified code in branch feat/notification-waitfor-generation

---

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]

🧹 Nitpick comments (1)

notification/job.go (1)

440-452: Check generation before the wait-for re-evaluation path.

This runs after shouldSkipNotificationDueToHealth, so Kubernetes-scraped configs can still trigger one more incremental scrape/requeue before the stale row is skipped. If the intent is to drop stale waitFor rows as soon as they become eligible, move the generation comparison ahead of that helper or fold it into that helper’s early-exit logic.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@notification/job.go` around lines 440 - 452, The generation check is
happening after the wait-for re-evaluation path
(shouldSkipNotificationDueToHealth), allowing one extra scrape; move the
generation comparison earlier so stale waitFor rows are dropped immediately:
call or inline shouldSkipDueToGeneration (and use generationChangedMessage for
the reason) before invoking shouldSkipNotificationDueToHealth, or fold the
generation logic into shouldSkipNotificationDueToHealth so it returns an
early-exit when generation changed; ensure the same DB update and traceLog
behavior (using notif.ID, currentHistory.ID, payload.EventName,
payload.ResourceID) is preserved when marking status as
NotificationStatusSkipped and setting the error reason.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@notification/job.go`:
- Around line 440-452: The generation check is happening after the wait-for
re-evaluation path (shouldSkipNotificationDueToHealth), allowing one extra
scrape; move the generation comparison earlier so stale waitFor rows are dropped
immediately: call or inline shouldSkipDueToGeneration (and use
generationChangedMessage for the reason) before invoking
shouldSkipNotificationDueToHealth, or fold the generation logic into
shouldSkipNotificationDueToHealth so it returns an early-exit when generation
changed; ensure the same DB update and traceLog behavior (using notif.ID,
currentHistory.ID, payload.EventName, payload.ResourceID) is preserved when
marking status as NotificationStatusSkipped and setting the error reason.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 941d9341-670d-4e17-8cf5-35e72e63fc39

📥 Commits

Reviewing files that changed from the base of the PR and between 7227582 and 7bf6f9c.

📒 Files selected for processing (4)

• notification/generation.go
• notification/job.go
• notification/notification_test.go
• notification/send.go