fix: Pod IP Deletion Leak in eBPF FilterMap

[alexcastilio]

Description

Fix: Pod IP Deletion Leak in eBPF FilterMap

Problem

Pod IPs accumulate indefinitely in the eBPF filtermap because DELETE operations fail in two ways:

1. PodCallBackFn guard drops delete events: When a namespace is removed from the include list or a pod annotation is removed, nsOfInterest() and podOfInterest() both return false — the PodDeleted event is silently discarded before reaching handlePodEvent().

2. applyDirtyPodsDelete uses wrong metadata: Even if the event reaches the delete path, Annotated and Namespaced flags are re-evaluated at delete time against current state (not the state when the IP was added). The filtermanager requires matching (Requestor, RequestMetadata) to remove a reference — a delete with the wrong metadata is a no-op.

This causes "no space left on device" errors when the eBPF filtermap fills up (255 entries).

Please provide a brief description of the changes made in this pull request.

Fix

Two changes in pkg/module/metrics/metrics_module.go:

1. Bypass guard for PodDeleted events — PodCallBackFn now skips the nsOfInterest/podOfInterest check when event.Type == EventTypePodDeleted, ensuring delete events always reach handlePodEvent.

2. Always delete with both metadata types — applyDirtyPodsDelete unconditionally issues DeleteIPs with both modulePodReqMetadata ("pod") and moduleReqMetadata ("namespace") for every IP in the delete list. The filtermanager's deleteIP is a safe no-op when the metadata doesn't exist for an IP, so extra calls cause no harm.

Additional minor fix: Replaced zap.Any with fmt.Sprint for []net.IP log fields to fix unsupported value type errors in log output.

Tests

Unit tests added and manual test done.

Manual validation

Scenario 1 — Namespace filter change (annotations mode)

Test:

helm install with: enableAnnotations=true, enablePodLevel=true

kubectl create namespace test-leak-ns
kubectl run test-pod -n test-leak-ns --image=nginx
kubectl annotate namespace test-leak-ns retina.sh=observe
# → Verify: "Adding IPs to filter manager" with pod IP in retina logs
kubectl annotate namespace test-leak-ns retina.sh-
kubectl delete pod test-pod -n test-leak-ns
# → Verify: "Adding pod IP to DELETE dirty pods cache" and "Deleting Ips in dirty pods from filtermap" in retina logs

Logs:

=== ADD phase ===
Defaulted container "retina" out of: retina, init-retina (init)
ts=2026-03-16T10:30:44.504Z level=info caller=metrics/metrics_module.go:391 msg="Namespaces to add" namespaces=
ts=2026-03-16T10:30:44.504Z level=info caller=metrics/metrics_module.go:391 msg="Namespaces to add" namespaces=test-leak-ns
ts=2026-03-16T10:30:44.504Z level=info caller=metrics/metrics_module.go:397 msg="Adding IPs to filter manager" namespace=test-leak-ns ips=[10.224.0.32]
namespace/test-leak-ns annotated
pod "test-pod" deleted
=== DELETE phase ===
Defaulted container "retina" out of: retina, init-retina (init)
ts=2026-03-16T10:31:12.955Z level=info caller=metrics/metrics_module.go:478 msg="Adding pod IP to DELETE dirty pods cache" pod name=test-leak-ns/test-pod
ts=2026-03-16T10:31:13.504Z level=debug caller=metrics/metrics_module.go:544 msg="Deleting Ips in dirty pods from filtermap" IPs=[10.224.0.32]

Scenario 2 — Namespace filter change (MetricsConfiguration CRD mode)

Test:

helm install with: enableAnnotations=false, enablePodLevel=true

kubectl create namespace test-leak-ns
kubectl run test-pod -n test-leak-ns --image=nginx
Apply MetricsConfiguration CRD with namespaces.include: [test-leak-ns]
# → Verify: "Adding IPs to filter manager" with pod IP in retina logs
Update MetricsConfiguration CRD to namespaces.include: [default]
kubectl delete pod test-pod -n test-leak-ns
# → Verify: "Adding pod IP to DELETE dirty pods cache" and "Deleting Ips in dirty pods from filtermap" in retina logs

Logs:

=== ADD phase ===
Defaulted container "retina" out of: retina, init-retina (init)
ts=2026-03-16T10:37:22.462Z level=info caller=metrics/metrics_module.go:158 msg="Reconciling metric module" spec= specError="unsupported value type"
ts=2026-03-16T10:37:22.462Z level=info caller=metrics/metrics_module.go:391 msg="Namespaces to add" namespaces=
ts=2026-03-16T10:37:22.462Z level=info caller=metrics/metrics_module.go:391 msg="Namespaces to add" namespaces=test-leak-ns
ts=2026-03-16T10:37:22.463Z level=info caller=metrics/metrics_module.go:397 msg="Adding IPs to filter manager" namespace=test-leak-ns ips=[10.224.0.36]
metricsconfiguration.retina.sh/test-metricsconfig configured
pod "test-pod" deleted
=== DELETE phase ===
Defaulted container "retina" out of: retina, init-retina (init)
ts=2026-03-16T10:37:49.138Z level=info caller=metrics/metrics_module.go:478 msg="Adding pod IP to DELETE dirty pods cache" pod name=test-leak-ns/test-pod
ts=2026-03-16T10:37:49.465Z level=debug caller=metrics/metrics_module.go:544 msg="Deleting Ips in dirty pods from filtermap" IPs=[10.224.0.36]
metricsconfiguration.retina.sh "test-metricsconfig" deleted
namespace "test-leak-ns" deleted

Scenario 3 — Pod annotation removed then deleted

Test:

helm install with: enableAnnotations=true, enablePodLevel=true

Create pod with annotation retina.sh=observe in default namespace
# → Verify: "Adding pod IP to ADD dirty pods cache" in retina logs
kubectl annotate pod annotated-pod -n default retina.sh-
kubectl delete pod annotated-pod -n default
# → Verify: "Adding pod IP to DELETE dirty pods cache" and "Deleting Ips in dirty pods from filtermap" in retina logs

Logs:

=== ADD phase ===
Defaulted container "retina" out of: retina, init-retina (init)
ts=2026-03-16T10:32:59.294Z level=info caller=metrics/metrics_module.go:475 msg="Adding pod IP to ADD dirty pods cache" pod name=default/annotated-pod
ts=2026-03-16T10:32:59.504Z level=debug caller=metrics/metrics_module.go:515 msg="Adding annotated pod IPs to filtermap" IPs=[10.224.0.31]
pod/annotated-pod annotated
pod "annotated-pod" deleted
=== DELETE phase ===
Defaulted container "retina" out of: retina, init-retina (init)
ts=2026-03-16T10:33:14.229Z level=info caller=metrics/metrics_module.go:470 msg="Adding pod IP to DELETE dirty pods cache. Pod not annotated or in namespace of interest." pod name=default/annotated-pod
ts=2026-03-16T10:33:14.505Z level=debug caller=metrics/metrics_module.go:544 msg="Deleting Ips in dirty pods from filtermap" IPs=[10.224.0.31]

Related Issue

#2085

Checklist

• I have read the contributing documentation.
• I signed and signed-off the commits (git commit -S -s ...). See this documentation on signing commits.
• I have correctly attributed the author(s) of the code.
• I have tested the changes locally.
• I have followed the project's style guidelines.
• I have updated the documentation, if necessary.
• I have added tests, if applicable.

Screenshots (if applicable) or Testing Completed

Please add any relevant screenshots or GIFs to showcase the changes made.

Additional Notes

Add any additional notes or context about the pull request here.

---

Please refer to the CONTRIBUTING.md file for more information on how to contribute to this project.

[github-actions]

Retina Code Coverage Report

Total coverage increased from 33.2% to 33.3% ✅

Increased diff

Impacted Files	Coverage
pkg/module/metrics/metrics_module.go	79.51% ... 81.79% (2.28%)	⬆️

Decreased diff

Impacted Files	Coverage
pkg/enricher/enricher.go	57.8% ... 56.4% (-1.4%)	⬇️

[aanchal22]

A few gaps I noticed from my investigation that the two PRs don't cover:

1. Spurious DELETE event protection
   When a pod DELETE event fires, neither PR verifies the pod is actually gone from the cache before processing. Due to the cache timing issue (cache updated before event published), spurious DELETE events during
   startup or rapid pod churn could remove valid IPs from the filtermap. Our branch added a cache check:

if endpoint := m.daemonCache.GetPodByIP(ip.String()); endpoint != nil {
     // Pod still exists in cache — ignore spurious DELETE
     return
 }

2. Forced Annotated = true on IP reuse (in handlePodEvent)
   When a pod IP is reused by an untracked pod, the current code forces podCacheEntry.Annotated = true before adding to the delete cache. This causes the delete to use pod-annotation metadata even if the original
   IP was added with namespace metadata, potentially leaving a stale entry. The brute-force "delete with both" approach in this PR may mask this, but the forced flag is still incorrect.

3. Filtermanager observability
   No warning logs are emitted when deleteIP fails in the filtermanager cache (requestor not found, IP not found). This makes it harder to diagnose leak issues in production. Adding warnings to
   pkg/managers/filtermanager/cache.go for these failure paths would improve debuggability.

4. eBPF filter map size configurability
   The retina_filter eBPF map max_entries is hardcoded at 255. For clusters with many tracked pods, this can cause "no space left on device" errors. I have a separate PR#2117 for making this configurable via Helmvalues / env var.

[agrawaliti]

A few gaps I noticed from my investigation that the two PRs don't cover:

1. Spurious DELETE event protection
   When a pod DELETE event fires, neither PR verifies the pod is actually gone from the cache before processing. Due to the cache timing issue (cache updated before event published), spurious DELETE events during
   startup or rapid pod churn could remove valid IPs from the filtermap. Our branch added a cache check:

if endpoint := m.daemonCache.GetPodByIP(ip.String()); endpoint != nil {
     // Pod still exists in cache — ignore spurious DELETE
     return
 }

2. Forced Annotated = true on IP reuse (in handlePodEvent)
   When a pod IP is reused by an untracked pod, the current code forces podCacheEntry.Annotated = true before adding to the delete cache. This causes the delete to use pod-annotation metadata even if the original
   IP was added with namespace metadata, potentially leaving a stale entry. The brute-force "delete with both" approach in this PR may mask this, but the forced flag is still incorrect.
3. Filtermanager observability
   No warning logs are emitted when deleteIP fails in the filtermanager cache (requestor not found, IP not found). This makes it harder to diagnose leak issues in production. Adding warnings to
   pkg/managers/filtermanager/cache.go for these failure paths would improve debuggability.
4. eBPF filter map size configurability
   The retina_filter eBPF map max_entries is hardcoded at 255. For clusters with many tracked pods, this can cause "no space left on device" errors. I have a separate PR#2117 for making this configurable via Helmvalues / env var.

Hello @aanchal22 Thanks for the review, addressed your comments.

[Copilot]

Pull request overview

Fixes an eBPF filtermap leak in the metrics module by ensuring pod IP DELETE events are processed and removed even when namespace/pod “interest” state changes after the IP was added.

Changes:

• Bypass nsOfInterest/podOfInterest early-return guard for EventTypePodDeleted so deletes always reach handlePodEvent.
• Make applyDirtyPodsDelete always attempt DeleteIPs with both request metadata types (pod and namespace) for each deleted IP.
• Adjust logging of []net.IP fields to avoid log encoding errors, and add unit tests covering leak scenarios and IP-reuse churn.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
pkg/module/metrics/metrics_module.go	Ensures delete events aren’t dropped and deletes IPs using both metadata types; tweaks IP logging and adds an IP-reuse DELETE guard.
pkg/module/metrics/metrics_module_linux_test.go	Adds unit tests covering namespace/annotation changes, normal lifecycle, never-tracked deletes, and IP reuse behavior.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The new EventTypePodDeleted handling includes an additional guard that ignores DELETE events when daemonCache.GetPodByIP(ip) still returns an endpoint (IP reuse / churn). This behavior change isn’t mentioned in the PR description; please document it there (or in a code comment that explains the ordering assumption between cache updates and pubsub delivery) so future maintainers understand why deletes may be skipped.

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

handlePodEvent uses nsOfInterest() / namespace maps when building podCacheEntry (just above this switch) without holding the module mutex. Since those maps are mutated under Module.Lock() in Reconcile()/appendIncludeList(), pod events racing with reconcile can cause concurrent map read/write panics. Consider computing Annotated/Namespaced under RLock() in PodCallBackFn and passing the booleans into handlePodEvent, or otherwise guaranteeing nsOfInterest() is only called while the module lock is held (without introducing RWMutex re-entrancy deadlocks).

[Copilot]

Log message uses "Ips"; since "IP" is an acronym, this should be "IPs" for consistency with other logs/metrics terminology and easier grepping.

Suggested change

	m.l.Debug("Deleting Ips in dirty pods from filtermap", zap.String("IPs", fmt.Sprint(ipsToDelete)))
	m.l.Debug("Deleting IPs in dirty pods from filtermap", zap.String("IPs", fmt.Sprint(ipsToDelete)))

[Copilot]

Using AnyTimes() here makes the test less strict and can hide unintended repeated DeleteIPs calls. Since the expected behavior is a single additional no-op delete attempt per lifecycle, prefer Times(1) (or otherwise asserting an upper bound) so regressions are caught.

Suggested change

	fm.EXPECT().DeleteIPs([]net.IP{ip1}, gomock.Any(), modulePodReqMetadata).Return(nil).AnyTimes()
	fm.EXPECT().DeleteIPs([]net.IP{ip1}, gomock.Any(), modulePodReqMetadata).Return(nil).Times(1)

[Copilot]

Same as above: AnyTimes() can mask extra/unexpected calls. If the code should only issue one extra delete for moduleReqMetadata, consider tightening this to Times(1) (or setting an explicit max) to keep the test deterministic and regression-resistant.

Suggested change

	// Allow the extra moduleReqMetadata delete (no-op) after fix
	fm.EXPECT().DeleteIPs([]net.IP{ip1}, gomock.Any(), moduleReqMetadata).Return(nil).AnyTimes()
	// Allow exactly one extra moduleReqMetadata delete (no-op) after fix
	fm.EXPECT().DeleteIPs([]net.IP{ip1}, gomock.Any(), moduleReqMetadata).Return(nil).Times(1)