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: implement ignore_resource_deletion server feature #9339

Merged
merged 8 commits into from Jul 8, 2022

Conversation

sergiitk
Copy link
Member

@sergiitk sergiitk commented Jul 7, 2022

As defined in the gRFC A53: Option for Ignoring xDS Resource Deletion.

Note for the reviewer: it'll probably be more convenient to review commit-by-commit. The test refactoring commit can be skipped: it's not required, but was very handy for implementing new tests, and modifying the old ones.

@sergiitk sergiitk added TODO:backport PR needs to be backported. Removed after backport complete TODO:release blocker Issue/PR is important enough to delay the release. Removed after release issues resolved labels Jul 7, 2022
@sergiitk sergiitk changed the title xds: Implement ignore_resource_deletion server feature xds: implement ignore_resource_deletion server feature Jul 7, 2022
@sergiitk sergiitk force-pushed the xds-ignore_resource_deletion branch 5 times, most recently from 6866bf3 to 1b81cfd Compare July 8, 2022 07:14
@sergiitk sergiitk force-pushed the xds-ignore_resource_deletion branch from 1b81cfd to 48f331c Compare July 8, 2022 07:28
@sergiitk sergiitk marked this pull request as ready for review July 8, 2022 07:30
@sergiitk sergiitk requested a review from ejona86 July 8, 2022 07:30
Copy link
Member

@ejona86 ejona86 left a comment

Choose a reason for hiding this comment

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

The parameterization of the test is fine, but I do think we should strive to be able to remove it in the future. Just a few test cases should care about the bootstrap value, so we could just run some tests with a different bootstrap. (For the few tests impacted, we can parameterize manually by passing in a boolean argument.) We're going to have more of these sorts of bootstrap options over time and adding another test dimension each time will lead to pain. But this is fine now for expediency.

xds/src/main/java/io/grpc/xds/ClientXdsClient.java Outdated Show resolved Hide resolved
xds/src/main/java/io/grpc/xds/ClientXdsClient.java Outdated Show resolved Hide resolved
@sergiitk
Copy link
Member Author

sergiitk commented Jul 8, 2022

FYI @erikjoh

@sergiitk sergiitk enabled auto-merge (squash) July 8, 2022 19:47
@sergiitk sergiitk merged commit ac23d33 into grpc:master Jul 8, 2022
@sergiitk sergiitk deleted the xds-ignore_resource_deletion branch July 8, 2022 20:10
sergiitk added a commit to sergiitk/grpc-java that referenced this pull request Jul 8, 2022
As defined in the gRFC [A53: Option for Ignoring xDS Resource Deletion](https://github.com/grpc/proposal/blob/master/A53-xds-ignore-resource-deletion.md).

This includes semi-related changes:
* Refactor ClientXdsClientTestBase: extract verify methods for golden resources
* Parameterize ClientXdsClientV2Test and ClientXdsClientV3Test with ignoreResourceDeletion enabled and disabled
* Add FORCE_INFO and FORCE_WARNING levels to XdsLogLevel
sergiitk added a commit that referenced this pull request Jul 8, 2022
As defined in the gRFC [A53: Option for Ignoring xDS Resource Deletion](https://github.com/grpc/proposal/blob/master/A53-xds-ignore-resource-deletion.md).

This includes semi-related changes:
* Refactor ClientXdsClientTestBase: extract verify methods for golden resources
* Parameterize ClientXdsClientV2Test and ClientXdsClientV3Test with ignoreResourceDeletion enabled and disabled
* Add FORCE_INFO and FORCE_WARNING levels to XdsLogLevel
@sergiitk
Copy link
Member Author

sergiitk commented Jul 8, 2022

Backported to 1.48.x.

@sergiitk sergiitk removed TODO:backport PR needs to be backported. Removed after backport complete TODO:release blocker Issue/PR is important enough to delay the release. Removed after release issues resolved labels Jul 8, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 7, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants