Skip to content

Commit 63e21eb

Browse files
ipcache: add goroutine-based race simulation for host-IP-as-world bug
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>
1 parent b2be3c8 commit 63e21eb

1 file changed

Lines changed: 208 additions & 0 deletions

File tree

pkg/ipcache/metadata_restart_test.go

Lines changed: 208 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,13 +30,15 @@ package ipcache
3030

3131
import (
3232
"net/netip"
33+
"sync"
3334
"testing"
3435

3536
"github.com/stretchr/testify/assert"
3637
"github.com/stretchr/testify/require"
3738

3839
cmtypes "github.com/cilium/cilium/pkg/clustermesh/types"
3940
"github.com/cilium/cilium/pkg/identity"
41+
ipcacheTypes "github.com/cilium/cilium/pkg/ipcache/types"
4042
"github.com/cilium/cilium/pkg/labels"
4143
"github.com/cilium/cilium/pkg/option"
4244
"github.com/cilium/cilium/pkg/source"
@@ -185,3 +187,209 @@ func TestWorldFallbackDoesNotOccurWhenHostLabelPresentFirst(t *testing.T) {
185187
"Identity must not have world label when reserved:host is present. Labels: %v",
186188
resolvedIdentity.Labels)
187189
}
190+
191+
// TestHostIPWorldFallbackRaceSimulation simulates the actual daemon startup
192+
// sequencing using real goroutines and explicit synchronisation barriers.
193+
//
194+
// This mirrors the two concurrent actors from daemon.go:
195+
// - goroutine A: daemon.go:202 — K8sWatcher processes CiliumCIDRGroups
196+
// - goroutine B: daemon.go:249 — syncHostIPs.StartAndWaitFirst() completes
197+
//
198+
// The barrier channel enforces the ordering that causes the bug: A completes
199+
// and triggers label injection BEFORE B starts. The test then confirms:
200+
// 1. During the window (after A, before B): world identity is assigned.
201+
// 2. After B completes: identity is corrected to reserved:host.
202+
//
203+
// Run with -race to confirm there are no concurrent memory access violations
204+
// (the bug is a *logical* ordering issue, not a data race):
205+
//
206+
// go test ./pkg/ipcache/... -run TestHostIPWorldFallbackRaceSimulation -race -v
207+
func TestHostIPWorldFallbackRaceSimulation(t *testing.T) {
208+
s := setupIPCacheTestSuite(t)
209+
ctx := t.Context()
210+
211+
oldVal := option.Config.PolicyCIDRMatchMode
212+
t.Cleanup(func() { option.Config.PolicyCIDRMatchMode = oldVal })
213+
option.Config.PolicyCIDRMatchMode = []string{}
214+
215+
hostIPPrefix := cmtypes.NewLocalPrefixCluster(netip.MustParsePrefix("10.0.1.1/32"))
216+
resource := ipcacheTypes.NewResourceID(ipcacheTypes.ResourceKindDaemon, "", "test")
217+
218+
// cidrGroupDone is closed when goroutine A has finished injecting CIDRGroup
219+
// labels — i.e. the end of the "window" during which the bug is present.
220+
cidrGroupDone := make(chan struct{})
221+
222+
// ── Goroutine A: K8sWatcher / CiliumCIDRGroup reconciler ─────────────────
223+
// Simulates daemon.go:202: K8sWatcher.InitK8sSubsystem() starts processing
224+
// CiliumCIDRGroups. Uses the same public API that the reconciler uses:
225+
// IPCache.UpsertMetadataBatch().
226+
var wg sync.WaitGroup
227+
wg.Add(1)
228+
go func() {
229+
defer wg.Done()
230+
defer close(cidrGroupDone)
231+
232+
// Simulate the CIDRGroup reconciler upserting cidrgroup labels for a
233+
// subnet that contains a host IP.
234+
s.IPIdentityCache.metadata.upsertLocked(
235+
hostIPPrefix,
236+
source.Generated,
237+
resource,
238+
cidrGroupLabels("example-local-subnet"),
239+
)
240+
// Synchronously inject labels to simulate the reconciler draining its queue.
241+
_, _ = s.IPIdentityCache.doInjectLabels(ctx, []cmtypes.PrefixCluster{hostIPPrefix})
242+
}()
243+
244+
// Wait for goroutine A to finish: we are now inside the race window.
245+
// The host IP has been assigned an identity, but syncHostIPs has not run yet.
246+
<-cidrGroupDone
247+
248+
// ── Observe the intermediate state (the race window) ─────────────────────
249+
// At this point in the real daemon, any endpoint regeneration triggered by
250+
// the CIDRGroup update would see world identity for the host IP.
251+
windowEntry, ok := s.IPIdentityCache.ipToIdentityCache["10.0.1.1/32"]
252+
require.True(t, ok, "host IP should have an identity during the race window")
253+
254+
windowIdentity := s.Allocator.LookupIdentityByID(ctx, windowEntry.ID)
255+
require.NotNil(t, windowIdentity)
256+
257+
// BUG: during the window, world identity is assigned.
258+
assert.NotEqual(t, identity.ReservedIdentityHost, windowEntry.ID,
259+
"RACE WINDOW OBSERVED: host IP has id=%d (not ReservedIdentityHost) "+
260+
"while syncHostIPs has not yet run", windowEntry.ID)
261+
assert.True(t,
262+
windowIdentity.Labels.HasWorldLabel() || windowIdentity.Labels.HasWorldIPv4Label(),
263+
"RACE WINDOW: host IP labels=%v should contain world label", windowIdentity.Labels)
264+
265+
// ── Goroutine B: syncHostIPs.StartAndWaitFirst() ─────────────────────────
266+
// Simulates daemon.go:249: syncHostIPs runs after daemon init completes.
267+
// This is the same call that syncHostIPs.sync() makes at line 205:
268+
// s.params.IPCache.UpsertMetadata(p, source.Local, daemonResourceID, lbls)
269+
wg.Add(1)
270+
go func() {
271+
defer wg.Done()
272+
273+
s.IPIdentityCache.metadata.upsertLocked(
274+
hostIPPrefix,
275+
source.Local,
276+
resource,
277+
labels.LabelHost,
278+
)
279+
_, _ = s.IPIdentityCache.doInjectLabels(ctx, []cmtypes.PrefixCluster{hostIPPrefix})
280+
}()
281+
282+
// Wait for goroutine B to finish (syncHostIPs has now run).
283+
wg.Wait()
284+
285+
// ── Verify correction ─────────────────────────────────────────────────────
286+
// After syncHostIPs runs, resolveLabels() sees HasHostLabel()=true,
287+
// sets isInCluster=true, removes the cidrgroup label, and does not add world.
288+
// The identity must be corrected to ReservedIdentityHost.
289+
correctedEntry, ok := s.IPIdentityCache.ipToIdentityCache["10.0.1.1/32"]
290+
require.True(t, ok)
291+
assert.Equal(t, identity.ReservedIdentityHost, correctedEntry.ID,
292+
"After syncHostIPs runs, identity must be corrected to ReservedIdentityHost (id=1). "+
293+
"Got id=%d.", correctedEntry.ID)
294+
}
295+
296+
// TestHostIPWorldFallbackStress exercises the ordering sensitivity by running
297+
// many iterations where goroutine A (CIDRGroup) and goroutine B (syncHostIPs)
298+
// race without an explicit ordering barrier.
299+
//
300+
// When A wins the race (cidrgroup before host), world identity is transiently
301+
// assigned. When B wins (host before cidrgroup), host identity is assigned
302+
// directly. The final state should always be ReservedIdentityHost regardless
303+
// of which goroutine "won" — but during the bug window, it may not be.
304+
//
305+
// Run this with:
306+
//
307+
// go test ./pkg/ipcache/... -run TestHostIPWorldFallbackStress -race -count=5 -v
308+
//
309+
// The -race flag will confirm there are no concurrent memory violations.
310+
// The bug is a logical ordering issue, invisible to the race detector.
311+
func TestHostIPWorldFallbackStress(t *testing.T) {
312+
const iterations = 50
313+
314+
oldVal := option.Config.PolicyCIDRMatchMode
315+
t.Cleanup(func() { option.Config.PolicyCIDRMatchMode = oldVal })
316+
option.Config.PolicyCIDRMatchMode = []string{}
317+
318+
resource := ipcacheTypes.NewResourceID(ipcacheTypes.ResourceKindDaemon, "", "stress-test")
319+
320+
worldWins := 0
321+
hostWins := 0
322+
323+
for i := 0; i < iterations; i++ {
324+
s := setupIPCacheTestSuite(t)
325+
ctx := t.Context()
326+
hostIPPrefix := cmtypes.NewLocalPrefixCluster(netip.MustParsePrefix("10.0.1.1/32"))
327+
328+
var wg sync.WaitGroup
329+
var mu sync.Mutex
330+
var intermediateID identity.NumericIdentity
331+
332+
// Goroutine A: CiliumCIDRGroup reconciler (daemon.go:202).
333+
wg.Add(1)
334+
go func() {
335+
defer wg.Done()
336+
s.IPIdentityCache.metadata.upsertLocked(
337+
hostIPPrefix, source.Generated, resource,
338+
cidrGroupLabels("example-local-subnet"),
339+
)
340+
prefixes := []cmtypes.PrefixCluster{hostIPPrefix}
341+
_, _ = s.IPIdentityCache.doInjectLabels(ctx, prefixes)
342+
343+
// Capture the identity at the moment A finishes.
344+
mu.Lock()
345+
if e, ok := s.IPIdentityCache.ipToIdentityCache["10.0.1.1/32"]; ok {
346+
intermediateID = e.ID
347+
}
348+
mu.Unlock()
349+
}()
350+
351+
// Goroutine B: syncHostIPs (daemon.go:249).
352+
wg.Add(1)
353+
go func() {
354+
defer wg.Done()
355+
s.IPIdentityCache.metadata.upsertLocked(
356+
hostIPPrefix, source.Local, resource,
357+
labels.LabelHost,
358+
)
359+
_, _ = s.IPIdentityCache.doInjectLabels(ctx, []cmtypes.PrefixCluster{hostIPPrefix})
360+
}()
361+
362+
wg.Wait()
363+
364+
// Check the intermediate identity captured by goroutine A.
365+
mu.Lock()
366+
id := intermediateID
367+
mu.Unlock()
368+
369+
if id == identity.ReservedIdentityHost {
370+
hostWins++
371+
} else {
372+
worldWins++
373+
}
374+
375+
// The FINAL state (after both goroutines) must always be host.
376+
finalEntry, ok := s.IPIdentityCache.ipToIdentityCache["10.0.1.1/32"]
377+
require.True(t, ok)
378+
assert.Equal(t, identity.ReservedIdentityHost, finalEntry.ID,
379+
"iteration %d: final identity must be ReservedIdentityHost after "+
380+
"both goroutines complete. Got id=%d.", i, finalEntry.ID)
381+
382+
s.IPIdentityCache.Shutdown()
383+
}
384+
385+
t.Logf("Over %d iterations: cidrgroup-first (world identity window)=%d, "+
386+
"host-first (no bug window)=%d",
387+
iterations, worldWins, hostWins)
388+
389+
// In most runs, at least some iterations will show the world identity window.
390+
// If worldWins==0 always, the goroutine scheduler always ran B before A,
391+
// which would be surprising over 50 iterations.
392+
t.Logf("NOTE: worldWins=%d means the race window was observable %d times. "+
393+
"A value of 0 does not mean the bug is fixed — it means the goroutine "+
394+
"scheduler happened to always run syncHostIPs first in this run.", worldWins, worldWins)
395+
}

0 commit comments

Comments
 (0)