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/internal/server: switch to generic xDS API for LDS/RDS #6726

Merged
merged 2 commits into from
Oct 13, 2023

Conversation

easwars
Copy link
Contributor

@easwars easwars commented Oct 13, 2023

The changes to switch to the generic xDS API on the server ended up being simple:

  • OnUpdate callback calls existing methods that handle updates
    • the error handling logic from the existing handler methods were removed
  • OnError and OnResourceDoesNotExist handle the errors inline

#resource-agnostic-xdsclient-api

RELEASE NOTES: none

@easwars easwars requested a review from zasweq October 13, 2023 01:16
@easwars easwars added the Type: Internal Cleanup Refactors, etc label Oct 13, 2023
@easwars easwars added this to the 1.60 Release milestone Oct 13, 2023
@codecov
Copy link

codecov bot commented Oct 13, 2023

Codecov Report

Merging #6726 (5c7e82b) into master (3e9b85c) will increase coverage by 0.06%.
Report is 3 commits behind head on master.
The diff coverage is 50.84%.

Additional details and impacted files

Copy link
Contributor

@zasweq zasweq left a comment

Choose a reason for hiding this comment

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

LGTM.

xds/internal/server/listener_wrapper.go Outdated Show resolved Hide resolved
xds/internal/server/listener_wrapper.go Show resolved Hide resolved
xds/internal/server/rds_handler.go Outdated Show resolved Hide resolved
xds/internal/server/rds_handler.go Outdated Show resolved Hide resolved

func (rw *rdsWatcher) OnResourceDoesNotExist() {
if rw.logger.V(2) {
rw.logger.Infof("RDS watch for resource %q reported resource-does-not-exits error: %v", rw.name)
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here. exist

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. Thanks.

xds/internal/server/rds_handler.go Outdated Show resolved Hide resolved
@zasweq zasweq assigned easwars and unassigned zasweq Oct 13, 2023
@easwars easwars merged commit 6fe6085 into grpc:master Oct 13, 2023
12 of 13 checks passed
@easwars easwars deleted the xds_server_generic_xds_api branch October 13, 2023 21:31
return
}
if lw.logger.V(2) {
lw.logger.Infof("LDS watch for resource %q reported resource-does-not-exist error: %v", lw.name)
Copy link
Contributor

Choose a reason for hiding this comment

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

What does the formatting directive %v correspond to?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants