test: deflaking */LoadStatsIntegrationTest.LocalityWeighted/*

source/common/protobuf/utility.h

@@ -142,15 +142,15 @@ 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 <typename ProtoType>
-  static ProtobufTypes::ConstMessagePtrVector
-  convertToConstMessagePtrVector(const Protobuf::RepeatedPtrField<ProtoType>& repeated_field) {
-    ProtobufTypes::ConstMessagePtrVector ret_vector;
+  template <typename ProtoType, typename ReturnType>
+  static ReturnType
+  convertToConstMessagePtrContainer(const Protobuf::RepeatedPtrField<ProtoType>& repeated_field) {
+    ReturnType ret_vector;
     std::transform(repeated_field.begin(), repeated_field.end(), std::back_inserter(ret_vector),
                    [](const ProtoType& proto_message) -> std::unique_ptr<const Protobuf::Message> {
                      Protobuf::Message* clone = proto_message.New();

source/common/router/scoped_rds.cc

@@ -36,7 +36,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<envoy::api::v2::ScopedRouteConfiguration,
+                                                           ProtobufTypes::ConstMessagePtrVector>(
             scoped_route_list.scoped_route_configurations()),
         factory_context,
         ScopedRoutesConfigProviderManagerOptArg(config.scoped_routes().name(),

test/integration/load_stats_integration_test.cc

@@ -191,15 +191,13 @@ class LoadStatsIntegrationTest : public testing::TestWithParam<Network::Address:
           upstream_locality_stats->set_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::TestWithParam<Network::Address:
         upstream_locality_stats->CopyFrom(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::TestWithParam<Network::Address:
       EXPECT_EQ("application/grpc",
                 loadstats_stream_->headers().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) {
@@ -461,22 +478,16 @@ TEST_P(LoadStatsIntegrationTest, LocalityWeighted) {
   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 +497,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.

test/test_common/utility.h

@@ -265,30 +265,59 @@ class TestUtility {
    *
    * @param lhs RepeatedPtrField on LHS.
    * @param rhs RepeatedPtrField on RHS.
+   * @param ignore_repeated_field_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 <class ProtoType>
+  template <typename ProtoType>
   static bool repeatedPtrFieldEqual(const Protobuf::RepeatedPtrField<ProtoType>& lhs,
-                                    const Protobuf::RepeatedPtrField<ProtoType>& rhs) {
+                                    const Protobuf::RepeatedPtrField<ProtoType>& 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<std::unique_ptr<const Protobuf::Message>> 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<ProtoType, ProtoList>(lhs);
+    ProtoList rhs_list =
+        RepeatedPtrUtil::convertToConstMessagePtrContainer<ProtoType, ProtoList>(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 <class ProtoType>
   static AssertionResult
   assertRepeatedPtrFieldEqual(const Protobuf::RepeatedPtrField<ProtoType>& lhs,
-                              const Protobuf::RepeatedPtrField<ProtoType>& rhs) {
-    if (!repeatedPtrFieldEqual(lhs, rhs)) {
+                              const Protobuf::RepeatedPtrField<ProtoType>& rhs,
+                              bool ignore_ordering = false) {
+    if (!repeatedPtrFieldEqual(lhs, rhs, ignore_ordering)) {
       return AssertionFailure() << RepeatedPtrUtil::debugString(lhs) << " does not match "
                                 << RepeatedPtrUtil::debugString(rhs);
     }