Skip to content

xds_test: Wait for server to start serving in RBAC test#8287

Merged
arjan-bal merged 3 commits into
grpc:masterfrom
arjan-bal:fix-rbac-test-flake
May 5, 2025
Merged

xds_test: Wait for server to start serving in RBAC test#8287
arjan-bal merged 3 commits into
grpc:masterfrom
arjan-bal:fix-rbac-test-flake

Conversation

@arjan-bal
Copy link
Copy Markdown
Contributor

@arjan-bal arjan-bal commented May 5, 2025

Fixes: #8286

This PR makes the test wait for the xds server to enter SERVING mode, otherwise it closes the net.Conn without returning any error.

if l.mode == connectivity.ServingModeNotServing {
// Close connections as soon as we accept them when we are in
// "not-serving" mode. Since we accept a net.Listener from the user
// in Serve(), we cannot close the listener when we move to
// "not-serving". Closing the connection immediately upon accepting
// is one of the other ways to implement the "not-serving" mode as
// outlined in gRFC A36.
l.mu.Unlock()
conn.Close()
continue
}

Analysis: #8286 (comment)

Testing

Verified the test passes 10^5 runs on forge.

RELEASE NOTES: N/A

@arjan-bal arjan-bal added this to the 1.73 Release milestone May 5, 2025
@arjan-bal arjan-bal requested a review from easwars May 5, 2025 13:11
@arjan-bal arjan-bal assigned arjan-bal and unassigned easwars May 5, 2025
@arjan-bal arjan-bal force-pushed the fix-rbac-test-flake branch from b5f5007 to ec7668d Compare May 5, 2025 15:03
@codecov
Copy link
Copy Markdown

codecov Bot commented May 5, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.31%. Comparing base (515f377) to head (7c642b1).
Report is 10 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #8287      +/-   ##
==========================================
+ Coverage   82.14%   82.31%   +0.17%     
==========================================
  Files         417      419       +2     
  Lines       41344    41904     +560     
==========================================
+ Hits        33961    34494     +533     
- Misses       5957     5960       +3     
- Partials     1426     1450      +24     

see 50 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@arjan-bal arjan-bal assigned easwars and unassigned arjan-bal May 5, 2025
Comment thread test/xds/xds_server_rbac_test.go Outdated
defer cleanup2()
// Initialize a test gRPC server, assign it to the stub server, and start
// the test service.
lis, err := testutils.LocalTCPListener()
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.

I don't know how many tests that use an xDS-enabled gRPC server have this boilerplate that creates a listener, a stub server, a gRPC server, and then starts serving. The stubserver.StartTestService is very convenient for tests that don't need an xDS-enabled gRPC server. Maybe we can have a helper for the tests that need an xDS-enabled gRPC server, similar to stubserver.StartTestService? (and maybe we can get some TVC to work on that?).

Copy link
Copy Markdown
Contributor Author

@arjan-bal arjan-bal May 5, 2025

Choose a reason for hiding this comment

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

setupGRPCServer in this package is a helper that creates an xDS enabled gRPC server. I changed the test to to not use it because I wanted to set a server option for setting the serving mode callback. The helper was already setting this server option. I made the setupGRPCServer accept a variadic list of server options, similar to StartTestService. Callers can override the default options set by setupGRPCServer as setupGRPCServer appends the passed options to the end of the list used to create the new server.

@easwars easwars assigned arjan-bal and unassigned easwars May 5, 2025
@arjan-bal arjan-bal merged commit 7fb5738 into grpc:master May 5, 2025
23 of 24 checks passed
purnesh42H pushed a commit to purnesh42H/grpc-go that referenced this pull request May 8, 2025
vinothkumarr227 pushed a commit to vinothkumarr227/grpc-go that referenced this pull request May 26, 2025
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Nov 2, 2025
@arjan-bal arjan-bal deleted the fix-rbac-test-flake branch January 15, 2026 12:33
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Flaky test: Test/RBAC_WithBadRouteConfiguration

2 participants