Skip to content

Commit

Permalink
v2 to v3 fragment changes for parseClusterFromV2Yaml (#11894)
Browse files Browse the repository at this point in the history
v2 to v3 fragment changes for parseClusterFromV2Yaml

Risk Level: low
Testing: unit, format and integration test
Docs Changes: NA
Release Notes:
Part of #10843

Signed-off-by: Abhay Narayan Katare <[email protected]>
  • Loading branch information
ankatare authored Jul 9, 2020
1 parent e5366cc commit d67246d
Show file tree
Hide file tree
Showing 12 changed files with 449 additions and 193 deletions.
15 changes: 5 additions & 10 deletions test/common/upstream/cluster_factory_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,6 @@ TEST_F(TestStaticClusterImplTest, CreateWithoutConfig) {
connect_timeout: 0.25s
lb_policy: ROUND_ROBIN
load_assignment:
cluster_name: staticcluster
endpoints:
- lb_endpoints:
- endpoint:
Expand All @@ -96,7 +95,7 @@ TEST_F(TestStaticClusterImplTest, CreateWithoutConfig) {
TestStaticClusterFactory factory;
Registry::InjectFactory<ClusterFactory> registered_factory(factory);

const envoy::config::cluster::v3::Cluster cluster_config = parseClusterFromV2Yaml(yaml);
const envoy::config::cluster::v3::Cluster cluster_config = parseClusterFromV3Yaml(yaml);
auto create_result = ClusterFactoryImplBase::create(
cluster_config, cm_, stats_, tls_, dns_resolver_, ssl_context_manager_, runtime_, random_,
dispatcher_, log_manager_, local_info_, admin_, singleton_manager_,
Expand Down Expand Up @@ -124,7 +123,6 @@ TEST_F(TestStaticClusterImplTest, CreateWithStructConfig) {
connect_timeout: 0.25s
lb_policy: ROUND_ROBIN
load_assignment:
cluster_name: staticcluster
endpoints:
- lb_endpoints:
- endpoint:
Expand All @@ -142,7 +140,7 @@ TEST_F(TestStaticClusterImplTest, CreateWithStructConfig) {
port_value: 80
)EOF";

const envoy::config::cluster::v3::Cluster cluster_config = parseClusterFromV2Yaml(yaml);
const envoy::config::cluster::v3::Cluster cluster_config = parseClusterFromV3Yaml(yaml);
auto create_result = ClusterFactoryImplBase::create(
cluster_config, cm_, stats_, tls_, dns_resolver_, ssl_context_manager_, runtime_, random_,
dispatcher_, log_manager_, local_info_, admin_, singleton_manager_,
Expand All @@ -169,7 +167,6 @@ TEST_F(TestStaticClusterImplTest, CreateWithTypedConfig) {
connect_timeout: 0.25s
lb_policy: ROUND_ROBIN
load_assignment:
cluster_name: staticcluster
endpoints:
- lb_endpoints:
- endpoint:
Expand All @@ -186,7 +183,7 @@ TEST_F(TestStaticClusterImplTest, CreateWithTypedConfig) {
port_value: 80
)EOF";

const envoy::config::cluster::v3::Cluster cluster_config = parseClusterFromV2Yaml(yaml);
const envoy::config::cluster::v3::Cluster cluster_config = parseClusterFromV3Yaml(yaml);
auto create_result = ClusterFactoryImplBase::create(
cluster_config, cm_, stats_, tls_, dns_resolver_, ssl_context_manager_, runtime_, random_,
dispatcher_, log_manager_, local_info_, admin_, singleton_manager_,
Expand All @@ -213,7 +210,6 @@ TEST_F(TestStaticClusterImplTest, UnsupportedClusterType) {
connect_timeout: 0.25s
lb_policy: ROUND_ROBIN
load_assignment:
cluster_name: staticcluster
endpoints:
- lb_endpoints:
- endpoint:
Expand All @@ -230,7 +226,7 @@ TEST_F(TestStaticClusterImplTest, UnsupportedClusterType) {
// the factory is not registered, expect to throw
EXPECT_THROW_WITH_MESSAGE(
{
const envoy::config::cluster::v3::Cluster cluster_config = parseClusterFromV2Yaml(yaml);
const envoy::config::cluster::v3::Cluster cluster_config = parseClusterFromV3Yaml(yaml);
ClusterFactoryImplBase::create(
cluster_config, cm_, stats_, tls_, dns_resolver_, ssl_context_manager_, runtime_,
random_, dispatcher_, log_manager_, local_info_, admin_, singleton_manager_,
Expand All @@ -250,7 +246,6 @@ TEST_F(TestStaticClusterImplTest, HostnameWithoutDNS) {
consistent_hashing_lb_config:
use_hostname_for_hashing: true
load_assignment:
cluster_name: staticcluster
endpoints:
- lb_endpoints:
- endpoint:
Expand All @@ -264,7 +259,7 @@ TEST_F(TestStaticClusterImplTest, HostnameWithoutDNS) {

EXPECT_THROW_WITH_MESSAGE(
{
const envoy::config::cluster::v3::Cluster cluster_config = parseClusterFromV2Yaml(yaml);
const envoy::config::cluster::v3::Cluster cluster_config = parseClusterFromV3Yaml(yaml);
ClusterFactoryImplBase::create(
cluster_config, cm_, stats_, tls_, dns_resolver_, ssl_context_manager_, runtime_,
random_, dispatcher_, log_manager_, local_info_, admin_, singleton_manager_,
Expand Down
4 changes: 2 additions & 2 deletions test/common/upstream/cluster_manager_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2665,7 +2665,7 @@ TEST_F(ClusterManagerImplTest, MergedUpdatesDestroyedOnUpdate) {
common_lb_config:
update_merge_window: 3s
)EOF";
EXPECT_TRUE(cluster_manager_->addOrUpdateCluster(parseClusterFromV2Yaml(yaml), "version1"));
EXPECT_TRUE(cluster_manager_->addOrUpdateCluster(parseClusterFromV3Yaml(yaml), "version1"));

Cluster& cluster = cluster_manager_->activeClusters().find("new_cluster")->second;
HostVectorSharedPtr hosts(
Expand Down Expand Up @@ -2734,7 +2734,7 @@ TEST_F(ClusterManagerImplTest, MergedUpdatesDestroyedOnUpdate) {
.gauge("cluster_manager.warming_clusters", Stats::Gauge::ImportMode::NeverImport)
.value());
EXPECT_TRUE(
cluster_manager_->addOrUpdateCluster(parseClusterFromV2Yaml(yaml_updated), "version2"));
cluster_manager_->addOrUpdateCluster(parseClusterFromV3Yaml(yaml_updated), "version2"));
EXPECT_EQ(2, factory_.stats_
.gauge("cluster_manager.active_clusters", Stats::Gauge::ImportMode::NeverImport)
.value());
Expand Down
2 changes: 1 addition & 1 deletion test/common/upstream/eds_speed_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ class EdsSpeedTest {

void resetCluster(const std::string& yaml_config, Cluster::InitializePhase initialize_phase) {
local_info_.node_.mutable_locality()->set_zone("us-east-1a");
eds_cluster_ = parseClusterFromV2Yaml(yaml_config);
eds_cluster_ = parseClusterFromV3Yaml(yaml_config);
Envoy::Stats::ScopePtr scope = stats_.createScope(fmt::format(
"cluster.{}.",
eds_cluster_.alt_stat_name().empty() ? eds_cluster_.name() : eds_cluster_.alt_stat_name()));
Expand Down
14 changes: 7 additions & 7 deletions test/common/upstream/eds_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ class EdsTest : public testing::Test {
connect_timeout: 0.25s
type: EDS
lb_policy: ROUND_ROBIN
drain_connections_on_host_removal: true
ignore_health_on_host_removal: true
eds_cluster_config:
service_name: fare
eds_config:
Expand Down Expand Up @@ -89,7 +89,7 @@ class EdsTest : public testing::Test {

void resetCluster(const std::string& yaml_config, Cluster::InitializePhase initialize_phase) {
local_info_.node_.mutable_locality()->set_zone("us-east-1a");
eds_cluster_ = parseClusterFromV2Yaml(yaml_config);
eds_cluster_ = parseClusterFromV3Yaml(yaml_config);
Envoy::Stats::ScopePtr scope = stats_.createScope(fmt::format(
"cluster.{}.",
eds_cluster_.alt_stat_name().empty() ? eds_cluster_.name() : eds_cluster_.alt_stat_name()));
Expand Down Expand Up @@ -138,8 +138,8 @@ class EdsWithHealthCheckUpdateTest : public EdsTest {

// Build the initial cluster with some endpoints.
void initializeCluster(const std::vector<uint32_t> endpoint_ports,
const bool drain_connections_on_host_removal) {
resetCluster(drain_connections_on_host_removal);
const bool ignore_health_on_host_removal) {
resetCluster(ignore_health_on_host_removal);

auto health_checker = std::make_shared<MockHealthChecker>();
EXPECT_CALL(*health_checker, start());
Expand Down Expand Up @@ -173,13 +173,13 @@ class EdsWithHealthCheckUpdateTest : public EdsTest {
}
}

void resetCluster(const bool drain_connections_on_host_removal) {
void resetCluster(const bool ignore_health_on_host_removal) {
const std::string config = R"EOF(
name: name
connect_timeout: 0.25s
type: EDS
lb_policy: ROUND_ROBIN
drain_connections_on_host_removal: {}
ignore_health_on_host_removal: {}
eds_cluster_config:
service_name: fare
eds_config:
Expand All @@ -189,7 +189,7 @@ class EdsWithHealthCheckUpdateTest : public EdsTest {
- eds
refresh_delay: 1s
)EOF";
EdsTest::resetCluster(fmt::format(config, drain_connections_on_host_removal),
EdsTest::resetCluster(fmt::format(config, ignore_health_on_host_removal),
Cluster::InitializePhase::Secondary);
}

Expand Down
93 changes: 59 additions & 34 deletions test/common/upstream/logical_dns_cluster_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,11 @@ class LogicalDnsClusterTest : public testing::Test {
protected:
LogicalDnsClusterTest() : api_(Api::createApiForTest(stats_store_)) {}

void setupFromV2Yaml(const std::string& yaml) {
void setupFromV3Yaml(const std::string& yaml, bool avoid_boosting = true) {
resolve_timer_ = new Event::MockTimer(&dispatcher_);
NiceMock<MockClusterManager> cm;
envoy::config::cluster::v3::Cluster cluster_config = parseClusterFromV2Yaml(yaml);
envoy::config::cluster::v3::Cluster cluster_config =
parseClusterFromV3Yaml(yaml, avoid_boosting);
Envoy::Stats::ScopePtr scope = stats_store_.createScope(fmt::format(
"cluster.{}.", cluster_config.alt_stat_name().empty() ? cluster_config.name()
: cluster_config.alt_stat_name()));
Expand Down Expand Up @@ -75,7 +76,7 @@ class LogicalDnsClusterTest : public testing::Test {
void testBasicSetup(const std::string& config, const std::string& expected_address,
uint32_t expected_port, uint32_t expected_hc_port) {
expectResolve(Network::DnsLookupFamily::V4Only, expected_address);
setupFromV2Yaml(config);
setupFromV3Yaml(config);

EXPECT_CALL(membership_updated_, ready());
EXPECT_CALL(initialized_, ready());
Expand Down Expand Up @@ -264,10 +265,14 @@ TEST_P(LogicalDnsParamTest, ImmediateResolve) {
lb_policy: round_robin
)EOF" + std::get<0>(GetParam()) +
R"EOF(
hosts:
- socket_address:
address: foo.bar.com
port_value: 443
load_assignment:
endpoints:
- lb_endpoints:
- endpoint:
address:
socket_address:
address: foo.bar.com
port_value: 443
)EOF";

EXPECT_CALL(membership_updated_, ready());
Expand All @@ -280,7 +285,7 @@ TEST_P(LogicalDnsParamTest, ImmediateResolve) {
TestUtility::makeDnsResponse(std::get<2>(GetParam())));
return nullptr;
}));
setupFromV2Yaml(yaml);
setupFromV3Yaml(yaml);
EXPECT_EQ(1UL, cluster_->prioritySet().hostSetsPerPriority()[0]->hosts().size());
EXPECT_EQ(1UL, cluster_->prioritySet().hostSetsPerPriority()[0]->healthyHosts().size());
EXPECT_EQ("foo.bar.com",
Expand All @@ -302,14 +307,18 @@ TEST_F(LogicalDnsParamTest, FailureRefreshRateBackoffResetsWhenSuccessHappens) {
# Since the following expectResolve() requires Network::DnsLookupFamily::V4Only we need to set
# dns_lookup_family to V4_ONLY explicitly for v2 .yaml config.
dns_lookup_family: V4_ONLY
hosts:
- socket_address:
address: foo.bar.com
port_value: 443
load_assignment:
endpoints:
- lb_endpoints:
- endpoint:
address:
socket_address:
address: foo.bar.com
port_value: 443
)EOF";

expectResolve(Network::DnsLookupFamily::V4Only, "foo.bar.com");
setupFromV2Yaml(yaml);
setupFromV3Yaml(yaml);

// Failing response kicks the failure refresh backoff strategy.
ON_CALL(random_, random()).WillByDefault(Return(8000));
Expand Down Expand Up @@ -342,14 +351,18 @@ TEST_F(LogicalDnsParamTest, TtlAsDnsRefreshRate) {
# Since the following expectResolve() requires Network::DnsLookupFamily::V4Only we need to set
# dns_lookup_family to V4_ONLY explicitly for v2 .yaml config.
dns_lookup_family: V4_ONLY
hosts:
- socket_address:
address: foo.bar.com
port_value: 443
load_assignment:
endpoints:
- lb_endpoints:
- endpoint:
address:
socket_address:
address: foo.bar.com
port_value: 443
)EOF";

expectResolve(Network::DnsLookupFamily::V4Only, "foo.bar.com");
setupFromV2Yaml(yaml);
setupFromV3Yaml(yaml);

// TTL is recorded when the DNS response is successful and not empty
EXPECT_CALL(membership_updated_, ready());
Expand Down Expand Up @@ -378,17 +391,25 @@ TEST_F(LogicalDnsClusterTest, BadConfig) {
dns_refresh_rate: 4s
connect_timeout: 0.25s
lb_policy: ROUND_ROBIN
hosts:
- socket_address:
address: foo.bar.com
port_value: 443
- socket_address:
address: foo2.bar.com
port_value: 443
load_assignment:
cluster_name: name
endpoints:
- lb_endpoints:
- endpoint:
address:
socket_address:
address: foo.bar.com
port_value: 443
- endpoint:
address:
socket_address:
address: foo2.bar.com
port_value: 443
)EOF";

EXPECT_THROW_WITH_MESSAGE(setupFromV2Yaml(multiple_hosts_yaml), EnvoyException,
"LOGICAL_DNS clusters must have a single host");
EXPECT_THROW_WITH_MESSAGE(
setupFromV3Yaml(multiple_hosts_yaml), EnvoyException,
"LOGICAL_DNS clusters must have a single locality_lb_endpoint and a single lb_endpoint");

const std::string multiple_lb_endpoints_yaml = R"EOF(
name: name
Expand Down Expand Up @@ -418,7 +439,7 @@ TEST_F(LogicalDnsClusterTest, BadConfig) {
)EOF";

EXPECT_THROW_WITH_MESSAGE(
setupFromV2Yaml(multiple_lb_endpoints_yaml), EnvoyException,
setupFromV3Yaml(multiple_lb_endpoints_yaml), EnvoyException,
"LOGICAL_DNS clusters must have a single locality_lb_endpoint and a single lb_endpoint");

const std::string multiple_endpoints_yaml = R"EOF(
Expand Down Expand Up @@ -451,7 +472,7 @@ TEST_F(LogicalDnsClusterTest, BadConfig) {
)EOF";

EXPECT_THROW_WITH_MESSAGE(
setupFromV2Yaml(multiple_endpoints_yaml), EnvoyException,
setupFromV3Yaml(multiple_endpoints_yaml), EnvoyException,
"LOGICAL_DNS clusters must have a single locality_lb_endpoint and a single lb_endpoint");

const std::string custom_resolver_yaml = R"EOF(
Expand All @@ -475,7 +496,7 @@ TEST_F(LogicalDnsClusterTest, BadConfig) {
port_value: 8000
)EOF";

EXPECT_THROW_WITH_MESSAGE(setupFromV2Yaml(custom_resolver_yaml), EnvoyException,
EXPECT_THROW_WITH_MESSAGE(setupFromV3Yaml(custom_resolver_yaml), EnvoyException,
"LOGICAL_DNS clusters must NOT have a custom resolver name set");
}

Expand All @@ -492,10 +513,14 @@ TEST_F(LogicalDnsClusterTest, Basic) {
# Since the following expectResolve() requires Network::DnsLookupFamily::V4Only we need to set
# dns_lookup_family to V4_ONLY explicitly for v2 .yaml config.
dns_lookup_family: V4_ONLY
hosts:
- socket_address:
address: foo.bar.com
port_value: 443
load_assignment:
endpoints:
- lb_endpoints:
- endpoint:
address:
socket_address:
address: foo.bar.com
port_value: 443
)EOF";

const std::string basic_yaml_load_assignment = R"EOF(
Expand Down
Loading

0 comments on commit d67246d

Please sign in to comment.