From a204d54f24648137106501eef2d0a684da283c6a Mon Sep 17 00:00:00 2001 From: Donna Dionne Date: Thu, 1 Oct 2020 10:26:17 -0700 Subject: [PATCH] Fixing ListenerRemoved test. In the ListenerRemoved test, we observed that XdsConfigSelector gets re-created after being destroyed upon removal of listener. This sometimes causes test to fail as RPC will continue to succeed which is not expected; more importantly the re-creation of XdsConfigSelector is not the correct behaviour. XdsConfigSelector gets recreated because the XdsConfigSelector destructor calls will check on removed clusters and then recreate XdsConfigSelector due to the change. This recreation uses a stored copy of the LDS/RDS update, the correct solution is to clear that update upon the removal of listener (ResourceDoesNotExist) and thus prvent the recreation of XdsConfigSelector. Fix is tested to make sure ListenerRemoved test is no longer flaky; logs are checked to ensure that after Listener Removal, the CDs and EDs requests contain no resources and the XDS server will unsubscribe to those resources and no RPC will succeed. --- .../ext/filters/client_channel/resolver/xds/xds_resolver.cc | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/core/ext/filters/client_channel/resolver/xds/xds_resolver.cc b/src/core/ext/filters/client_channel/resolver/xds/xds_resolver.cc index 980fbf6c98e..9a4cf351761 100644 --- a/src/core/ext/filters/client_channel/resolver/xds/xds_resolver.cc +++ b/src/core/ext/filters/client_channel/resolver/xds/xds_resolver.cc @@ -615,9 +615,10 @@ void XdsResolver::OnError(grpc_error* error) { void XdsResolver::OnResourceDoesNotExist() { gpr_log(GPR_ERROR, - "[xds_resolver %p] LDS/RDS resource does not exist -- returning " - "empty service config", + "[xds_resolver %p] LDS/RDS resource does not exist -- clearing " + "update and returning empty service config", this); + current_update_.clear(); Result result; result.service_config = ServiceConfig::Create(args_, "{}", &result.service_config_error); @@ -659,6 +660,7 @@ grpc_error* XdsResolver::CreateServiceConfig( } void XdsResolver::GenerateResult() { + if (current_update_.empty()) return; // First create XdsConfigSelector, which may add new entries to the cluster // state map, and then CreateServiceConfig for LB policies. auto config_selector =