You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by ji...@apache.org on 2018/02/08 01:25:58 UTC

[08/12] mesos git commit: Fixed the flakiness in the SLRP metrics test.

Fixed the flakiness in the SLRP metrics test.

The metrics test unnecessarily set up the disk profile module without
providing the profile config file. This may cause the profile module to
pull a non-existent file. Also, it could kill the plugin before SLRP
connecting to the endpoint, making SLRP wait for 1 minute.

Review: https://reviews.apache.org/r/65523/


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/59c7e69c
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/59c7e69c
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/59c7e69c

Branch: refs/heads/master
Commit: 59c7e69c9bc4bc5dfde46535479f61dbc03c08b0
Parents: 81b46f9
Author: Chun-Hung Hsiao <ch...@mesosphere.io>
Authored: Wed Feb 7 16:04:09 2018 -0800
Committer: Jie Yu <yu...@gmail.com>
Committed: Wed Feb 7 16:04:09 2018 -0800

----------------------------------------------------------------------
 .../storage_local_resource_provider_tests.cpp   | 21 ++++++--------------
 1 file changed, 6 insertions(+), 15 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/59c7e69c/src/tests/storage_local_resource_provider_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/storage_local_resource_provider_tests.cpp b/src/tests/storage_local_resource_provider_tests.cpp
index ae1dc2c..fcac4c4 100644
--- a/src/tests/storage_local_resource_provider_tests.cpp
+++ b/src/tests/storage_local_resource_provider_tests.cpp
@@ -2952,8 +2952,6 @@ TEST_F(
 // properly reported.
 TEST_F(StorageLocalResourceProviderTest, ROOT_Metrics)
 {
-  loadUriDiskProfileModule();
-
   setupResourceProviderConfig(Gigabytes(4));
 
   Try<Owned<cluster::Master>> master = StartMaster();
@@ -2978,7 +2976,6 @@ TEST_F(StorageLocalResourceProviderTest, ROOT_Metrics)
       {capabilities.begin(), capabilities.end()});
 
   slaveFlags.resource_provider_config_dir = resourceProviderConfigDir;
-  slaveFlags.disk_profile_adaptor = URI_DISK_PROFILE_ADAPTOR_NAME;
 
   slave::Fetcher fetcher(slaveFlags);
 
@@ -2989,16 +2986,8 @@ TEST_F(StorageLocalResourceProviderTest, ROOT_Metrics)
 
   Owned<slave::MesosContainerizer> containerizer(_containerizer.get());
 
-  // Since the local resource provider daemon is started after the agent
-  // is registered, it is guaranteed that the slave will send two
-  // `UpdateSlaveMessage`s, where the latter one contains resources from
-  // the storage local resource provider.
-  // NOTE: The order of the two `FUTURE_PROTOBUF`s are reversed because
-  // Google Mock will search the expectations in reverse order.
-  Future<UpdateSlaveMessage> updateSlave2 =
-    FUTURE_PROTOBUF(UpdateSlaveMessage(), _, _);
-  Future<UpdateSlaveMessage> updateSlave1 =
-    FUTURE_PROTOBUF(UpdateSlaveMessage(), _, _);
+  Future<Nothing> pluginConnected =
+    FUTURE_DISPATCH(_, &ContainerDaemonProcess::waitContainer);
 
   Try<Owned<cluster::Slave>> slave = StartSlave(
       detector.get(),
@@ -3007,8 +2996,7 @@ TEST_F(StorageLocalResourceProviderTest, ROOT_Metrics)
 
   ASSERT_SOME(slave);
 
-  AWAIT_READY(updateSlave1);
-  AWAIT_READY(updateSlave2);
+  AWAIT_READY(pluginConnected);
 
   const string prefix =
     "resource_providers/" + stringify(TEST_SLRP_TYPE) +
@@ -3037,6 +3025,9 @@ TEST_F(StorageLocalResourceProviderTest, ROOT_Metrics)
     FUTURE_DISPATCH(_, &ContainerDaemonProcess::launchContainer);
 
   // Kill the plugin container and wait for it to restart.
+  // NOTE: We need to wait for `pluginConnected` before issuing the
+  // kill, or it may kill the plugin before it created the endpoint
+  // socket and the resource provider would wait for one minute.
   Future<int> pluginKilled = containerizer->status(pluginContainerId)
     .then([](const ContainerStatus& status) {
       return os::kill(status.executor_pid(), SIGKILL);