From e8c1669f2452a65e5d2fbd4ca5b2a416fbd217d0 Mon Sep 17 00:00:00 2001 From: Jianfei Hu Date: Tue, 9 Jul 2019 19:56:45 +0000 Subject: [PATCH 1/2] listeners are added to warming lst if !worker_stated Signed-off-by: Jianfei Hu --- source/server/listener_manager_impl.cc | 19 ++++++++++++++----- test/server/listener_manager_impl_test.cc | 4 ++-- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/source/server/listener_manager_impl.cc b/source/server/listener_manager_impl.cc index 82f7b2261084..00978ccbb933 100644 --- a/source/server/listener_manager_impl.cc +++ b/source/server/listener_manager_impl.cc @@ -441,13 +441,22 @@ ProtobufTypes::MessagePtr ListenerManagerImpl::dumpListenerConfigs() { static_listener.mutable_listener()->MergeFrom(listener->config()); TimestampUtil::systemClockToTimestamp(listener->last_updated_, *(static_listener.mutable_last_updated())); + continue; + } + envoy::admin::v2alpha::ListenersConfigDump_DynamicListener* dump_listener; + // Listeners are always added to active_listeners_ list before workers are started. + // This applies even when the listeners are still waiting for initialization. + // To avoid confusion in config dump, in that case, we add these listeners to warming + // listeners config dump rather than active ones. + if (workers_started_) { + dump_listener = config_dump->mutable_dynamic_active_listeners()->Add(); } else { - auto& dynamic_listener = *config_dump->mutable_dynamic_active_listeners()->Add(); - dynamic_listener.set_version_info(listener->versionInfo()); - dynamic_listener.mutable_listener()->MergeFrom(listener->config()); - TimestampUtil::systemClockToTimestamp(listener->last_updated_, - *(dynamic_listener.mutable_last_updated())); + dump_listener = config_dump->mutable_dynamic_warming_listeners()->Add(); } + dump_listener->set_version_info(listener->versionInfo()); + dump_listener->mutable_listener()->MergeFrom(listener->config()); + TimestampUtil::systemClockToTimestamp(listener->last_updated_, + *(dump_listener->mutable_last_updated())); } for (const auto& listener : warming_listeners_) { diff --git a/test/server/listener_manager_impl_test.cc b/test/server/listener_manager_impl_test.cc index cbdeb23e8c6b..bf7a4818288b 100644 --- a/test/server/listener_manager_impl_test.cc +++ b/test/server/listener_manager_impl_test.cc @@ -766,6 +766,7 @@ filter_chains: {} version_info: version1 static_listeners: dynamic_active_listeners: +dynamic_warming_listeners: version_info: "version1" listener: name: "foo" @@ -777,7 +778,6 @@ version_info: version1 last_updated: seconds: 1001001001 nanos: 1000000 -dynamic_warming_listeners: dynamic_draining_listeners: )EOF"); @@ -808,6 +808,7 @@ per_connection_buffer_limit_bytes: 10 version_info: version2 static_listeners: dynamic_active_listeners: +dynamic_warming_listeners: version_info: "version2" listener: name: "foo" @@ -820,7 +821,6 @@ version_info: version2 last_updated: seconds: 2002002002 nanos: 2000000 -dynamic_warming_listeners: dynamic_draining_listeners: )EOF"); From b32cbbabae3cff1f870720d6be9e55234ef93d2e Mon Sep 17 00:00:00 2001 From: Jianfei Hu Date: Wed, 10 Jul 2019 17:26:26 +0000 Subject: [PATCH 2/2] add check in ads_integration_test.cc Signed-off-by: Jianfei Hu --- test/integration/ads_integration_test.cc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/integration/ads_integration_test.cc b/test/integration/ads_integration_test.cc index 3ef9f4df589e..3745513ee431 100644 --- a/test/integration/ads_integration_test.cc +++ b/test/integration/ads_integration_test.cc @@ -969,6 +969,9 @@ TEST_P(AdsIntegrationTest, ListenerDrainBeforeServerStart) { Config::TypeUrl::get().Listener, {buildListener("listener_0", "route_config_0")}, {buildListener("listener_0", "route_config_0")}, {}, "1"); test_server_->waitForGaugeGe("listener_manager.total_listeners_active", 1); + // Before server is started, even though listeners are added to active list + // we mark them as "warming" in config dump since they're not initialized yet. + EXPECT_EQ(getListenersConfigDump().dynamic_warming_listeners().size(), 1); // Remove listener. sendDiscoveryResponse(Config::TypeUrl::get().Listener, {}, {}, {}, "1");