From b99cfe8ffe2d7de1ae940536c505ac3ac1bb5eb8 Mon Sep 17 00:00:00 2001 From: Shriram Rajagopalan Date: Mon, 5 Feb 2018 17:32:55 -0500 Subject: [PATCH 1/7] make mixer cluster configurable --- src/envoy/mixer/config.cc | 9 +++++++++ src/envoy/mixer/grpc_transport.cc | 12 +++++------- src/envoy/mixer/grpc_transport.h | 1 + src/envoy/mixer/mixer_control.cc | 16 ++++++++++++---- 4 files changed, 27 insertions(+), 11 deletions(-) diff --git a/src/envoy/mixer/config.cc b/src/envoy/mixer/config.cc index ec635f2647d..f4260ad0f4f 100644 --- a/src/envoy/mixer/config.cc +++ b/src/envoy/mixer/config.cc @@ -51,6 +51,12 @@ const std::string kDisableTcpCheckCalls("disable_tcp_check_calls"); const std::string kV2Config("v2"); +// The name for the mixer server cluster. +const std::string kDefaultMixerClusterName("mixer_server"); +// The Json object name for check_cluster and report_cluster +const std::string kCheckCluster("check_cluster"); +const std::string kReportCluster("report_cluster"); + void ReadStringMap(const Json::Object& json, const std::string& name, Attributes* attributes) { if (json.hasObject(name)) { @@ -73,6 +79,9 @@ void ReadTransportConfig(const Json::Object& json, TransportConfig* config) { config->set_disable_check_cache(json.getBoolean(kDisableCheckCache, false)); config->set_disable_quota_cache(json.getBoolean(kDisableQuotaCache, false)); config->set_disable_report_batch(json.getBoolean(kDisableReportBatch, false)); + + config->set_check_cluster(json.getString(kCheckCluster, kDefaultMixerClusterName)); + config->set_report_cluster(json.getString(kReportCluster, kDefaultMixerClusterName)); } bool ReadV2Config(const Json::Object& json, Message* message) { diff --git a/src/envoy/mixer/grpc_transport.cc b/src/envoy/mixer/grpc_transport.cc index 4f44be63a09..f443525e741 100644 --- a/src/envoy/mixer/grpc_transport.cc +++ b/src/envoy/mixer/grpc_transport.cc @@ -35,9 +35,6 @@ const LowerCaseString kB3Sampled("x-b3-sampled"); const LowerCaseString kB3Flags("x-b3-flags"); const LowerCaseString kOtSpanContext("x-ot-span-context"); -// The name for the mixer server cluster. -const char* kMixerServerClusterName = "mixer_server"; - inline void CopyHeaderEntry(const HeaderEntry* entry, const LowerCaseString& key, Http::HeaderMap& headers) { @@ -110,13 +107,14 @@ void GrpcTransport::Cancel() { template typename GrpcTransport::Func GrpcTransport::GetFunc(Upstream::ClusterManager& cm, + const std::string& cluster_name, const HeaderMap* headers) { - return [&cm, headers](const RequestType& request, ResponseType* response, + return [&cm, &cluster_name, headers](const RequestType& request, ResponseType* response, istio::mixer_client::DoneFunc on_done) -> istio::mixer_client::CancelFunc { auto transport = new GrpcTransport( Grpc::AsyncClientPtr( - new Grpc::AsyncClientImpl(cm, kMixerServerClusterName)), + new Grpc::AsyncClientImpl(cm, cluster_name.c_str())), request, headers, response, on_done); return [transport]() { transport->Cancel(); }; }; @@ -142,9 +140,9 @@ const google::protobuf::MethodDescriptor& ReportTransport::descriptor() { // explicitly instantiate CheckTransport and ReportTransport template CheckTransport::Func CheckTransport::GetFunc( - Upstream::ClusterManager& cm, const HeaderMap* headers); + Upstream::ClusterManager& cm, const std::string& cluster_name, const HeaderMap* headers); template ReportTransport::Func ReportTransport::GetFunc( - Upstream::ClusterManager& cm, const HeaderMap* headers); + Upstream::ClusterManager& cm, const std::string& cluster_name, const HeaderMap* headers); } // namespace Mixer } // namespace Http diff --git a/src/envoy/mixer/grpc_transport.h b/src/envoy/mixer/grpc_transport.h index fd12a63c207..b50464c47f1 100644 --- a/src/envoy/mixer/grpc_transport.h +++ b/src/envoy/mixer/grpc_transport.h @@ -39,6 +39,7 @@ class GrpcTransport : public Grpc::TypedAsyncRequestCallbacks, istio::mixer_client::DoneFunc on_done)>; static Func GetFunc(Upstream::ClusterManager& cm, + const std::string& cluster_name, const HeaderMap* headers = nullptr); GrpcTransport(Grpc::AsyncClientPtr async_client, const RequestType& request, diff --git a/src/envoy/mixer/mixer_control.cc b/src/envoy/mixer/mixer_control.cc index 2430b533852..4951c3f1f0f 100644 --- a/src/envoy/mixer/mixer_control.cc +++ b/src/envoy/mixer/mixer_control.cc @@ -45,9 +45,11 @@ class EnvoyTimer : public ::istio::mixer_client::Timer { void CreateEnvironment(Upstream::ClusterManager& cm, Event::Dispatcher& dispatcher, Runtime::RandomGenerator& random, + const std::string& check_cluster, + const std::string& report_cluster, ::istio::mixer_client::Environment* env) { - env->check_transport = CheckTransport::GetFunc(cm, nullptr); - env->report_transport = ReportTransport::GetFunc(cm); + env->check_transport = CheckTransport::GetFunc(cm, check_cluster, nullptr); + env->report_transport = ReportTransport::GetFunc(cm, report_cluster); env->timer_create_func = [&dispatcher](std::function timer_cb) -> std::unique_ptr<::istio::mixer_client::Timer> { @@ -70,7 +72,10 @@ 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, &options.env); + CreateEnvironment(cm, dispatcher, random, + mixer_config.transport().check_cluster(), + mixer_config.transport().report_cluster(), + &options.env); controller_ = ::istio::mixer_control::http::Controller::Create(options); @@ -85,7 +90,10 @@ TcpMixerControl::TcpMixerControl(const TcpMixerConfig& mixer_config, ::istio::mixer_control::tcp::Controller::Options options( mixer_config.tcp_config); - CreateEnvironment(cm, dispatcher, random, &options.env); + CreateEnvironment(cm, dispatcher, random, + mixer_config.transport().check_cluster(), + mixer_config.transport().report_cluster(), + &options.env); controller_ = ::istio::mixer_control::tcp::Controller::Create(options); From 7cf9598184e77800860cb7a0afcbba71c9b2fdf2 Mon Sep 17 00:00:00 2001 From: Shriram Rajagopalan Date: Mon, 5 Feb 2018 22:20:55 -0500 Subject: [PATCH 2/7] nits --- src/envoy/mixer/config.cc | 16 ++++++++++------ src/envoy/mixer/grpc_transport.cc | 2 +- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/src/envoy/mixer/config.cc b/src/envoy/mixer/config.cc index f4260ad0f4f..39d56f3dd2e 100644 --- a/src/envoy/mixer/config.cc +++ b/src/envoy/mixer/config.cc @@ -68,7 +68,7 @@ void ReadStringMap(const Json::Object& json, const std::string& name, } } -void ReadTransportConfig(const Json::Object& json, TransportConfig* config) { +void ReadLegacyTransportConfig(const Json::Object& json, TransportConfig* config) { // Default is open, unless it specifically set to "close" config->set_network_fail_policy(TransportConfig::FAIL_OPEN); if (json.hasObject(kNetworkFailPolicy) && @@ -79,9 +79,6 @@ void ReadTransportConfig(const Json::Object& json, TransportConfig* config) { config->set_disable_check_cache(json.getBoolean(kDisableCheckCache, false)); config->set_disable_quota_cache(json.getBoolean(kDisableQuotaCache, false)); config->set_disable_report_batch(json.getBoolean(kDisableReportBatch, false)); - - config->set_check_cluster(json.getString(kCheckCluster, kDefaultMixerClusterName)); - config->set_report_cluster(json.getString(kReportCluster, kDefaultMixerClusterName)); } bool ReadV2Config(const Json::Object& json, Message* message) { @@ -119,13 +116,17 @@ void HttpMixerConfig::Load(const Json::Object& json) { legacy_quotas.push_back({json.getString(kQuotaName), amount}); } - ReadTransportConfig(json, http_config.mutable_transport()); + TransportConfig* transport_config = http_config.mutable_transport(); + ReadLegacyTransportConfig(json, transport_config); has_v2_config = ReadV2Config(json, &http_config); if (has_v2_config) { // If v2 config is valid, clear v1 legacy_quotas. legacy_quotas.clear(); } + + transport_config->set_check_cluster(json.getString(kCheckCluster, kDefaultMixerClusterName)); + transport_config->set_report_cluster(json.getString(kReportCluster, kDefaultMixerClusterName)); } void HttpMixerConfig::CreateLegacyRouteConfig( @@ -144,12 +145,15 @@ void HttpMixerConfig::CreateLegacyRouteConfig( void TcpMixerConfig::Load(const Json::Object& json) { ReadStringMap(json, kMixerAttributes, tcp_config.mutable_mixer_attributes()); - ReadTransportConfig(json, tcp_config.mutable_transport()); + TransportConfig *transport_config = tcp_config.mutable_transport(); + ReadLegacyTransportConfig(json, transport_config); tcp_config.set_disable_check_calls( json.getBoolean(kDisableTcpCheckCalls, false)); ReadV2Config(json, &tcp_config); + transport_config->set_check_cluster(json.getString(kCheckCluster, kDefaultMixerClusterName)); + transport_config->set_report_cluster(json.getString(kReportCluster, kDefaultMixerClusterName)); } } // namespace Mixer diff --git a/src/envoy/mixer/grpc_transport.cc b/src/envoy/mixer/grpc_transport.cc index f443525e741..9b6eb13a2e5 100644 --- a/src/envoy/mixer/grpc_transport.cc +++ b/src/envoy/mixer/grpc_transport.cc @@ -109,7 +109,7 @@ typename GrpcTransport::Func GrpcTransport::GetFunc(Upstream::ClusterManager& cm, const std::string& cluster_name, const HeaderMap* headers) { - return [&cm, &cluster_name, headers](const RequestType& request, ResponseType* response, + return [&cm, cluster_name, headers](const RequestType& request, ResponseType* response, istio::mixer_client::DoneFunc on_done) -> istio::mixer_client::CancelFunc { auto transport = new GrpcTransport( From 136d9e87c6e848fabbfbedf11d9612825118f738 Mon Sep 17 00:00:00 2001 From: Shriram Rajagopalan Date: Mon, 5 Feb 2018 22:30:55 -0500 Subject: [PATCH 3/7] format --- src/envoy/mixer/config.cc | 17 +++++++++++------ src/envoy/mixer/grpc_transport.cc | 17 ++++++++++------- src/envoy/mixer/mixer_control.cc | 6 ++---- 3 files changed, 23 insertions(+), 17 deletions(-) diff --git a/src/envoy/mixer/config.cc b/src/envoy/mixer/config.cc index 39d56f3dd2e..1b9b57b92fc 100644 --- a/src/envoy/mixer/config.cc +++ b/src/envoy/mixer/config.cc @@ -68,7 +68,8 @@ void ReadStringMap(const Json::Object& json, const std::string& name, } } -void ReadLegacyTransportConfig(const Json::Object& json, TransportConfig* config) { +void ReadLegacyTransportConfig(const Json::Object& json, + TransportConfig* config) { // Default is open, unless it specifically set to "close" config->set_network_fail_policy(TransportConfig::FAIL_OPEN); if (json.hasObject(kNetworkFailPolicy) && @@ -125,8 +126,10 @@ void HttpMixerConfig::Load(const Json::Object& json) { legacy_quotas.clear(); } - transport_config->set_check_cluster(json.getString(kCheckCluster, kDefaultMixerClusterName)); - transport_config->set_report_cluster(json.getString(kReportCluster, kDefaultMixerClusterName)); + transport_config->set_check_cluster( + json.getString(kCheckCluster, kDefaultMixerClusterName)); + transport_config->set_report_cluster( + json.getString(kReportCluster, kDefaultMixerClusterName)); } void HttpMixerConfig::CreateLegacyRouteConfig( @@ -145,15 +148,17 @@ 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(); + TransportConfig* transport_config = tcp_config.mutable_transport(); ReadLegacyTransportConfig(json, transport_config); tcp_config.set_disable_check_calls( json.getBoolean(kDisableTcpCheckCalls, false)); ReadV2Config(json, &tcp_config); - transport_config->set_check_cluster(json.getString(kCheckCluster, kDefaultMixerClusterName)); - transport_config->set_report_cluster(json.getString(kReportCluster, kDefaultMixerClusterName)); + transport_config->set_check_cluster( + json.getString(kCheckCluster, kDefaultMixerClusterName)); + transport_config->set_report_cluster( + json.getString(kReportCluster, kDefaultMixerClusterName)); } } // namespace Mixer diff --git a/src/envoy/mixer/grpc_transport.cc b/src/envoy/mixer/grpc_transport.cc index 9b6eb13a2e5..22ac2c48808 100644 --- a/src/envoy/mixer/grpc_transport.cc +++ b/src/envoy/mixer/grpc_transport.cc @@ -106,11 +106,12 @@ void GrpcTransport::Cancel() { template typename GrpcTransport::Func -GrpcTransport::GetFunc(Upstream::ClusterManager& cm, - const std::string& cluster_name, - const HeaderMap* headers) { - return [&cm, cluster_name, headers](const RequestType& request, ResponseType* response, - istio::mixer_client::DoneFunc on_done) +GrpcTransport::GetFunc( + Upstream::ClusterManager& cm, const std::string& cluster_name, + const HeaderMap* headers) { + return [&cm, cluster_name, headers](const RequestType& request, + ResponseType* response, + istio::mixer_client::DoneFunc on_done) -> istio::mixer_client::CancelFunc { auto transport = new GrpcTransport( Grpc::AsyncClientPtr( @@ -140,9 +141,11 @@ const google::protobuf::MethodDescriptor& ReportTransport::descriptor() { // explicitly instantiate CheckTransport and ReportTransport template CheckTransport::Func CheckTransport::GetFunc( - Upstream::ClusterManager& cm, const std::string& cluster_name, const HeaderMap* headers); + Upstream::ClusterManager& cm, const std::string& cluster_name, + const HeaderMap* headers); template ReportTransport::Func ReportTransport::GetFunc( - Upstream::ClusterManager& cm, const std::string& cluster_name, const HeaderMap* headers); + Upstream::ClusterManager& cm, const std::string& cluster_name, + const HeaderMap* headers); } // namespace Mixer } // namespace Http diff --git a/src/envoy/mixer/mixer_control.cc b/src/envoy/mixer/mixer_control.cc index 4951c3f1f0f..7b071192e94 100644 --- a/src/envoy/mixer/mixer_control.cc +++ b/src/envoy/mixer/mixer_control.cc @@ -74,8 +74,7 @@ HttpMixerControl::HttpMixerControl(const HttpMixerConfig& mixer_config, CreateEnvironment(cm, dispatcher, random, mixer_config.transport().check_cluster(), - mixer_config.transport().report_cluster(), - &options.env); + mixer_config.transport().report_cluster(), &options.env); controller_ = ::istio::mixer_control::http::Controller::Create(options); @@ -92,8 +91,7 @@ TcpMixerControl::TcpMixerControl(const TcpMixerConfig& mixer_config, CreateEnvironment(cm, dispatcher, random, mixer_config.transport().check_cluster(), - mixer_config.transport().report_cluster(), - &options.env); + mixer_config.transport().report_cluster(), &options.env); controller_ = ::istio::mixer_control::tcp::Controller::Create(options); From 2b4e7fdbd13abaec241ceafdbafe5e174f5e8be8 Mon Sep 17 00:00:00 2001 From: Shriram Rajagopalan Date: Mon, 5 Feb 2018 22:43:23 -0500 Subject: [PATCH 4/7] fixes --- src/envoy/mixer/mixer_control.cc | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/envoy/mixer/mixer_control.cc b/src/envoy/mixer/mixer_control.cc index 7b071192e94..ab5b4306c20 100644 --- a/src/envoy/mixer/mixer_control.cc +++ b/src/envoy/mixer/mixer_control.cc @@ -73,8 +73,9 @@ HttpMixerControl::HttpMixerControl(const HttpMixerConfig& mixer_config, mixer_config.http_config, mixer_config.legacy_quotas); CreateEnvironment(cm, dispatcher, random, - mixer_config.transport().check_cluster(), - mixer_config.transport().report_cluster(), &options.env); + mixer_config.http_config.transport().check_cluster(), + mixer_config.http_config.transport().report_cluster(), + &options.env); controller_ = ::istio::mixer_control::http::Controller::Create(options); @@ -90,8 +91,9 @@ TcpMixerControl::TcpMixerControl(const TcpMixerConfig& mixer_config, mixer_config.tcp_config); CreateEnvironment(cm, dispatcher, random, - mixer_config.transport().check_cluster(), - mixer_config.transport().report_cluster(), &options.env); + mixer_config.tcp_config.transport().check_cluster(), + mixer_config.tcp_config.transport().report_cluster(), + &options.env); controller_ = ::istio::mixer_control::tcp::Controller::Create(options); From f4fb057bde8e6489e74f394d37aa125697b1dafa Mon Sep 17 00:00:00 2001 From: Shriram Rajagopalan Date: Tue, 6 Feb 2018 11:12:19 -0500 Subject: [PATCH 5/7] fixes --- src/envoy/mixer/config.cc | 18 +++++++----------- src/envoy/mixer/config.h | 8 ++++++++ src/envoy/mixer/http_filter.cc | 2 +- src/envoy/mixer/mixer_control.cc | 17 +++++++++-------- src/envoy/mixer/mixer_control.h | 25 +++++++++++++++++++++++++ 5 files changed, 50 insertions(+), 20 deletions(-) diff --git a/src/envoy/mixer/config.cc b/src/envoy/mixer/config.cc index 1b9b57b92fc..5fdf3b877fd 100644 --- a/src/envoy/mixer/config.cc +++ b/src/envoy/mixer/config.cc @@ -25,6 +25,9 @@ using ::istio::mixer_client::AttributesBuilder; using ::istio::mixer::v1::config::client::ServiceConfig; using ::istio::mixer::v1::config::client::TransportConfig; +#define PROTOBUF_GET_STRING_OR_DEFAULT(message, field_name, default_value) \ + ((message).has_##field_name() ? (message).field_name() : (default_value)) + namespace Envoy { namespace Http { namespace Mixer { @@ -53,9 +56,6 @@ const std::string kV2Config("v2"); // The name for the mixer server cluster. const std::string kDefaultMixerClusterName("mixer_server"); -// The Json object name for check_cluster and report_cluster -const std::string kCheckCluster("check_cluster"); -const std::string kReportCluster("report_cluster"); void ReadStringMap(const Json::Object& json, const std::string& name, Attributes* attributes) { @@ -126,10 +126,8 @@ void HttpMixerConfig::Load(const Json::Object& json) { legacy_quotas.clear(); } - transport_config->set_check_cluster( - json.getString(kCheckCluster, kDefaultMixerClusterName)); - transport_config->set_report_cluster( - json.getString(kReportCluster, kDefaultMixerClusterName)); + check_cluster = PROTOBUF_GET_STRING_OR_DEFAULT(transport_config, check_cluster, kDefaultMixerClusterName); + report_cluster = PROTOBUF_GET_STRING_OR_DEFAULT(transport_config, report_cluster, kDefaultMixerClusterName); } void HttpMixerConfig::CreateLegacyRouteConfig( @@ -155,10 +153,8 @@ void TcpMixerConfig::Load(const Json::Object& json) { json.getBoolean(kDisableTcpCheckCalls, false)); ReadV2Config(json, &tcp_config); - transport_config->set_check_cluster( - json.getString(kCheckCluster, kDefaultMixerClusterName)); - transport_config->set_report_cluster( - json.getString(kReportCluster, kDefaultMixerClusterName)); + check_cluster = PROTOBUF_GET_STRING_OR_DEFAULT(transport_config, check_cluster, kDefaultMixerClusterName); + report_cluster = PROTOBUF_GET_STRING_OR_DEFAULT(transport_config, report_cluster, kDefaultMixerClusterName); } } // namespace Mixer diff --git a/src/envoy/mixer/config.h b/src/envoy/mixer/config.h index 490ad2347f9..453c2c4bdf7 100644 --- a/src/envoy/mixer/config.h +++ b/src/envoy/mixer/config.h @@ -39,6 +39,10 @@ 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; + // report cluster + std::string report_cluster; // Create per route legacy config. static void CreateLegacyRouteConfig( @@ -54,6 +58,10 @@ struct TcpMixerConfig { // The Tcp client config. ::istio::mixer::v1::config::client::TcpClientConfig tcp_config; + // check cluster + std::string check_cluster; + // report cluster + std::string report_cluster; }; } // namespace Mixer diff --git a/src/envoy/mixer/http_filter.cc b/src/envoy/mixer/http_filter.cc index 700038e88f8..beb1860f276 100644 --- a/src/envoy/mixer/http_filter.cc +++ b/src/envoy/mixer/http_filter.cc @@ -438,7 +438,7 @@ class Instance : public Http::StreamDecoderFilter, HeaderUpdate header_update(&headers); cancel_check_ = handler_->Check( &check_data, &header_update, - CheckTransport::GetFunc(mixer_control_.cm(), &headers), + CheckTransport::GetFunc(mixer_control_.cm(), mixer_control_.check_cluster(), &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 ab5b4306c20..3ae7a19daa9 100644 --- a/src/envoy/mixer/mixer_control.cc +++ b/src/envoy/mixer/mixer_control.cc @@ -72,14 +72,14 @@ HttpMixerControl::HttpMixerControl(const HttpMixerConfig& mixer_config, ::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, - mixer_config.http_config.transport().check_cluster(), - mixer_config.http_config.transport().report_cluster(), - &options.env); + check_cluster_, report_cluster_, &options.env); controller_ = ::istio::mixer_control::http::Controller::Create(options); - - has_v2_config_ = mixer_config.has_v2_config; + has_v2_config_ = mixer_config.has_v2_config; } TcpMixerControl::TcpMixerControl(const TcpMixerConfig& mixer_config, @@ -90,10 +90,11 @@ TcpMixerControl::TcpMixerControl(const TcpMixerConfig& mixer_config, ::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, - mixer_config.tcp_config.transport().check_cluster(), - mixer_config.tcp_config.transport().report_cluster(), - &options.env); + check_cluster_, 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 d9e97b83f11..f813166d902 100644 --- a/src/envoy/mixer/mixer_control.h +++ b/src/envoy/mixer/mixer_control.h @@ -36,6 +36,14 @@ class HttpMixerControl final : public ThreadLocal::ThreadLocalObject { Upstream::ClusterManager& cm() { return cm_; } + std::string& check_cluster() const { + return check_cluster_; + } + + std::string& report_cluster() const { + return report_cluster_; + } + ::istio::mixer_control::http::Controller* controller() { return controller_.get(); } @@ -49,6 +57,10 @@ class HttpMixerControl final : public ThreadLocal::ThreadLocalObject { 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 { @@ -66,6 +78,14 @@ class TcpMixerControl final : public ThreadLocal::ThreadLocalObject { return report_interval_ms_; } + std::string& check_cluster() const { + return check_cluster_; + } + + std::string& report_cluster() const { + return report_cluster_; + } + Event::Dispatcher& dispatcher() { return dispatcher_; } private: @@ -76,6 +96,11 @@ 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 4ea060db5141490cda4244ed4210b0111f9a47b7 Mon Sep 17 00:00:00 2001 From: Shriram Rajagopalan Date: Tue, 6 Feb 2018 16:32:00 -0500 Subject: [PATCH 6/7] fix --- src/envoy/mixer/config.cc | 16 +++++++++------- src/envoy/mixer/mixer_control.h | 8 ++++---- 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/src/envoy/mixer/config.cc b/src/envoy/mixer/config.cc index 5fdf3b877fd..48fab80082c 100644 --- a/src/envoy/mixer/config.cc +++ b/src/envoy/mixer/config.cc @@ -25,9 +25,6 @@ using ::istio::mixer_client::AttributesBuilder; using ::istio::mixer::v1::config::client::ServiceConfig; using ::istio::mixer::v1::config::client::TransportConfig; -#define PROTOBUF_GET_STRING_OR_DEFAULT(message, field_name, default_value) \ - ((message).has_##field_name() ? (message).field_name() : (default_value)) - namespace Envoy { namespace Http { namespace Mixer { @@ -126,8 +123,10 @@ void HttpMixerConfig::Load(const Json::Object& json) { legacy_quotas.clear(); } - check_cluster = PROTOBUF_GET_STRING_OR_DEFAULT(transport_config, check_cluster, kDefaultMixerClusterName); - report_cluster = PROTOBUF_GET_STRING_OR_DEFAULT(transport_config, report_cluster, kDefaultMixerClusterName); + check_cluster = transport_config->check_cluster().empty() ? kDefaultMixerClusterName : + transport_config->check_cluster(); + report_cluster = transport_config->report_cluster().empty() ? kDefaultMixerClusterName : + transport_config->report_cluster(); } void HttpMixerConfig::CreateLegacyRouteConfig( @@ -153,8 +152,11 @@ void TcpMixerConfig::Load(const Json::Object& json) { json.getBoolean(kDisableTcpCheckCalls, false)); ReadV2Config(json, &tcp_config); - check_cluster = PROTOBUF_GET_STRING_OR_DEFAULT(transport_config, check_cluster, kDefaultMixerClusterName); - report_cluster = PROTOBUF_GET_STRING_OR_DEFAULT(transport_config, report_cluster, kDefaultMixerClusterName); + + check_cluster = transport_config->check_cluster().empty() ? kDefaultMixerClusterName : + transport_config->check_cluster(); + report_cluster = transport_config->report_cluster().empty() ? kDefaultMixerClusterName : + transport_config->report_cluster(); } } // namespace Mixer diff --git a/src/envoy/mixer/mixer_control.h b/src/envoy/mixer/mixer_control.h index f813166d902..f4057a817a1 100644 --- a/src/envoy/mixer/mixer_control.h +++ b/src/envoy/mixer/mixer_control.h @@ -36,11 +36,11 @@ class HttpMixerControl final : public ThreadLocal::ThreadLocalObject { Upstream::ClusterManager& cm() { return cm_; } - std::string& check_cluster() const { + const std::string& check_cluster() const { return check_cluster_; } - std::string& report_cluster() const { + const std::string& report_cluster() const { return report_cluster_; } @@ -78,11 +78,11 @@ class TcpMixerControl final : public ThreadLocal::ThreadLocalObject { return report_interval_ms_; } - std::string& check_cluster() const { + const std::string& check_cluster() const { return check_cluster_; } - std::string& report_cluster() const { + const std::string& report_cluster() const { return report_cluster_; } From d8bce6f2bbaa53c1fadf218073e69f77b184d14a Mon Sep 17 00:00:00 2001 From: Shriram Rajagopalan Date: Tue, 6 Feb 2018 16:32:13 -0500 Subject: [PATCH 7/7] format --- src/envoy/mixer/config.cc | 20 ++++++++++++-------- src/envoy/mixer/http_filter.cc | 3 ++- src/envoy/mixer/mixer_control.cc | 14 +++++++------- src/envoy/mixer/mixer_control.h | 16 ++++------------ 4 files changed, 25 insertions(+), 28 deletions(-) diff --git a/src/envoy/mixer/config.cc b/src/envoy/mixer/config.cc index 48fab80082c..2a4ae7f260d 100644 --- a/src/envoy/mixer/config.cc +++ b/src/envoy/mixer/config.cc @@ -123,10 +123,12 @@ 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(); + check_cluster = transport_config->check_cluster().empty() + ? kDefaultMixerClusterName + : transport_config->check_cluster(); + report_cluster = transport_config->report_cluster().empty() + ? kDefaultMixerClusterName + : transport_config->report_cluster(); } void HttpMixerConfig::CreateLegacyRouteConfig( @@ -153,10 +155,12 @@ void TcpMixerConfig::Load(const Json::Object& json) { 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(); + check_cluster = transport_config->check_cluster().empty() + ? kDefaultMixerClusterName + : transport_config->check_cluster(); + report_cluster = transport_config->report_cluster().empty() + ? kDefaultMixerClusterName + : transport_config->report_cluster(); } } // namespace Mixer diff --git a/src/envoy/mixer/http_filter.cc b/src/envoy/mixer/http_filter.cc index beb1860f276..b0fd86a28f4 100644 --- a/src/envoy/mixer/http_filter.cc +++ b/src/envoy/mixer/http_filter.cc @@ -438,7 +438,8 @@ class Instance : public Http::StreamDecoderFilter, HeaderUpdate header_update(&headers); cancel_check_ = handler_->Check( &check_data, &header_update, - CheckTransport::GetFunc(mixer_control_.cm(), mixer_control_.check_cluster(), &headers), + CheckTransport::GetFunc(mixer_control_.cm(), + mixer_control_.check_cluster(), &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 3ae7a19daa9..356f36c996f 100644 --- a/src/envoy/mixer/mixer_control.cc +++ b/src/envoy/mixer/mixer_control.cc @@ -72,14 +72,14 @@ HttpMixerControl::HttpMixerControl(const HttpMixerConfig& mixer_config, ::istio::mixer_control::http::Controller::Options options( mixer_config.http_config, mixer_config.legacy_quotas); - check_cluster_ = mixer_config.check_cluster; + 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, check_cluster_, report_cluster_, + &options.env); controller_ = ::istio::mixer_control::http::Controller::Create(options); - has_v2_config_ = mixer_config.has_v2_config; + has_v2_config_ = mixer_config.has_v2_config; } TcpMixerControl::TcpMixerControl(const TcpMixerConfig& mixer_config, @@ -90,11 +90,11 @@ TcpMixerControl::TcpMixerControl(const TcpMixerConfig& mixer_config, ::istio::mixer_control::tcp::Controller::Options options( mixer_config.tcp_config); - check_cluster_ = mixer_config.check_cluster; + 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, check_cluster_, 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 f4057a817a1..a72679f4ab1 100644 --- a/src/envoy/mixer/mixer_control.h +++ b/src/envoy/mixer/mixer_control.h @@ -36,13 +36,9 @@ class HttpMixerControl final : public ThreadLocal::ThreadLocalObject { Upstream::ClusterManager& cm() { return cm_; } - const std::string& check_cluster() const { - return check_cluster_; - } + const std::string& check_cluster() const { return check_cluster_; } - const std::string& report_cluster() const { - return report_cluster_; - } + const std::string& report_cluster() const { return report_cluster_; } ::istio::mixer_control::http::Controller* controller() { return controller_.get(); @@ -78,13 +74,9 @@ class TcpMixerControl final : public ThreadLocal::ThreadLocalObject { return report_interval_ms_; } - const std::string& check_cluster() const { - return check_cluster_; - } + const std::string& check_cluster() const { return check_cluster_; } - const std::string& report_cluster() const { - return report_cluster_; - } + const std::string& report_cluster() const { return report_cluster_; } Event::Dispatcher& dispatcher() { return dispatcher_; }