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

change CSDS to populate new generic_xds_configs field #27794

Merged
merged 1 commit into from
Oct 26, 2021

Conversation

markdroth
Copy link
Member

@markdroth markdroth added the release notes: no Indicates if PR should not be in release notes label Oct 21, 2021
Copy link
Contributor

@lidizheng lidizheng left a comment

Choose a reason for hiding this comment

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

The new CSDS is indeed much easier to use. Besides C++, we might also need to update:

  • Python's CSDS test: src/python/grpcio_tests/tests/csds/test_csds.py (we can add unittest.skip("TODO(lidiz) use generic_xds_config field") to both test classes for now)
  • xDS interop test framework: I will update them in parallel of this PR.

src/core/ext/xds/xds_api.h Show resolved Hide resolved
@markdroth
Copy link
Member Author

If you want to send me a PR with the changes for src/python/grpcio_tests/tests/csds/test_csds.py, I can merge that into this PR, so that we can merge both changes into master at the same time.

Also, were you going to change the xDS interop tests to accept both old and new CSDS formats, or just change it to use the new one? If the former, then that probably needs to be done before this is merged. If the latter, then we probably need to coordinate the changes in all languages, or else we'll break the dashboard. I think the former would be easier.

We probably also shouldn't merge this until the grpcdebug changes are done.

@lidizheng
Copy link
Contributor

I'm going to let both GCE and GKE framework accept both CSDS formats. grpcdebug should have lower priority, depends on whether we want this change in v1.42.0.

@lidizheng
Copy link
Contributor

I end up updating the GCE/GKE framework and Python tests in #27796 to support both CSDS standards. Those places should be all the CSDS use cases in grpc/grpc.

Copy link
Contributor

@lidizheng lidizheng left a comment

Choose a reason for hiding this comment

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

The testing frameworks, Python tests, debug clients have added support to both CSDS standards. This PR should be unblocked.

src/core/ext/xds/xds_api.h Show resolved Hide resolved
@markdroth
Copy link
Member Author

Test failures look unrelated.

@markdroth markdroth merged commit 06b5061 into grpc:master Oct 26, 2021
@markdroth markdroth deleted the csds_update branch October 26, 2021 20:24
@copybara-service copybara-service bot added the imported Specifies if the PR has been imported to the internal repository label Oct 27, 2021
dapengzhang0 added a commit to grpc/grpc-java that referenced this pull request Nov 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
imported Specifies if the PR has been imported to the internal repository lang/c++ lang/core release notes: no Indicates if PR should not be in release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants