Listener: respect the connection balancer of the redirected listener (e…

…nvoyproxy#15842)

If listener1 redirects the connection to listener2, the balancer field in listener2 decides whether to rebalance.
Previously we rely on the rebalancing at listener1, however, the rebalance is weak because listener1 is likely to
not own any connection and the rebalance is no-op.

Risk Level: MID. Rebalance may introduce latency. User needs to clear rebalancer field of listener2 to recover the original behavior.

Fix envoyproxy#15146 envoyproxy#16113

Signed-off-by: Yuchen Dai <silentdai@gmail.com>
Signed-off-by: Gokul Nair <gnair@twitter.com>

docs/root/version_history/current.rst

@@ -22,6 +22,9 @@ Minor Behavior Changes
   defined within the listener where the sockets are redirected to. Clear that field to restore the previous behavior.
 
 
+* listener: respect the :ref:`connection balance config <envoy_v3_api_field_config.listener.v3.Listener.connection_balance_config>`
+  defined within the listener where the sockets are redirected to. Clear that field to restore the previous behavior.
+
 
 Bug Fixes
 ---------

test/integration/BUILD

@@ -1550,7 +1550,6 @@ envoy_cc_test(
         "//source/extensions/filters/network/tcp_proxy:config",
         "//test/common/grpc:grpc_client_integration_lib",
         "//test/integration/filters:address_restore_listener_filter_lib",
-        "//test/test_common:network_utility_lib",
         "//test/test_common:resources_lib",
         "//test/test_common:utility_lib",
         "@envoy_api//envoy/config/bootstrap/v3:pkg_cc_proto",

test/integration/listener_lds_integration_test.cc

@@ -412,72 +412,6 @@ TEST_P(ListenerIntegrationTest, MultipleLdsUpdatesSharingListenSocketFactory) {
   }
 }
 
-TEST_P(ListenerIntegrationTest, ChangeListenerAddress) {
-  on_server_init_function_ = [&]() {
-    createLdsStream();
-    sendLdsResponse({MessageUtil::getYamlStringFromMessage(listener_config_)}, "1");
-    createRdsStream(route_table_name_);
-  };
-  initialize();
-  test_server_->waitForCounterGe("listener_manager.lds.update_success", 1);
-  // testing-listener-0 is not initialized as we haven't pushed any RDS yet.
-  EXPECT_EQ(test_server_->server().initManager().state(), Init::Manager::State::Initializing);
-  // Workers not started, the LDS added listener 0 is in active_listeners_ list.
-  EXPECT_EQ(test_server_->server().listenerManager().listeners().size(), 1);
-  registerTestServerPorts({listener_name_});
-
-  const std::string route_config_tmpl = R"EOF(
-      name: {}
-      virtual_hosts:
-      - name: integration
-        domains: ["*"]
-        routes:
-        - match: {{ prefix: "/" }}
-          route: {{ cluster: {} }}
-)EOF";
-  sendRdsResponse(fmt::format(route_config_tmpl, route_table_name_, "cluster_0"), "1");
-  test_server_->waitForCounterGe(
-      fmt::format("http.config_test.rds.{}.update_success", route_table_name_), 1);
-  // Now testing-listener-0 finishes initialization, Server initManager will be ready.
-  EXPECT_EQ(test_server_->server().initManager().state(), Init::Manager::State::Initialized);
-
-  test_server_->waitUntilListenersReady();
-  // NOTE: The line above doesn't tell you if listener is up and listening.
-  test_server_->waitForCounterGe("listener_manager.listener_create_success", 1);
-  const uint32_t old_port = lookupPort(listener_name_);
-  // Make a connection to the listener from version 1.
-  codec_client_ = makeHttpConnection(old_port);
-
-  // Change the listener address from loopback to wildcard.
-  const std::string new_address = Network::Test::getAnyAddressString(ipVersion());
-  listener_config_.mutable_address()->mutable_socket_address()->set_address(new_address);
-  sendLdsResponse({MessageUtil::getYamlStringFromMessage(listener_config_)}, "2");
-  sendRdsResponse(fmt::format(route_config_tmpl, route_table_name_, "cluster_0"), "2");
-
-  test_server_->waitForCounterGe("listener_manager.listener_create_success", 2);
-  registerTestServerPorts({listener_name_});
-  // Verify that the listener was updated and that the next connection will be to the new listener.
-  // (Note that connecting to 127.0.0.1 works whether the listener address is 127.0.0.1 or 0.0.0.0.)
-  const uint32_t new_port = lookupPort(listener_name_);
-  EXPECT_NE(old_port, new_port);
-
-  // Wait for the client to be disconnected.
-  ASSERT_TRUE(codec_client_->waitForDisconnect());
-  // Make a new connection to the new listener.
-  codec_client_ = makeHttpConnection(new_port);
-  const uint32_t response_size = 800;
-  const uint32_t request_size = 10;
-  Http::TestResponseHeaderMapImpl response_headers{{":status", "200"},
-                                                   {"server_id", "cluster_0, backend_0"}};
-  auto response = sendRequestAndWaitForResponse(
-      Http::TestResponseHeaderMapImpl{
-          {":method", "GET"}, {":path", "/"}, {":authority", "host"}, {":scheme", "http"}},
-      request_size, response_headers, response_size, /*cluster_0*/ 0);
-  verifyResponse(std::move(response), "200", response_headers, std::string(response_size, 'a'));
-  EXPECT_TRUE(upstream_request_->complete());
-  EXPECT_EQ(request_size, upstream_request_->bodyLength());
-}
-
 class RebalancerTest : public testing::TestWithParam<Network::Address::IpVersion>,
                        public BaseIntegrationTest {
 public: