From 9793af2b0923670bb3c2cb2bc47ba8a25fa31002 Mon Sep 17 00:00:00 2001 From: Donna Dionne Date: Thu, 30 Apr 2020 11:55:02 -0700 Subject: [PATCH] Consolidating LDS and RDS tests together And refactoring the code to be able to reuse the same helper methods. Updating all tests to use helper methods. --- test/cpp/end2end/xds_end2end_test.cc | 351 ++++++++------------------- 1 file changed, 105 insertions(+), 246 deletions(-) diff --git a/test/cpp/end2end/xds_end2end_test.cc b/test/cpp/end2end/xds_end2end_test.cc index 0b4d671fa84..c109c6834a3 100644 --- a/test/cpp/end2end/xds_end2end_test.cc +++ b/test/cpp/end2end/xds_end2end_test.cc @@ -1074,22 +1074,27 @@ class LrsServiceImpl : public LrsService, class TestType { public: - TestType(bool use_xds_resolver, bool enable_load_reporting) + TestType(bool use_xds_resolver, bool enable_load_reporting, + bool enable_rds_testing = false) : use_xds_resolver_(use_xds_resolver), - enable_load_reporting_(enable_load_reporting) {} + enable_load_reporting_(enable_load_reporting), + enable_rds_testing_(enable_rds_testing) {} bool use_xds_resolver() const { return use_xds_resolver_; } bool enable_load_reporting() const { return enable_load_reporting_; } + bool enable_rds_testing() const { return enable_rds_testing_; } grpc::string AsString() const { grpc::string retval = (use_xds_resolver_ ? "XdsResolver" : "FakeResolver"); if (enable_load_reporting_) retval += "WithLoadReporting"; + if (enable_rds_testing_) retval += "Rds"; return retval; } private: const bool use_xds_resolver_; const bool enable_load_reporting_; + const bool enable_rds_testing_; }; class XdsEnd2endTest : public ::testing::TestWithParam { @@ -1133,6 +1138,9 @@ class XdsEnd2endTest : public ::testing::TestWithParam { ? client_load_reporting_interval_seconds_ : 0)); balancers_.back()->Start(); + if (GetParam().enable_rds_testing()) { + balancers_[i]->ads_service()->SetLdsToUseDynamicRds(); + } } ResetStub(); } @@ -1468,6 +1476,24 @@ class XdsEnd2endTest : public ::testing::TestWithParam { } } + void SetRouteConfiguration(int idx, const RouteConfiguration& route_config) { + if (GetParam().enable_rds_testing()) { + balancers_[idx]->ads_service()->SetRdsResource(route_config, + kDefaultResourceName); + } else { + balancers_[idx]->ads_service()->SetLdsResource( + AdsServiceImpl::BuildListener(route_config), kDefaultResourceName); + } + } + + AdsServiceImpl::ResponseState RouteConfigurationResponseState(int idx) const { + AdsServiceImpl* ads_service = balancers_[idx]->ads_service(); + if (GetParam().enable_rds_testing()) { + return ads_service->rds_response_state(); + } + return ads_service->lds_response_state(); + } + public: // This method could benefit test subclasses; to make it accessible // via bind with a qualified name, it needs to be public. @@ -2128,16 +2154,6 @@ TEST_P(SecureNamingTest, TargetNameIsUnexpected) { using LdsTest = BasicTest; -// Tests that LDS client should send an ACK upon correct LDS response (with -// inlined RDS result). -TEST_P(LdsTest, Vanilla) { - SetNextResolution({}); - SetNextResolutionForLbChannelAllBalancers(); - (void)SendRpc(); - EXPECT_EQ(balancers_[0]->ads_service()->lds_response_state(), - AdsServiceImpl::ACKED); -} - // Tests that LDS client should send a NACK if there is no API listener in the // Listener in the LDS response. TEST_P(LdsTest, NoApiListener) { @@ -2167,25 +2183,34 @@ TEST_P(LdsTest, WrongRouteSpecifier) { AdsServiceImpl::NACKED); } +using LdsRdsTest = BasicTest; + +// Tests that LDS client should send an ACK upon correct LDS response (with +// inlined RDS result). +TEST_P(LdsRdsTest, Vanilla) { + SetNextResolution({}); + SetNextResolutionForLbChannelAllBalancers(); + (void)SendRpc(); + EXPECT_EQ(RouteConfigurationResponseState(0), AdsServiceImpl::ACKED); +} + // Tests that LDS client should send a NACK if matching domain can't be found in // the LDS response. -TEST_P(LdsTest, NoMatchedDomain) { +TEST_P(LdsRdsTest, NoMatchedDomain) { RouteConfiguration route_config = balancers_[0]->ads_service()->default_route_config(); route_config.mutable_virtual_hosts(0)->clear_domains(); route_config.mutable_virtual_hosts(0)->add_domains("unmatched_domain"); - balancers_[0]->ads_service()->SetLdsResource( - AdsServiceImpl::BuildListener(route_config), kDefaultResourceName); + SetRouteConfiguration(0, route_config); SetNextResolution({}); SetNextResolutionForLbChannelAllBalancers(); CheckRpcSendFailure(); - EXPECT_EQ(balancers_[0]->ads_service()->lds_response_state(), - AdsServiceImpl::NACKED); + EXPECT_EQ(RouteConfigurationResponseState(0), AdsServiceImpl::NACKED); } // Tests that LDS client should choose the virtual host with matching domain if // multiple virtual hosts exist in the LDS response. -TEST_P(LdsTest, ChooseMatchedDomain) { +TEST_P(LdsRdsTest, ChooseMatchedDomain) { RouteConfiguration route_config = balancers_[0]->ads_service()->default_route_config(); *(route_config.add_virtual_hosts()) = route_config.virtual_hosts(0); @@ -2195,18 +2220,16 @@ TEST_P(LdsTest, ChooseMatchedDomain) { ->mutable_routes(0) ->mutable_route() ->mutable_cluster_header(); - balancers_[0]->ads_service()->SetLdsResource( - AdsServiceImpl::BuildListener(route_config), kDefaultResourceName); + SetRouteConfiguration(0, route_config); SetNextResolution({}); SetNextResolutionForLbChannelAllBalancers(); (void)SendRpc(); - EXPECT_EQ(balancers_[0]->ads_service()->lds_response_state(), - AdsServiceImpl::ACKED); + EXPECT_EQ(RouteConfigurationResponseState(0), AdsServiceImpl::ACKED); } // Tests that LDS client should choose the last route in the virtual host if // multiple routes exist in the LDS response. -TEST_P(LdsTest, ChooseLastRoute) { +TEST_P(LdsRdsTest, ChooseLastRoute) { RouteConfiguration route_config = balancers_[0]->ads_service()->default_route_config(); *(route_config.mutable_virtual_hosts(0)->add_routes()) = @@ -2215,36 +2238,32 @@ TEST_P(LdsTest, ChooseLastRoute) { ->mutable_routes(0) ->mutable_route() ->mutable_cluster_header(); - balancers_[0]->ads_service()->SetLdsResource( - AdsServiceImpl::BuildListener(route_config), kDefaultResourceName); + SetRouteConfiguration(0, route_config); SetNextResolution({}); SetNextResolutionForLbChannelAllBalancers(); (void)SendRpc(); - EXPECT_EQ(balancers_[0]->ads_service()->lds_response_state(), - AdsServiceImpl::ACKED); + EXPECT_EQ(RouteConfigurationResponseState(0), AdsServiceImpl::ACKED); } // Tests that LDS client should send a NACK if route match has non-empty prefix // as the only route (default) in the LDS response. -TEST_P(LdsTest, RouteMatchHasNonemptyPrefix) { +TEST_P(LdsRdsTest, RouteMatchHasNonemptyPrefix) { RouteConfiguration route_config = balancers_[0]->ads_service()->default_route_config(); route_config.mutable_virtual_hosts(0) ->mutable_routes(0) ->mutable_match() ->set_prefix("nonempty_prefix"); - balancers_[0]->ads_service()->SetLdsResource( - AdsServiceImpl::BuildListener(route_config), kDefaultResourceName); + SetRouteConfiguration(0, route_config); SetNextResolution({}); SetNextResolutionForLbChannelAllBalancers(); CheckRpcSendFailure(); - EXPECT_EQ(balancers_[0]->ads_service()->lds_response_state(), - AdsServiceImpl::NACKED); + EXPECT_EQ(RouteConfigurationResponseState(0), AdsServiceImpl::NACKED); } // Tests that LDS client should send a NACK if route match has a prefix // string with no "/". -TEST_P(LdsTest, RouteMatchHasInvalidPrefixNonEmptyNoSlash) { +TEST_P(LdsRdsTest, RouteMatchHasInvalidPrefixNonEmptyNoSlash) { ResetStub(/*failover_timeout=*/0, /*expected_targets=*/"", /*xds_resource_does_not_exist_timeout*/ 0, @@ -2256,18 +2275,16 @@ TEST_P(LdsTest, RouteMatchHasInvalidPrefixNonEmptyNoSlash) { auto* default_route = route_config.mutable_virtual_hosts(0)->add_routes(); default_route->mutable_match()->set_prefix(""); default_route->mutable_route()->set_cluster(kDefaultResourceName); - balancers_[0]->ads_service()->SetLdsResource( - AdsServiceImpl::BuildListener(route_config), kDefaultResourceName); + SetRouteConfiguration(0, route_config); SetNextResolution({}); SetNextResolutionForLbChannelAllBalancers(); CheckRpcSendFailure(); - EXPECT_EQ(balancers_[0]->ads_service()->lds_response_state(), - AdsServiceImpl::NACKED); + EXPECT_EQ(RouteConfigurationResponseState(0), AdsServiceImpl::NACKED); } // Tests that LDS client should send a NACK if route match has a prefix // string does not end with "/". -TEST_P(LdsTest, RouteMatchHasInvalidPrefixNoEndingSlash) { +TEST_P(LdsRdsTest, RouteMatchHasInvalidPrefixNoEndingSlash) { ResetStub(/*failover_timeout=*/0, /*expected_targets=*/"", /*xds_resource_does_not_exist_timeout*/ 0, @@ -2276,18 +2293,16 @@ TEST_P(LdsTest, RouteMatchHasInvalidPrefixNoEndingSlash) { balancers_[0]->ads_service()->default_route_config(); auto* route1 = route_config.mutable_virtual_hosts(0)->mutable_routes(0); route1->mutable_match()->set_prefix("/grpc.testing.EchoTest1Service"); - balancers_[0]->ads_service()->SetLdsResource( - AdsServiceImpl::BuildListener(route_config), kDefaultResourceName); + SetRouteConfiguration(0, route_config); SetNextResolution({}); SetNextResolutionForLbChannelAllBalancers(); CheckRpcSendFailure(); - EXPECT_EQ(balancers_[0]->ads_service()->lds_response_state(), - AdsServiceImpl::NACKED); + EXPECT_EQ(RouteConfigurationResponseState(0), AdsServiceImpl::NACKED); } // Tests that LDS client should send a NACK if route match has a prefix // string does not start with "/". -TEST_P(LdsTest, RouteMatchHasInvalidPrefixNoLeadingSlash) { +TEST_P(LdsRdsTest, RouteMatchHasInvalidPrefixNoLeadingSlash) { ResetStub(/*failover_timeout=*/0, /*expected_targets=*/"", /*xds_resource_does_not_exist_timeout*/ 0, @@ -2296,18 +2311,16 @@ TEST_P(LdsTest, RouteMatchHasInvalidPrefixNoLeadingSlash) { balancers_[0]->ads_service()->default_route_config(); auto* route1 = route_config.mutable_virtual_hosts(0)->mutable_routes(0); route1->mutable_match()->set_prefix("grpc.testing.EchoTest1Service/"); - balancers_[0]->ads_service()->SetLdsResource( - AdsServiceImpl::BuildListener(route_config), kDefaultResourceName); + SetRouteConfiguration(0, route_config); SetNextResolution({}); SetNextResolutionForLbChannelAllBalancers(); CheckRpcSendFailure(); - EXPECT_EQ(balancers_[0]->ads_service()->lds_response_state(), - AdsServiceImpl::NACKED); + EXPECT_EQ(RouteConfigurationResponseState(0), AdsServiceImpl::NACKED); } // Tests that LDS client should send a NACK if route match has a prefix // string with extra content outside of "/service/". -TEST_P(LdsTest, RouteMatchHasInvalidPrefixExtraContent) { +TEST_P(LdsRdsTest, RouteMatchHasInvalidPrefixExtraContent) { ResetStub(/*failover_timeout=*/0, /*expected_targets=*/"", /*xds_resource_does_not_exist_timeout*/ 0, @@ -2316,18 +2329,16 @@ TEST_P(LdsTest, RouteMatchHasInvalidPrefixExtraContent) { balancers_[0]->ads_service()->default_route_config(); auto* route1 = route_config.mutable_virtual_hosts(0)->mutable_routes(0); route1->mutable_match()->set_prefix("/grpc.testing.EchoTest1Service/Echo1"); - balancers_[0]->ads_service()->SetLdsResource( - AdsServiceImpl::BuildListener(route_config), kDefaultResourceName); + SetRouteConfiguration(0, route_config); SetNextResolution({}); SetNextResolutionForLbChannelAllBalancers(); CheckRpcSendFailure(); - EXPECT_EQ(balancers_[0]->ads_service()->lds_response_state(), - AdsServiceImpl::NACKED); + EXPECT_EQ(RouteConfigurationResponseState(0), AdsServiceImpl::NACKED); } // Tests that LDS client should send a NACK if route match has a prefix // string "//". -TEST_P(LdsTest, RouteMatchHasInvalidPrefixNoContent) { +TEST_P(LdsRdsTest, RouteMatchHasInvalidPrefixNoContent) { ResetStub(/*failover_timeout=*/0, /*expected_targets=*/"", /*xds_resource_does_not_exist_timeout*/ 0, @@ -2336,18 +2347,16 @@ TEST_P(LdsTest, RouteMatchHasInvalidPrefixNoContent) { balancers_[0]->ads_service()->default_route_config(); auto* route1 = route_config.mutable_virtual_hosts(0)->mutable_routes(0); route1->mutable_match()->set_prefix("//"); - balancers_[0]->ads_service()->SetLdsResource( - AdsServiceImpl::BuildListener(route_config), kDefaultResourceName); + SetRouteConfiguration(0, route_config); SetNextResolution({}); SetNextResolutionForLbChannelAllBalancers(); CheckRpcSendFailure(); - EXPECT_EQ(balancers_[0]->ads_service()->lds_response_state(), - AdsServiceImpl::NACKED); + EXPECT_EQ(RouteConfigurationResponseState(0), AdsServiceImpl::NACKED); } // Tests that LDS client should send a NACK if route match has path // but it's empty. -TEST_P(LdsTest, RouteMatchHasInvalidPathEmptyPath) { +TEST_P(LdsRdsTest, RouteMatchHasInvalidPathEmptyPath) { ResetStub(/*failover_timeout=*/0, /*expected_targets=*/"", /*xds_resource_does_not_exist_timeout*/ 0, @@ -2359,18 +2368,16 @@ TEST_P(LdsTest, RouteMatchHasInvalidPathEmptyPath) { default_route->mutable_match()->set_prefix(""); default_route->mutable_route()->set_cluster(kDefaultResourceName); route1->mutable_match()->set_path(""); - balancers_[0]->ads_service()->SetLdsResource( - AdsServiceImpl::BuildListener(route_config), kDefaultResourceName); + SetRouteConfiguration(0, route_config); SetNextResolution({}); SetNextResolutionForLbChannelAllBalancers(); CheckRpcSendFailure(); - EXPECT_EQ(balancers_[0]->ads_service()->lds_response_state(), - AdsServiceImpl::NACKED); + EXPECT_EQ(RouteConfigurationResponseState(0), AdsServiceImpl::NACKED); } // Tests that LDS client should send a NACK if route match has path // string does not start with "/". -TEST_P(LdsTest, RouteMatchHasInvalidPathNoLeadingSlash) { +TEST_P(LdsRdsTest, RouteMatchHasInvalidPathNoLeadingSlash) { ResetStub(/*failover_timeout=*/0, /*expected_targets=*/"", /*xds_resource_does_not_exist_timeout*/ 0, @@ -2382,18 +2389,16 @@ TEST_P(LdsTest, RouteMatchHasInvalidPathNoLeadingSlash) { default_route->mutable_match()->set_prefix(""); default_route->mutable_route()->set_cluster(kDefaultResourceName); route1->mutable_match()->set_path("grpc.testing.EchoTest1Service/Echo1"); - balancers_[0]->ads_service()->SetLdsResource( - AdsServiceImpl::BuildListener(route_config), kDefaultResourceName); + SetRouteConfiguration(0, route_config); SetNextResolution({}); SetNextResolutionForLbChannelAllBalancers(); CheckRpcSendFailure(); - EXPECT_EQ(balancers_[0]->ads_service()->lds_response_state(), - AdsServiceImpl::NACKED); + EXPECT_EQ(RouteConfigurationResponseState(0), AdsServiceImpl::NACKED); } // Tests that LDS client should send a NACK if route match has path // string that ends with "/". -TEST_P(LdsTest, RouteMatchHasInvalidPathEndsWithSlash) { +TEST_P(LdsRdsTest, RouteMatchHasInvalidPathEndsWithSlash) { ResetStub(/*failover_timeout=*/0, /*expected_targets=*/"", /*xds_resource_does_not_exist_timeout*/ 0, @@ -2405,18 +2410,16 @@ TEST_P(LdsTest, RouteMatchHasInvalidPathEndsWithSlash) { default_route->mutable_match()->set_prefix(""); default_route->mutable_route()->set_cluster(kDefaultResourceName); route1->mutable_match()->set_path("/grpc.testing.EchoTest1Service/Echo1/"); - balancers_[0]->ads_service()->SetLdsResource( - AdsServiceImpl::BuildListener(route_config), kDefaultResourceName); + SetRouteConfiguration(0, route_config); SetNextResolution({}); SetNextResolutionForLbChannelAllBalancers(); CheckRpcSendFailure(); - EXPECT_EQ(balancers_[0]->ads_service()->lds_response_state(), - AdsServiceImpl::NACKED); + EXPECT_EQ(RouteConfigurationResponseState(0), AdsServiceImpl::NACKED); } // Tests that LDS client should send a NACK if route match has path // string that misses "/" between service and method. -TEST_P(LdsTest, RouteMatchHasInvalidPathMissingMiddleSlash) { +TEST_P(LdsRdsTest, RouteMatchHasInvalidPathMissingMiddleSlash) { ResetStub(/*failover_timeout=*/0, /*expected_targets=*/"", /*xds_resource_does_not_exist_timeout*/ 0, @@ -2428,18 +2431,16 @@ TEST_P(LdsTest, RouteMatchHasInvalidPathMissingMiddleSlash) { default_route->mutable_match()->set_prefix(""); default_route->mutable_route()->set_cluster(kDefaultResourceName); route1->mutable_match()->set_path("/grpc.testing.EchoTest1Service.Echo1"); - balancers_[0]->ads_service()->SetLdsResource( - AdsServiceImpl::BuildListener(route_config), kDefaultResourceName); + SetRouteConfiguration(0, route_config); SetNextResolution({}); SetNextResolutionForLbChannelAllBalancers(); CheckRpcSendFailure(); - EXPECT_EQ(balancers_[0]->ads_service()->lds_response_state(), - AdsServiceImpl::NACKED); + EXPECT_EQ(RouteConfigurationResponseState(0), AdsServiceImpl::NACKED); } // Tests that LDS client should send a NACK if route match has path // string that is missing service. -TEST_P(LdsTest, RouteMatchHasInvalidPathMissingService) { +TEST_P(LdsRdsTest, RouteMatchHasInvalidPathMissingService) { ResetStub(/*failover_timeout=*/0, /*expected_targets=*/"", /*xds_resource_does_not_exist_timeout*/ 0, @@ -2451,18 +2452,16 @@ TEST_P(LdsTest, RouteMatchHasInvalidPathMissingService) { default_route->mutable_match()->set_prefix(""); default_route->mutable_route()->set_cluster(kDefaultResourceName); route1->mutable_match()->set_path("//Echo1"); - balancers_[0]->ads_service()->SetLdsResource( - AdsServiceImpl::BuildListener(route_config), kDefaultResourceName); + SetRouteConfiguration(0, route_config); SetNextResolution({}); SetNextResolutionForLbChannelAllBalancers(); CheckRpcSendFailure(); - EXPECT_EQ(balancers_[0]->ads_service()->lds_response_state(), - AdsServiceImpl::NACKED); + EXPECT_EQ(RouteConfigurationResponseState(0), AdsServiceImpl::NACKED); } // Tests that LDS client should send a NACK if route match has path // string that is missing method. -TEST_P(LdsTest, RouteMatchHasInvalidPathMissingMethod) { +TEST_P(LdsRdsTest, RouteMatchHasInvalidPathMissingMethod) { ResetStub(/*failover_timeout=*/0, /*expected_targets=*/"", /*xds_resource_does_not_exist_timeout*/ 0, @@ -2474,28 +2473,24 @@ TEST_P(LdsTest, RouteMatchHasInvalidPathMissingMethod) { default_route->mutable_match()->set_prefix(""); default_route->mutable_route()->set_cluster(kDefaultResourceName); route1->mutable_match()->set_path("/grpc.testing.EchoTest1Service/"); - balancers_[0]->ads_service()->SetLdsResource( - AdsServiceImpl::BuildListener(route_config), kDefaultResourceName); + SetRouteConfiguration(0, route_config); SetNextResolution({}); SetNextResolutionForLbChannelAllBalancers(); CheckRpcSendFailure(); - EXPECT_EQ(balancers_[0]->ads_service()->lds_response_state(), - AdsServiceImpl::NACKED); + EXPECT_EQ(RouteConfigurationResponseState(0), AdsServiceImpl::NACKED); } // Tests that LDS client should send a NACK if route has an action other than // RouteAction in the LDS response. -TEST_P(LdsTest, RouteHasNoRouteAction) { +TEST_P(LdsRdsTest, RouteHasNoRouteAction) { RouteConfiguration route_config = balancers_[0]->ads_service()->default_route_config(); route_config.mutable_virtual_hosts(0)->mutable_routes(0)->mutable_redirect(); - balancers_[0]->ads_service()->SetLdsResource( - AdsServiceImpl::BuildListener(route_config), kDefaultResourceName); + SetRouteConfiguration(0, route_config); SetNextResolution({}); SetNextResolutionForLbChannelAllBalancers(); CheckRpcSendFailure(); - EXPECT_EQ(balancers_[0]->ads_service()->lds_response_state(), - AdsServiceImpl::NACKED); + EXPECT_EQ(RouteConfigurationResponseState(0), AdsServiceImpl::NACKED); } // TODO@donnadionne: Add more invalid config tests to cover all errors in @@ -2503,26 +2498,28 @@ TEST_P(LdsTest, RouteHasNoRouteAction) { // Tests that LDS client should send a NACK if RouteAction has a // cluster_specifier other than cluster in the LDS response. -TEST_P(LdsTest, RouteActionHasNoCluster) { +TEST_P(LdsRdsTest, RouteActionHasNoCluster) { RouteConfiguration route_config = balancers_[0]->ads_service()->default_route_config(); route_config.mutable_virtual_hosts(0) ->mutable_routes(0) ->mutable_route() ->mutable_cluster_header(); - balancers_[0]->ads_service()->SetLdsResource( - AdsServiceImpl::BuildListener(route_config), kDefaultResourceName); + SetRouteConfiguration(0, route_config); SetNextResolution({}); SetNextResolutionForLbChannelAllBalancers(); CheckRpcSendFailure(); - EXPECT_EQ(balancers_[0]->ads_service()->lds_response_state(), - AdsServiceImpl::NACKED); + EXPECT_EQ(RouteConfigurationResponseState(0), AdsServiceImpl::NACKED); } // Tests that LDS client times out when no response received. -TEST_P(LdsTest, Timeout) { +TEST_P(LdsRdsTest, Timeout) { ResetStub(0, "", 500); - balancers_[0]->ads_service()->SetResourceIgnore(kLdsTypeUrl); + if (GetParam().enable_rds_testing()) { + balancers_[0]->ads_service()->SetResourceIgnore(kRdsTypeUrl); + } else { + balancers_[0]->ads_service()->SetResourceIgnore(kLdsTypeUrl); + } SetNextResolution({}); SetNextResolutionForLbChannelAllBalancers(); CheckRpcSendFailure(); @@ -2530,7 +2527,7 @@ TEST_P(LdsTest, Timeout) { // Tests that LDS client should choose the default route (with no matching // specified) after unable to find a match with previous routes. -TEST_P(LdsTest, XdsRoutingPathMatching) { +TEST_P(LdsRdsTest, XdsRoutingPathMatching) { ResetStub(/*failover_timeout=*/0, /*expected_targets=*/"", /*xds_resource_does_not_exist_timeout*/ 0, @@ -2582,9 +2579,7 @@ TEST_P(LdsTest, XdsRoutingPathMatching) { auto* default_route = new_route_config.mutable_virtual_hosts(0)->add_routes(); default_route->mutable_match()->set_prefix(""); default_route->mutable_route()->set_cluster(kDefaultResourceName); - Listener listener = - balancers_[0]->ads_service()->BuildListener(new_route_config); - balancers_[0]->ads_service()->SetLdsResource(listener, kDefaultResourceName); + SetRouteConfiguration(0, new_route_config); WaitForAllBackends(0, 2); CheckRpcSendOk(kNumEchoRpcs, RpcOptions().set_wait_for_ready(true)); CheckRpcSendOk(kNumEcho1Rpcs, RpcOptions() @@ -2610,7 +2605,7 @@ TEST_P(LdsTest, XdsRoutingPathMatching) { EXPECT_EQ(kNumEcho2Rpcs, backends_[3]->backend_service2()->request_count()); } -TEST_P(LdsTest, XdsRoutingPrefixMatching) { +TEST_P(LdsRdsTest, XdsRoutingPrefixMatching) { ResetStub(/*failover_timeout=*/0, /*expected_targets=*/"", /*xds_resource_does_not_exist_timeout*/ 0, @@ -2659,9 +2654,7 @@ TEST_P(LdsTest, XdsRoutingPrefixMatching) { auto* default_route = new_route_config.mutable_virtual_hosts(0)->add_routes(); default_route->mutable_match()->set_prefix(""); default_route->mutable_route()->set_cluster(kDefaultResourceName); - Listener listener = - balancers_[0]->ads_service()->BuildListener(new_route_config); - balancers_[0]->ads_service()->SetLdsResource(listener, kDefaultResourceName); + SetRouteConfiguration(0, new_route_config); WaitForAllBackends(0, 2); CheckRpcSendOk(kNumEchoRpcs, RpcOptions().set_wait_for_ready(true)); CheckRpcSendOk( @@ -2685,142 +2678,6 @@ TEST_P(LdsTest, XdsRoutingPrefixMatching) { EXPECT_EQ(kNumEcho2Rpcs, backends_[3]->backend_service2()->request_count()); } -using RdsTest = BasicTest; - -// Tests that RDS client should send an ACK upon correct RDS response. -TEST_P(RdsTest, Vanilla) { - balancers_[0]->ads_service()->SetLdsToUseDynamicRds(); - SetNextResolution({}); - SetNextResolutionForLbChannelAllBalancers(); - (void)SendRpc(); - EXPECT_EQ(balancers_[0]->ads_service()->rds_response_state(), - AdsServiceImpl::ACKED); -} - -// Tests that RDS client should send a NACK if matching domain can't be found in -// the RDS response. -TEST_P(RdsTest, NoMatchedDomain) { - balancers_[0]->ads_service()->SetLdsToUseDynamicRds(); - RouteConfiguration route_config = - balancers_[0]->ads_service()->default_route_config(); - route_config.mutable_virtual_hosts(0)->clear_domains(); - route_config.mutable_virtual_hosts(0)->add_domains("unmatched_domain"); - balancers_[0]->ads_service()->SetRdsResource(route_config, - kDefaultResourceName); - SetNextResolution({}); - SetNextResolutionForLbChannelAllBalancers(); - CheckRpcSendFailure(); - EXPECT_EQ(balancers_[0]->ads_service()->rds_response_state(), - AdsServiceImpl::NACKED); -} - -// Tests that RDS client should choose the virtual host with matching domain if -// multiple virtual hosts exist in the RDS response. -TEST_P(RdsTest, ChooseMatchedDomain) { - balancers_[0]->ads_service()->SetLdsToUseDynamicRds(); - RouteConfiguration route_config = - balancers_[0]->ads_service()->default_route_config(); - *(route_config.add_virtual_hosts()) = route_config.virtual_hosts(0); - route_config.mutable_virtual_hosts(0)->clear_domains(); - route_config.mutable_virtual_hosts(0)->add_domains("unmatched_domain"); - route_config.mutable_virtual_hosts(0) - ->mutable_routes(0) - ->mutable_route() - ->mutable_cluster_header(); - balancers_[0]->ads_service()->SetRdsResource(route_config, - kDefaultResourceName); - SetNextResolution({}); - SetNextResolutionForLbChannelAllBalancers(); - (void)SendRpc(); - EXPECT_EQ(balancers_[0]->ads_service()->rds_response_state(), - AdsServiceImpl::ACKED); -} - -// Tests that RDS client should choose the last route in the virtual host if -// multiple routes exist in the RDS response. -TEST_P(RdsTest, ChooseLastRoute) { - balancers_[0]->ads_service()->SetLdsToUseDynamicRds(); - RouteConfiguration route_config = - balancers_[0]->ads_service()->default_route_config(); - *(route_config.mutable_virtual_hosts(0)->add_routes()) = - route_config.virtual_hosts(0).routes(0); - route_config.mutable_virtual_hosts(0) - ->mutable_routes(0) - ->mutable_route() - ->mutable_cluster_header(); - balancers_[0]->ads_service()->SetRdsResource(route_config, - kDefaultResourceName); - SetNextResolution({}); - SetNextResolutionForLbChannelAllBalancers(); - (void)SendRpc(); - EXPECT_EQ(balancers_[0]->ads_service()->rds_response_state(), - AdsServiceImpl::ACKED); -} - -// Tests that RDS client should send a NACK if route match has non-empty prefix -// as the only route (default) in the RDS response. -TEST_P(RdsTest, RouteMatchHasNonemptyPrefix) { - balancers_[0]->ads_service()->SetLdsToUseDynamicRds(); - RouteConfiguration route_config = - balancers_[0]->ads_service()->default_route_config(); - route_config.mutable_virtual_hosts(0) - ->mutable_routes(0) - ->mutable_match() - ->set_prefix("/nonempty_prefix/"); - balancers_[0]->ads_service()->SetRdsResource(route_config, - kDefaultResourceName); - SetNextResolution({}); - SetNextResolutionForLbChannelAllBalancers(); - CheckRpcSendFailure(); - EXPECT_EQ(balancers_[0]->ads_service()->rds_response_state(), - AdsServiceImpl::NACKED); -} - -// Tests that RDS client should send a NACK if route has an action other than -// RouteAction in the RDS response. -TEST_P(RdsTest, RouteHasNoRouteAction) { - balancers_[0]->ads_service()->SetLdsToUseDynamicRds(); - RouteConfiguration route_config = - balancers_[0]->ads_service()->default_route_config(); - route_config.mutable_virtual_hosts(0)->mutable_routes(0)->mutable_redirect(); - balancers_[0]->ads_service()->SetRdsResource(route_config, - kDefaultResourceName); - SetNextResolution({}); - SetNextResolutionForLbChannelAllBalancers(); - CheckRpcSendFailure(); - EXPECT_EQ(balancers_[0]->ads_service()->rds_response_state(), - AdsServiceImpl::NACKED); -} - -// Tests that RDS client should send a NACK if RouteAction has a -// cluster_specifier other than cluster in the RDS response. -TEST_P(RdsTest, RouteActionHasNoCluster) { - balancers_[0]->ads_service()->SetLdsToUseDynamicRds(); - RouteConfiguration route_config = - balancers_[0]->ads_service()->default_route_config(); - route_config.mutable_virtual_hosts(0) - ->mutable_routes(0) - ->mutable_route() - ->mutable_cluster_header(); - balancers_[0]->ads_service()->SetRdsResource(route_config, - kDefaultResourceName); - SetNextResolution({}); - SetNextResolutionForLbChannelAllBalancers(); - CheckRpcSendFailure(); - EXPECT_EQ(balancers_[0]->ads_service()->rds_response_state(), - AdsServiceImpl::NACKED); -} - -// Tests that RDS client times out when no response received. -TEST_P(RdsTest, Timeout) { - ResetStub(0, "", 500); - balancers_[0]->ads_service()->SetResourceIgnore(kRdsTypeUrl); - balancers_[0]->ads_service()->SetLdsToUseDynamicRds(); - SetNextResolution({}); - SetNextResolutionForLbChannelAllBalancers(); - CheckRpcSendFailure(); -} - using CdsTest = BasicTest; // Tests that CDS client should send an ACK upon correct CDS response. @@ -4060,10 +3917,12 @@ INSTANTIATE_TEST_SUITE_P(XdsTest, LdsTest, TestType(true, true)), &TestTypeName); -// RDS depends on XdsResolver. -INSTANTIATE_TEST_SUITE_P(XdsTest, RdsTest, +// LDS RDS Commmon tests depends on XdsResolver. +INSTANTIATE_TEST_SUITE_P(XdsTest, LdsRdsTest, ::testing::Values(TestType(true, false), - TestType(true, true)), + TestType(true, true), + TestType(true, false, true), + TestType(true, true, true)), &TestTypeName); // CDS depends on XdsResolver.