From ad5920ec14fa03399c8137ace336c99eebdba86e Mon Sep 17 00:00:00 2001 From: Wayne Zhang Date: Tue, 6 Feb 2018 23:29:27 +0000 Subject: [PATCH 1/2] Clean up duplicated cluster names --- src/envoy/mixer/http_filter.cc | 4 +--- src/envoy/mixer/mixer_control.cc | 19 ++++++------------- src/envoy/mixer/mixer_control.h | 26 +++++++++----------------- 3 files changed, 16 insertions(+), 33 deletions(-) diff --git a/src/envoy/mixer/http_filter.cc b/src/envoy/mixer/http_filter.cc index b0fd86a28f4..631cc186187 100644 --- a/src/envoy/mixer/http_filter.cc +++ b/src/envoy/mixer/http_filter.cc @@ -437,9 +437,7 @@ class Instance : public Http::StreamDecoderFilter, CheckData check_data(headers, decoder_callbacks_->connection()); HeaderUpdate header_update(&headers); cancel_check_ = handler_->Check( - &check_data, &header_update, - CheckTransport::GetFunc(mixer_control_.cm(), - mixer_control_.check_cluster(), &headers), + &check_data, &header_update, mixer_control_.GetCheckTransport(&headers), [this](const Status& status) { completeCheck(status); }); initiating_call_ = false; diff --git a/src/envoy/mixer/mixer_control.cc b/src/envoy/mixer/mixer_control.cc index 356f36c996f..7e376bf7761 100644 --- a/src/envoy/mixer/mixer_control.cc +++ b/src/envoy/mixer/mixer_control.cc @@ -14,7 +14,6 @@ */ #include "src/envoy/mixer/mixer_control.h" -#include "src/envoy/mixer/grpc_transport.h" namespace Envoy { namespace Http { @@ -68,15 +67,12 @@ HttpMixerControl::HttpMixerControl(const HttpMixerConfig& mixer_config, Upstream::ClusterManager& cm, Event::Dispatcher& dispatcher, Runtime::RandomGenerator& random) - : cm_(cm) { + : config_(mixer_config), cm_(cm) { ::istio::mixer_control::http::Controller::Options options( mixer_config.http_config, mixer_config.legacy_quotas); - check_cluster_ = mixer_config.check_cluster; - report_cluster_ = mixer_config.report_cluster; - - CreateEnvironment(cm, dispatcher, random, check_cluster_, report_cluster_, - &options.env); + CreateEnvironment(cm, dispatcher, random, config_.check_cluster, + config_.report_cluster, &options.env); controller_ = ::istio::mixer_control::http::Controller::Create(options); has_v2_config_ = mixer_config.has_v2_config; @@ -86,15 +82,12 @@ TcpMixerControl::TcpMixerControl(const TcpMixerConfig& mixer_config, Upstream::ClusterManager& cm, Event::Dispatcher& dispatcher, Runtime::RandomGenerator& random) - : dispatcher_(dispatcher) { + : config_(mixer_config), dispatcher_(dispatcher) { ::istio::mixer_control::tcp::Controller::Options options( mixer_config.tcp_config); - check_cluster_ = mixer_config.check_cluster; - report_cluster_ = mixer_config.report_cluster; - - CreateEnvironment(cm, dispatcher, random, check_cluster_, report_cluster_, - &options.env); + CreateEnvironment(cm, dispatcher, random, config_.check_cluster, + config_.report_cluster, &options.env); controller_ = ::istio::mixer_control::tcp::Controller::Create(options); diff --git a/src/envoy/mixer/mixer_control.h b/src/envoy/mixer/mixer_control.h index a72679f4ab1..ab4a6836bc4 100644 --- a/src/envoy/mixer/mixer_control.h +++ b/src/envoy/mixer/mixer_control.h @@ -22,6 +22,7 @@ #include "envoy/thread_local/thread_local.h" #include "envoy/upstream/cluster_manager.h" #include "src/envoy/mixer/config.h" +#include "src/envoy/mixer/grpc_transport.h" namespace Envoy { namespace Http { @@ -36,27 +37,25 @@ class HttpMixerControl final : public ThreadLocal::ThreadLocalObject { Upstream::ClusterManager& cm() { return cm_; } - const std::string& check_cluster() const { return check_cluster_; } - - const std::string& report_cluster() const { return report_cluster_; } - ::istio::mixer_control::http::Controller* controller() { return controller_.get(); } bool has_v2_config() const { return has_v2_config_; } + CheckTransport::Func GetCheckTransport(const HeaderMap* headers) { + return CheckTransport::GetFunc(cm_, config_.check_cluster, headers); + } + private: + // The mixer config. + const HttpMixerConfig& config_; // Envoy cluster manager for making gRPC calls. Upstream::ClusterManager& cm_; // The mixer control std::unique_ptr<::istio::mixer_control::http::Controller> controller_; // has v2 config; bool has_v2_config_; - // check cluster - std::string check_cluster_; - // report cluster - std::string report_cluster_; }; class TcpMixerControl final : public ThreadLocal::ThreadLocalObject { @@ -74,13 +73,11 @@ class TcpMixerControl final : public ThreadLocal::ThreadLocalObject { return report_interval_ms_; } - const std::string& check_cluster() const { return check_cluster_; } - - const std::string& report_cluster() const { return report_cluster_; } - Event::Dispatcher& dispatcher() { return dispatcher_; } private: + // The mixer config. + const TcpMixerConfig& config_; // The mixer control std::unique_ptr<::istio::mixer_control::tcp::Controller> controller_; @@ -88,11 +85,6 @@ class TcpMixerControl final : public ThreadLocal::ThreadLocalObject { std::chrono::milliseconds report_interval_ms_; Event::Dispatcher& dispatcher_; - - // check cluster - std::string check_cluster_; - // report cluster - std::string report_cluster_; }; } // namespace Mixer From be067e23c1ea9be92b71959a9e75b0ef1b5a7607 Mon Sep 17 00:00:00 2001 From: Wayne Zhang Date: Wed, 7 Feb 2018 01:15:17 +0000 Subject: [PATCH 2/2] Fix a crash --- src/envoy/mixer/config.cc | 29 +++++++++++++---------------- src/envoy/mixer/config.h | 18 ++++++++++++++---- src/envoy/mixer/mixer_control.cc | 8 ++++---- src/envoy/mixer/mixer_control.h | 2 +- 4 files changed, 32 insertions(+), 25 deletions(-) diff --git a/src/envoy/mixer/config.cc b/src/envoy/mixer/config.cc index 2a4ae7f260d..ec94134104d 100644 --- a/src/envoy/mixer/config.cc +++ b/src/envoy/mixer/config.cc @@ -79,6 +79,15 @@ void ReadLegacyTransportConfig(const Json::Object& json, config->set_disable_report_batch(json.getBoolean(kDisableReportBatch, false)); } +void SetDefaultMixerClusters(TransportConfig* config) { + if (config->check_cluster().empty()) { + config->set_check_cluster(kDefaultMixerClusterName); + } + if (config->report_cluster().empty()) { + config->set_report_cluster(kDefaultMixerClusterName); + } +} + bool ReadV2Config(const Json::Object& json, Message* message) { if (!json.hasObject(kV2Config)) { return false; @@ -114,8 +123,7 @@ void HttpMixerConfig::Load(const Json::Object& json) { legacy_quotas.push_back({json.getString(kQuotaName), amount}); } - TransportConfig* transport_config = http_config.mutable_transport(); - ReadLegacyTransportConfig(json, transport_config); + ReadLegacyTransportConfig(json, http_config.mutable_transport()); has_v2_config = ReadV2Config(json, &http_config); if (has_v2_config) { @@ -123,12 +131,7 @@ void HttpMixerConfig::Load(const Json::Object& json) { legacy_quotas.clear(); } - check_cluster = transport_config->check_cluster().empty() - ? kDefaultMixerClusterName - : transport_config->check_cluster(); - report_cluster = transport_config->report_cluster().empty() - ? kDefaultMixerClusterName - : transport_config->report_cluster(); + SetDefaultMixerClusters(http_config.mutable_transport()); } void HttpMixerConfig::CreateLegacyRouteConfig( @@ -147,20 +150,14 @@ void HttpMixerConfig::CreateLegacyRouteConfig( void TcpMixerConfig::Load(const Json::Object& json) { ReadStringMap(json, kMixerAttributes, tcp_config.mutable_mixer_attributes()); - TransportConfig* transport_config = tcp_config.mutable_transport(); - ReadLegacyTransportConfig(json, transport_config); + ReadLegacyTransportConfig(json, tcp_config.mutable_transport()); tcp_config.set_disable_check_calls( json.getBoolean(kDisableTcpCheckCalls, false)); ReadV2Config(json, &tcp_config); - check_cluster = transport_config->check_cluster().empty() - ? kDefaultMixerClusterName - : transport_config->check_cluster(); - report_cluster = transport_config->report_cluster().empty() - ? kDefaultMixerClusterName - : transport_config->report_cluster(); + SetDefaultMixerClusters(tcp_config.mutable_transport()); } } // namespace Mixer diff --git a/src/envoy/mixer/config.h b/src/envoy/mixer/config.h index 453c2c4bdf7..1b4116aad64 100644 --- a/src/envoy/mixer/config.h +++ b/src/envoy/mixer/config.h @@ -39,10 +39,15 @@ struct HttpMixerConfig { std::vector<::istio::quota::Requirement> legacy_quotas; // If true, v2 config is valid. bool has_v2_config; + // check cluster - std::string check_cluster; + const std::string& check_cluster() const { + return http_config.transport().check_cluster(); + } // report cluster - std::string report_cluster; + const std::string& report_cluster() const { + return http_config.transport().report_cluster(); + } // Create per route legacy config. static void CreateLegacyRouteConfig( @@ -58,10 +63,15 @@ struct TcpMixerConfig { // The Tcp client config. ::istio::mixer::v1::config::client::TcpClientConfig tcp_config; + // check cluster - std::string check_cluster; + const std::string& check_cluster() const { + return tcp_config.transport().check_cluster(); + } // report cluster - std::string report_cluster; + const std::string& report_cluster() const { + return tcp_config.transport().report_cluster(); + } }; } // namespace Mixer diff --git a/src/envoy/mixer/mixer_control.cc b/src/envoy/mixer/mixer_control.cc index 7e376bf7761..a2fac956628 100644 --- a/src/envoy/mixer/mixer_control.cc +++ b/src/envoy/mixer/mixer_control.cc @@ -71,8 +71,8 @@ HttpMixerControl::HttpMixerControl(const HttpMixerConfig& mixer_config, ::istio::mixer_control::http::Controller::Options options( mixer_config.http_config, mixer_config.legacy_quotas); - CreateEnvironment(cm, dispatcher, random, config_.check_cluster, - config_.report_cluster, &options.env); + CreateEnvironment(cm, dispatcher, random, config_.check_cluster(), + config_.report_cluster(), &options.env); controller_ = ::istio::mixer_control::http::Controller::Create(options); has_v2_config_ = mixer_config.has_v2_config; @@ -86,8 +86,8 @@ TcpMixerControl::TcpMixerControl(const TcpMixerConfig& mixer_config, ::istio::mixer_control::tcp::Controller::Options options( mixer_config.tcp_config); - CreateEnvironment(cm, dispatcher, random, config_.check_cluster, - config_.report_cluster, &options.env); + CreateEnvironment(cm, dispatcher, random, config_.check_cluster(), + config_.report_cluster(), &options.env); controller_ = ::istio::mixer_control::tcp::Controller::Create(options); diff --git a/src/envoy/mixer/mixer_control.h b/src/envoy/mixer/mixer_control.h index ab4a6836bc4..cb4d0de5f10 100644 --- a/src/envoy/mixer/mixer_control.h +++ b/src/envoy/mixer/mixer_control.h @@ -44,7 +44,7 @@ class HttpMixerControl final : public ThreadLocal::ThreadLocalObject { bool has_v2_config() const { return has_v2_config_; } CheckTransport::Func GetCheckTransport(const HeaderMap* headers) { - return CheckTransport::GetFunc(cm_, config_.check_cluster, headers); + return CheckTransport::GetFunc(cm_, config_.check_cluster(), headers); } private: