admin: fix config dump order #4197
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Rama <rama.rao@salesforce.com>
Signed-off-by: Rama <rama.rao@salesforce.com>
htuch
suggested changes
Aug 20, 2018
test/server/http/admin_test.cc
Outdated
| Buffer::OwnedImpl response; | ||
| Http::HeaderMapImpl header_map; | ||
| EXPECT_EQ(Http::Code::OK, getCallback("/config_dump", header_map, response)); | ||
| std::string output = response.toString(); |
| msg->set_value("bootstrap_config"); | ||
| return msg; | ||
| }); | ||
| auto route_entry = admin_.getConfigTracker().add("routes", [] { |
There was a problem hiding this comment.
Nit: you don't need to assign to a variable/retain results here, they aren't used below.
There was a problem hiding this comment.
With out that config_dump is not build as expected. Not sure why though. The test above this ConfigDump test also has the same issue. Could n't quite figure out why.
There was a problem hiding this comment.
Oh, maybe it's because these are smart pointers, RAII, got it.
Signed-off-by: Rama <rama.rao@salesforce.com>
|
yes, The ConfigTracker keeps them in a map which is lexically ordered by key. |
snowp
pushed a commit
to snowp/envoy
that referenced
this pull request
Aug 20, 2018
* origin/master: (34 commits) fuzz: fixes oss-fuzz: 9895 (envoyproxy#4189) docs: added developer docs for pprof/tcmalloc testing (envoyproxy#4194) fix sometimes returns response body to HEAD requests (envoyproxy#3985) admin: fix config dump order (envoyproxy#4197) thrift: allow translation between downstream and upstream protocols (envoyproxy#4136) Use local_info.node() instead of bootstrap.node() whenever possible (envoyproxy#4120) upstream: allow extension protocol options to be used with http filters (envoyproxy#4165) [thrift_proxy] Replace local HeaderMap with Http::HeaderMap[Impl] (envoyproxy#4169) docs: improve gRPC-JSON filter doc (envoyproxy#4184) stats: refactoring MetricImpl without strings (envoyproxy#4190) fuzz: coverage profile generation instructions. (envoyproxy#4193) upstream: rebuild cluster when health check config is changed (envoyproxy#4075) build: use clang-6.0. (envoyproxy#4168) thrift_proxy: introduce header transport (envoyproxy#4082) tcp: allow connection pool callers to store protocol state (envoyproxy#4131) configs: match latest API changes (envoyproxy#4185) Fix wrong mock function name. (envoyproxy#4187) Bump yaml-cpp so it builds with Visual Studio 15.8 (envoyproxy#4182) Converting envoy configs to V2 (envoyproxy#2957) Add timestamp to HealthCheckEvent definition (envoyproxy#4119) ... Signed-off-by: Snow Pettersen <snowp@squareup.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Signed-off-by: Rama rama.rao@salesforce.com
Description: Fixes the order of config_dump elements - bootstrap, clusters, listeners and routes will come in that order. Note that inside each of them, ordering is not guaranteed. This is a step in moving towards complete ordering.
Risk Level: Low
Testing: Added Automated tests
Docs Changes: Updated
Release Notes: N/A