Skip to content

Clean up duplicated cluster names #1001

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.

Sign up for GitHub

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

Merged
istio-merge-robot merged 2 commits into from
Feb 7, 2018
Merged

Clean up duplicated cluster names #1001

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ad5920e
Clean up duplicated cluster names
qiwzhang Feb 6, 2018
be067e2
Fix a crash
qiwzhang Feb 7, 2018
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
29 changes: 13 additions & 16 deletions src/envoy/mixer/config.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -114,21 +123,15 @@ 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) {
// If v2 config is valid, clear v1 legacy_quotas.
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(
Expand All @@ -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
Expand Down
18 changes: 14 additions & 4 deletions src/envoy/mixer/config.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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
Expand Down
4 changes: 1 addition & 3 deletions src/envoy/mixer/http_filter.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
19 changes: 6 additions & 13 deletions src/envoy/mixer/mixer_control.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
*/

#include "src/envoy/mixer/mixer_control.h"
#include "src/envoy/mixer/grpc_transport.h"

namespace Envoy {
namespace Http {
Expand Down Expand Up @@ -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;
Expand All @@ -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);

Expand Down
26 changes: 9 additions & 17 deletions src/envoy/mixer/mixer_control.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 {
Expand All @@ -74,25 +73,18 @@ 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_;

// Time interval in milliseconds for sending periodical delta reports.
std::chrono::milliseconds report_interval_ms_;

Event::Dispatcher& dispatcher_;

// check cluster
std::string check_cluster_;
// report cluster
std::string report_cluster_;
};

} // namespace Mixer
Expand Down