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

xds/federation: support populating resource template in xds-resolver #4900

Merged
merged 5 commits into from Nov 8, 2021

Conversation

menghanl
Copy link
Contributor

@menghanl menghanl commented Oct 26, 2021

And also handle percent encoding for new style resource names.

RELEASE NOTES:

  • [bug]xds/resolver: release leaked ClientConn if resolver initialization fails

@menghanl menghanl requested a review from easwars Oct 26, 2021
@menghanl menghanl added this to the 1.42 Release milestone Oct 26, 2021
@menghanl
Copy link
Contributor Author

@menghanl menghanl commented Oct 26, 2021

This PR is based on #4892. All new changes are in the last commit.

@menghanl menghanl force-pushed the federation_target_parsing branch from 448cb60 to cfba88f Compare Oct 28, 2021
@menghanl menghanl removed this from the 1.42 Release milestone Oct 29, 2021
@menghanl menghanl added this to the 1.43 release milestone Oct 29, 2021
xds/internal/resolver/xds_resolver.go Outdated Show resolved Hide resolved
xds/internal/xdsclient/bootstrap/template.go Outdated Show resolved Hide resolved
xds/internal/xdsclient/bootstrap/template.go Show resolved Hide resolved
xds/internal/testutils/fakeclient/client.go Outdated Show resolved Hide resolved
xds/internal/xdsclient/bootstrap/template_test.go Outdated Show resolved Hide resolved
xds/internal/xdsclient/bootstrap/template.go Outdated Show resolved Hide resolved
@easwars easwars assigned menghanl and unassigned easwars Nov 1, 2021
@menghanl menghanl force-pushed the federation_target_parsing branch from 85457ab to da6d4c7 Compare Nov 2, 2021
@menghanl
Copy link
Contributor Author

@menghanl menghanl commented Nov 2, 2021

This is rebased on master. But the commit history is lost.
The commit that fixed the review comment was 85457ab

@menghanl menghanl assigned easwars and unassigned menghanl Nov 2, 2021
xds/internal/resolver/xds_resolver.go Show resolved Hide resolved
xds/internal/resolver/xds_resolver_test.go Show resolved Hide resolved
xds/internal/resolver/xds_resolver_test.go Show resolved Hide resolved
xds/internal/resolver/xds_resolver_test.go Outdated Show resolved Hide resolved
xds/internal/xdsclient/bootstrap/template_test.go Outdated Show resolved Hide resolved
@easwars easwars assigned menghanl and unassigned easwars Nov 3, 2021
@menghanl menghanl assigned easwars and unassigned menghanl Nov 3, 2021
@menghanl menghanl force-pushed the federation_target_parsing branch from 53a360b to 40ab629 Compare Nov 3, 2021
xds/internal/resolver/xds_resolver_test.go Show resolved Hide resolved
tgt := target
if opts.target != nil {
tgt = *opts.target
}
Copy link
Contributor

@easwars easwars Nov 4, 2021

Choose a reason for hiding this comment

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

This is a bit of magic where we get the target from a global var if not set in opts. This makes the tests harder to read since the reader will have to keep track of all this. Instead, could we just always pass a valid target from tests?

Copy link
Contributor Author

@menghanl menghanl Nov 8, 2021

Choose a reason for hiding this comment

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

Done

@easwars easwars assigned menghanl and unassigned easwars Nov 4, 2021
@menghanl menghanl assigned easwars and unassigned menghanl Nov 8, 2021
@menghanl menghanl force-pushed the federation_target_parsing branch from 6c8bf77 to 27f50c7 Compare Nov 8, 2021
menghanl added 3 commits Nov 8, 2021
[federation_target_parsing] template populate

[federation_target_parsing] default bootstrap for test client

[federation_target_parsing] fixed resolver tests, new tests not added yet

[federation_target_parsing] new tests

[federation_target_parsing] fix test

[federation_target_parsing] c1
@menghanl menghanl force-pushed the federation_target_parsing branch from 27f50c7 to ec1a81f Compare Nov 8, 2021
easwars
easwars approved these changes Nov 8, 2021
@easwars easwars assigned menghanl and unassigned easwars Nov 8, 2021
@menghanl menghanl merged commit c53203c into grpc:master Nov 8, 2021
10 checks passed
@menghanl menghanl deleted the federation_target_parsing branch Nov 8, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 8, 2022
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.

None yet

2 participants