Skip to content

do not skip host containers in NNC ContainerCallback#780

Merged
matthyx merged 1 commit into
mainfrom
host-fix
Apr 20, 2026
Merged

do not skip host containers in NNC ContainerCallback#780
matthyx merged 1 commit into
mainfrom
host-fix

Conversation

@matthyx
Copy link
Copy Markdown
Contributor

@matthyx matthyx commented Apr 16, 2026

Summary by CodeRabbit

  • Bug Fixes
    • Improved handling of host container events to ensure they are processed correctly through standard filtering logic rather than being skipped.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 16, 2026

Warning

Rate limit exceeded

@matthyx has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 43 minutes and 11 seconds before requesting another review.

Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 43 minutes and 11 seconds.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: c76445f9-43db-4e12-871f-3b32f75c9281

📥 Commits

Reviewing files that changed from the base of the PR and between fc86565 and d270025.

📒 Files selected for processing (2)
  • pkg/objectcache/applicationprofilecache/applicationprofilecache.go
  • pkg/objectcache/networkneighborhoodcache/networkneighborhoodcache.go
📝 Walkthrough

Walkthrough

The PR modifies container event handling in the network neighborhood cache. Instead of skipping host-container add events early, the code now forces their namespace to "host" and allows them to proceed through the ignore-filter and subsequent processing logic. Remove events receive the same namespace override treatment.

Changes

Cohort / File(s) Summary
Host Container Event Processing
pkg/objectcache/networkneighborhoodcache/networkneighborhoodcache.go
Modified ContainerCallback to force K8s.Namespace = "host" for host-container events instead of returning early on add, enabling them to flow through the standard ignore-filter and async add/delete logic for both add and remove operations.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

Poem

🐰 Host containers now flow free,
No early returns, just processing spree,
Namespace "host" marks the way,
Through filters and logic they'll stay! 🏃‍♂️✨

🚥 Pre-merge checks | ✅ 3
✅ 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 clearly and specifically describes the main change: preventing host containers from being skipped in the NNC ContainerCallback, which matches the core functionality alteration in the diff.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch host-fix

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

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

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@pkg/objectcache/networkneighborhoodcache/networkneighborhoodcache.go`:
- Around line 336-347: Notification handling sets host containers' namespace to
"host" then still calls nnc.cfg.IgnoreContainer, causing host events to be
dropped; fix by treating host containers as always allowed: after detecting a
host container via utils.IsHostContainer(notif.Container), set
notif.Container.K8s.Namespace = "host" and skip the subsequent
nnc.cfg.IgnoreContainer check (i.e., do not call IgnoreContainer for host
containers) in both the add (where addContainerWithTimeout is launched) and
remove (containercollection.EventTypeRemoveContainer) branches so host events
are never filtered out by namespace include/exclude rules.
- Around line 336-346: You're mutating the shared notif.Container.K8s.Namespace
in place which causes data races when callbacks run concurrently; instead, read
utils.IsHostContainer(notif.Container) and store the namespace in a local
variable for filtering with nnc.cfg.IgnoreContainer(namespace, ...) and only
create a deep copy of notif.Container (or at least copy K8s metadata) before
passing it to async calls like go nnc.addContainerWithTimeout(...) so the
goroutine operates on an owned copy; apply the same pattern in
applicationprofilecache where notif.Container is mutated.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 8cf9049e-1c33-4e39-a64a-fba524e4700a

📥 Commits

Reviewing files that changed from the base of the PR and between c34943b and fc86565.

📒 Files selected for processing (1)
  • pkg/objectcache/networkneighborhoodcache/networkneighborhoodcache.go

Comment thread pkg/objectcache/networkneighborhoodcache/networkneighborhoodcache.go Outdated
Comment thread pkg/objectcache/networkneighborhoodcache/networkneighborhoodcache.go Outdated
@github-actions
Copy link
Copy Markdown

Performance Benchmark Results

Node-Agent Resource Usage
Metric BEFORE AFTER Delta
Avg CPU (cores) 0.159 0.155 -2.9%
Peak CPU (cores) 0.167 0.159 -4.7%
Avg Memory (MiB) 394.495 311.582 -21.0%
Peak Memory (MiB) 401.109 314.441 -21.6%
Dedup Effectiveness (AFTER only)
Event Type Passed Deduped Ratio
capabilities 0 0 N/A
dns 1199 0 0.0%
hardlink 6000 0 0.0%
http 1700 119460 98.6%
network 900 78000 98.9%
open 33831 622288 94.8%
symlink 6000 0 0.0%
syscall 974 1875 65.8%
Event Counters
Metric BEFORE AFTER
capability_counter 8 7
dns_counter 1445 1401
exec_counter 7227 7011
network_counter 95069 92223
open_counter 792461 767238
syscall_counter 3519 3337

@matthyx matthyx added the release Create release label Apr 16, 2026
Mirror the APC ContainerCallback behavior: remap host containers to
the "host" namespace instead of returning early, so the NNC can track
network neighborhoods for host-mode workloads.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: Matthias Bertschy <matthias.bertschy@gmail.com>
@github-actions
Copy link
Copy Markdown

Performance Benchmark Results

Node-Agent Resource Usage
Metric BEFORE AFTER Delta
Avg CPU (cores) 0.159 0.162 +1.4%
Peak CPU (cores) 0.166 0.167 +0.7%
Avg Memory (MiB) 433.437 311.514 -28.1%
Peak Memory (MiB) 435.461 313.125 -28.1%
Dedup Effectiveness (AFTER only)
Event Type Passed Deduped Ratio
capabilities 1 0 0.0%
dns 1193 0 0.0%
hardlink 6000 0 0.0%
http 1704 119456 98.6%
network 900 77937 98.9%
open 33959 622293 94.8%
symlink 6000 0 0.0%
syscall 986 1887 65.7%
Event Counters
Metric BEFORE AFTER
capability_counter 10 10
dns_counter 1424 1417
exec_counter 7124 7125
network_counter 93691 93672
open_counter 779922 780944
syscall_counter 3496 3609

@matthyx matthyx moved this to Needs Reviewer in KS PRs tracking Apr 17, 2026
@matthyx matthyx merged commit d6fb040 into main Apr 20, 2026
28 checks passed
@matthyx matthyx deleted the host-fix branch April 20, 2026 15:56
@matthyx matthyx moved this from Needs Reviewer to To Archive in KS PRs tracking Apr 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release Create release

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

1 participant