Strip Config of all service specific configuration for now #694
Conversation
XAMPPRocky
requested review from
markmandel,
No-ops,
rezvaneh and
vallfors
as code owners
XAMPPRocky
force-pushed
the
ep/strip-config
branch
5 times, most recently
from
307037b to
3a95f1d
Compare
XAMPPRocky
force-pushed
the
ep/strip-config
branch
2 times, most recently
from
11e3216 to
d385802
Compare
XAMPPRocky
force-pushed
the
ep/strip-config
branch
from
d385802 to
7828a1f
Compare
|
This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size. |
|
Build Succeeded 🥳 Build Id: 50545872-e0f6-45dc-ae54-1ada4f310103 The following development images have been built, and will exist for the next 30 days: To build this version: |
| @@ -59,34 +55,17 @@ pub(crate) const CONNECTION_TIMEOUT: u64 = 5; | |||
| #[serde(deny_unknown_fields)] | |||
| #[non_exhaustive] | |||
| pub struct Config { | |||
| #[serde(default = "Slot::<Admin>::with_default")] | |||
| pub admin: Slot<Admin>, | |||
| #[serde(default)] | |||
| pub clusters: Slot<ClusterMap>, | |||
There was a problem hiding this comment.
(non blocking) what are your thoughts on keeping static cluster config? (since it's proxy specific)
There was a problem hiding this comment.
Well clusters is one of the least proxy specific pieces of information because the control plane and relay create a cluster map for proxies to consume through xDS.
We do still have static configuration, the API just also handles being updated through other sources if provided, unless I'm missing the question?
There was a problem hiding this comment.
Ah yes - that is a good point - everyone needs clusters/endpoints, so it's not actually proxy specific 👍🏻
Question answered!
More work on #600. This removes all service specific configuration from
Config(well specifically non-shared service configuration, filters is still there because filters are shared between services even though only proxy uses them), as currentlyConfigis a little too proxy focused, and with the introduction of a third service, I wanted to restrictConfigto just what is currently shared between all services, and service specific configuration should be set using CLI or environment parameters (or using theclifrom the lib). This also removes xds' dependence onConfigin order to better support running multiple types of clients and different services handling resource updates differently.We could always later reintroduce these properties into
Configwhen we've set up a good way to actually handle updates to these properties as they're currently only set once (such as binding new sockets when the port changes), but I think we should do that after #600 is finished.