diff --git a/source/server/filter_chain_manager_impl.cc b/source/server/filter_chain_manager_impl.cc index 01e63097f2c63..f1a74a70e78cd 100644 --- a/source/server/filter_chain_manager_impl.cc +++ b/source/server/filter_chain_manager_impl.cc @@ -31,9 +31,7 @@ Network::Address::InstanceConstSharedPtr fakeAddress() { PerFilterChainFactoryContextImpl::PerFilterChainFactoryContextImpl( Configuration::FactoryContext& parent_context, Init::Manager& init_manager) - : parent_context_(parent_context), scope_(parent_context_.scope().createScope("")), - filter_chain_scope_(parent_context_.listenerScope().createScope("")), - init_manager_(init_manager) {} + : parent_context_(parent_context), init_manager_(init_manager) {} bool PerFilterChainFactoryContextImpl::drainClose() const { return is_draining_.load() || parent_context_.drainDecision().drainClose(); @@ -103,7 +101,7 @@ Envoy::Runtime::Loader& PerFilterChainFactoryContextImpl::runtime() { return parent_context_.runtime(); } -Stats::Scope& PerFilterChainFactoryContextImpl::scope() { return *scope_; } +Stats::Scope& PerFilterChainFactoryContextImpl::scope() { return parent_context_.scope(); } Singleton::Manager& PerFilterChainFactoryContextImpl::singletonManager() { return parent_context_.singletonManager(); @@ -137,7 +135,9 @@ PerFilterChainFactoryContextImpl::getTransportSocketFactoryContext() const { return parent_context_.getTransportSocketFactoryContext(); } -Stats::Scope& PerFilterChainFactoryContextImpl::listenerScope() { return *filter_chain_scope_; } +Stats::Scope& PerFilterChainFactoryContextImpl::listenerScope() { + return parent_context_.listenerScope(); +} bool PerFilterChainFactoryContextImpl::isQuicListener() const { return parent_context_.isQuicListener(); diff --git a/source/server/filter_chain_manager_impl.h b/source/server/filter_chain_manager_impl.h index 3919aeb81073d..8218e89777127 100644 --- a/source/server/filter_chain_manager_impl.h +++ b/source/server/filter_chain_manager_impl.h @@ -87,10 +87,6 @@ class PerFilterChainFactoryContextImpl : public Configuration::FilterChainFactor private: Configuration::FactoryContext& parent_context_; - // The scope that has empty prefix. - Stats::ScopePtr scope_; - // filter_chain_scope_ has the same prefix as listener owners scope. - Stats::ScopePtr filter_chain_scope_; Init::Manager& init_manager_; std::atomic is_draining_{false}; }; diff --git a/test/integration/server.h b/test/integration/server.h index da51efe2eabf7..8ad4c633d6892 100644 --- a/test/integration/server.h +++ b/test/integration/server.h @@ -477,11 +477,6 @@ class IntegrationTestServer : public Logger::Loggable, notifyingStatsAllocator().waitForCounterExists(name); } - // TODO(#17956): Add Gauge type to NotifyingAllocator and adopt it in this method. - void waitForGaugeDestroyed(const std::string& name) override { - ASSERT_TRUE(TestUtility::waitForGaugeDestroyed(statStore(), name, time_system_)); - } - void waitUntilHistogramHasSamples( const std::string& name, std::chrono::milliseconds timeout = std::chrono::milliseconds::zero()) override { diff --git a/test/integration/server_stats.h b/test/integration/server_stats.h index d4520d3456db5..66cb7e07e7d27 100644 --- a/test/integration/server_stats.h +++ b/test/integration/server_stats.h @@ -66,12 +66,6 @@ class IntegrationTestServerStats { waitForGaugeEq(const std::string& name, uint64_t value, std::chrono::milliseconds timeout = std::chrono::milliseconds::zero()) PURE; - /** - * Wait for a gauge to be destroyed. Note that MockStatStore does not destroy stat. - * @param name gauge name. - */ - virtual void waitForGaugeDestroyed(const std::string& name) PURE; - /** * Counter lookup. This is not thread safe, since we don't get a consistent * snapshot, uses counters() instead for this behavior. diff --git a/test/integration/xds_integration_test.cc b/test/integration/xds_integration_test.cc index 8e866132782b7..7a11e6a156470 100644 --- a/test/integration/xds_integration_test.cc +++ b/test/integration/xds_integration_test.cc @@ -301,9 +301,6 @@ class LdsInplaceUpdateHttpIntegrationTest std::string tls_inspector_config = ConfigHelper::tlsInspectorFilter(); config_helper_.addListenerFilter(tls_inspector_config); config_helper_.addSslConfig(); - config_helper_.addConfigModifier( - [&](envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager& - hcm) { hcm.mutable_stat_prefix()->assign("hcm0"); }); config_helper_.addConfigModifier([this, add_default_filter_chain]( envoy::config::bootstrap::v3::Bootstrap& bootstrap) { if (!use_default_balancer_) { @@ -338,7 +335,6 @@ class LdsInplaceUpdateHttpIntegrationTest ->mutable_routes(0) ->mutable_route() ->set_cluster("cluster_1"); - hcm_config.mutable_stat_prefix()->assign("hcm1"); config_blob->PackFrom(hcm_config); bootstrap.mutable_static_resources()->mutable_clusters()->Add()->MergeFrom( *bootstrap.mutable_static_resources()->mutable_clusters(0)); @@ -385,7 +381,7 @@ class LdsInplaceUpdateHttpIntegrationTest } } - void expectConnectionServed(std::string alpn = "alpn0") { + void expectConnenctionServed(std::string alpn = "alpn0") { auto codec_client_after_config_update = createHttpCodec(alpn); expectResponseHeaderConnectionClose(*codec_client_after_config_update, false); codec_client_after_config_update->close(); @@ -399,7 +395,7 @@ class LdsInplaceUpdateHttpIntegrationTest }; // Verify that http response on filter chain 1 and default filter chain have "Connection: close" -// header when these 2 filter chains are deleted during the listener update. +// header when these 2 filter chains are deleted during the listener update. TEST_P(LdsInplaceUpdateHttpIntegrationTest, ReloadConfigDeletingFilterChain) { inplaceInitialize(/*add_default_filter_chain=*/true); @@ -407,6 +403,12 @@ TEST_P(LdsInplaceUpdateHttpIntegrationTest, ReloadConfigDeletingFilterChain) { auto codec_client_0 = createHttpCodec("alpn0"); auto codec_client_default = createHttpCodec("alpndefault"); + Cleanup cleanup([c1 = codec_client_1.get(), c0 = codec_client_0.get(), + c_default = codec_client_default.get()]() { + c1->close(); + c0->close(); + c_default->close(); + }); ConfigHelper new_config_helper( version_, *api_, MessageUtil::getJsonStringFromMessageOrDie(config_helper_.bootstrap())); new_config_helper.addConfigModifier( @@ -420,20 +422,12 @@ TEST_P(LdsInplaceUpdateHttpIntegrationTest, ReloadConfigDeletingFilterChain) { test_server_->waitForCounterGe("listener_manager.listener_in_place_updated", 1); test_server_->waitForGaugeGe("listener_manager.total_filter_chains_draining", 1); - test_server_->waitForGaugeGe("http.hcm0.downstream_cx_active", 1); - test_server_->waitForGaugeGe("http.hcm1.downstream_cx_active", 1); - expectResponseHeaderConnectionClose(*codec_client_1, true); expectResponseHeaderConnectionClose(*codec_client_default, true); test_server_->waitForGaugeGe("listener_manager.total_filter_chains_draining", 0); expectResponseHeaderConnectionClose(*codec_client_0, false); - expectConnectionServed(); - - codec_client_1->close(); - test_server_->waitForGaugeDestroyed("http.hcm1.downstream_cx_active"); - codec_client_0->close(); - codec_client_default->close(); + expectConnenctionServed(); } // Verify that http clients of filter chain 0 survives if new listener config adds new filter @@ -444,19 +438,15 @@ TEST_P(LdsInplaceUpdateHttpIntegrationTest, ReloadConfigAddingFilterChain) { auto codec_client_0 = createHttpCodec("alpn0"); Cleanup cleanup0([c0 = codec_client_0.get()]() { c0->close(); }); - test_server_->waitForGaugeGe("http.hcm0.downstream_cx_active", 1); - ConfigHelper new_config_helper( version_, *api_, MessageUtil::getJsonStringFromMessageOrDie(config_helper_.bootstrap())); new_config_helper.addConfigModifier([&](envoy::config::bootstrap::v3::Bootstrap& bootstrap) -> void { auto* listener = bootstrap.mutable_static_resources()->mutable_listeners(0); - // Note that HCM2 copies the stats prefix from HCM0 - listener->mutable_filter_chains()->Add()->MergeFrom(*listener->mutable_filter_chains(0)); + listener->mutable_filter_chains()->Add()->MergeFrom(*listener->mutable_filter_chains(1)); *listener->mutable_filter_chains(2) ->mutable_filter_chain_match() ->mutable_application_protocols(0) = "alpn2"; - auto default_filter_chain = bootstrap.mutable_static_resources()->mutable_listeners(0)->mutable_default_filter_chain(); default_filter_chain->MergeFrom(*listener->mutable_filter_chains(1)); @@ -468,9 +458,6 @@ TEST_P(LdsInplaceUpdateHttpIntegrationTest, ReloadConfigAddingFilterChain) { auto codec_client_2 = createHttpCodec("alpn2"); auto codec_client_default = createHttpCodec("alpndefault"); - // 1 connection from filter chain 0 and 1 connection from filter chain 2. - test_server_->waitForGaugeGe("http.hcm0.downstream_cx_active", 2); - Cleanup cleanup2([c2 = codec_client_2.get(), c_default = codec_client_default.get()]() { c2->close(); c_default->close(); @@ -478,7 +465,7 @@ TEST_P(LdsInplaceUpdateHttpIntegrationTest, ReloadConfigAddingFilterChain) { expectResponseHeaderConnectionClose(*codec_client_2, false); expectResponseHeaderConnectionClose(*codec_client_default, false); expectResponseHeaderConnectionClose(*codec_client_0, false); - expectConnectionServed(); + expectConnenctionServed(); } // Verify that http clients of default filter chain is drained and recreated if the default filter @@ -506,7 +493,7 @@ TEST_P(LdsInplaceUpdateHttpIntegrationTest, ReloadConfigUpdatingDefaultFilterCha Cleanup cleanup2([c_default_v3 = codec_client_default_v3.get()]() { c_default_v3->close(); }); expectResponseHeaderConnectionClose(*codec_client_default, true); expectResponseHeaderConnectionClose(*codec_client_default_v3, false); - expectConnectionServed(); + expectConnenctionServed(); } // Verify that balancer is inherited. Test only default balancer because ExactConnectionBalancer @@ -528,7 +515,7 @@ TEST_P(LdsInplaceUpdateHttpIntegrationTest, OverlappingFilterChainServesNewConne new_config_helper.setLds("1"); test_server_->waitForCounterGe("listener_manager.listener_in_place_updated", 1); expectResponseHeaderConnectionClose(*codec_client_0, false); - expectConnectionServed(); + expectConnenctionServed(); } // Verify default filter chain update is filter chain only update. diff --git a/test/test_common/utility.cc b/test/test_common/utility.cc index c2f68cc000c6d..fe0db0bcf7ee8 100644 --- a/test/test_common/utility.cc +++ b/test/test_common/utility.cc @@ -238,14 +238,6 @@ AssertionResult TestUtility::waitForGaugeEq(Stats::Store& store, const std::stri return AssertionSuccess(); } -AssertionResult TestUtility::waitForGaugeDestroyed(Stats::Store& store, const std::string& name, - Event::TestTimeSystem& time_system) { - while (findGauge(store, name) == nullptr) { - time_system.advanceTimeWait(std::chrono::milliseconds(10)); - } - return AssertionSuccess(); -} - AssertionResult TestUtility::waitUntilHistogramHasSamples(Stats::Store& store, const std::string& name, Event::TestTimeSystem& time_system, diff --git a/test/test_common/utility.h b/test/test_common/utility.h index e7fa952614c92..c355b50643394 100644 --- a/test/test_common/utility.h +++ b/test/test_common/utility.h @@ -277,17 +277,6 @@ class TestUtility { Event::TestTimeSystem& time_system, std::chrono::milliseconds timeout = std::chrono::milliseconds::zero()); - /** - * Wait for a gauge to be destroyed. - * @param store supplies the stats store. - * @param name gauge name. - * @param time_system the time system to use for waiting. - * @return AssertionSuccess() if the gauge was == to the value within the timeout, else - * AssertionFailure(). - */ - static AssertionResult waitForGaugeDestroyed(Stats::Store& store, const std::string& name, - Event::TestTimeSystem& time_system); - /** * Wait for a histogram to have samples. * @param store supplies the stats store.