examples: fixed fault-injection delay demo #11715
Conversation
Without that part Envoy assumes that delay is disabled. Changed Envoy docker file to run as root: required for access to stdout. Signed-off-by: Christoph Pakulski <christoph@tetrate.io>
| @@ -13,6 +13,8 @@ services: | |||
| ports: | |||
| - 9211:9211 | |||
| - 9901:9901 | |||
| environment: | |||
| ENVOY_UID: 0 | |||
There was a problem hiding this comment.
It was necessary to give Envoy permission to access /dev/stdout. Without that setting Envoy container exists complaining that it cannot write logs to /dev/stdout (specified in envoy.yaml as sink for logs).
There was a problem hiding this comment.
Optional, bonus points for commenting that - I ran into that trying to repro and it'd be lovely if a local grep for stdout turned up the fix :-)
There was a problem hiding this comment.
Added comment to explain why Envoy must run as root.
| @@ -36,6 +36,11 @@ static_resources: | |||
| percentage: | |||
| numerator: 0 | |||
| denominator: HUNDRED | |||
| delay: | |||
There was a problem hiding this comment.
This is a great find, thanks for tracking it down!
I do wonder if we should tweak the fault filter to delay / abort if there's a runtime override even if the proto doesn't include the config. Even though it's pretty clearly documented, I can imagine it confusing folks.
cc @mattklein123
either way it shouldn't block this PR, more musing if we should file a tracking issue.
There was a problem hiding this comment.
I don't recall the history of why this is implemented the way it is. I agree it's potentially confusing. I think the idea is we want the user to opt-in explicitly to the config they want but I don't feel super strongly about it. @Augustyniak any thoughts here?
There was a problem hiding this comment.
I think explicitness here is good since we support injecting faults via different methods including HTTP headers and we want these faults to be injected only if our Envoy is explicitly configured to accept them.
- Without Envoy requiring people to opt-in with this config, it may be confusing as to what kind of faults a given Envoy may potentially inject - the ones controlled via runtime or via HTTP headers. With config in, it becomes clear what’s supported.
- If we decide that we don’t want to require a config for faults controlled via runtime we should probably do the same for faults controlled with HTTP headers which is risky because of security implications.
For all of the aforementioned reasons, I would leave the presence of the config as the required step of FaultFilter’s setup.
For reference, an example of a config that supports delay faults via HTTP headers:
delay:
header_delay: {}
percentage:
numerator: 100
Signed-off-by: Christoph Pakulski <christoph@tetrate.io>
|
@alyssawilk I think this is ready for merge. |
fault injection example did not work for delay option. After gdb debugging it was discovered that fault injection config requires delay part to be present. Without delay part Envoy assumes that Delay is disabled and does not scan RTDS for delay config updates. Risk Level: Low: Testing: Did manual testing as described in https://www.envoyproxy.io/docs/envoy/latest/start/sandboxes/fault_injection Docs Changes: No Release Notes: No Fixes: envoyproxy#11095 Signed-off-by: Christoph Pakulski <christoph@tetrate.io> Signed-off-by: chaoqinli <chaoqinli@google.com>
fault injection example did not work for delay option. After gdb debugging it was discovered that fault injection config requires delay part to be present. Without delay part Envoy assumes that Delay is disabled and does not scan RTDS for delay config updates. Risk Level: Low: Testing: Did manual testing as described in https://www.envoyproxy.io/docs/envoy/latest/start/sandboxes/fault_injection Docs Changes: No Release Notes: No Fixes: envoyproxy#11095 Signed-off-by: Christoph Pakulski <christoph@tetrate.io>
fault injection example did not work for delay option. After gdb debugging it was discovered that fault injection config requires delay part to be present. Without delay part Envoy assumes that Delay is disabled and does not scan RTDS for delay config updates. Risk Level: Low: Testing: Did manual testing as described in https://www.envoyproxy.io/docs/envoy/latest/start/sandboxes/fault_injection Docs Changes: No Release Notes: No Fixes: envoyproxy#11095 Signed-off-by: Christoph Pakulski <christoph@tetrate.io> Signed-off-by: yashwant121 <yadavyashwant36@gmail.com>
Description:
fault injection example did not work for delay option. After gdb debugging it was discovered that fault injection config requires
delaypart to be present. Withoutdelaypart Envoy assumes that Delay is disabled and does not scan RTDS for delay config updates.Risk Level:
Low:
Testing:
Did manual testing as described in https://www.envoyproxy.io/docs/envoy/latest/start/sandboxes/fault_injection
Docs Changes:
No
Release Notes:
No
Fixes: #11095