config: test binary to load configuration files against #969
Conversation
|
|
||
| int main(int argc, char* argv[]) { | ||
| if (argc != 2) { | ||
| return EXIT_FAILURE; |
There was a problem hiding this comment.
Can you add a usage string on std:cerr?
| return EXIT_FAILURE; | ||
| } | ||
| try { | ||
| uint32_t num_tested = ConfigTest::run(std::string(argv[1])); |
|
|
||
| #include "test/config_test/config_test.h" | ||
|
|
||
| #include "spdlog/spdlog.h" |
There was a problem hiding this comment.
This is unrelated to this review, but I've noticed this anti-pattern across the Envoy code base, where we're forcing a dependency on spdlog just to get at fmt::format. It would be nice if we could clean this up one day by wrapping this in a shim and not require spdlog dependency just for basic string formatting. I think spdlog is just using https://github.com/fmtlib/fmt in any case, which it bundles. I've created #975 to track, feel free to keep using spdlog in this review as it's a wider issue.
| uint32_t num_tested = ConfigTest::run(std::string(argv[1])); | ||
| std::cout << fmt::format("Successfully tested: {}", num_tested) << std::endl; | ||
| return EXIT_SUCCESS; | ||
| } catch (const std::runtime_error& e) { |
There was a problem hiding this comment.
Can you comment on which exception you plan on catching here?
|
@junr03 CI indicates this fails to build, can you take a look? Thanks. |
|
|
||
| Input | ||
| The tool expects a PATH to the root of a directory that holds json envoy configuration files. The tool | ||
| will recursively go through the filesystem tree and run a configuration test for each configuration file found. |
There was a problem hiding this comment.
can you expand on how it identifies a configuration file?
| } | ||
| try { | ||
| const uint32_t num_tested = Envoy::ConfigTest::run(std::string(argv[1])); | ||
| std::cout << fmt::format("Successfully tested: {}", num_tested) << std::endl; |
There was a problem hiding this comment.
we can get rid of that "spdlog/spdlog.h"
this can be changed to
std::cout << "Successfully tested: " << num_tested << " configs" << std::endl;
|
|
||
| int main(int argc, char* argv[]) { | ||
| if (argc != 2) { | ||
| std::cerr << "Usage: config_load_check PATH" << std::endl; |
There was a problem hiding this comment.
up to you to remove bunch of std::cerr and replace by single std::cerr
|
LGTM with small nits. |
| the tool will try to load all files found in the path. | ||
|
|
||
| Output | ||
| The tool will output envoy logs as it initializes the server configuration with the config it is currently testing. |
There was a problem hiding this comment.
Nit: s/envoy/Envoy/, s/json/JSON/ in this file.
Automatic merge from submit-queue. [DO NOT MERGE] Auto PR to update dependencies of proxy This PR will be merged automatically once checks are successful. ```release-note none ```
Cleaning up the sonatype upload script to enable easier local deployment. The change here is just to remove some required fields in the local publish case. Signed-off-by: Alan Chiu <[email protected]> For an explanation of how to fill out the fields, please see the relevant section in [PULL_REQUESTS.md](https://github.com/envoyproxy/envoy/blob/master/PULL_REQUESTS.md) Description: android: clean up sonatype upload to support local deploy Risk Level: low Testing: local Docs Changes: n/a Release Notes: n/a [Optional Fixes #Issue] [Optional Deprecated:] Signed-off-by: JP Simard <[email protected]>
Cleaning up the sonatype upload script to enable easier local deployment. The change here is just to remove some required fields in the local publish case. Signed-off-by: Alan Chiu <[email protected]> For an explanation of how to fill out the fields, please see the relevant section in [PULL_REQUESTS.md](https://github.com/envoyproxy/envoy/blob/master/PULL_REQUESTS.md) Description: android: clean up sonatype upload to support local deploy Risk Level: low Testing: local Docs Changes: n/a Release Notes: n/a [Optional Fixes #Issue] [Optional Deprecated:] Signed-off-by: JP Simard <[email protected]>
This may fix issue envoyproxy#969. When testing OOM handling we set up sys allocator that fails memory allocation. But debugallocator itself allocates some internal metadata memory via malloc and crashes if those allocations fail. So occasionally this test failed when debugallocator's internal malloc ended up causing sys allocator. So instead of failing tests from time to time, we drop it for debug allocator. It's OOM handling is already crashy anyways.
This PR adds a small binary that wraps around existing config_tests. The binary enables developers to validate their configuration files for well formed JSON and JSON that conforms to Envoy's JSON schema.
This tool is different from #863. #863, runs a deeper configuration validation as it goes through the motion of setting up Envoy's event loop up to the point where Envoy would be ready to listen for traffic. This tool only validates configuration files as far as their JSON structure and conformation to Envoy's JSON schema.