Docs and tests: Struct deprecation for Any and hosts deprecation for load_assignment#6409
Docs and tests: Struct deprecation for Any and hosts deprecation for load_assignment#6409moderation wants to merge 13 commits intoenvoyproxy:masterfrom moderation:master
Conversation
…onfigs, updates Sandbox docs. Signed-off-by: Michael Payne <michael@sooper.org>
Signed-off-by: Michael Payne <michael@sooper.org>
…hine running DoH / DoT - enable successful download of libjaegertracing_plugin.linux_amd64.so for jaeger-native-tracing sandbox. Signed-off-by: Michael Payne <michael@sooper.org>
Signed-off-by: Michael Payne <michael@sooper.org>
Signed-off-by: Michael Payne <michael@sooper.org>
Signed-off-by: Michael Payne <michael@sooper.org>
Signed-off-by: Michael Payne <michael@sooper.org>
…load_assignment. Signed-off-by: Michael Payne <michael@sooper.org>
|
/cc @dio @htuch I didn't update https://github.com/envoyproxy/envoy/blob/master/test/config/integration/server.json which is a config that is very out of date. I couldn't find a way to trigger this in tests and for example could delete sections of the JSON to cause invalid JSON and all tests would pass. The references for server.json are https://github.com/envoyproxy/envoy/search?q=%22server.json%22&unscoped_q=%22server.json%22. If this config is note being used at all can we just remove it? (converting server.yaml was a huge undertaking) |
|
/retest |
|
🙀 failed invoking rebuild of |
|
@moderation if you find a config that is not used and not tested, definitely delete it. Can you confirm that all configs that you have been fixing are covered by some test? I just want to make sure that we don't have a test bug somewhere that is causing a config to not get tested. |
mattklein123
left a comment
mattklein123
left a comment
There was a problem hiding this comment.
Thanks so much for cranking on this. I have 1 question and if you could delete the dead config(s) that would be great (and also confirm that all the configs you are touching are getting tested somewhere).
Thank you!!!
| name: redirect | ||
| - routes: | ||
| - prefix: "/" | ||
| typed_config: |
There was a problem hiding this comment.
I'm a little confused why all this churn was needed. Isn't it as simple as just specifying the type param in various places as was done elsewhere? I must be missing something?
There was a problem hiding this comment.
The places where we switch to load_assignment make sense to me, but this particular file has a lot of churn. Most of this seems legit, but it would be helpful if @moderation could walk reviewers through this.
| address: <ADS management server IP address> | ||
| port_value: <ADS management server port> | ||
| load_assignment: | ||
| cluster_name: ads_cluster |
There was a problem hiding this comment.
I wonder if we should make cluster_name optional for these embedded scenarios? This information is strictly redundant. @dio @mattklein123 ?
There was a problem hiding this comment.
@dio sort of. I think we can do this today by removing the check from PGV and putting it in Envoy at the consuming sites. I'm not sure how to do it generically though, ideas?
| name: redirect | ||
| - routes: | ||
| - prefix: "/" | ||
| typed_config: |
There was a problem hiding this comment.
The places where we switch to load_assignment make sense to me, but this particular file has a lot of churn. Most of this seems legit, but it would be helpful if @moderation could walk reviewers through this.
|
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
|
/wait Not stale, just haven't had a chance to work on this this week. |
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
|
Still not stale. |
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! |
4 participants
Description: update docs and tests for Struct -> Any and hosts -> load_assignment. This should complete the docs, examples, sandboxes and testing configs. TODO is to update the code tests.
Risk Level: Low, documentation and tests
Testing:
bazel test //test/...Docs Changes: XDS protocol, Zookeeper filter & flow control
Release Notes: None required
Related: #6346