envoy icon indicating copy to clipboard operation
envoy copied to clipboard

rlqs: Add per route runtime overrides

Open sergiitk opened this issue 1 year ago • 7 comments

Signed-off-by: Sergii Tkachenko [email protected]

Per initial review comment: https://github.com/envoyproxy/envoy/pull/19793#discussion_r906161595 CC @yanavlasov @mattklein123

Commit Message: rlqs: Add per route runtime overrides 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:]

sergiitk avatar Jul 12 '22 19:07 sergiitk

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to (api/envoy/|docs/root/api-docs/). envoyproxy/api-shepherds assignee is @markdroth CC @envoyproxy/api-watchers: FYI only for changes made to (api/envoy/|docs/root/api-docs/).

:cat:

Caused by: https://github.com/envoyproxy/envoy/pull/22150 was opened by sergiitk.

see: more, trace.

Why are these being added to the override config but not the top-level config? Wouldn't it also be useful to set these in the top-level config? Although if we do add them to the top-level config, we also need to define precedence rules for them.

Also, note that gRPC does not have Envoy's concept of runtime settings, so we would be able to support these only as fixed percentages.

markdroth avatar Jul 12 '22 19:07 markdroth

@markdroth Because they already present at the top level. This is a follow up for adding them to the overrides, see this comment: https://github.com/envoyproxy/envoy/pull/19793#discussion_r906161595:

You might consider adding per route runtime overrides here also, default to 100%, but up to you. That could be added later also if needed.
— @mattklein123

I caught up with @yanavlasov on this after the PR got merged, so adding this post-factum.

sergiitk avatar Jul 12 '22 21:07 sergiitk

Ah, okay, I hadn't realized that these had already been added at the top level.

What is the override behavior here? If one of these fields is set in the top-level config but not set in the override config, do we inherit the value from the top-level config? Or is the idea that if an override config is present, if these fields are not set in the override config, then we treat these fields as unset, even if they are set in the top-level config? I think the latter makes more sense, but we should explicitly state how this works in the proto comments.

markdroth avatar Jul 12 '22 21:07 markdroth

@markdroth Don't know how it should work - as you mentioned we don't support it in gRPC. Handing this over to @tyxia.

sergiitk avatar Jul 14 '22 19:07 sergiitk

/wait-any

phlax avatar Jul 19 '22 07:07 phlax

/assign @tyxia

tyxia avatar Aug 10 '22 14:08 tyxia

I think we're still waiting on answers/doc updates to Mark's question.

zuercher avatar Aug 16 '22 16:08 zuercher

/wait-any

zuercher avatar Aug 16 '22 16:08 zuercher

@markdroth Don't know how it should work - as you mentioned we don't support it in gRPC. Handing this over to @tyxia.

gRPC doesn't support the runtime setting, but the config.core.v3.RuntimeFractionalPercent field allows a fixed percentage to be specified (in the default_value field, which we use in gRPC in the context of route matching). So we will need to answer this question in gRPC as well.

I think the question of override semantics applies equally to gRPC and Envoy. We need to define this before this PR can be merged.

markdroth avatar Aug 18 '22 00:08 markdroth

This pull request has been automatically marked as stale because it has not had activity in the last 30 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!

github-actions[bot] avatar Sep 28 '22 20:09 github-actions[bot]

This pull request has been automatically closed because it has not had activity in the last 37 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!

github-actions[bot] avatar Oct 05 '22 20:10 github-actions[bot]