ipcache: add tests reproducing host-IP-as-world misclassification during agent restart - #657
Closed
seanduncandatadog wants to merge 3 commits into
Conversation
…restart Add two test files that reproduce and document a bug where node/host IPs inside a local-DC CiliumCIDRGroup are misclassified as world identity during rolling Cilium agent restarts. Root cause (two cooperating code paths): 1. local_identity_restorer.go:128 - dumpOldIPCache() only restores identities with IdentityScopeLocal or ReservedIdentityIngress. ReservedIdentityHost (id=1, scope=global) is excluded. After ipcachemap.Recreate() wipes the BPF map, host IP entries are gone until syncHostIPs runs. 2. daemon.go startup ordering - K8sWatcher.InitK8sSubsystem() (line 202) starts CiliumCIDRGroup processing before syncHostIPs.StartAndWaitFirst() (line 249). During this window a host IP in a local-DC CiliumCIDRGroup receives only a cidrgroup label. 3. metadata.go:798 resolveLabels() - without reserved:host, HasHostLabel()=false, isInCluster=false, AddWorldLabel() fires. The IP is assigned world identity. Impact: traffic from node IPs to cluster-dns is denied because the CNP uses fromEntities:cluster (which excludes world). Confirmed in production at us1.fed.dog (plain1, us-gov-west-1a) on 2026-03-20 with 10,551 drops over 48h. pkg/ipcache/metadata_restart_test.go: - TestHostIPWorldFallbackDuringRestartWindow: asserts the buggy behaviour where a host IP with only a cidrgroup label gets world identity. Expected to fail once the root cause is fixed. - TestWorldFallbackDoesNotOccurWhenHostLabelPresentFirst: asserts the correct steady-state behaviour where reserved:host arrives before cidrgroup labels. pkg/ipcache/restore/local_identity_restorer_test.go: - TestHostIdentityExcludedFromIPCacheRestoration: directly tests the filter condition at local_identity_restorer.go:128 for all relevant identity types. - TestHostIdentityScopeIsGlobal: verifies that ReservedIdentityHost.Scope() returns IdentityScopeGlobal, which is why it is excluded from restoration. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
…restart Add two test files that reproduce and document a bug where host IPs covered by a CiliumCIDRGroup subnet are misclassified as world identity during rolling Cilium agent restarts. Root cause (two cooperating code paths): 1. local_identity_restorer.go:128 - dumpOldIPCache() only restores identities with IdentityScopeLocal or ReservedIdentityIngress. ReservedIdentityHost (id=1, scope=global) is excluded. After ipcachemap.Recreate() wipes the BPF map, host IP entries are gone until syncHostIPs runs. 2. daemon.go startup ordering - K8sWatcher.InitK8sSubsystem() (line 202) starts CiliumCIDRGroup processing before syncHostIPs.StartAndWaitFirst() (line 249). During this window a host IP covered by a CiliumCIDRGroup receives only a cidrgroup label — no reserved:host. 3. metadata.go:798 resolveLabels() - without reserved:host, HasHostLabel()=false, isInCluster=false, AddWorldLabel() fires. The IP is assigned world identity. Impact: traffic from the misclassified host IP is denied by CNPs that use fromEntities:cluster, because world (id=2) is not in the cluster entity. pkg/ipcache/metadata_restart_test.go: - TestHostIPWorldFallbackDuringRestartWindow: inserts only a cidrgroup label for a host IP (no reserved:host), runs doInjectLabels, and asserts that world identity is assigned. Expected to fail once the root cause is fixed. - TestWorldFallbackDoesNotOccurWhenHostLabelPresentFirst: verifies correct steady-state behaviour when reserved:host arrives before cidrgroup labels. pkg/ipcache/restore/local_identity_restorer_test.go: - TestHostIdentityExcludedFromIPCacheRestoration: tests the exact filter predicate from dumpOldIPCache line 128 for all relevant identity types. - TestHostIdentityScopeIsGlobal: verifies that ReservedIdentityHost.Scope() returns IdentityScopeGlobal, the mechanical reason it is excluded. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Extend the test suite with two additional tests that simulate the race condition using real goroutines, mirroring the actual concurrent actors in the daemon startup sequence. TestHostIPWorldFallbackRaceSimulation: - Uses two goroutines separated by an explicit ordering barrier (channel). - Goroutine A (K8sWatcher / CIDRGroup reconciler): upserts cidrgroup labels and injects. Represents daemon.go:202 completing before syncHostIPs. - After A closes cidrGroupDone, the test observes the intermediate state: world identity is assigned (the bug window is open). - Goroutine B (syncHostIPs): unblocked by cidrGroupDone, upserts host label. - After B completes the test verifies identity is corrected to host. - Run with -race to confirm there are no data races (the bug is a logical ordering issue, not a concurrent memory violation). TestHostIPWorldFallbackStress: - Runs 50 iterations of truly concurrent goroutines (A and B race without a barrier) and records how often A wins the race (world window observed) vs B wins (host identity assigned directly). - Asserts the FINAL state is always ReservedIdentityHost after both goroutines complete, regardless of scheduling order. - Useful for amplifying the race: run with -count=5 -race across many iterations to observe the ordering sensitivity. Neither test modifies the existing sequential tests. All four tests remain passing with the bug present and the goroutine-based tests provide a more faithful simulation of the daemon startup race. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Collaborator
|
Feel free to open a PR in the upstream Cilium repo. The main in this fork is kept in sync with the upstream main. We only push to our v1.xx-dd branches in case we need to apply a patch before it is released in the official upstream version. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
During rolling Cilium agent restarts, host IPs that fall within a
CiliumCIDRGroupsubnet can be transiently misclassified asworldidentity. This causespolicy_denieddrops for traffic from those IPs to endpoints whose CNP usesfromEntities: cluster(which does not includeworld, id=2).Root Cause
Three code paths cooperate to produce the bug:
1.
local_identity_restorer.go:128— host identity excluded from restorationReservedIdentityHost(id=1) hasIdentityScopeGlobal(scope bits = 0). It is excluded from the restored identity set. Afteripcachemap.Recreate()wipes the BPF map (cell.go:118), host IP entries are gone untilsyncHostIPsruns.2.
daemon.gostartup ordering — CiliumCIDRGroup processing racessyncHostIPsBetween these two lines, the Kubernetes watcher processes
CiliumCIDRGroupobjects and callsUpsertMetadatawith only acidrgroup:*label for IPs in the covered subnet — beforesyncHostIPshas insertedreserved:host.3.
metadata.go:798—resolveLabels()world fallbackThe host IP ends up with
worldidentity and is denied byfromEntities: clusterCNP rules.Changes
pkg/ipcache/metadata_restart_test.goTestHostIPWorldFallbackDuringRestartWindow: inserts only acidrgroup:*label for a host IP (simulating the restart window beforesyncHostIPsruns), callsdoInjectLabels, and asserts that world identity is assigned. This directly reproduces the bug and is expected to fail once a fix is applied.TestWorldFallbackDoesNotOccurWhenHostLabelPresentFirst: verifies the correct steady-state behaviour wherereserved:hostis present beforecidrgrouplabels arrive.pkg/ipcache/restore/local_identity_restorer_test.goTestHostIdentityExcludedFromIPCacheRestoration: tests the exact filter predicate fromdumpOldIPCache(line 128) against all relevant identity types, documenting which survive restoration.TestHostIdentityScopeIsGlobal: verifies thatReservedIdentityHost.Scope() == IdentityScopeGlobal, the direct mechanical reason it is excluded from restoration.Test plan
TestHostIPWorldFallbackDuringRestartWindowcurrently passes (the bug is present). It should be updated to assertReservedIdentityHostonce a fix is implemented.🤖 Generated with Claude Code