remove expired runtime flag: envoy.reloadable_features.validate_detailed_override_host_statuses (envoyproxy#30446)

code · web-flow · commit 3f73002574de · 2023-10-24T08:42:19.000-04:00
Commit Message: remove expired runtime flag: envoy.reloadable_features.validate_detailed_override_host_statuses Additional Description: Fixes envoyproxy#29103. Signed-off-by: wbpcode <wbphub@live.com>
diff --git a/changelogs/current.yaml b/changelogs/current.yaml
@@ -26,6 +26,11 @@ removed_config_or_runtime:
     Removed the deprecated ``envoy.reloadable_features.prohibit_route_refresh_after_response_headers_sent``
     runtime flag and legacy code path.
 
+- area: upstream
+  change: |
+    Removed the deprecated ``envoy.reloadable_features.validate_detailed_override_host_statuses``
+    runtime flag and legacy code path.
+
 new_features:
 
 deprecated:
diff --git a/source/common/runtime/runtime_features.cc b/source/common/runtime/runtime_features.cc
@@ -89,7 +89,6 @@ RUNTIME_GUARD(envoy_reloadable_features_upstream_wait_for_response_headers_befor
 RUNTIME_GUARD(envoy_reloadable_features_use_cluster_cache_for_alt_protocols_filter);
 RUNTIME_GUARD(envoy_reloadable_features_use_http3_header_normalisation);
 RUNTIME_GUARD(envoy_reloadable_features_validate_connect);
-RUNTIME_GUARD(envoy_reloadable_features_validate_detailed_override_host_statuses);
 RUNTIME_GUARD(envoy_reloadable_features_validate_grpc_header_before_log_grpc_status);
 RUNTIME_GUARD(envoy_reloadable_features_validate_upstream_headers);
 RUNTIME_GUARD(envoy_restart_features_send_goaway_for_premature_rst_streams);
diff --git a/source/common/upstream/cluster_manager_impl.h b/source/common/upstream/cluster_manager_impl.h
@@ -654,14 +654,6 @@ class ClusterManagerImpl : public ClusterManager,
       // * 0b010000: envoy::config::core::v3::HealthStatus::TIMEOUT
       // * 0b100000: envoy::config::core::v3::HealthStatus::DEGRADED
       //
-      // If runtime flag `envoy.reloadable_features.validate_detailed_override_host_statuses` is
-      // disabled, the old coarse health status Host::Health will be used. The specific
-      // correspondence is shown below:
-      //
-      // * 0b001: Host::Health::Unhealthy
-      // * 0b010: Host::Health::Degraded
-      // * 0b100: Host::Health::Healthy
-      //
       // If multiple bit fields are set, it is acceptable as long as the status of override host is
       // in any of these statuses.
       const HostUtility::HostStatusSet override_host_statuses_{};
diff --git a/source/common/upstream/host_utility.cc b/source/common/upstream/host_utility.cc
@@ -97,38 +97,6 @@ HostUtility::HostStatusSet HostUtility::createOverrideHostStatus(
     const envoy::config::cluster::v3::Cluster::CommonLbConfig& common_config) {
   HostStatusSet override_host_status;
 
-  if (!Runtime::runtimeFeatureEnabled(
-          "envoy.reloadable_features.validate_detailed_override_host_statuses")) {
-    // Old code path that should be removed once the runtime flag is removed directly.
-    // Coarse health status is used here.
-
-    if (!common_config.has_override_host_status()) {
-      // No override host status and 'Healthy' and 'Degraded' will be applied by default.
-      override_host_status.set(static_cast<size_t>(Host::Health::Healthy));
-      override_host_status.set(static_cast<size_t>(Host::Health::Degraded));
-      return override_host_status;
-    }
-
-    for (auto single_status : common_config.override_host_status().statuses()) {
-      switch (static_cast<envoy::config::core::v3::HealthStatus>(single_status)) {
-        PANIC_ON_PROTO_ENUM_SENTINEL_VALUES;
-      case envoy::config::core::v3::HealthStatus::UNKNOWN:
-      case envoy::config::core::v3::HealthStatus::HEALTHY:
-        override_host_status.set(static_cast<size_t>(Host::Health::Healthy));
-        break;
-      case envoy::config::core::v3::HealthStatus::UNHEALTHY:
-      case envoy::config::core::v3::HealthStatus::DRAINING:
-      case envoy::config::core::v3::HealthStatus::TIMEOUT:
-        override_host_status.set(static_cast<size_t>(Host::Health::Unhealthy));
-        break;
-      case envoy::config::core::v3::HealthStatus::DEGRADED:
-        override_host_status.set(static_cast<size_t>(Host::Health::Degraded));
-        break;
-      }
-    }
-    return override_host_status;
-  }
-
   if (!common_config.has_override_host_status()) {
     // No override host status and [UNKNOWN, HEALTHY, DEGRADED] will be applied by default.
     override_host_status.set(static_cast<uint32_t>(envoy::config::core::v3::HealthStatus::UNKNOWN));
@@ -179,17 +147,6 @@ HostConstSharedPtr HostUtility::selectOverrideHost(const HostMap* host_map, Host
   HostConstSharedPtr host = host_iter->second;
   ASSERT(host != nullptr);
 
-  if (!Runtime::runtimeFeatureEnabled(
-          "envoy.reloadable_features.validate_detailed_override_host_statuses")) {
-    // Old code path that should be removed once the runtime flag is removed directly.
-    // Coarse health status is used here.
-
-    if (status[static_cast<size_t>(host->coarseHealth())]) {
-      return host;
-    }
-    return nullptr;
-  }
-
   if (status[static_cast<uint32_t>(host->healthStatus())]) {
     return host;
   }
diff --git a/test/common/upstream/host_utility_test.cc b/test/common/upstream/host_utility_test.cc
@@ -225,154 +225,6 @@ TEST(HostUtilityTest, SelectOverrideHostTest) {
   }
 }
 
-TEST(HostUtilityTest, CreateOverrideHostStatusWithRuntimeFlagFlase) {
-  TestScopedRuntime scoped_runtime;
-  scoped_runtime.mergeValues(
-      {{"envoy.reloadable_features.validate_detailed_override_host_statuses", "false"}});
-
-  // Deprecated status that will be removed after runtime flag is removed.
-  static constexpr HostUtility::HostStatusSet UnhealthyStatus =
-      1u << static_cast<size_t>(Host::Health::Unhealthy);
-  static constexpr HostUtility::HostStatusSet DegradedStatus =
-      1u << static_cast<size_t>(Host::Health::Degraded);
-  static constexpr HostUtility::HostStatusSet HealthyStatus =
-      1u << static_cast<size_t>(Host::Health::Healthy);
-
-  {
-    envoy::config::cluster::v3::Cluster::CommonLbConfig lb_config;
-    lb_config.mutable_override_host_status()->add_statuses(
-        ::envoy::config::core::v3::HealthStatus::UNKNOWN);
-    lb_config.mutable_override_host_status()->add_statuses(
-        ::envoy::config::core::v3::HealthStatus::HEALTHY);
-    EXPECT_EQ(HostUtility::createOverrideHostStatus(lb_config), HealthyStatus);
-  }
-  {
-    envoy::config::cluster::v3::Cluster::CommonLbConfig lb_config;
-    lb_config.mutable_override_host_status()->add_statuses(
-        ::envoy::config::core::v3::HealthStatus::UNHEALTHY);
-    lb_config.mutable_override_host_status()->add_statuses(
-        ::envoy::config::core::v3::HealthStatus::DRAINING);
-    lb_config.mutable_override_host_status()->add_statuses(
-        ::envoy::config::core::v3::HealthStatus::TIMEOUT);
-
-    EXPECT_EQ(HostUtility::createOverrideHostStatus(lb_config), UnhealthyStatus);
-  }
-  {
-    envoy::config::cluster::v3::Cluster::CommonLbConfig lb_config;
-    lb_config.mutable_override_host_status()->add_statuses(
-        ::envoy::config::core::v3::HealthStatus::DEGRADED);
-    EXPECT_EQ(HostUtility::createOverrideHostStatus(lb_config), DegradedStatus);
-  }
-  {
-    envoy::config::cluster::v3::Cluster::CommonLbConfig lb_config;
-    EXPECT_EQ(HostUtility::createOverrideHostStatus(lb_config), 0b110u);
-  }
-  {
-    envoy::config::cluster::v3::Cluster::CommonLbConfig lb_config;
-    lb_config.mutable_override_host_status()->add_statuses(
-        ::envoy::config::core::v3::HealthStatus::UNHEALTHY);
-    lb_config.mutable_override_host_status()->add_statuses(
-        ::envoy::config::core::v3::HealthStatus::DRAINING);
-    lb_config.mutable_override_host_status()->add_statuses(
-        ::envoy::config::core::v3::HealthStatus::TIMEOUT);
-    lb_config.mutable_override_host_status()->add_statuses(
-        ::envoy::config::core::v3::HealthStatus::UNKNOWN);
-    lb_config.mutable_override_host_status()->add_statuses(
-        ::envoy::config::core::v3::HealthStatus::HEALTHY);
-
-    EXPECT_EQ(HostUtility::createOverrideHostStatus(lb_config), 0b101u);
-  }
-
-  {
-    envoy::config::cluster::v3::Cluster::CommonLbConfig lb_config;
-    lb_config.mutable_override_host_status()->add_statuses(
-        ::envoy::config::core::v3::HealthStatus::UNHEALTHY);
-    lb_config.mutable_override_host_status()->add_statuses(
-        ::envoy::config::core::v3::HealthStatus::DRAINING);
-    lb_config.mutable_override_host_status()->add_statuses(
-        ::envoy::config::core::v3::HealthStatus::TIMEOUT);
-    lb_config.mutable_override_host_status()->add_statuses(
-        ::envoy::config::core::v3::HealthStatus::UNKNOWN);
-    lb_config.mutable_override_host_status()->add_statuses(
-        ::envoy::config::core::v3::HealthStatus::HEALTHY);
-    lb_config.mutable_override_host_status()->add_statuses(
-        ::envoy::config::core::v3::HealthStatus::DEGRADED);
-    EXPECT_EQ(HostUtility::createOverrideHostStatus(lb_config), 0b111u);
-  }
-}
-
-TEST(HostUtilityTest, SelectOverrideHostTestRuntimeFlagFlase) {
-  TestScopedRuntime scoped_runtime;
-  scoped_runtime.mergeValues(
-      {{"envoy.reloadable_features.validate_detailed_override_host_statuses", "false"}});
-
-  // Deprecated status that will be removed after runtime flag is removed.
-  static constexpr HostUtility::HostStatusSet UnhealthyStatus =
-      1u << static_cast<size_t>(Host::Health::Unhealthy);
-  static constexpr HostUtility::HostStatusSet DegradedStatus =
-      1u << static_cast<size_t>(Host::Health::Degraded);
-  static constexpr HostUtility::HostStatusSet HealthyStatus =
-      1u << static_cast<size_t>(Host::Health::Healthy);
-
-  NiceMock<Upstream::MockLoadBalancerContext> context;
-
-  const HostUtility::HostStatusSet all_health_statuses =
-      UnhealthyStatus | DegradedStatus | HealthyStatus;
-
-  {
-    // No valid host map.
-    EXPECT_EQ(nullptr, HostUtility::selectOverrideHost(nullptr, all_health_statuses, &context));
-  }
-  {
-    // No valid load balancer context.
-    auto host_map = std::make_shared<HostMap>();
-    EXPECT_EQ(nullptr,
-              HostUtility::selectOverrideHost(host_map.get(), all_health_statuses, nullptr));
-  }
-  {
-    // No valid expected host.
-    EXPECT_CALL(context, overrideHostToSelect()).WillOnce(Return(absl::nullopt));
-    auto host_map = std::make_shared<HostMap>();
-    EXPECT_EQ(nullptr,
-              HostUtility::selectOverrideHost(host_map.get(), all_health_statuses, &context));
-  }
-  {
-    // The host map does not contain the expected host.
-    LoadBalancerContext::OverrideHost override_host{"1.2.3.4"};
-    EXPECT_CALL(context, overrideHostToSelect())
-        .WillOnce(Return(absl::make_optional(override_host)));
-    auto host_map = std::make_shared<HostMap>();
-    EXPECT_EQ(nullptr, HostUtility::selectOverrideHost(host_map.get(), HealthyStatus, &context));
-  }
-  {
-    // The status of host is not as expected.
-    auto mock_host = std::make_shared<NiceMock<MockHost>>();
-    EXPECT_CALL(*mock_host, coarseHealth()).WillOnce(Return(Host::Health::Unhealthy));
-
-    LoadBalancerContext::OverrideHost override_host{"1.2.3.4"};
-    EXPECT_CALL(context, overrideHostToSelect())
-        .WillOnce(Return(absl::make_optional(override_host)));
-
-    auto host_map = std::make_shared<HostMap>();
-    host_map->insert({"1.2.3.4", mock_host});
-    EXPECT_EQ(nullptr, HostUtility::selectOverrideHost(host_map.get(), HealthyStatus, &context));
-  }
-  {
-    // Get expected host.
-    auto mock_host = std::make_shared<NiceMock<MockHost>>();
-    EXPECT_CALL(*mock_host, coarseHealth()).WillOnce(Return(Host::Health::Degraded));
-
-    LoadBalancerContext::OverrideHost override_host{"1.2.3.4"};
-    EXPECT_CALL(context, overrideHostToSelect())
-        .WillOnce(Return(absl::make_optional(override_host)));
-
-    auto host_map = std::make_shared<HostMap>();
-    host_map->insert({"1.2.3.4", mock_host});
-    EXPECT_EQ(mock_host, HostUtility::selectOverrideHost(host_map.get(),
-                                                         HealthyStatus | DegradedStatus, &context));
-  }
-}
-
 } // namespace
 } // namespace Upstream
 } // namespace Envoy
