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
Copy link
Contributor Author

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

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
Copy link
Contributor Author

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
xds/internal/resolver/xds_resolver_test.go Show resolved Hide resolved
Comment on lines 263 to 266
tgt := target
if opts.target != nil {
tgt = *opts.target
}
Copy link
Contributor

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

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
[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
@easwars easwars assigned menghanl and unassigned easwars Nov 8, 2021
@menghanl menghanl merged commit c53203c into grpc:master Nov 8, 2021
@menghanl menghanl deleted the federation_target_parsing branch November 8, 2021 22:17
@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