-
Notifications
You must be signed in to change notification settings - Fork 343
feat(xds): rework DeltaXds to use metadata instead of proto #13467
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
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: Lukasz Dziedziak <[email protected]>
Signed-off-by: Lukasz Dziedziak <[email protected]>
Signed-off-by: Lukasz Dziedziak <[email protected]>
Reviewer Checklist🔍 Each of these sections need to be checked by the reviewer of the PR 🔍:
|
Signed-off-by: Lukasz Dziedziak <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lukidzi Please double check pkg/plugins/runtime/k8s/controllers/pod_converter.go
PodConverter struct DeltaXds
property, it seems not used anymore
Signed-off-by: Lukasz Dziedziak <[email protected]>
if d.EnvoyXdsTransportProtocolVariant != "" { | ||
switch d.EnvoyXdsTransportProtocolVariant { | ||
case "DELTA_GRPC": | ||
case "GRPC": |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we call this one SOTW_GRPC
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Envoy configuration reference to SOTW as GRPC
so I wanted to keep them the same
Resources types.ProxyResources | ||
SystemCaPath string | ||
TransparentProxy *tproxy_config.DataplaneConfig | ||
UseDeltaXds bool |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do we need UseDeltaXds
if you can always call params.Features.HasFeature(xds_types.FeatureDeltaGRPC)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need since you can always enable it on the control-plane level and later you need to set it
Signed-off-by: Lukasz Dziedziak <[email protected]>
Signed-off-by: Lukasz Dziedziak <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Signed-off-by: Lukasz Dziedziak <[email protected]>
Motivation
We shouldn't change the proto schema but instead use metadata.
Implementation information
KUMA_DATAPLANE_RUNTIME_ENVOY_XDS_TRANSPORT_PROTOCOL_VARIANT
which allow setting specific variant of the xds protocolkuma.io/xds-transport-protocol-variant
in the k8s to simplify delta testing for the userSupporting documentation
Fix #13461