api: allow multiple type URLs for xDS#10788
api: allow multiple type URLs for xDS#10788Shikugawa wants to merge 6 commits intoenvoyproxy:masterfrom
Conversation
Signed-off-by: shikugawa <rei@tetrate.io>
Signed-off-by: shikugawa <rei@tetrate.io>
Shikugawa
force-pushed
the
remove_hardcoded_type_url_sec2
branch
from
89306d4 to
eab6fe3
Compare
|
/review htuch |
htuch
left a comment
htuch
left a comment
There was a problem hiding this comment.
Thanks, looks good but would be cleanest if two PRs..
| Cleanup eds_resume( | ||
| [this] { cm_.adsMux()->resume(Config::TypeUrl::get().ClusterLoadAssignment); }); | ||
| for (auto&& type_url : target_type_urls) { | ||
| if (!cm_.adsMux()->paused(type_url)) { |
There was a problem hiding this comment.
FWIW, I think we're still mixing two unrelated PRs; switching to non-hardcoded type URLs and supporting multiple type URLs.
| if (!started_secondary_initialize_) { | ||
| ENVOY_LOG(info, "cm init: initializing secondary clusters"); | ||
| const auto target_type_urls = | ||
| Config::getAllVersionTypeUrls<envoy::config::endpoint::v3::ClusterLoadAssignment>(); |
There was a problem hiding this comment.
This seems a regression in terms of reducing the amount of version hardcoding; before we could just do Config::TypeUrl::get().ClusterLoadAssignment(), now we need to mention v3..
| * requests may later be resumed with resume(). | ||
| * @param type_urls all of type URLs corresponding to xDS APIs. | ||
| */ | ||
| void pause(const std::vector<std::string>& type_urls) { |
There was a problem hiding this comment.
Do we need some unit tests for these vectored pause/resume methods?
Shikugawa
changed the title
|
This pull request has been automatically marked as stale because it has not had activity in the last 7 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions! |
stale
bot
added
the
stale
|
/nostale |
stale
bot
removed
the
stale
|
This pull request has been automatically marked as stale because it has not had activity in the last 7 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions! |
stale
bot
added
the
stale
|
This pull request has been automatically closed because it has not had activity in the last 14 days. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions! |
stale
bot
removed
the
stale
|
This pull request has been automatically marked as stale because it has not had activity in the last 7 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions! |
stale
bot
added
the
stale
|
This pull request has been automatically closed because it has not had activity in the last 14 days. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions! |
2 participants
For an explanation of how to fill out the fields, please see the relevant section
in PULL_REQUESTS.md
Description: allow multiple versions of type URLs. For example, we can set
type.googleapis.com/envoy.api.v2.ScopedRouteConfigurationand"type.googleapis.com/envoy.config.route.v3.ScopedRouteConfiguration"simultaneously.Risk Level: Mid
Testing: Unit / Integration
Docs Changes: N/A
Release Notes: N/A
[Optional Fixes #Issue]
[Optional Deprecated:]