Skip to content

test(usm): fix flakiness in resolver periodic-update tests #55883

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
chouquette wants to merge 1 commit into
base: main
Choose a base branch
from
+44 −11
Open

test(usm): fix flakiness in resolver periodic-update tests #55883

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 44 additions & 11 deletions pkg/process/net/resolver/resolver_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -448,6 +448,12 @@ func TestLocalResolverPeriodicUpdates(t *testing.T) {
resolver.Run()
mockedClock.Add(11 * time.Second)

assert.Eventually(func() bool {

@github-actions github-actions Bot Sep 4, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Eventually still depends on wall-clock polling and an arbitrary 2-second deadline, so these tests can remain flaky under sufficient contention. Synchronize with the mocked GetContainers call (for example, signal a channel from DoAndReturn) and wait for that completion instead. The same issue applies to the other added Eventually calls.

@chouquette chouquette Sep 4, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

My understanding is that relying on a channel here would need to also change the code being tested, which I don't want to do.

I'll let the reviewers comment further since I don't know this code enough to make the call IMHO

@github-actions github-actions Bot Sep 4, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The wall-clock-based Eventually polling remains timing-dependent and can still fail when the resolver goroutine is delayed beyond two seconds; the same pattern is added in four places. Please synchronize deterministically with the mocked GetContainers call/update completion (and use a fatal assertion if later checks depend on it) instead of imposing a real-time deadline.

resolver.mux.Lock()
defer resolver.mux.Unlock()
return len(resolver.addrToCtrID) == 4
}, 2*time.Second, 5*time.Millisecond)

connections := &model.Connections{
Conns: []*model.Connection{
// connection 0
Expand Down Expand Up @@ -538,6 +544,12 @@ func TestLocalResolverCachePersistence(t *testing.T) {
resolver.Run()
mockedClock.Add(11 * time.Second)

assert.Eventually(func() bool {
resolver.mux.Lock()
defer resolver.mux.Unlock()
return len(resolver.addrToCtrID) == 4
}, 2*time.Second, 5*time.Millisecond)

func() {
resolver.mux.Lock()
defer resolver.mux.Unlock()
Expand Down Expand Up @@ -628,17 +640,6 @@ func TestLocalResolverCachePersistence(t *testing.T) {
}

mockContainerProvider.EXPECT().GetContainers(2*time.Second, nil).Return(containers, nil, nil, nil)
mockedClock.Add(10 * time.Second)

// still should have 4 entries in the addr cache,
// missing entries should be just marked as not
// in use
func() {
resolver.mux.Lock()
defer resolver.mux.Unlock()

assert.Len(resolver.addrToCtrID, 4)
}()

missingAddrs := []model.ContainerAddr{
{
Expand All @@ -653,6 +654,32 @@ func TestLocalResolverCachePersistence(t *testing.T) {
},
}

mockedClock.Add(10 * time.Second)

assert.Eventually(func() bool {
resolver.mux.Lock()
defer resolver.mux.Unlock()

for addr, cid := range resolver.addrToCtrID {
for _, missing := range missingAddrs {
if missing == addr && cid.inUse {
return false
}
}
}
return len(resolver.addrToCtrID) == 4
}, 2*time.Second, 5*time.Millisecond)

// still should have 4 entries in the addr cache,
// missing entries should be just marked as not
// in use
func() {
resolver.mux.Lock()
defer resolver.mux.Unlock()

assert.Len(resolver.addrToCtrID, 4)
}()

// verify the missing address entries were marked
// as not in use
func() {
Expand Down Expand Up @@ -743,6 +770,12 @@ func TestLocalResolverCacheLimits(t *testing.T) {
resolver.Run()
mockedClock.Add(11 * time.Second)

assert.Eventually(func() bool {
resolver.mux.Lock()
defer resolver.mux.Unlock()
return len(resolver.addrToCtrID) == 1 && len(resolver.ctrForPid) == 1
}, 2*time.Second, 5*time.Millisecond)

func() {
resolver.mux.Lock()
defer resolver.mux.Unlock()
Expand Down
Loading