[mesos] 01/02: Introduced the `CSIPluginInfo.target_path_exists` field.

[qi...@apache.org]

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

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

commit 2d2265de7df7801612fc2f104f9c8f455a97a1fd
Author: Qian Zhang <zh...@gmail.com>
AuthorDate: Thu Aug 20 17:08:32 2020 +0800

    Introduced the `CSIPluginInfo.target_path_exists` field.
    
    Review: https://reviews.apache.org/r/72788
---
 include/mesos/mesos.proto     |  7 +++++++
 include/mesos/v1/mesos.proto  |  7 +++++++
 src/csi/v1_volume_manager.cpp | 39 ++++++++++++++++++++++++++++-----------
 3 files changed, 42 insertions(+), 11 deletions(-)

diff --git a/include/mesos/mesos.proto b/include/mesos/mesos.proto
index 661f746..a100844 100644
--- a/include/mesos/mesos.proto
+++ b/include/mesos/mesos.proto
@@ -1140,6 +1140,13 @@ message CSIPluginInfo {
   // Each volume will be published by the CSI plugin at a sub-directory
   // under this path.
   optional string target_path_root = 5;
+
+  // For some CSI plugins which implement CSI v1 spec, they expect the target
+  // path is an existing path which is actually not CSI v1 spec compliant. In
+  // such case this field should be set to `true` as a work around for those
+  // plugins. For the CSI plugins which implement CSI v0 spec, this field will
+  // be just ignored.
+  optional bool target_path_exists = 6;
 }
 
 
diff --git a/include/mesos/v1/mesos.proto b/include/mesos/v1/mesos.proto
index ffe45c3..09973ab 100644
--- a/include/mesos/v1/mesos.proto
+++ b/include/mesos/v1/mesos.proto
@@ -1128,6 +1128,13 @@ message CSIPluginInfo {
   // Each volume will be published by the CSI plugin at a sub-directory
   // under this path.
   optional string target_path_root = 5;
+
+  // For some CSI plugins which implement CSI v1 spec, they expect the target
+  // path is an existing path which is actually not CSI v1 spec compliant. In
+  // such case this field should be set to `true` as a work around for those
+  // plugins. For the CSI plugins which implement CSI v0 spec, this field will
+  // be just ignored.
+  optional bool target_path_exists = 6;
 }
 
 
diff --git a/src/csi/v1_volume_manager.cpp b/src/csi/v1_volume_manager.cpp
index 1a1b97c..29ae821 100644
--- a/src/csi/v1_volume_manager.cpp
+++ b/src/csi/v1_volume_manager.cpp
@@ -952,16 +952,29 @@ Future<Nothing> VolumeManagerProcess::_publishVolume(const string& volumeId)
 
   const string targetPath = paths::getMountTargetPath(mountRootDir, volumeId);
 
-  // Ensure the parent directory of the target path exists. The target path
-  // itself will be created by the plugin.
-  //
-  // NOTE: The target path will be removed by the plugin as well, and The parent
-  // directory of the target path will be cleaned up during volume removal.
-  Try<Nothing> mkdir = os::mkdir(Path(targetPath).dirname());
-  if (mkdir.isError()) {
-    return Failure(
-        "Failed to create parent directory of target path '" + targetPath +
-        "': " + mkdir.error());
+  if (info.target_path_exists()) {
+    // For some CSI plugins, they expect the target path is an existing path
+    // rather than creating the target path. So here we create the target path
+    // for such CSI plugins.
+    Try<Nothing> mkdir = os::mkdir(targetPath);
+    if (mkdir.isError()) {
+      return Failure(
+          "Failed to create the target path '" + targetPath +
+          "': " + mkdir.error());
+    }
+  } else {
+    // Ensure the parent directory of the target path exists. The
+    // target path itself will be created by the plugin.
+    //
+    // NOTE: The target path will be removed by the plugin as well,
+    // and the parent directory of the target path will be cleaned
+    // up during volume removal.
+    Try<Nothing> mkdir = os::mkdir(Path(targetPath).dirname());
+    if (mkdir.isError()) {
+      return Failure(
+          "Failed to create parent directory of target path '" + targetPath +
+          "': " + mkdir.error());
+    }
   }
 
   if (volumeState.state() == VolumeState::VOL_READY) {
@@ -1244,7 +1257,11 @@ Future<Nothing> VolumeManagerProcess::__unpublishVolume(const string& volumeId)
   return call(NODE_SERVICE, &Client::nodeUnpublishVolume, std::move(request))
     .then(process::defer(self(), [this, volumeId, targetPath]()
         -> Future<Nothing> {
-      if (os::exists(targetPath)) {
+      // For the CSI plugins which expect the target path is an existing path,
+      // they do not remove the target path as part of the `NodeUnpublishVolume`
+      // operation. So here we should not verify the target path is already
+      // removed by such CSI plugins.
+      if (!info.target_path_exists() && os::exists(targetPath)) {
         return Failure("Target path '" + targetPath + "' not removed");
       }