diff --git a/test/config/utility.cc b/test/config/utility.cc index e27a922b53263..23ba4e3e8551d 100644 --- a/test/config/utility.cc +++ b/test/config/utility.cc @@ -245,8 +245,6 @@ name: squash )EOF"; } -// TODO(fredlas) set_node_on_first_message_only was true; the delta+SotW unification -// work restores it here. // TODO(#6327) cleaner approach to testing with static config. std::string ConfigHelper::discoveredClustersBootstrap(const std::string& api_type) { return fmt::format( @@ -265,7 +263,7 @@ std::string ConfigHelper::discoveredClustersBootstrap(const std::string& api_typ grpc_services: envoy_grpc: cluster_name: my_cds_cluster - set_node_on_first_message_only: false + set_node_on_first_message_only: true static_resources: clusters: - name: my_cds_cluster @@ -331,6 +329,7 @@ std::string ConfigHelper::adsBootstrap(const std::string& api_type, ads_config: transport_api_version: {1} api_type: {0} + set_node_on_first_message_only: true static_resources: clusters: name: dummy_cluster diff --git a/test/extensions/clusters/aggregate/cluster_integration_test.cc b/test/extensions/clusters/aggregate/cluster_integration_test.cc index 5e153e0165fb0..3a1c13d1df71a 100644 --- a/test/extensions/clusters/aggregate/cluster_integration_test.cc +++ b/test/extensions/clusters/aggregate/cluster_integration_test.cc @@ -44,7 +44,7 @@ const std::string& config() { grpc_services: envoy_grpc: cluster_name: my_cds_cluster - set_node_on_first_message_only: false + set_node_on_first_message_only: true static_resources: clusters: - name: my_cds_cluster diff --git a/test/integration/ads_integration_test.cc b/test/integration/ads_integration_test.cc index fc86aee90c91b..8592bb8c89449 100644 --- a/test/integration/ads_integration_test.cc +++ b/test/integration/ads_integration_test.cc @@ -188,7 +188,7 @@ TEST_P(AdsIntegrationTest, Failure) { EXPECT_TRUE(compareDiscoveryRequest(Config::TypeUrl::get().Listener, "", {}, {}, {})); EXPECT_TRUE(compareDiscoveryRequest( - Config::TypeUrl::get().Cluster, "", {}, {}, {}, true, + Config::TypeUrl::get().Cluster, "", {}, {}, {}, false, Grpc::Status::WellKnownGrpcStatus::Internal, fmt::format("does not match the message-wide type URL {}", Config::TypeUrl::get().Cluster))); sendDiscoveryResponse(Config::TypeUrl::get().Cluster, @@ -203,7 +203,7 @@ TEST_P(AdsIntegrationTest, Failure) { EXPECT_TRUE(compareDiscoveryRequest(Config::TypeUrl::get().Cluster, "1", {}, {}, {})); EXPECT_TRUE(compareDiscoveryRequest(Config::TypeUrl::get().ClusterLoadAssignment, "", - {"cluster_0"}, {}, {}, true, + {"cluster_0"}, {}, {}, false, Grpc::Status::WellKnownGrpcStatus::Internal, fmt::format("does not match the message-wide type URL {}", Config::TypeUrl::get().ClusterLoadAssignment))); @@ -218,7 +218,7 @@ TEST_P(AdsIntegrationTest, Failure) { {buildRouteConfig("listener_0", "route_config_0")}, {}, "1"); EXPECT_TRUE(compareDiscoveryRequest( - Config::TypeUrl::get().Listener, "", {}, {}, {}, true, + Config::TypeUrl::get().Listener, "", {}, {}, {}, false, Grpc::Status::WellKnownGrpcStatus::Internal, fmt::format("does not match the message-wide type URL {}", Config::TypeUrl::get().Listener))); sendDiscoveryResponse( @@ -233,7 +233,7 @@ TEST_P(AdsIntegrationTest, Failure) { EXPECT_TRUE(compareDiscoveryRequest(Config::TypeUrl::get().Listener, "1", {}, {}, {})); EXPECT_TRUE(compareDiscoveryRequest(Config::TypeUrl::get().RouteConfiguration, "", - {"route_config_0"}, {}, {}, true, + {"route_config_0"}, {}, {}, false, Grpc::Status::WellKnownGrpcStatus::Internal, fmt::format("does not match the message-wide type URL {}", Config::TypeUrl::get().RouteConfiguration))); @@ -397,7 +397,7 @@ TEST_P(AdsIntegrationTest, CdsEdsReplacementWarming) { Config::TypeUrl::get().ClusterLoadAssignment, {buildTlsClusterLoadAssignment("cluster_0")}, {buildTlsClusterLoadAssignment("cluster_0")}, {}, "2"); - EXPECT_TRUE(compareDiscoveryRequest(Config::TypeUrl::get().Cluster, "2", {}, {}, {}, true)); + EXPECT_TRUE(compareDiscoveryRequest(Config::TypeUrl::get().Cluster, "2", {}, {}, {})); EXPECT_TRUE(compareDiscoveryRequest(Config::TypeUrl::get().ClusterLoadAssignment, "2", {"cluster_0"}, {}, {})); } @@ -1148,6 +1148,29 @@ TEST_P(AdsIntegrationTest, NodeMessage) { xds_stream_->finishGrpcStream(Grpc::Status::Ok); } +TEST_P(AdsIntegrationTest, SetNodeAlways) { + config_helper_.addConfigModifier([](envoy::config::bootstrap::v3::Bootstrap& bootstrap) { + auto* ads_config = bootstrap.mutable_dynamic_resources()->mutable_ads_config(); + ads_config->set_set_node_on_first_message_only(false); + }); + initialize(); + + // Check that the node is sent in each request. + EXPECT_TRUE(compareDiscoveryRequest(Config::TypeUrl::get().Cluster, "", {}, {}, {}, true)); + sendDiscoveryResponse(Config::TypeUrl::get().Cluster, + {buildCluster("cluster_0")}, + {buildCluster("cluster_0")}, {}, "1"); + + EXPECT_TRUE(compareDiscoveryRequest(Config::TypeUrl::get().ClusterLoadAssignment, "", + {"cluster_0"}, {"cluster_0"}, {}, true)); + sendDiscoveryResponse( + Config::TypeUrl::get().ClusterLoadAssignment, {buildClusterLoadAssignment("cluster_0")}, + {buildClusterLoadAssignment("cluster_0")}, {}, "1"); + + EXPECT_TRUE(compareDiscoveryRequest(Config::TypeUrl::get().Cluster, "1", {}, {}, {}, true)); + EXPECT_TRUE(compareDiscoveryRequest(Config::TypeUrl::get().Listener, "", {}, {}, {}, true)); +}; + // Check if EDS cluster defined in file is loaded before ADS request and used as xDS server class AdsClusterFromFileIntegrationTest : public Grpc::DeltaSotwIntegrationParamTest, public HttpIntegrationTest { @@ -1227,7 +1250,7 @@ TEST_P(AdsClusterFromFileIntegrationTest, BasicTestWidsAdsEndpointLoadedFromFile xds_stream_->startGrpcStream(); EXPECT_TRUE(compareDiscoveryRequest(Config::TypeUrl::get().ClusterLoadAssignment, "", - {"ads_eds_cluster"}, {"ads_eds_cluster"}, {})); + {"ads_eds_cluster"}, {"ads_eds_cluster"}, {}, true)); sendDiscoveryResponse( Config::TypeUrl::get().ClusterLoadAssignment, {buildClusterLoadAssignment("ads_eds_cluster")}, {buildClusterLoadAssignment("ads_eds_cluster")}, {}, "1"); @@ -1240,8 +1263,6 @@ TEST_P(AdsClusterFromFileIntegrationTest, BasicTestWidsAdsEndpointLoadedFromFile class AdsIntegrationTestWithRtds : public AdsIntegrationTest { public: - AdsIntegrationTestWithRtds() = default; - void initialize() override { config_helper_.addConfigModifier([](envoy::config::bootstrap::v3::Bootstrap& bootstrap) { auto* layered_runtime = bootstrap.mutable_layered_runtime(); @@ -1274,9 +1295,9 @@ class AdsIntegrationTestWithRtds : public AdsIntegrationTest { Config::TypeUrl::get().Runtime, {some_rtds_layer}, {some_rtds_layer}, {}, "1"); test_server_->waitForCounterGe("runtime.load_success", 1); - EXPECT_TRUE(compareDiscoveryRequest(Config::TypeUrl::get().Cluster, "", {}, {}, {}, false)); - EXPECT_TRUE(compareDiscoveryRequest(Config::TypeUrl::get().Runtime, "1", {"ads_rtds_layer"}, {}, - {}, false)); + EXPECT_TRUE(compareDiscoveryRequest(Config::TypeUrl::get().Cluster, "", {}, {}, {})); + EXPECT_TRUE( + compareDiscoveryRequest(Config::TypeUrl::get().Runtime, "1", {"ads_rtds_layer"}, {}, {})); } }; @@ -1290,8 +1311,6 @@ TEST_P(AdsIntegrationTestWithRtds, Basic) { class AdsIntegrationTestWithRtdsAndSecondaryClusters : public AdsIntegrationTestWithRtds { public: - AdsIntegrationTestWithRtdsAndSecondaryClusters() = default; - void initialize() override { config_helper_.addConfigModifier([](envoy::config::bootstrap::v3::Bootstrap& bootstrap) { // Add secondary cluster to the list of static resources. diff --git a/test/integration/base_integration_test.h b/test/integration/base_integration_test.h index f72d4d96b7dbf..a0b947f533d12 100644 --- a/test/integration/base_integration_test.h +++ b/test/integration/base_integration_test.h @@ -140,13 +140,11 @@ class BaseIntegrationTest : protected Logger::Loggable { // sending/receiving to/from the (imaginary) xDS server. You should almost always use // compareDiscoveryRequest() and sendDiscoveryResponse(), but the SotW/delta-specific versions are // available if you're writing a SotW/delta-specific test. - // TODO(fredlas) expect_node was defaulting false here; the delta+SotW unification work restores - // it. AssertionResult compareDiscoveryRequest( const std::string& expected_type_url, const std::string& expected_version, const std::vector& expected_resource_names, const std::vector& expected_resource_names_added, - const std::vector& expected_resource_names_removed, bool expect_node = true, + const std::vector& expected_resource_names_removed, bool expect_node = false, const Protobuf::int32 expected_error_code = Grpc::Status::WellKnownGrpcStatus::Ok, const std::string& expected_error_message = ""); template @@ -179,11 +177,9 @@ class BaseIntegrationTest : protected Logger::Loggable { const Protobuf::int32 expected_error_code = Grpc::Status::WellKnownGrpcStatus::Ok, const std::string& expected_error_message = ""); - // TODO(fredlas) expect_node was defaulting false here; the delta+SotW unification work restores - // it. AssertionResult compareSotwDiscoveryRequest( const std::string& expected_type_url, const std::string& expected_version, - const std::vector& expected_resource_names, bool expect_node = true, + const std::vector& expected_resource_names, bool expect_node = false, const Protobuf::int32 expected_error_code = Grpc::Status::WellKnownGrpcStatus::Ok, const std::string& expected_error_message = ""); diff --git a/test/integration/rtds_integration_test.cc b/test/integration/rtds_integration_test.cc index 17a3553519059..e54e355092b4a 100644 --- a/test/integration/rtds_integration_test.cc +++ b/test/integration/rtds_integration_test.cc @@ -61,7 +61,7 @@ std::string tdsBootstrapConfig(absl::string_view api_type) { grpc_services: envoy_grpc: cluster_name: rtds_cluster - set_node_on_first_message_only: false + set_node_on_first_message_only: true - name: some_admin_layer admin_layer: {{}} admin: