Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert "admin: add host priority and hostname to cluster response" #88

Merged
merged 1 commit into from
Jul 30, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 0 additions & 6 deletions api/envoy/admin/v2alpha/clusters.proto
Original file line number Diff line number Diff line change
Expand Up @@ -65,12 +65,6 @@ message HostStatus {

// The host's weight. If not configured, the value defaults to 1.
uint32 weight = 5;

// The hostname of the host, if applicable.
string hostname = 6;

// The host's priority. If not configured, the value defaults to 0 (highest priority).
uint32 priority = 7;
}

// Health status for a host.
Expand Down
5 changes: 0 additions & 5 deletions docs/root/intro/version_history.rst
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,6 @@ Version history
* access log: added a new field for downstream TLS session ID to file and gRPC access logger.
* access log: added a new field for response code details in :ref:`file access logger<config_access_log_format_response_code_details>` and :ref:`gRPC access logger<envoy_api_field_data.accesslog.v2.HTTPResponseProperties.response_code_details>`.
* admin: the administration interface now includes a :ref:`/ready endpoint <operations_admin_interface>` for easier readiness checks.
* admin: extend :ref:`/runtime_modify endpoint <operations_admin_interface_runtime_modify>` to support parameters within the request body.
* admin: the :ref:`/listener endpoint <operations_admin_interface_listeners>` now returns :ref:`listeners.proto<envoy_api_msg_admin.v2alpha.Listeners>` which includes listener names and ports.
* admin: added host priority to :http:get:`/clusters` and :http:get:`/clusters?format=json` end point response
* admin: the :ref:`/clusters endpoint <operations_admin_interface_clusters>` now shows hostname
for each host, useful for DNS based clusters.
* api: track and report requests issued since last load report.
* build: releases are built with Clang and linked with LLD.
* dubbo_proxy: support the :ref:`Dubbo proxy filter <config_network_filters_dubbo_proxy>`.
Expand Down
7 changes: 0 additions & 7 deletions source/server/http/admin.cc
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,6 @@ void AdminImpl::writeClustersAsJson(Buffer::Instance& response) {
envoy::admin::v2alpha::HostStatus& host_status = *cluster_status.add_host_statuses();
Network::Utility::addressToProtobufAddress(*host->address(),
*host_status.mutable_address());
host_status.set_hostname(host->hostname());
std::vector<Stats::CounterSharedPtr> sorted_counters;
for (const Stats::CounterSharedPtr& counter : host->counters()) {
sorted_counters.push_back(counter);
Expand Down Expand Up @@ -388,8 +387,6 @@ void AdminImpl::writeClustersAsJson(Buffer::Instance& response) {
}

host_status.set_weight(host->weight());

host_status.set_priority(host->priority());
}
}
}
Expand Down Expand Up @@ -427,8 +424,6 @@ void AdminImpl::writeClustersAsText(Buffer::Instance& response) {
host->address()->asString(), stat.first, stat.second));
}

response.add(fmt::format("{}::{}::hostname::{}\n", cluster.second.get().info()->name(),
host->address()->asString(), host->hostname()));
response.add(fmt::format("{}::{}::health_flags::{}\n", cluster.second.get().info()->name(),
host->address()->asString(),
Upstream::HostUtility::healthFlagsToString(*host)));
Expand All @@ -445,8 +440,6 @@ void AdminImpl::writeClustersAsText(Buffer::Instance& response) {
response.add(fmt::format("{}::{}::success_rate::{}\n", cluster.second.get().info()->name(),
host->address()->asString(),
host->outlierDetector().successRate()));
response.add(fmt::format("{}::{}::priority::{}\n", cluster.second.get().info()->name(),
host->address()->asString(), host->priority()));
}
}
}
Expand Down
40 changes: 5 additions & 35 deletions test/server/http/admin_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1038,8 +1038,6 @@ TEST_P(AdminInstanceTest, ClustersJson) {
Network::Address::InstanceConstSharedPtr address =
Network::Utility::resolveUrl("tcp://1.2.3.4:80");
ON_CALL(*host, address()).WillByDefault(Return(address));
const std::string hostname = "foo.com";
ON_CALL(*host, hostname()).WillByDefault(ReturnRef(hostname));

// Add stats in random order and validate that they come in order.
Stats::IsolatedStoreImpl store;
Expand All @@ -1066,7 +1064,6 @@ TEST_P(AdminInstanceTest, ClustersJson) {

ON_CALL(host->outlier_detector_, successRate()).WillByDefault(Return(43.2));
ON_CALL(*host, weight()).WillByDefault(Return(5));
ON_CALL(*host, priority()).WillByDefault(Return(6));

Buffer::OwnedImpl response;
Http::HeaderMapImpl header_map;
Expand Down Expand Up @@ -1129,9 +1126,7 @@ TEST_P(AdminInstanceTest, ClustersJson) {
"success_rate": {
"value": 43.2
},
"weight": 5,
"hostname": "foo.com",
"priority": 6
"weight": 5
}
]
}
Expand All @@ -1146,35 +1141,10 @@ TEST_P(AdminInstanceTest, ClustersJson) {
EXPECT_THAT(output_proto, ProtoEq(expected_proto));

// Ensure that the normal text format is used by default.
Buffer::OwnedImpl response2;
EXPECT_EQ(Http::Code::OK, getCallback("/clusters", header_map, response2));
const std::string expected_text = R"EOF(fake_cluster::outlier::success_rate_average::0
fake_cluster::outlier::success_rate_ejection_threshold::6
fake_cluster::default_priority::max_connections::1
fake_cluster::default_priority::max_pending_requests::1024
fake_cluster::default_priority::max_requests::1024
fake_cluster::default_priority::max_retries::1
fake_cluster::high_priority::max_connections::1
fake_cluster::high_priority::max_pending_requests::1024
fake_cluster::high_priority::max_requests::1024
fake_cluster::high_priority::max_retries::1
fake_cluster::added_via_api::true
fake_cluster::1.2.3.4:80::arest_counter::5
fake_cluster::1.2.3.4:80::atest_gauge::10
fake_cluster::1.2.3.4:80::rest_counter::10
fake_cluster::1.2.3.4:80::test_counter::10
fake_cluster::1.2.3.4:80::test_gauge::11
fake_cluster::1.2.3.4:80::hostname::foo.com
fake_cluster::1.2.3.4:80::health_flags::/failed_active_hc/failed_outlier_check/degraded_active_hc/degraded_eds_health/pending_dynamic_removal
fake_cluster::1.2.3.4:80::weight::5
fake_cluster::1.2.3.4:80::region::test_region
fake_cluster::1.2.3.4:80::zone::test_zone
fake_cluster::1.2.3.4:80::sub_zone::test_sub_zone
fake_cluster::1.2.3.4:80::canary::false
fake_cluster::1.2.3.4:80::success_rate::43.2
fake_cluster::1.2.3.4:80::priority::6
)EOF";
EXPECT_EQ(expected_text, response2.toString());
EXPECT_EQ(Http::Code::OK, getCallback("/clusters", header_map, response));
std::string text_output = response.toString();
envoy::admin::v2alpha::Clusters failed_conversion_proto;
EXPECT_THROW(MessageUtil::loadFromJson(text_output, failed_conversion_proto), EnvoyException);
}

TEST_P(AdminInstanceTest, GetRequest) {
Expand Down