Skip to content
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.

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

rls: fix flaky test introduced by #6514 #6535

Merged
merged 1 commit into from
Aug 10, 2023
Merged

Conversation

easwars
Copy link
Contributor

@easwars easwars commented Aug 10, 2023

#6514 changed some tests to use the stub balancer package instead of defining custom test balancers, but it introduced a bug which made one of the tests flaky.

The bug was that a testCCWrapper was being passed as the balancer.ClientConn for the child policies of RLS. This meant that any state change reported by the child policy ended up changing the state of the main channel. This is not how it is supposed to work. The way it works is that the RLS LB policy intercepts state updates from the child policies, aggregates them and pushes a final state to the parent ClientConn.

The fix for this PR ensures that the balancer.ClientConn passed by RLS LB policy (when it creates a child policy) is the one which gets passed to the child policy in the test as well.

RELEASE NOTES: none

@easwars easwars requested a review from dfawley August 10, 2023 00:20
@easwars easwars added this to the 1.58 Release milestone Aug 10, 2023
@easwars easwars changed the title rls: fix flaky test rls: fix flaky test introduced by #6514 Aug 10, 2023
@dfawley dfawley assigned easwars and unassigned dfawley Aug 10, 2023
@dfawley
Copy link
Member

dfawley commented Aug 10, 2023

Tricky one. Thanks for the fix.

@easwars easwars merged commit e274152 into grpc:master Aug 10, 2023
1 check passed
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 7, 2024
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.

2 participants