-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
docs: add cluster_name to load assignment entry for static and mostly static #4123
docs: add cluster_name to load assignment entry for static and mostly static #4123
Conversation
Signed-off-by: Achmad Gozali <gozali@gmail.com>
@dio it seems a little odd that this field would be required for the static cases when the cluster name is already specified in the config. But indeed it is required so this doc update is correct. Any thoughts here? |
@mattklein123 it is odd, but since it is also required for the management server to get informed that this field is required is important. A wild idea will be: since we do validation in bootstrap object level here: Line 161 in 0a4bffc
we do additional checking and treatment for static resources (i.e. if static, all requirements for cluster name to be filled [ |
Sorry what do you mean? I think it's just an artifact of the message being shared between EDS responses and the local config?
Yeah this might be a good thing to do? I will go ahead and merge this since it's required but it seems like we should check they are the same and maybe also do a doc update to say that? |
Yeah, I meant this. Sorry for not putting the context that the issue also wants to remove the First, since we also need that to be required for EDS envoy/source/common/upstream/eds.cc Lines 61 to 65 in a614808
Second, I think the only reason why we keep it for now, is to let this validation be generated: https://github.com/envoyproxy/go-control-plane/blob/5f1c666fa3aaa02d95e4fdd95e725f0c5eb7239f/envoy/api/v2/eds.pb.validate.go#L45-L50 |
@dio makes sense. I would probably update the docs to be more clear about how cluster_name is used for load assignment and maybe also put in a check as you pointed out for static clusters that enforces they are the same for clarity? |
* origin/master: (38 commits) test: add tests for corner-cases around sending requests before run() starts or after run() ends. (envoyproxy#4114) perf: reduce the memory usage of LC Trie construction (envoyproxy#4117) test: moving redundant code in websocket_integration_test to utilities (envoyproxy#4127) test: make YamlLoadFromStringFail less picky about error msg. (envoyproxy#4141) rbac: add rbac network filter. (envoyproxy#4083) fuzz: route lookup and header finalization fuzzer. (envoyproxy#4116) Set content-type and content-length (envoyproxy#4113) fault: use FractionalPercent for percent (envoyproxy#3978) test: Fix inverted exact match logic in IntegrationTcpClient::waitForData() (envoyproxy#4134) Added cluster_name to load assignment config for static cluster (envoyproxy#4123) ssl: refactor ContextConfig to use TlsCertificateConfig (envoyproxy#4115) syscall: refactor OsSysCalls for deeper errno latching (envoyproxy#4111) thrift_proxy: fix oneway bugs (envoyproxy#4025) Do not crash when converting YAML to JSON fails (envoyproxy#4110) config: allow unknown fields flag (take 2) (envoyproxy#4096) Use a jittered backoff strategy for handling HdsDelegate stream/connection failures (envoyproxy#4108) bazel: use GCS remote cache (envoyproxy#4050) Add thread local cache of overload action states (envoyproxy#4090) Added TCP healthcheck capabilities to the HdsDelegate (envoyproxy#4079) secret: add secret provider interface and use it for TlsCertificates (envoyproxy#4086) ... Signed-off-by: Snow Pettersen <snowp@squareup.com>
Description:
This PR adds missing
cluster_name
ofload_assignment
configfor static and mostly-static bootstrap configuration.
Risk Level: Low
Testing:
./docs/build.sh
Docs Changes: Added
cluster_name
toload_assignment
config for static and mostly-static bootstrap configuration.Release Notes: N/A
Fixes #4056
Signed-off-by: Achmad Gozali gozali@gmail.com