v2 to v3 fragment changes for parseClusterFromV2Yaml (envoyproxy#11894)

v2 to v3 fragment changes for parseClusterFromV2Yaml Risk Level: low Testing: unit, format and integration test Docs Changes: NA Release Notes: Part of envoyproxy#10843 Signed-off-by: Abhay Narayan Katare <[email protected]> Signed-off-by: scheler <[email protected]>
scheler · Aug 4, 2020 · ab82fc2 · ab82fc2
1 parent ac2239f
commit ab82fc2
Show file tree

Hide file tree

Showing 12 changed files with 449 additions and 193 deletions.
diff --git a/test/common/upstream/cluster_factory_impl_test.cc b/test/common/upstream/cluster_factory_impl_test.cc
@@ -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:
@@ -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_,
@@ -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:
@@ -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_,
@@ -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:
@@ -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_,
@@ -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:
@@ -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_,
@@ -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:
@@ -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_,

diff --git a/test/common/upstream/cluster_manager_impl_test.cc b/test/common/upstream/cluster_manager_impl_test.cc
@@ -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(
@@ -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());

diff --git a/test/common/upstream/eds_speed_test.cc b/test/common/upstream/eds_speed_test.cc
@@ -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()));

diff --git a/test/common/upstream/eds_test.cc b/test/common/upstream/eds_test.cc
@@ -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:
@@ -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()));
@@ -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());
@@ -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:
@@ -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);
   }
 

diff --git a/test/common/upstream/logical_dns_cluster_test.cc b/test/common/upstream/logical_dns_cluster_test.cc
@@ -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()));
@@ -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());
@@ -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());
@@ -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",
@@ -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));
@@ -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());
@@ -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
@@ -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(
@@ -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(
@@ -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");
 }
 
@@ -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(