runtime: add static layer case to loader impl#7932
runtime: add static layer case to loader impl#7932mattklein123 merged 5 commits intoenvoyproxy:masterfrom
Conversation
Signed-off-by: Asra Ali <asraa@google.com>
mattklein123
left a comment
There was a problem hiding this comment.
Thanks is it possible to have some test that covers this? You can use EXPECT_LOG_CONTAINS to test the presence/absence of the warning.
/wait
Signed-off-by: Asra Ali <asraa@google.com>
|
|
||
| // Validate that Static Layer does not log as unsupported. | ||
| TEST_F(StaticLoaderImplTest, NoUnsupportedStaticLog) { | ||
| EXPECT_LOG_NOT_CONTAINS("warning", "Skipping unsupported runtime layer", setup()); |
There was a problem hiding this comment.
While you are in here can you add a test for the warning also? Thank you!
/wait
There was a problem hiding this comment.
Hm, we actually shouldn't be able to get to this condition ever since there's a proto constraint that validates that we specify one of static/disk/admin/rtds. It'd run in to a proto validation error first. I suppose the warning would only come up if a new layered_runtime was added without support in LoaderImpl.
There was a problem hiding this comment.
Yeah if there is a constraint, just remove the warning instead and use NOT_REACHED
There was a problem hiding this comment.
I don't think this test is useful anymore. Can you revert these changes? Thank you!
/wait
There was a problem hiding this comment.
Sorry about that, should just be the NOT_REACHED case now (with proto validator).
Signed-off-by: Asra Ali <asraa@google.com>
Signed-off-by: Asra Ali <asraa@google.com>
Signed-off-by: Asra Ali <asraa@google.com>
When using a static layer, Envoy should catch the case instead of falling through to an unsupported layer specifier.
Risk Level: Low
Fixes Issues #7919
istio/istio#16083
Signed-off-by: Asra Ali asraa@google.com