[mesos] 04/05: Fixed `AgentResourceProviderConfigApiTest.Update` for `vendor` field.

[ch...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

chhsiao pushed a commit to branch 1.7.x
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit 30dbd9644a9f8b7078b7e81618c87f1a82c02971
Author: Chun-Hung Hsiao <ch...@mesosphere.io>
AuthorDate: Thu Dec 6 20:20:46 2018 -0800

    Fixed `AgentResourceProviderConfigApiTest.Update` for `vendor` field.
    
    The introduction of the `Resource.DiskInfo.Source.vendor` changes the
    resource comparison and thus breaks this test. This patch fixes the test
    and make it more robust.
    
    NOTE: The updated test will fail unless the previous patch (which sets
    up the `vendor` field) is also applied.
    
    Review: https://reviews.apache.org/r/69521
---
 .../agent_resource_provider_config_api_tests.cpp   | 51 ++++++++++++++++------
 1 file changed, 38 insertions(+), 13 deletions(-)

diff --git a/src/tests/agent_resource_provider_config_api_tests.cpp b/src/tests/agent_resource_provider_config_api_tests.cpp
index e6a68ba..f55953e 100644
--- a/src/tests/agent_resource_provider_config_api_tests.cpp
+++ b/src/tests/agent_resource_provider_config_api_tests.cpp
@@ -85,18 +85,19 @@ public:
 
     // This extra closure is necessary in order to use `ASSERT_*`, as
     // these macros require a void return type.
-    [&]() {
-      // Randomize the plugin name so we get a clean work directory for
-      // each created config.
+    [&] {
+      // Randomize the plugin name so every created config uses its own CSI
+      // plugin instance. We use this to check if the config has been updated in
+      // some tests.
       const string testCsiPluginName =
-        "test_csi_plugin_" +
-        strings::remove(id::UUID::random().toString(), "-");
+        "local_" + strings::remove(id::UUID::random().toString(), "-");
 
       const string testCsiPluginPath =
         path::join(tests::flags.build_dir, "src", "test-csi-plugin");
 
       const string testCsiPluginWorkDir =
         path::join(sandbox.get(), testCsiPluginName);
+
       ASSERT_SOME(os::mkdir(testCsiPluginWorkDir));
 
       Try<string> resourceProviderConfig = strings::format(
@@ -508,10 +509,12 @@ TEST_P(AgentResourceProviderConfigApiTest, ROOT_Update)
   slaveFlags.resource_provider_config_dir = resourceProviderConfigDir;
 
   // Generate a pre-existing config.
+  const string volumes = "volume1:4GB";
   const string configPath = path::join(resourceProviderConfigDir, "test.json");
+
   ASSERT_SOME(os::write(
       configPath,
-      stringify(JSON::protobuf(createResourceProviderInfo("volume1:4GB")))));
+      stringify(JSON::protobuf(createResourceProviderInfo(volumes)))));
 
   Future<SlaveRegisteredMessage> slaveRegisteredMessage =
     FUTURE_PROTOBUF(SlaveRegisteredMessage(), _, _);
@@ -544,14 +547,22 @@ TEST_P(AgentResourceProviderConfigApiTest, ROOT_Update)
   Future<vector<Offer>> oldOffers;
 
   EXPECT_CALL(sched, resourceOffers(&driver, OffersHaveAnyResource(
-      std::bind(&Resources::hasResourceProvider, lambda::_1))))
+      &Resources::hasResourceProvider)))
     .WillOnce(FutureArg<1>(&oldOffers));
 
   driver.start();
 
   // Wait for an offer having the old provider resource.
   AWAIT_READY(oldOffers);
-  ASSERT_FALSE(oldOffers->empty());
+  ASSERT_EQ(1u, oldOffers->size());
+
+  Resource oldResource = *Resources(oldOffers->at(0).resources())
+    .filter(&Resources::hasResourceProvider)
+    .begin();
+
+  ASSERT_TRUE(oldResource.has_disk());
+  ASSERT_TRUE(oldResource.disk().has_source());
+  ASSERT_TRUE(oldResource.disk().source().has_vendor());
 
   Future<OfferID> rescinded;
 
@@ -564,7 +575,8 @@ TEST_P(AgentResourceProviderConfigApiTest, ROOT_Update)
       std::bind(&Resources::hasResourceProvider, lambda::_1))))
     .WillOnce(FutureArg<1>(&newOffers));
 
-  ResourceProviderInfo info = createResourceProviderInfo("volume1:2GB");
+  // Create a new config that serves the same volumes with a different vendor.
+  ResourceProviderInfo info = createResourceProviderInfo(volumes);
 
   AWAIT_EXPECT_RESPONSE_STATUS_EQ(
       http::OK().status,
@@ -585,6 +597,7 @@ TEST_P(AgentResourceProviderConfigApiTest, ROOT_Update)
 
   Try<ResourceProviderInfo> _info =
     ::protobuf::parse<ResourceProviderInfo>(json.get());
+
   ASSERT_SOME(_info);
   EXPECT_EQ(_info.get(), info);
 
@@ -593,10 +606,22 @@ TEST_P(AgentResourceProviderConfigApiTest, ROOT_Update)
 
   // Wait for an offer having the new provider resource.
   AWAIT_READY(newOffers);
-
-  // The new provider resource is smaller than the old provider resource.
-  EXPECT_FALSE(Resources(newOffers->at(0).resources()).contains(
-      oldOffers->at(0).resources()));
+  ASSERT_EQ(1u, newOffers->size());
+
+  Resource newResource = *Resources(newOffers->at(0).resources())
+    .filter(&Resources::hasResourceProvider)
+    .begin();
+
+  ASSERT_TRUE(newResource.has_disk());
+  ASSERT_TRUE(newResource.disk().has_source());
+  ASSERT_TRUE(newResource.disk().source().has_vendor());
+
+  // The resource from the new resource provider has the same provider ID but a
+  // different vendor as that from the old one.
+  EXPECT_EQ(oldResource.provider_id(), newResource.provider_id());
+  EXPECT_NE(
+      oldResource.disk().source().vendor(),
+      newResource.disk().source().vendor());
 }