envoy
envoy copied to clipboard
envoyproxy
relax backing cluster check for sds
Checking to see what CI thinks Fixes https://github.com/envoyproxy/envoy/issues/12954 Commit Message: Additional Description: Risk Level: Testing: Docs Changes: Release Notes: Platform Specific Features: [Optional Runtime guard:] [Optional Fixes #Issue] [Optional Fixes commit #PR or SHA] [Optional Deprecated:] [Optional API Considerations:]
As a reminder, PRs marked as draft will not be automatically assigned reviewers, or be handled by maintainer-oncall triage.
Please mark your PR as ready when you want it to be reviewed!
:cat:
Caused by: https://github.com/envoyproxy/envoy/pull/36694 was opened by ramaraochavali.
![repokitteh-read-only[bot] avatar](https://avatars.githubusercontent.com/in/16729?v=4 "Published by a pub.dev verified publisher"){kind=small}
would there be cases where SDS is used, and depends on some cluster that isn't ready (and is not checked now), and will cause a cluster/listener to not have its secret?
If cluster/listener not able to get secrets because of SDS is not ready, the requests would fail and downstream_context_secrets_not_ready stat is incremented.
BTW, The behaviour is same - even with static cluster (defined in bootstrap) with static load assignments with a non existing server listening to that endpoint and port. So there is no change in behaviour for dynamic clusters
If cluster/listener not able to get secrets because of SDS is not ready, the requests would fail and
downstream_context_secrets_not_readystat is incremented.BTW, The behaviour is same - even with static cluster (defined in bootstrap) with static load assignments with a non existing server listening to that endpoint and port. So there is no change in behaviour for dynamic clusters
Thanks! So I'm assuming this will mainly impact the initialization of Envoy, i.e., up until now Envoy would wait for the static clusters, and the dynamic secrets to arrive before initialization, whereas now it will finish initialization without waiting (or probably failing) to receive the secrets. Is that correct?
So I'm assuming this will mainly impact the initialization of Envoy, i.e., up until now Envoy would wait for the static clusters, and the dynamic secrets to arrive before initialization, whereas now it will finish initialization without waiting (or probably failing) to receive the secrets. Is that correct?
Not really. The current initialization sequence is
- It initializes the static cluster
- It initializes the listener/cluster that referred SDS. It throws a validation if the SDS apiconfig source refers to a non static/non EDS cluster. If secret is provided it considers. If the SDS server is unavailable or not send secret, it finishes initialization but puts the secret in warming state.
if a request comes to the listener if fails with
downstream_context_secrets_not_readystat.
After this change
- It will accept any cluster - dynamic/eds
- It initializes the listener/cluster that referred SDS. It does not do any validation. . If cluster referred in SDS is not available/or it does not respond, the initialization is finished by putting the secret in warming state
if a request comes to the listener if fails with
downstream_context_secrets_not_readystat.
So there is absolutely no change in the behaviour either case - except the removal of validation.
Ok, thanks. I suggest going forward with this, and maybe guard it with a restart-feature-flag.
CC @envoyproxy/runtime-guard-changes: FYI only for changes made to (source/common/runtime/runtime_features.cc).
:cat:
Caused by: https://github.com/envoyproxy/envoy/pull/36694 was synchronize by ramaraochavali.
@envoyproxy/senior-maintainers assignee is @ggreenway
:cat:
Caused by: a https://github.com/envoyproxy/envoy/pull/36694#pullrequestreview-2399075852 was submitted by @adisuissa.
Please also add a test that uses a bootstrap-defined EDS cluster, as that is also explicitly allowed by this change.
Added. PTAL