From df142de6a76f5463012c20fbf8b1baf1e621d998 Mon Sep 17 00:00:00 2001
From: Flavio Crisciani <f.crisciani@gmail.com>
Date: Thu, 22 Aug 2019 18:48:48 +0000
Subject: [PATCH] Enhance comment about MonotonicTime

Depending on the execution environment in which envoy is being run, it
is possible that some of the assumption on the clock are maybe not
holding as previously commented. With some sandboxing technologies the
clock does not reference the machine boot time but the sandbox boot
time. This invalidates the assumtpion that the first update in the
cluster_manager will most likely fall out of the windows and ends up
showing a non intuitive behavior difficult to catch.
This PR simply adds a comment that will allow the reader to consider
this option while reading to the code.

Signed-off-by: Flavio Crisciani <f.crisciani@gmail.com>
---
 api/envoy/api/v2/cds.proto                    |  3 ++-
 source/common/upstream/cluster_manager_impl.h | 10 +++++++---
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/api/envoy/api/v2/cds.proto b/api/envoy/api/v2/cds.proto
index 647a5015063f..6f98e1bcc6e2 100644
--- a/api/envoy/api/v2/cds.proto
+++ b/api/envoy/api/v2/cds.proto
@@ -553,7 +553,8 @@ message Cluster {
     // the first update happens. This is useful for big clusters, with potentially noisy deploys
     // that might trigger excessive CPU usage due to a constant stream of healthcheck state changes
     // or metadata updates. The first set of updates to be seen apply immediately (e.g.: a new
-    // cluster).
+    // cluster). Please always keep in mind that the use of sandbox technologies may change this
+    // behavior.
     //
     // If this is not set, we default to a merge window of 1000ms. To disable it, set the merge
     // window to 0.
diff --git a/source/common/upstream/cluster_manager_impl.h b/source/common/upstream/cluster_manager_impl.h
index ae0ecb2dbd40..f00088deb7a7 100644
--- a/source/common/upstream/cluster_manager_impl.h
+++ b/source/common/upstream/cluster_manager_impl.h
@@ -419,9 +419,13 @@ class ClusterManagerImpl : public ClusterManager, Logger::Loggable<Logger::Id::u
     // This is default constructed to the clock's epoch:
     // https://en.cppreference.com/w/cpp/chrono/time_point/time_point
     //
-    // This will usually be the computer's boot time, which means that given a not very large
-    // `Cluster.CommonLbConfig.update_merge_window`, the first update will trigger immediately
-    // (the expected behavior).
+    // Depending on your execution environment this value can be different.
+    // When running as host process: This will usually be the computer's boot time, which means that
+    // given a not very large `Cluster.CommonLbConfig.update_merge_window`, the first update will
+    // trigger immediately (the expected behavior). When running in some sandboxed environment this
+    // value can be set to the start time of the sandbox, which means that the delta calculated
+    // between now and the start time may fall within the
+    // `Cluster.CommonLbConfig.update_merge_window`, with the side effect to delay the first update.
     MonotonicTime last_updated_;
   };