diff --git a/source/common/protobuf/utility.h b/source/common/protobuf/utility.h index fa60e8ede2f43..5adba55f0f666 100644 --- a/source/common/protobuf/utility.h +++ b/source/common/protobuf/utility.h @@ -142,22 +142,22 @@ class RepeatedPtrUtil { } /** - * Converts a proto repeated field into a generic vector of const Protobuf::Message unique_ptr's. + * Converts a proto repeated field into a container of const Protobuf::Message unique_ptr's. * * @param repeated_field the proto repeated field to convert. - * @return ProtobufType::ConstMessagePtrVector the vector of const Message pointers. + * @return ReturnType the container of const Message pointers. */ - template - static ProtobufTypes::ConstMessagePtrVector - convertToConstMessagePtrVector(const Protobuf::RepeatedPtrField& repeated_field) { - ProtobufTypes::ConstMessagePtrVector ret_vector; - std::transform(repeated_field.begin(), repeated_field.end(), std::back_inserter(ret_vector), + template + static ReturnType + convertToConstMessagePtrContainer(const Protobuf::RepeatedPtrField& repeated_field) { + ReturnType ret_container; + std::transform(repeated_field.begin(), repeated_field.end(), std::back_inserter(ret_container), [](const ProtoType& proto_message) -> std::unique_ptr { Protobuf::Message* clone = proto_message.New(); clone->MergeFrom(proto_message); return std::unique_ptr(clone); }); - return ret_vector; + return ret_container; } }; diff --git a/source/common/router/scoped_rds.cc b/source/common/router/scoped_rds.cc index 9694c38f78f9b..0ea1c4ed5a2da 100644 --- a/source/common/router/scoped_rds.cc +++ b/source/common/router/scoped_rds.cc @@ -35,7 +35,8 @@ create(const envoy::config::filter::network::http_connection_manager::v2::HttpCo ScopedRouteConfigurationsList& scoped_route_list = config.scoped_routes().scoped_route_configurations_list(); return scoped_routes_config_provider_manager.createStaticConfigProvider( - RepeatedPtrUtil::convertToConstMessagePtrVector( + RepeatedPtrUtil::convertToConstMessagePtrContainer( scoped_route_list.scoped_route_configurations()), factory_context, ScopedRoutesConfigProviderManagerOptArg(config.scoped_routes().name(), diff --git a/test/integration/load_stats_integration_test.cc b/test/integration/load_stats_integration_test.cc index c846a5649580e..de9a3e3434cb0 100644 --- a/test/integration/load_stats_integration_test.cc +++ b/test/integration/load_stats_integration_test.cc @@ -191,15 +191,13 @@ class LoadStatsIntegrationTest : public testing::TestWithParamset_total_successful_requests( upstream_locality_stats->total_successful_requests() + local_upstream_locality_stats.total_successful_requests()); - upstream_locality_stats->set_total_requests_in_progress( - upstream_locality_stats->total_requests_in_progress() + - local_upstream_locality_stats.total_requests_in_progress()); upstream_locality_stats->set_total_error_requests( upstream_locality_stats->total_error_requests() + local_upstream_locality_stats.total_error_requests()); upstream_locality_stats->set_total_issued_requests( upstream_locality_stats->total_issued_requests() + local_upstream_locality_stats.total_issued_requests()); + // Unlike most stats, current requests in progress replaces old requests in progress. break; } } @@ -208,6 +206,25 @@ class LoadStatsIntegrationTest : public testing::TestWithParamCopyFrom(local_upstream_locality_stats); } } + + // Unfortunately because we don't issue an update when total_requests_in_progress goes from + // non-zero to zero, we have to go through and zero it out for any locality stats we didn't see. + for (int i = 0; i < cluster_stats->upstream_locality_stats_size(); ++i) { + auto upstream_locality_stats = cluster_stats->mutable_upstream_locality_stats(i); + bool found = false; + for (int j = 0; j < local_cluster_stats.upstream_locality_stats_size(); ++j) { + auto& local_upstream_locality_stats = local_cluster_stats.upstream_locality_stats(j); + if (TestUtility::protoEqual(upstream_locality_stats->locality(), + local_upstream_locality_stats.locality()) && + upstream_locality_stats->priority() == local_upstream_locality_stats.priority()) { + found = true; + break; + } + } + if (!found) { + upstream_locality_stats->set_total_requests_in_progress(0); + } + } } void waitForLoadStatsRequest( @@ -257,7 +274,7 @@ class LoadStatsIntegrationTest : public testing::TestWithParamheaders().ContentType()->value().getStringView()); } while (!TestUtility::assertRepeatedPtrFieldEqual(expected_cluster_stats, - loadstats_request.cluster_stats())); + loadstats_request.cluster_stats(), true)); } void waitForUpstreamResponse(uint32_t endpoint_index, uint32_t response_code = 200) { @@ -460,23 +477,16 @@ TEST_P(LoadStatsIntegrationTest, LocalityWeighted) { locality_weighted_lb_ = true; initialize(); - // Debug logs for #6874 - std::cerr << "Waiting for load stats stream." << std::endl; waitForLoadStatsStream(); - std::cerr << "Waiting for load stats request." << std::endl; waitForLoadStatsRequest({}); - std::cerr << "Done waiting." << std::endl; loadstats_stream_->startGrpcStream(); - std::cerr << "Starting response." << std::endl; requestLoadStatsResponse({"cluster_0"}); - std::cerr << "Updating assignments." << std::endl; // Simple 33%/67% split between dragon/winter localities. // Even though there are more endpoints in the dragon locality, the winter locality gets the // expected weighting in the WRR locality schedule. updateClusterLoadAssignment({{0}, 2}, {{1, 2}, 1}, {}, {}); - std::cerr << "Sending traffic." << std::endl; sendAndReceiveUpstream(0); sendAndReceiveUpstream(1); @@ -486,10 +496,8 @@ TEST_P(LoadStatsIntegrationTest, LocalityWeighted) { sendAndReceiveUpstream(0); // Verify we get the expect request distribution. - std::cerr << "Waiting for load stats request 2." << std::endl; waitForLoadStatsRequest( {localityStats("winter", 4, 0, 0, 4), localityStats("dragon", 2, 0, 0, 2)}); - std::cerr << "Done waiting." << std::endl; EXPECT_EQ(1, test_server_->counter("load_reporter.requests")->value()); // On slow machines, more than one load stats response may be pushed while we are simulating load. diff --git a/test/test_common/utility.h b/test/test_common/utility.h index 2c80f49b431f0..b075f5f22542c 100644 --- a/test/test_common/utility.h +++ b/test/test_common/utility.h @@ -265,30 +265,59 @@ class TestUtility { * * @param lhs RepeatedPtrField on LHS. * @param rhs RepeatedPtrField on RHS. + * @param ignore_ordering if ordering should be ignored. Note if true this turns + * comparison into an N^2 operation. * @return bool indicating whether the RepeatedPtrField are equal. TestUtility::protoEqual() is * used for individual element testing. */ - template + template static bool repeatedPtrFieldEqual(const Protobuf::RepeatedPtrField& lhs, - const Protobuf::RepeatedPtrField& rhs) { + const Protobuf::RepeatedPtrField& rhs, + bool ignore_ordering = false) { if (lhs.size() != rhs.size()) { return false; } - for (int i = 0; i < lhs.size(); ++i) { - if (!TestUtility::protoEqual(lhs[i], rhs[i], /*ignore_repeated_field_ordering=*/false)) { + if (!ignore_ordering) { + for (int i = 0; i < lhs.size(); ++i) { + if (!TestUtility::protoEqual(lhs[i], rhs[i], /*ignore_ordering=*/false)) { + return false; + } + } + + return true; + } + typedef std::list> ProtoList; + // Iterate through using protoEqual as ignore_ordering is true, and fields + // in the sub-protos may also be out of order. + ProtoList lhs_list = + RepeatedPtrUtil::convertToConstMessagePtrContainer(lhs); + ProtoList rhs_list = + RepeatedPtrUtil::convertToConstMessagePtrContainer(rhs); + while (!lhs_list.empty()) { + bool found = false; + for (auto it = rhs_list.begin(); it != rhs_list.end(); ++it) { + if (TestUtility::protoEqual(*lhs_list.front(), **it, + /*ignore_ordering=*/true)) { + lhs_list.pop_front(); + rhs_list.erase(it); + found = true; + break; + } + } + if (!found) { return false; } } - return true; } template static AssertionResult assertRepeatedPtrFieldEqual(const Protobuf::RepeatedPtrField& lhs, - const Protobuf::RepeatedPtrField& rhs) { - if (!repeatedPtrFieldEqual(lhs, rhs)) { + const Protobuf::RepeatedPtrField& rhs, + bool ignore_ordering = false) { + if (!repeatedPtrFieldEqual(lhs, rhs, ignore_ordering)) { return AssertionFailure() << RepeatedPtrUtil::debugString(lhs) << " does not match " << RepeatedPtrUtil::debugString(rhs); }