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 24c70aa15c566b6f15a3f3636664507bae3af3b6
Author: Chun-Hung Hsiao <chhs...@mesosphere.io>
AuthorDate: Thu Apr 18 12:50:54 2019 -0700

    Used full paths as volume IDs for the test CSI plugin.
    
    The full paths of simulated volumes are now in their ID instead of their
    contextual information. This simplifies SLRP tests, and makes it cleaner
    if we want to customize the contextual information in the future.
    
    Review: https://reviews.apache.org/r/70621
---
 src/examples/test_csi_plugin.cpp                   | 245 ++++++++++++---------
 .../storage_local_resource_provider_tests.cpp      | 170 +++-----------
 2 files changed, 172 insertions(+), 243 deletions(-)

diff --git a/src/examples/test_csi_plugin.cpp b/src/examples/test_csi_plugin.cpp
index 03f782e..7ff08b8 100644
--- a/src/examples/test_csi_plugin.cpp
+++ b/src/examples/test_csi_plugin.cpp
@@ -42,6 +42,7 @@
 #include <stout/bytes.hpp>
 #include <stout/flags.hpp>
 #include <stout/foreach.hpp>
+#include <stout/fs.hpp>
 #include <stout/hashmap.hpp>
 #include <stout/none.hpp>
 #include <stout/option.hpp>
@@ -59,13 +60,14 @@
 
 #include "csi/v0_utils.hpp"
 #include "csi/v1_utils.hpp"
+#include "csi/volume_manager.hpp"
 
 #include "linux/fs.hpp"
 
 #include "logging/logging.hpp"
 
 namespace http = process::http;
-namespace fs = mesos::internal::fs;
+namespace internal = mesos::internal;
 
 using std::cerr;
 using std::cout;
@@ -95,6 +97,8 @@ using grpc::ServerContext;
 using grpc::Status;
 using grpc::WriteOptions;
 
+using mesos::csi::VolumeInfo;
+
 using mesos::csi::types::VolumeCapability;
 
 using process::grpc::StatusError;
@@ -192,41 +196,46 @@ public:
     defaultVolumeCapability.mutable_access_mode()
       ->set_mode(VolumeCapability::AccessMode::SINGLE_NODE_WRITER);
 
-    // Scan for preprovisioned volumes.
+    Bytes usedCapacity(0);
+
+    // Scan for created volumes.
     //
     // TODO(jieyu): Consider not using CHECKs here.
-    Try<list<string>> paths = os::ls(workDir);
-    CHECK_SOME(paths);
-
-    foreach (const string& path, paths.get()) {
-      Try<VolumeInfo> volumeInfo = parseVolumePath(path);
-      CHECK_SOME(volumeInfo);
-
-      CHECK(!volumes.contains(volumeInfo->id));
-      volumes.put(volumeInfo->id, volumeInfo.get());
-
-      if (!_volumes.contains(volumeInfo->id)) {
-        CHECK_GE(availableCapacity, volumeInfo->size);
-        availableCapacity -= volumeInfo->size;
-      }
+    Try<list<string>> paths = fs::list(path::join(workDir, "*-*"));
+    foreach (const string& path, CHECK_NOTERROR(paths)) {
+      volumes.put(path, CHECK_NOTERROR(parseVolumePath(path)));
+      usedCapacity += volumes.at(path).capacity;
     }
 
+    // Create preprovisioned volumes if they have not existed yet.
     foreachpair (const string& name, const Bytes& capacity, _volumes) {
-      if (volumes.contains(name)) {
+      Option<VolumeInfo> found = findVolumeByName(name);
+
+      if (found.isSome()) {
+        CHECK_EQ(found->capacity, capacity)
+          << "Expected preprovisioned volume '" << name << "' to be "
+          << capacity << " but found " << found->capacity << " instead";
+
+        usedCapacity -= found->capacity;
         continue;
       }
 
-      VolumeInfo volumeInfo;
-      volumeInfo.id = name;
-      volumeInfo.size = capacity;
-
-      const string path = getVolumePath(volumeInfo);
+      VolumeInfo volumeInfo{
+        capacity, getVolumePath(capacity, name), Map<string, string>()};
 
-      Try<Nothing> mkdir = os::mkdir(path);
-      CHECK_SOME(mkdir);
+      Try<Nothing> mkdir = os::mkdir(volumeInfo.id);
+      CHECK_SOME(mkdir)
+        << "Failed to create directory for preprovisioned volume '" << name
+        << "': " << mkdir.error();
 
-      volumes.put(volumeInfo.id, volumeInfo);
+      volumes.put(volumeInfo.id, std::move(volumeInfo));
     }
+
+    CHECK_GE(availableCapacity, usedCapacity)
+      << "Insufficient available capacity for volumes, expected to be at least 
"
+      << usedCapacity;
+
+    availableCapacity -= usedCapacity;
   }
 
   void run();
@@ -406,14 +415,9 @@ public:
       csi::v1::NodeGetInfoResponse* response) override;
 
 private:
-  struct VolumeInfo
-  {
-    string id;
-    Bytes size;
-  };
-
-  string getVolumePath(const VolumeInfo& volumeInfo);
-  Try<VolumeInfo> parseVolumePath(const string& path);
+  string getVolumePath(const Bytes& capacity, const string& name);
+  Try<VolumeInfo> parseVolumePath(const string& dir);
+  Option<VolumeInfo> findVolumeByName(const string& name);
 
   Try<VolumeInfo, StatusError> createVolume(
       const string& name,
@@ -568,9 +572,8 @@ Status TestCSIPlugin::CreateVolume(
   }
 
   response->mutable_volume()->set_id(result->id);
-  response->mutable_volume()->set_capacity_bytes(result->size.bytes());
-  (*response->mutable_volume()->mutable_attributes())["path"] =
-    getVolumePath(result.get());
+  response->mutable_volume()->set_capacity_bytes(result->capacity.bytes());
+  *response->mutable_volume()->mutable_attributes() = result->context;
 
   return Status::OK;
 }
@@ -688,8 +691,8 @@ Status TestCSIPlugin::ListVolumes(
   foreach (const VolumeInfo& volumeInfo, result.get()) {
     csi::v0::Volume* volume = response->add_entries()->mutable_volume();
     volume->set_id(volumeInfo.id);
-    volume->set_capacity_bytes(volumeInfo.size.bytes());
-    (*volume->mutable_attributes())["path"] = getVolumePath(volumeInfo);
+    volume->set_capacity_bytes(volumeInfo.capacity.bytes());
+    *volume->mutable_attributes() = volumeInfo.context;
   }
 
   return Status::OK;
@@ -925,9 +928,8 @@ Status TestCSIPlugin::CreateVolume(
   }
 
   response->mutable_volume()->set_volume_id(result->id);
-  response->mutable_volume()->set_capacity_bytes(result->size.bytes());
-  (*response->mutable_volume()->mutable_volume_context())["path"] =
-    getVolumePath(result.get());
+  response->mutable_volume()->set_capacity_bytes(result->capacity.bytes());
+  *response->mutable_volume()->mutable_volume_context() = result->context;
 
   return Status::OK;
 }
@@ -1050,8 +1052,8 @@ Status TestCSIPlugin::ListVolumes(
   foreach (const VolumeInfo& volumeInfo, result.get()) {
     csi::v1::Volume* volume = response->add_entries()->mutable_volume();
     volume->set_volume_id(volumeInfo.id);
-    volume->set_capacity_bytes(volumeInfo.size.bytes());
-    (*volume->mutable_volume_context())["path"] = getVolumePath(volumeInfo);
+    volume->set_capacity_bytes(volumeInfo.capacity.bytes());
+    *volume->mutable_volume_context() = volumeInfo.context;
   }
 
   return Status::OK;
@@ -1079,11 +1081,9 @@ Status TestCSIPlugin::GetCapacity(
 }
 
 
-string TestCSIPlugin::getVolumePath(const VolumeInfo& volumeInfo)
+string TestCSIPlugin::getVolumePath(const Bytes& capacity, const string& name)
 {
-  return path::join(
-      workDir,
-      strings::join("-", stringify(volumeInfo.size), volumeInfo.id));
+  return path::join(workDir, strings::join("-", capacity, http::encode(name)));
 }
 
 
@@ -1250,31 +1250,62 @@ Status TestCSIPlugin::NodeGetInfo(
 }
 
 
-Try<TestCSIPlugin::VolumeInfo> TestCSIPlugin::parseVolumePath(
-    const string& path)
+Try<VolumeInfo> TestCSIPlugin::parseVolumePath(const string& dir)
 {
-  size_t pos = path.find_first_of("-");
-  if (pos == string::npos) {
-    return Error("Cannot find the delimiter");
+  // TODO(chhsiao): Consider using `<regex>`, which requires GCC 4.9+.
+
+  // Make sure there's a separator at the end of the prefix so that we
+  // don't accidentally slice off part of a directory.
+  const string prefix = path::join(workDir, "");
+
+  if (!strings::startsWith(dir, prefix)) {
+    return Error(
+        "Directory '" + dir + "' does not fall under work directory '" +
+        prefix + "'");
+  }
+
+  const string basename = Path(dir).basename();
+
+  vector<string> tokens = strings::tokenize(basename, "-", 2);
+  if (tokens.size() != 2) {
+    return Error("Cannot find delimiter '-' in '" + basename + "'");
   }
 
-  string bytesString = path.substr(0, path.find_first_of("-"));
-  string id = path.substr(path.find_first_of("-") + 1);
+  Try<Bytes> capacity = Bytes::parse(tokens[0]);
+  if (capacity.isError()) {
+    return Error(
+        "Failed to parse capacity from '" + tokens[0] +
+        "': " + capacity.error());
+  }
 
-  Try<Bytes> bytes = Bytes::parse(bytesString);
-  if (bytes.isError()) {
-    return Error("Failed to parse bytes: " + bytes.error());
+  Try<string> name = http::decode(tokens[1]);
+  if (name.isError()) {
+    return Error(
+        "Failed to decode volume name from '" + tokens[1] +
+        "': " + name.error());
   }
 
-  VolumeInfo volumeInfo;
-  volumeInfo.id = id;
-  volumeInfo.size = bytes.get();
+  CHECK_EQ(dir, getVolumePath(capacity.get(), name.get()))
+    << "Cannot reconstruct volume path '" << dir << "' from volume name '"
+    << name.get() << "' and capacity " << capacity.get();
 
-  return volumeInfo;
+  return VolumeInfo{capacity.get(), dir, Map<string, string>()};
 }
 
 
-Try<TestCSIPlugin::VolumeInfo, StatusError> TestCSIPlugin::createVolume(
+Option<VolumeInfo> TestCSIPlugin::findVolumeByName(const string& name)
+{
+  foreachvalue (const VolumeInfo& volumeInfo, volumes) {
+    if (volumeInfo.id == getVolumePath(volumeInfo.capacity, name)) {
+      return volumeInfo;
+    }
+  }
+
+  return None();
+}
+
+
+Try<VolumeInfo, StatusError> TestCSIPlugin::createVolume(
     const string& name,
     const Bytes& requiredBytes,
     const Bytes& limitBytes,
@@ -1296,43 +1327,42 @@ Try<TestCSIPlugin::VolumeInfo, StatusError> 
TestCSIPlugin::createVolume(
         grpc::INVALID_ARGUMENT, "Unsupported create parameters"));
   }
 
-  if (volumes.contains(volumeId)) {
-    const VolumeInfo& volumeInfo = volumes.at(volumeId);
+  Option<VolumeInfo> found = findVolumeByName(name);
 
-    if (volumeInfo.size > limitBytes) {
+  if (found.isSome()) {
+    if (found->capacity > limitBytes) {
       return StatusError(Status(
           grpc::ALREADY_EXISTS, "Cannot satisfy limit bytes"));
     }
 
-    if (volumeInfo.size < requiredBytes) {
+    if (found->capacity < requiredBytes) {
       return StatusError(Status(
           grpc::ALREADY_EXISTS, "Cannot satisfy required bytes"));
     }
 
-    return volumeInfo;
+    return *found;
   } else {
     if (availableCapacity < requiredBytes) {
       return StatusError(Status(grpc::OUT_OF_RANGE, "Insufficient capacity"));
     }
 
-    VolumeInfo volumeInfo;
-    volumeInfo.id = volumeId;
 
     // We assume that `requiredBytes <= limitBytes` has been verified.
     const Bytes defaultSize = min(availableCapacity, DEFAULT_VOLUME_CAPACITY);
-    volumeInfo.size = min(max(defaultSize, requiredBytes), limitBytes);
 
-    const string path = getVolumePath(volumeInfo);
+    VolumeInfo volumeInfo{min(max(defaultSize, requiredBytes), limitBytes),
+                          getVolumePath(volumeInfo.capacity, name),
+                          Map<string, string>()};
 
-    Try<Nothing> mkdir = os::mkdir(path);
+    Try<Nothing> mkdir = os::mkdir(volumeInfo.id);
     if (mkdir.isError()) {
       return StatusError(Status(
           grpc::INTERNAL,
           "Failed to create volume '" + volumeInfo.id + "': " + 
mkdir.error()));
     }
 
-    CHECK_GE(availableCapacity, volumeInfo.size);
-    availableCapacity -= volumeInfo.size;
+    CHECK_GE(availableCapacity, volumeInfo.capacity);
+    availableCapacity -= volumeInfo.capacity;
     volumes.put(volumeInfo.id, volumeInfo);
 
     return volumeInfo;
@@ -1350,16 +1380,15 @@ Try<Nothing, StatusError> 
TestCSIPlugin::deleteVolume(const string& volumeId)
   }
 
   const VolumeInfo& volumeInfo = volumes.at(volumeId);
-  const string path = getVolumePath(volumeInfo);
 
-  Try<Nothing> rmdir = os::rmdir(path);
+  Try<Nothing> rmdir = os::rmdir(volumeInfo.id);
   if (rmdir.isError()) {
     return StatusError(Status(
         grpc::INTERNAL,
         "Failed to delete volume '" + volumeId + "': " + rmdir.error()));
   }
 
-  availableCapacity += volumeInfo.size;
+  availableCapacity += volumeInfo.capacity;
   volumes.erase(volumeInfo.id);
 
   return Nothing();
@@ -1394,9 +1423,8 @@ Try<Nothing, StatusError> 
TestCSIPlugin::controllerPublishVolume(
   }
 
   const VolumeInfo& volumeInfo = volumes.at(volumeId);
-  const string path = getVolumePath(volumeInfo);
 
-  if (!volumeContext.count("path") || volumeContext.at("path") != path) {
+  if (volumeContext != volumeInfo.context) {
     return StatusError(Status(
         grpc::INVALID_ARGUMENT, "Invalid volume context"));
   }
@@ -1436,9 +1464,8 @@ Try<Option<Error>, StatusError> 
TestCSIPlugin::validateVolumeCapabilities(
   }
 
   const VolumeInfo& volumeInfo = volumes.at(volumeId);
-  const string path = getVolumePath(volumeInfo);
 
-  if (!volumeContext.count("path") || volumeContext.at("path") != path) {
+  if (volumeContext != volumeInfo.context) {
     return StatusError(Status(
         grpc::INVALID_ARGUMENT, "Invalid volume context"));
   }
@@ -1457,9 +1484,8 @@ Try<Option<Error>, StatusError> 
TestCSIPlugin::validateVolumeCapabilities(
 }
 
 
-Try<vector<TestCSIPlugin::VolumeInfo>, StatusError> TestCSIPlugin::listVolumes(
-    const Option<int32_t>& maxEntries,
-    const Option<string>& startingToken)
+Try<vector<VolumeInfo>, StatusError> TestCSIPlugin::listVolumes(
+    const Option<int32_t>& maxEntries, const Option<string>& startingToken)
 {
   // TODO(chhsiao): Support max entries.
   if (maxEntries.isSome()) {
@@ -1527,14 +1553,15 @@ Try<Nothing, StatusError> 
TestCSIPlugin::nodeStageVolume(
   }
 
   const VolumeInfo& volumeInfo = volumes.at(volumeId);
-  const string path = getVolumePath(volumeInfo);
 
-  if (!volumeContext.count("path") || volumeContext.at("path") != path) {
+  if (volumeContext != volumeInfo.context) {
     return StatusError(Status(
         grpc::INVALID_ARGUMENT, "Invalid volume context"));
   }
 
-  Try<fs::MountInfoTable> table = fs::MountInfoTable::read();
+  Try<internal::fs::MountInfoTable> table =
+    internal::fs::MountInfoTable::read();
+
   if (table.isError()) {
     return StatusError(Status(
         grpc::INTERNAL, "Failed to get mount table: " + table.error()));
@@ -1543,17 +1570,19 @@ Try<Nothing, StatusError> 
TestCSIPlugin::nodeStageVolume(
   if (std::any_of(
           table->entries.begin(),
           table->entries.end(),
-          [&](const fs::MountInfoTable::Entry& entry) {
+          [&](const internal::fs::MountInfoTable::Entry& entry) {
             return entry.target == stagingPath;
           })) {
     return Nothing();
   }
 
-  Try<Nothing> mount = fs::mount(path, stagingPath, None(), MS_BIND, None());
+  Try<Nothing> mount =
+    internal::fs::mount(volumeInfo.id, stagingPath, None(), MS_BIND, None());
+
   if (mount.isError()) {
     return StatusError(Status(
         grpc::INTERNAL,
-        "Failed to mount from '" + path + "' to '" + stagingPath +
+        "Failed to mount from '" + volumeInfo.id + "' to '" + stagingPath +
           "': " + mount.error()));
   }
 
@@ -1569,7 +1598,9 @@ Try<Nothing, StatusError> 
TestCSIPlugin::nodeUnstageVolume(
         grpc::NOT_FOUND, "Volume '" + volumeId + "' does not exist"));
   }
 
-  Try<fs::MountInfoTable> table = fs::MountInfoTable::read();
+  Try<internal::fs::MountInfoTable> table =
+    internal::fs::MountInfoTable::read();
+
   if (table.isError()) {
     return StatusError(Status(
         grpc::INTERNAL, "Failed to get mount table: " + table.error()));
@@ -1578,13 +1609,13 @@ Try<Nothing, StatusError> 
TestCSIPlugin::nodeUnstageVolume(
   if (std::none_of(
           table->entries.begin(),
           table->entries.end(),
-          [&](const fs::MountInfoTable::Entry& entry) {
+          [&](const internal::fs::MountInfoTable::Entry& entry) {
             return entry.target == stagingPath;
           })) {
     return Nothing();
   }
 
-  Try<Nothing> unmount = fs::unmount(stagingPath);
+  Try<Nothing> unmount = internal::fs::unmount(stagingPath);
   if (unmount.isError()) {
     return StatusError(Status(
         grpc::INTERNAL,
@@ -1626,14 +1657,15 @@ Try<Nothing, StatusError> 
TestCSIPlugin::nodePublishVolume(
   }
 
   const VolumeInfo& volumeInfo = volumes.at(volumeId);
-  const string path = getVolumePath(volumeInfo);
 
-  if (!volumeContext.count("path") || volumeContext.at("path") != path) {
+  if (volumeContext != volumeInfo.context) {
     return StatusError(Status(
         grpc::INVALID_ARGUMENT, "Invalid volume context"));
   }
 
-  Try<fs::MountInfoTable> table = fs::MountInfoTable::read();
+  Try<internal::fs::MountInfoTable> table =
+    internal::fs::MountInfoTable::read();
+
   if (table.isError()) {
     return StatusError(Status(
         grpc::INTERNAL, "Failed to get mount table: " + table.error()));
@@ -1642,7 +1674,7 @@ Try<Nothing, StatusError> 
TestCSIPlugin::nodePublishVolume(
   if (std::none_of(
           table->entries.begin(),
           table->entries.end(),
-          [&](const fs::MountInfoTable::Entry& entry) {
+          [&](const internal::fs::MountInfoTable::Entry& entry) {
             return entry.target == stagingPath;
           })) {
     return StatusError(Status(
@@ -1653,13 +1685,13 @@ Try<Nothing, StatusError> 
TestCSIPlugin::nodePublishVolume(
   if (std::any_of(
           table->entries.begin(),
           table->entries.end(),
-          [&](const fs::MountInfoTable::Entry& entry) {
+          [&](const internal::fs::MountInfoTable::Entry& entry) {
             return entry.target == targetPath;
           })) {
     return Nothing();
   }
 
-  Try<Nothing> mount = fs::mount(
+  Try<Nothing> mount = internal::fs::mount(
       stagingPath,
       targetPath,
       None(),
@@ -1685,7 +1717,9 @@ Try<Nothing, StatusError> 
TestCSIPlugin::nodeUnpublishVolume(
         grpc::NOT_FOUND, "Volume '" + volumeId + "' does not exist"));
   }
 
-  Try<fs::MountInfoTable> table = fs::MountInfoTable::read();
+  Try<internal::fs::MountInfoTable> table =
+    internal::fs::MountInfoTable::read();
+
   if (table.isError()) {
     return StatusError(Status(
         grpc::INTERNAL, "Failed to get mount table: " + table.error()));
@@ -1694,13 +1728,13 @@ Try<Nothing, StatusError> 
TestCSIPlugin::nodeUnpublishVolume(
   if (std::none_of(
           table->entries.begin(),
           table->entries.end(),
-          [&](const fs::MountInfoTable::Entry& entry) {
+          [&](const internal::fs::MountInfoTable::Entry& entry) {
             return entry.target == targetPath;
           })) {
     return Nothing();
   }
 
-  Try<Nothing> unmount = fs::unmount(targetPath);
+  Try<Nothing> unmount = internal::fs::unmount(targetPath);
   if (unmount.isError()) {
     return StatusError(Status(
         grpc::INTERNAL,
@@ -1985,13 +2019,8 @@ int main(int argc, char** argv)
                  strings::pairs(flags.volumes.get(), ";", ":")) {
       Option<Error> error;
 
-      if (strings::contains(name, stringify(os::PATH_SEPARATOR))) {
-        error =
-          "Volume name cannot contain '" + stringify(os::PATH_SEPARATOR) + "'";
-      } else if (capacities.size() != 1) {
+      if (capacities.size() != 1) {
         error = "Volume name must be unique";
-      } else if (volumes.contains(name)) {
-        error = "Volume '" + name + "' already exists";
       } else {
         Try<Bytes> capacity = Bytes::parse(capacities[0]);
         if (capacity.isError()) {
diff --git a/src/tests/storage_local_resource_provider_tests.cpp 
b/src/tests/storage_local_resource_provider_tests.cpp
index 609aebc..7dd08be 100644
--- a/src/tests/storage_local_resource_provider_tests.cpp
+++ b/src/tests/storage_local_resource_provider_tests.cpp
@@ -1011,22 +1011,13 @@ TEST_P(StorageLocalResourceProviderTest, 
CreateDestroyDisk)
     .filter(std::bind(isMountDisk<Resource>, lambda::_1, "test"))
     .begin();
 
-  ASSERT_TRUE(created.disk().source().has_metadata());
   ASSERT_TRUE(created.disk().source().has_mount());
   ASSERT_TRUE(created.disk().source().mount().has_root());
   EXPECT_FALSE(path::absolute(created.disk().source().mount().root()));
 
   // Check if the volume is actually created by the test CSI plugin.
-  Option<string> volumePath;
-  foreach (const Label& label, created.disk().source().metadata().labels()) {
-    if (label.key() == "path") {
-      volumePath = label.value();
-      break;
-    }
-  }
-
-  ASSERT_SOME(volumePath);
-  EXPECT_TRUE(os::exists(volumePath.get()));
+  const string& volumePath = created.disk().source().id();
+  EXPECT_TRUE(os::exists(volumePath));
 
   // Destroy the MOUNT disk with pipelined `CREATE`, `DESTROY` and `UNRESERVE`
   // operations. Note that `UNRESERVE` can come before `DESTROY_DISK`.
@@ -1056,7 +1047,7 @@ TEST_P(StorageLocalResourceProviderTest, 
CreateDestroyDisk)
   AWAIT_READY(offers);
 
   // Check if the volume is actually deleted by the test CSI plugin.
-  EXPECT_FALSE(os::exists(volumePath.get()));
+  EXPECT_FALSE(os::exists(volumePath));
 }
 
 
@@ -1150,22 +1141,13 @@ TEST_P(StorageLocalResourceProviderTest, 
CreateDestroyDiskWithRecovery)
     .filter(std::bind(isMountDisk<Resource>, lambda::_1, "test"))
     .begin();
 
-  ASSERT_TRUE(created.disk().source().has_metadata());
   ASSERT_TRUE(created.disk().source().has_mount());
   ASSERT_TRUE(created.disk().source().mount().has_root());
   EXPECT_FALSE(path::absolute(created.disk().source().mount().root()));
 
   // Check if the volume is actually created by the test CSI plugin.
-  Option<string> volumePath;
-  foreach (const Label& label, created.disk().source().metadata().labels()) {
-    if (label.key() == "path") {
-      volumePath = label.value();
-      break;
-    }
-  }
-
-  ASSERT_SOME(volumePath);
-  EXPECT_TRUE(os::exists(volumePath.get()));
+  const string& volumePath = created.disk().source().id();
+  EXPECT_TRUE(os::exists(volumePath));
 
   // Restart the agent.
   EXPECT_CALL(sched, offerRescinded(_, _));
@@ -1250,7 +1232,7 @@ TEST_P(StorageLocalResourceProviderTest, 
CreateDestroyDiskWithRecovery)
   AWAIT_READY(offers);
 
   // Check if the volume is actually deleted by the test CSI plugin.
-  EXPECT_FALSE(os::exists(volumePath.get()));
+  EXPECT_FALSE(os::exists(volumePath));
 }
 
 
@@ -1882,17 +1864,9 @@ TEST_P(StorageLocalResourceProviderTest, 
ROOT_AgentRegisteredWithNewId)
     ASSERT_NE(sourceIdToProviderId.at(volume.disk().source().id()),
               volume.provider_id());
 
-    Option<string> volumePath;
-    foreach (const Label& label, volume.disk().source().metadata().labels()) {
-      if (label.key() == "path") {
-        volumePath = label.value();
-        break;
-      }
-    }
-
-    ASSERT_SOME(volumePath);
-    ASSERT_TRUE(os::exists(volumePath.get()));
-    volumePaths.push_back(volumePath.get());
+    const string& volumePath = volume.disk().source().id();
+    ASSERT_TRUE(os::exists(volumePath));
+    volumePaths.push_back(volumePath);
   }
 
   // Apply profiles 'good' and 'bad' to the preprovisioned volumes. The first
@@ -2080,22 +2054,13 @@ TEST_P(
     .filter(std::bind(isMountDisk<Resource>, lambda::_1, "test"))
     .begin();
 
-  ASSERT_TRUE(created.disk().source().has_metadata());
   ASSERT_TRUE(created.disk().source().has_mount());
   ASSERT_TRUE(created.disk().source().mount().has_root());
   EXPECT_FALSE(path::absolute(created.disk().source().mount().root()));
 
   // Check if the CSI volume is actually created.
-  Option<string> volumePath;
-  foreach (const Label& label, created.disk().source().metadata().labels()) {
-    if (label.key() == "path") {
-      volumePath = label.value();
-      break;
-    }
-  }
-
-  ASSERT_SOME(volumePath);
-  EXPECT_TRUE(os::exists(volumePath.get()));
+  const string& volumePath = created.disk().source().id();
+  EXPECT_TRUE(os::exists(volumePath));
 
   // Create a persistent MOUNT volume then launch a task to write a file.
   Resource persistentVolume = created;
@@ -2148,8 +2113,8 @@ TEST_P(
   EXPECT_EQ(OPERATION_FINISHED, 
updateOperationStatusMessage->status().state());
 
   // Check if the CSI volume still exists but has being cleaned up.
-  EXPECT_TRUE(os::exists(volumePath.get()));
-  EXPECT_FALSE(os::exists(path::join(volumePath.get(), "file")));
+  EXPECT_TRUE(os::exists(volumePath));
+  EXPECT_FALSE(os::exists(path::join(volumePath, "file")));
 
   AWAIT_READY(offers);
   ASSERT_EQ(1u, offers->size());
@@ -2169,7 +2134,7 @@ TEST_P(
   EXPECT_EQ(OPERATION_FINISHED, 
updateOperationStatusMessage->status().state());
 
   // Check if the CSI volume is actually deleted.
-  EXPECT_FALSE(os::exists(volumePath.get()));
+  EXPECT_FALSE(os::exists(volumePath));
 
   AWAIT_READY(offers);
 }
@@ -2411,18 +2376,9 @@ TEST_P(
   EXPECT_EQ(OPERATION_FINISHED, 
updateOperationStatusMessage->status().state());
 
   // Check if the CSI volume still exists but has being cleaned up.
-  Option<string> volumePath;
-
-  foreach (const Label& label, created.disk().source().metadata().labels()) {
-    if (label.key() == "path") {
-      volumePath = label.value();
-      break;
-    }
-  }
-
-  ASSERT_SOME(volumePath);
-  EXPECT_TRUE(os::exists(volumePath.get()));
-  EXPECT_FALSE(os::exists(path::join(volumePath.get(), "file")));
+  const string& volumePath = created.disk().source().id();
+  EXPECT_TRUE(os::exists(volumePath));
+  EXPECT_FALSE(os::exists(path::join(volumePath, "file")));
 
   AWAIT_READY(offers);
   ASSERT_EQ(1u, offers->size());
@@ -2442,7 +2398,7 @@ TEST_P(
   EXPECT_EQ(OPERATION_FINISHED, 
updateOperationStatusMessage->status().state());
 
   // Check if the CSI volume is actually deleted.
-  EXPECT_FALSE(os::exists(volumePath.get()));
+  EXPECT_FALSE(os::exists(volumePath));
 
   AWAIT_READY(offers);
 }
@@ -2728,18 +2684,9 @@ TEST_P(
   EXPECT_EQ(OPERATION_FINISHED, 
updateOperationStatusMessage->status().state());
 
   // Check if the CSI volume still exists but has being cleaned up.
-  Option<string> volumePath;
-
-  foreach (const Label& label, created.disk().source().metadata().labels()) {
-    if (label.key() == "path") {
-      volumePath = label.value();
-      break;
-    }
-  }
-
-  ASSERT_SOME(volumePath);
-  EXPECT_TRUE(os::exists(volumePath.get()));
-  EXPECT_FALSE(os::exists(path::join(volumePath.get(), "file")));
+  const string& volumePath = created.disk().source().id();
+  EXPECT_TRUE(os::exists(volumePath));
+  EXPECT_FALSE(os::exists(path::join(volumePath, "file")));
 
   AWAIT_READY(offers);
   ASSERT_EQ(1u, offers->size());
@@ -2759,7 +2706,7 @@ TEST_P(
   EXPECT_EQ(OPERATION_FINISHED, 
updateOperationStatusMessage->status().state());
 
   // Check if the CSI volume is actually deleted.
-  EXPECT_FALSE(os::exists(volumePath.get()));
+  EXPECT_FALSE(os::exists(volumePath));
 
   AWAIT_READY(offers);
 }
@@ -2909,23 +2856,13 @@ TEST_P(
   ASSERT_TRUE(volume->disk().source().has_vendor());
   EXPECT_EQ(TEST_CSI_VENDOR, volume->disk().source().vendor());
   ASSERT_TRUE(volume->disk().source().has_id());
-  ASSERT_TRUE(volume->disk().source().has_metadata());
   ASSERT_TRUE(volume->disk().source().has_mount());
   ASSERT_TRUE(volume->disk().source().mount().has_root());
   EXPECT_FALSE(path::absolute(volume->disk().source().mount().root()));
 
   // Check if the volume is actually created by the test CSI plugin.
-  Option<string> volumePath;
-
-  foreach (const Label& label, volume->disk().source().metadata().labels()) {
-    if (label.key() == "path") {
-      volumePath = label.value();
-      break;
-    }
-  }
-
-  ASSERT_SOME(volumePath);
-  EXPECT_TRUE(os::exists(volumePath.get()));
+  const string& volumePath = volume->disk().source().id();
+  EXPECT_TRUE(os::exists(volumePath));
 
   pluginRestarted =
     FUTURE_DISPATCH(_, &ContainerDaemonProcess::launchContainer);
@@ -2940,7 +2877,7 @@ TEST_P(
   AWAIT_READY(pluginRestarted);
 
   // Put a file into the volume.
-  ASSERT_SOME(os::touch(path::join(volumePath.get(), "file")));
+  ASSERT_SOME(os::touch(path::join(volumePath, "file")));
 
   // Create a persistent volume on the CSI volume, then launch a task to
   // use the persistent volume.
@@ -3011,7 +2948,7 @@ TEST_P(
   ASSERT_FALSE(volumeDestroyedOffers->empty());
 
   // Check if the volume is actually deleted by the test CSI plugin.
-  EXPECT_FALSE(os::exists(volumePath.get()));
+  EXPECT_FALSE(os::exists(volumePath));
 }
 
 
@@ -3908,17 +3845,8 @@ TEST_P(
   offer = offers->at(0);
 
   // Bind-mount the file in the CSI volume to fail persistent volume cleanup.
-  Option<string> volumePath;
-  foreach (const Label& label, created.disk().source().metadata().labels()) {
-    if (label.key() == "path") {
-      volumePath = label.value();
-      break;
-    }
-  }
-
-  ASSERT_SOME(volumePath);
-
-  const string filePath = path::join(volumePath.get(), "file");
+  const string& volumePath = created.disk().source().id();
+  const string filePath = path::join(volumePath, "file");
   ASSERT_TRUE(os::exists(filePath));
   ASSERT_SOME(fs::mount(filePath, filePath, None(), MS_BIND, None()));
 
@@ -4052,17 +3980,9 @@ TEST_P(StorageLocalResourceProviderTest, 
CreateDestroyPreprovisionedVolume)
   // Get the volume paths of the preprovisioned volumes.
   vector<string> volumePaths;
   foreach (const Resource& volume, preprovisioned) {
-    Option<string> volumePath;
-    foreach (const Label& label, volume.disk().source().metadata().labels()) {
-      if (label.key() == "path") {
-        volumePath = label.value();
-        break;
-      }
-    }
-
-    ASSERT_SOME(volumePath);
-    ASSERT_TRUE(os::exists(volumePath.get()));
-    volumePaths.push_back(volumePath.get());
+    const string& volumePath = volume.disk().source().id();
+    ASSERT_TRUE(os::exists(volumePath));
+    volumePaths.push_back(volumePath);
   }
 
   // Apply profile 'test' and 'unknown' to the preprovisioned volumes. The 
first
@@ -4872,7 +4792,6 @@ TEST_P(StorageLocalResourceProviderTest, 
OperationStateMetrics)
   ASSERT_TRUE(volume->disk().source().has_vendor());
   EXPECT_EQ(TEST_CSI_VENDOR, volume->disk().source().vendor());
   ASSERT_TRUE(volume->disk().source().has_id());
-  ASSERT_TRUE(volume->disk().source().has_metadata());
   ASSERT_TRUE(volume->disk().source().has_mount());
   ASSERT_TRUE(volume->disk().source().mount().has_root());
   EXPECT_FALSE(path::absolute(volume->disk().source().mount().root()));
@@ -4893,17 +4812,8 @@ TEST_P(StorageLocalResourceProviderTest, 
OperationStateMetrics)
       "operations/destroy_disk/dropped")));
 
   // Remove the volume out of band to fail `DESTROY_DISK`.
-  Option<string> volumePath;
-
-  foreach (const Label& label, volume->disk().source().metadata().labels()) {
-    if (label.key() == "path") {
-      volumePath = label.value();
-      break;
-    }
-  }
-
-  ASSERT_SOME(volumePath);
-  ASSERT_SOME(os::rmdir(volumePath.get()));
+  const string& volumePath = volume->disk().source().id();
+  ASSERT_SOME(os::rmdir(volumePath));
 
   // Destroy the created volume, which will fail.
   EXPECT_CALL(sched, resourceOffers(&driver, OffersHaveResource(volume.get())))
@@ -5095,7 +5005,6 @@ TEST_P(StorageLocalResourceProviderTest, 
CsiPluginRpcMetrics)
   ASSERT_TRUE(volume->disk().source().has_vendor());
   EXPECT_EQ(TEST_CSI_VENDOR, volume->disk().source().vendor());
   ASSERT_TRUE(volume->disk().source().has_id());
-  ASSERT_TRUE(volume->disk().source().has_metadata());
   ASSERT_TRUE(volume->disk().source().has_mount());
   ASSERT_TRUE(volume->disk().source().mount().has_root());
   EXPECT_FALSE(path::absolute(volume->disk().source().mount().root()));
@@ -5106,17 +5015,8 @@ TEST_P(StorageLocalResourceProviderTest, 
CsiPluginRpcMetrics)
   EXPECT_TRUE(metricEquals(metricName("csi_plugin/rpcs_failed"), 0));
 
   // Remove the volume out of band to fail `DESTROY_DISK`.
-  Option<string> volumePath;
-
-  foreach (const Label& label, volume->disk().source().metadata().labels()) {
-    if (label.key() == "path") {
-      volumePath = label.value();
-      break;
-    }
-  }
-
-  ASSERT_SOME(volumePath);
-  ASSERT_SOME(os::rmdir(volumePath.get()));
+  const string& volumePath = volume->disk().source().id();
+  ASSERT_SOME(os::rmdir(volumePath));
 
   // Destroy the created volume, which will fail.
   EXPECT_CALL(sched, resourceOffers(&driver, OffersHaveResource(volume.get())))

Reply via email to