You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by ch...@apache.org on 2018/11/29 18:31:28 UTC

[mesos] 04/14: Rewrote test `ConvertPreExistingVolume` for `CREATE_DISK`.

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

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

commit 4c9f1a05e35c70918bd21cf5f9b536efda1da28c
Author: Chun-Hung Hsiao <ch...@mesosphere.io>
AuthorDate: Sat Nov 10 16:56:36 2018 -0800

    Rewrote test `ConvertPreExistingVolume` for `CREATE_DISK`.
    
    Due to the changes of the `CREATE_DISK` semantics, this test is
    rewritten to convert a preprovisioned volume to a profile volumes, and
    then to destroy it to return the space back to the storage pool.
    
    NOTE: The updated test will fail unless r/69361 (which implements the
    new `CREATE_DISK` semantics) is also applied.
    
    Review: https://reviews.apache.org/r/69360
---
 .../storage_local_resource_provider_tests.cpp      | 243 +++++++++------------
 1 file changed, 104 insertions(+), 139 deletions(-)

diff --git a/src/tests/storage_local_resource_provider_tests.cpp b/src/tests/storage_local_resource_provider_tests.cpp
index 514e56d..e2a4d02 100644
--- a/src/tests/storage_local_resource_provider_tests.cpp
+++ b/src/tests/storage_local_resource_provider_tests.cpp
@@ -2651,51 +2651,35 @@ TEST_F(
 }
 
 
-// This test verifies that the storage local resource provider can
-// convert pre-existing CSI volumes into mount or block volumes.
-TEST_F(StorageLocalResourceProviderTest, ConvertPreExistingVolume)
+// This test verifies that the storage local resource provider can import a
+// preprovisioned CSI volume as a MOUNT disk of a given profile, and return the
+// space back to the storage pool after destroying the volume.
+TEST_F(StorageLocalResourceProviderTest, ImportPreprovisionedVolume)
 {
-  Clock::pause();
+  const string profilesPath = path::join(sandbox.get(), "profiles.json");
+  ASSERT_SOME(os::write(profilesPath, createDiskProfileMapping("test")));
 
-  setupResourceProviderConfig(Bytes(0), "volume1:2GB;volume2:2GB");
+  loadUriDiskProfileAdaptorModule(profilesPath);
 
-  master::Flags masterFlags = CreateMasterFlags();
-  Try<Owned<cluster::Master>> master = StartMaster(masterFlags);
+  // NOTE: We setup up the resource provider with an extra storage pool, so that
+  // when the storage pool is offered, we know that the corresponding profile is
+  // known to the resource provider.
+  setupResourceProviderConfig(Gigabytes(2), "volume1:2GB");
+
+  Try<Owned<cluster::Master>> master = StartMaster();
   ASSERT_SOME(master);
 
   Owned<MasterDetector> detector = master.get()->createDetector();
 
   slave::Flags slaveFlags = CreateSlaveFlags();
+  slaveFlags.disk_profile_adaptor = URI_DISK_PROFILE_ADAPTOR_NAME;
 
-  // 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 is reversed because
-  // Google Mock will search the expectations in reverse order.
-  Future<UpdateSlaveMessage> updateSlave2 =
-    FUTURE_PROTOBUF(UpdateSlaveMessage(), _, _);
-  Future<UpdateSlaveMessage> updateSlave1 =
-    FUTURE_PROTOBUF(UpdateSlaveMessage(), _, _);
+  Future<SlaveRegisteredMessage> slaveRegisteredMessage =
+    FUTURE_PROTOBUF(SlaveRegisteredMessage(), _, _);
 
   Try<Owned<cluster::Slave>> slave = StartSlave(detector.get(), slaveFlags);
   ASSERT_SOME(slave);
 
-  // Advance the clock to trigger agent registration and prevent retry.
-  Clock::advance(slaveFlags.registration_backoff_factor);
-
-  AWAIT_READY(updateSlave1);
-
-  // NOTE: We need to resume the clock so that the resource provider can
-  // periodically check if the CSI endpoint socket has been created by
-  // the plugin container, which runs in another Linux process.
-  Clock::resume();
-
-  AWAIT_READY(updateSlave2);
-  ASSERT_TRUE(updateSlave2->has_resource_providers());
-
-  Clock::pause();
-
   // Register a framework to exercise operations.
   FrameworkInfo framework = DEFAULT_FRAMEWORK_INFO;
   framework.set_roles(0, "storage");
@@ -2707,147 +2691,128 @@ TEST_F(StorageLocalResourceProviderTest, ConvertPreExistingVolume)
   EXPECT_CALL(sched, registered(&driver, _, _));
 
   // The framework is expected to see the following offers in sequence:
-  //   1. One containing two RAW pre-existing volumes before `CREATE_DISK`s.
-  //   2. One containing a MOUNT and a BLOCK disk resources after
-  //      `CREATE_DISK`s.
-  //   3. One containing two RAW pre-existing volumes after `DESTROY_DISK`s.
+  //   1. One containing a RAW preprovisioned volumes before `CREATE_DISK`.
+  //   2. One containing a MOUNT disk resources after `CREATE_DISK`.
+  //   3. One containing a RAW storage pool after `DESTROY_DISK`.
   //
   // We set up the expectations for these offers as the test progresses.
-  Future<vector<Offer>> rawDisksOffers;
-  Future<vector<Offer>> disksConvertedOffers;
-  Future<vector<Offer>> disksRevertedOffers;
+  Future<vector<Offer>> rawDiskOffers;
+  Future<vector<Offer>> diskCreatedOffers;
+  Future<vector<Offer>> diskDestroyedOffers;
 
-  // We are only interested in any pre-existing volume, which has an ID
-  // but no profile.
-  auto isPreExistingVolume = [](const Resource& r) {
+  // We use the following filter to filter offers that do not have
+  // wanted resources for 365 days (the maximum).
+  Filters declineFilters;
+  declineFilters.set_refuse_seconds(Days(365).secs());
+
+  // Decline offers that contain only the agent's default resources.
+  EXPECT_CALL(sched, resourceOffers(&driver, _))
+    .WillRepeatedly(DeclineOffers(declineFilters));
+
+  auto isStoragePool = [](const Resource& r, const string& profile) {
     return r.has_disk() &&
       r.disk().has_source() &&
-      r.disk().source().has_id() &&
-      !r.disk().source().has_profile();
+      r.disk().source().type() == Resource::DiskInfo::Source::RAW &&
+      !r.disk().source().has_id() &&
+      r.disk().source().has_profile() &&
+      r.disk().source().profile() == profile;
   };
 
+  // NOTE: Instead of expecting a preprovisioned volume, we expect an offer with
+  // a 'test1' storage pool as an indication that the profile is known to the
+  // resource provider. The offer should also have the preprovisioned volume.
+  // But, an extra offer with the storage pool may be received as a side effect
+  // of this workaround, so we decline it if this happens.
   EXPECT_CALL(sched, resourceOffers(&driver, OffersHaveAnyResource(
-      isPreExistingVolume)))
-    .WillOnce(FutureArg<1>(&rawDisksOffers));
+      std::bind(isStoragePool, lambda::_1, "test"))))
+    .WillOnce(FutureArg<1>(&rawDiskOffers))
+    .WillRepeatedly(DeclineOffers(declineFilters));
 
   driver.start();
 
-  AWAIT_READY(rawDisksOffers);
-  ASSERT_FALSE(rawDisksOffers->empty());
-
-  vector<Resource> sources;
-
-  foreach (const Resource& resource, rawDisksOffers->at(0).resources()) {
-    if (isPreExistingVolume(resource) &&
-        resource.disk().source().type() == Resource::DiskInfo::Source::RAW) {
-      sources.push_back(resource);
-    }
-  }
-
-  ASSERT_EQ(2u, sources.size());
-
-  // Create a volume and a block.
-  EXPECT_CALL(sched, resourceOffers(&driver, OffersHaveAnyResource(
-      isPreExistingVolume)))
-    .WillOnce(FutureArg<1>(&disksConvertedOffers));
+  AWAIT_READY(rawDiskOffers);
+  ASSERT_EQ(1u, rawDiskOffers->size());
 
-  // NOTE: The order of the two `FUTURE_PROTOBUF`s is reversed because
-  // Google Mock will search the expectations in reverse order.
-  Future<UpdateOperationStatusMessage> createBlockStatusUpdate =
-    FUTURE_PROTOBUF(UpdateOperationStatusMessage(), _, _);
-  Future<UpdateOperationStatusMessage> createVolumeStatusUpdate =
-    FUTURE_PROTOBUF(UpdateOperationStatusMessage(), _, _);
+  auto isPreprovisionedVolume = [](const Resource& r) {
+    return r.has_disk() &&
+      r.disk().has_source() &&
+      r.disk().source().type() == Resource::DiskInfo::Source::RAW &&
+      r.disk().source().has_id() &&
+      !r.disk().source().has_profile();
+  };
 
-  driver.acceptOffers(
-      {rawDisksOffers->at(0).id()},
-      {CREATE_DISK(sources.at(0), Resource::DiskInfo::Source::MOUNT),
-       CREATE_DISK(sources.at(1), Resource::DiskInfo::Source::BLOCK)});
+  Resources _preprovisioned = Resources(rawDiskOffers->at(0).resources())
+    .filter(isPreprovisionedVolume);
 
-  AWAIT_READY(createVolumeStatusUpdate);
-  AWAIT_READY(createBlockStatusUpdate);
+  ASSERT_SOME_EQ(Gigabytes(2), _preprovisioned.disk());
 
-  // Advance the clock to trigger another allocation.
-  Clock::advance(masterFlags.allocation_interval);
+  Resource preprovisioned = *_preprovisioned.begin();
 
-  AWAIT_READY(disksConvertedOffers);
-  ASSERT_FALSE(disksConvertedOffers->empty());
+  // Get the volume path of the preprovisioned volume.
+  Option<string> volumePath;
 
-  Option<Resource> volume;
-  Option<Resource> block;
-
-  foreach (const Resource& resource, disksConvertedOffers->at(0).resources()) {
-    if (isPreExistingVolume(resource)) {
-      if (resource.disk().source().type() ==
-            Resource::DiskInfo::Source::MOUNT) {
-        volume = resource;
-      } else if (resource.disk().source().type() ==
-                   Resource::DiskInfo::Source::BLOCK) {
-        block = resource;
-      }
+  foreach (const Label& label,
+           preprovisioned.disk().source().metadata().labels()) {
+    if (label.key() == "path") {
+      volumePath = label.value();
+      break;
     }
   }
 
-  ASSERT_SOME(volume);
-  ASSERT_TRUE(volume->disk().source().has_mount());
-  ASSERT_TRUE(volume->disk().source().mount().has_root());
-  EXPECT_FALSE(path::absolute(volume->disk().source().mount().root()));
+  ASSERT_SOME(volumePath);
+  ASSERT_TRUE(os::exists(volumePath.get()));
 
-  ASSERT_SOME(block);
+  auto isMountDisk = [](const Resource& r, const string& profile) {
+    return r.has_disk() &&
+      r.disk().has_source() &&
+      r.disk().source().type() == Resource::DiskInfo::Source::MOUNT &&
+      r.disk().source().has_id() &&
+      r.disk().source().has_profile() &&
+      r.disk().source().profile() == profile;
+  };
 
-  // Destroy the created volume.
+  // Apply profile 'test' to the preprovisioned volume.
   EXPECT_CALL(sched, resourceOffers(&driver, OffersHaveAnyResource(
-      isPreExistingVolume)))
-    .WillOnce(FutureArg<1>(&disksRevertedOffers));
+      std::bind(isMountDisk, lambda::_1, "test"))))
+    .WillOnce(FutureArg<1>(&diskCreatedOffers));
 
-  // NOTE: The order of the two `FUTURE_PROTOBUF`s is reversed because
-  // Google Mock will search the expectations in reverse order.
-  Future<UpdateOperationStatusMessage> destroyBlockStatusUpdate =
-    FUTURE_PROTOBUF(UpdateOperationStatusMessage(), _, _);
-  Future<UpdateOperationStatusMessage> destroyVolumeStatusUpdate =
-    FUTURE_PROTOBUF(UpdateOperationStatusMessage(), _, _);
+  // We use the following filter so that the resources will not be
+  // filtered for 5 seconds (the default).
+  Filters acceptFilters;
+  acceptFilters.set_refuse_seconds(0);
 
   driver.acceptOffers(
-      {disksConvertedOffers->at(0).id()},
-      {DESTROY_DISK(volume.get()),
-       DESTROY_DISK(block.get())});
-
-  AWAIT_READY(destroyVolumeStatusUpdate);
-  AWAIT_READY(destroyBlockStatusUpdate);
-
-  // Advance the clock to trigger another allocation.
-  Clock::advance(masterFlags.allocation_interval);
+      {rawDiskOffers->at(0).id()},
+      {CREATE_DISK(preprovisioned, Resource::DiskInfo::Source::MOUNT, "test")},
+      acceptFilters);
 
-  AWAIT_READY(disksRevertedOffers);
-  ASSERT_FALSE(disksRevertedOffers->empty());
+  AWAIT_READY(diskCreatedOffers);
+  ASSERT_EQ(1u, diskCreatedOffers->size());
 
-  vector<Resource> destroyed;
+  Resource created = *Resources(diskCreatedOffers->at(0).resources())
+    .filter(std::bind(isMountDisk, lambda::_1, "test"))
+    .begin();
 
-  foreach (const Resource& resource, disksRevertedOffers->at(0).resources()) {
-    if (isPreExistingVolume(resource) &&
-        resource.disk().source().type() == Resource::DiskInfo::Source::RAW) {
-      destroyed.push_back(resource);
-    }
-  }
+  // Destroy the created disk.
+  EXPECT_CALL(sched, resourceOffers(&driver, OffersHaveAnyResource(
+      std::bind(isStoragePool, lambda::_1, "test"))))
+    .WillOnce(FutureArg<1>(&diskDestroyedOffers));
 
-  ASSERT_EQ(2u, destroyed.size());
+  driver.acceptOffers(
+      {diskCreatedOffers->at(0).id()},
+      {DESTROY_DISK(created)},
+      acceptFilters);
 
-  foreach (const Resource& resource, destroyed) {
-    ASSERT_FALSE(resource.disk().source().has_mount());
-    ASSERT_TRUE(resource.disk().source().has_metadata());
+  AWAIT_READY(diskDestroyedOffers);
+  ASSERT_EQ(1u, diskDestroyedOffers->size());
 
-    // Check if the volume is not deleted by the test CSI plugin.
-    Option<string> volumePath;
+  Resources raw = Resources(diskDestroyedOffers->at(0).resources())
+    .filter(std::bind(isStoragePool, lambda::_1, "test"));
 
-    foreach (const Label& label, resource.disk().source().metadata().labels()) {
-      if (label.key() == "path") {
-        volumePath = label.value();
-        break;
-      }
-    }
+  EXPECT_SOME_EQ(Gigabytes(4), raw.disk());
 
-    ASSERT_SOME(volumePath);
-    EXPECT_TRUE(os::exists(volumePath.get()));
-  }
+  // Check if the volume is deleted by the test CSI plugin.
+  EXPECT_FALSE(os::exists(volumePath.get()));
 }