runtime: remove runtime guard listener_in_place_filterchain_update (envoyproxy#14921)

Fixes: envoyproxy#14647

Signed-off-by: Yuchen Dai <[email protected]>

Author: lambdai

docs/root/version_history/current.rst

@@ -65,6 +65,7 @@ Removed Config or Runtime
 * http: removed legacy connection close behavior and runtime guard `envoy.reloadable_features.fixed_connection_close`.
 * http: removed legacy HTTP/1.1 error reporting path and runtime guard `envoy.reloadable_features.early_errors_via_hcm`.
 * http: removed legacy sanitization path for upgrade response headers and runtime guard `envoy.reloadable_features.fix_upgrade_response`.
+* listener: removed legacy runtime guard `envoy.reloadable_features.listener_in_place_filterchain_update`.
 * router: removed `envoy.reloadable_features.consume_all_retry_headers` and legacy code path.
 
 New Features

source/common/runtime/runtime_features.cc

@@ -74,7 +74,6 @@ constexpr const char* runtime_features[] = {
     "envoy.reloadable_features.http_transport_failure_reason_in_body",
     "envoy.reloadable_features.http_upstream_wait_connect_response",
     "envoy.reloadable_features.http2_skip_encoding_empty_trailers",
-    "envoy.reloadable_features.listener_in_place_filterchain_update",
     "envoy.reloadable_features.overload_manager_disable_keepalive_drain_http2",
     "envoy.reloadable_features.prefer_quic_kernel_bpf_packet_routing",
     "envoy.reloadable_features.preserve_query_string_in_path_redirects",

source/server/listener_impl.cc

@@ -713,11 +713,6 @@ void ListenerImpl::setSocketFactory(const Network::ListenSocketFactorySharedPtr&
 
 bool ListenerImpl::supportUpdateFilterChain(const envoy::config::listener::v3::Listener& config,
                                             bool worker_started) {
-  if (!Runtime::runtimeFeatureEnabled(
-          "envoy.reloadable_features.listener_in_place_filterchain_update")) {
-    return false;
-  }
-
   // The in place update needs the active listener in worker thread. worker_started guarantees the
   // existence of that active listener.
   if (!worker_started) {

test/server/listener_manager_impl_test.cc

@@ -4845,107 +4845,6 @@ TEST_F(ListenerManagerImplForInPlaceFilterChainUpdateTest,
   EXPECT_EQ(0, server_.stats_store_.counter("listener_manager.listener_in_place_updated").value());
 }
 
-// This test execute an in place update first, then a traditional listener update.
-// The second update is enforced by runtime.
-TEST_F(ListenerManagerImplTest, RuntimeDisabledInPlaceUpdateFallbacksToTraditionalUpdate) {
-  InSequence s;
-  EXPECT_CALL(*worker_, start(_));
-  manager_->startWorkers(guard_dog_, callback_.AsStdFunction());
-
-  // Add foo listener.
-  const std::string listener_foo_yaml = R"EOF(
-name: foo
-address:
-  socket_address:
-    address: 127.0.0.1
-    port_value: 1234
-filter_chains:
-- filters: []
-  )EOF";
-
-  ListenerHandle* listener_foo = expectListenerCreate(false, true);
-  EXPECT_CALL(listener_factory_, createListenSocket(_, _, _, {true}));
-
-  EXPECT_CALL(*worker_, addListener(_, _, _));
-
-  EXPECT_TRUE(manager_->addOrUpdateListener(parseListenerFromV3Yaml(listener_foo_yaml), "", true));
-  EXPECT_EQ(0, server_.stats_store_.counter("listener_manager.listener_in_place_updated").value());
-
-  worker_->callAddCompletion(true);
-  EXPECT_EQ(1UL, manager_->listeners().size());
-  checkStats(__LINE__, 1, 0, 0, 0, 1, 0, 0);
-
-  // Add foo listener again. Will execute in place filter chain update path.
-  const std::string listener_foo_update1_yaml = R"EOF(
-  name: foo
-  address:
-    socket_address:
-      address: 127.0.0.1
-      port_value: 1234
-  filter_chains:
-  - filters: []
-    filter_chain_match:
-      destination_port: 1234
-    )EOF";
-
-  ListenerHandle* listener_foo_update1 = expectListenerOverridden(false, listener_foo);
-  EXPECT_CALL(*worker_, addListener(_, _, _));
-  auto* timer = new Event::MockTimer(dynamic_cast<Event::MockDispatcher*>(&server_.dispatcher()));
-  EXPECT_CALL(*timer, enableTimer(_, _));
-  EXPECT_TRUE(
-      manager_->addOrUpdateListener(parseListenerFromV3Yaml(listener_foo_update1_yaml), "", true));
-  EXPECT_EQ(1, server_.stats_store_.counter("listener_manager.listener_in_place_updated").value());
-
-  EXPECT_EQ(1UL, manager_->listeners().size());
-  worker_->callAddCompletion(true);
-
-  EXPECT_CALL(*worker_, removeFilterChains(_, _, _));
-  timer->invokeCallback();
-  EXPECT_CALL(*listener_foo, onDestroy());
-  worker_->callDrainFilterChainsComplete();
-
-  // Update foo again. This time we disable in place filter chain update in runtime.
-  // The traditional full listener update path is used.
-  auto in_place_update_disabled_guard = disableInplaceUpdateForThisTest();
-  const std::string listener_foo_update2_yaml = R"EOF(
-  name: foo
-  address:
-    socket_address:
-      address: 127.0.0.1
-      port_value: 1234
-  filter_chains:
-  - filters:
-    filter_chain_match:
-      destination_port: 2345
-    )EOF";
-
-  ListenerHandle* listener_foo_update2 = expectListenerCreate(false, true);
-  EXPECT_CALL(*worker_, addListener(_, _, _));
-  EXPECT_CALL(*worker_, stopListener(_, _));
-  EXPECT_CALL(*listener_foo_update1->drain_manager_, startDrainSequence(_));
-  EXPECT_TRUE(
-      manager_->addOrUpdateListener(parseListenerFromV3Yaml(listener_foo_update2_yaml), "", true));
-  EXPECT_EQ(1, server_.stats_store_.counter("listener_manager.listener_in_place_updated").value());
-
-  EXPECT_CALL(*worker_, removeListener(_, _));
-  listener_foo_update1->drain_manager_->drain_sequence_completion_();
-
-  EXPECT_CALL(*listener_foo_update1, onDestroy());
-  worker_->callRemovalCompletion();
-
-  EXPECT_CALL(*worker_, stopListener(_, _));
-  EXPECT_CALL(*listener_factory_.socket_, close());
-  EXPECT_CALL(*listener_foo_update2->drain_manager_, startDrainSequence(_));
-  EXPECT_TRUE(manager_->removeListener("foo"));
-
-  EXPECT_CALL(*worker_, removeListener(_, _));
-  listener_foo_update2->drain_manager_->drain_sequence_completion_();
-
-  EXPECT_CALL(*listener_foo_update2, onDestroy());
-  worker_->callRemovalCompletion();
-  EXPECT_EQ(0UL, manager_->listeners().size());
-}
-
 // This test verifies that on default initialization the UDP Packet Writer
 // is initialized in passthrough mode. (i.e. by using UdpDefaultWriter).
 TEST_F(ListenerManagerImplTest, UdpDefaultWriterConfig) {

test/server/listener_manager_impl_test.h

@@ -271,14 +271,6 @@ class ListenerManagerImplTest : public testing::Test {
     EXPECT_EQ(expected_listeners_config_dump.DebugString(), listeners_config_dump.DebugString());
   }
 
-  ABSL_MUST_USE_RESULT
-  auto disableInplaceUpdateForThisTest() {
-    auto scoped_runtime = std::make_unique<TestScopedRuntime>();
-    Runtime::LoaderSingleton::getExisting()->mergeValues(
-        {{"envoy.reloadable_features.listener_in_place_filterchain_update", "false"}});
-    return scoped_runtime;
-  }
-
   ABSL_MUST_USE_RESULT
   auto enableTlsInspectorInjectionForThisTest() {
     auto scoped_runtime = std::make_unique<TestScopedRuntime>();