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/12/14 02:03:40 UTC

[mesos] 02/05: Added an optional `vendor` field to `Resource.DiskInfo.Source`.

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 5e49943cfb15684dbea91c3323dfee68ca9b31e0
Author: Chun-Hung Hsiao <ch...@apache.org>
AuthorDate: Thu Nov 15 23:06:22 2018 -0800

    Added an optional `vendor` field to `Resource.DiskInfo.Source`.
    
    The new `vendor` field together with the `id` field provide the means of
    uniquely identifying a CSI volume upon a CSI plugin migration (e.g.,
    there is a change in the agent ID).
    
    Review: https://reviews.apache.org/r/69037/
---
 include/mesos/mesos.proto    | 28 ++++++++++++++++++++--------
 include/mesos/v1/mesos.proto | 28 ++++++++++++++++++++--------
 src/common/resources.cpp     | 36 ++++++++++++++++++------------------
 src/v1/resources.cpp         | 36 ++++++++++++++++++------------------
 4 files changed, 76 insertions(+), 52 deletions(-)

diff --git a/include/mesos/mesos.proto b/include/mesos/mesos.proto
index 043a737..e984541 100644
--- a/include/mesos/mesos.proto
+++ b/include/mesos/mesos.proto
@@ -1026,7 +1026,7 @@ message CSIPluginContainerInfo {
  * Describes a CSI plugin.
  */
 message CSIPluginInfo {
-  // The type of the CSI service. This uniquely identifies a CSI
+  // The type of the CSI plugin. This uniquely identifies a CSI
   // implementation. For instance:
   //     org.apache.mesos.csi.test
   //
@@ -1035,11 +1035,17 @@ message CSIPluginInfo {
   // to avoid conflicts on type names.
   required string type = 1;
 
-  // The name of the CSI service. There could be mutliple instances of a
-  // type of CSI service. The name field is used to distinguish these
-  // instances. It should be a legal Java identifier
+  // The name of the CSI plugin. There could be multiple instances of a
+  // type of CSI plugin within a Mesos cluster. The name field is used to
+  // distinguish these instances. It should be a legal Java identifier
   // (https://docs.oracle.com/javase/tutorial/java/nutsandbolts/variables.html)
   // to avoid conflicts on concatenation of type and name.
+  //
+  // The type and name together provide the means to uniquely identify a storage
+  // backend and its resources in the cluster, so the operator should ensure
+  // that the concatenation of type and name is unique in the cluster, and it
+  // remains the same if the instance is migrated to another agent (e.g., there
+  // is a change in the agent ID).
   required string name = 2;
 
   // A list of container configurations to run CSI plugin components.
@@ -1451,12 +1457,18 @@ message Resource {
       optional Path path = 2;
       optional Mount mount = 3;
 
-      // An identifier for this source. This field maps onto CSI
-      // volume IDs and is not expected to be set by frameworks.
+      // The vendor of this source. If present, this field provides the means to
+      // uniquely identify the storage backend of this source in the cluster.
+      optional string vendor = 7; // EXPERIMENTAL.
+
+      // The identifier of this source. This field maps onto CSI volume IDs and
+      // is not expected to be set by frameworks. If both `vendor` and `id` are
+      // present, these two fields together provide the means to uniquely
+      // identify this source in the cluster.
       optional string id = 4; // EXPERIMENTAL.
 
-      // Additional metadata for this source. This field maps onto CSI
-      // volume metadata and is not expected to be set by frameworks.
+      // Additional metadata for this source. This field maps onto CSI volume
+      // attributes and is not expected to be set by frameworks.
       optional Labels metadata = 5; // EXPERIMENTAL.
 
       // This field serves as an indirection to a set of storage
diff --git a/include/mesos/v1/mesos.proto b/include/mesos/v1/mesos.proto
index 9830c0b..dac7f51 100644
--- a/include/mesos/v1/mesos.proto
+++ b/include/mesos/v1/mesos.proto
@@ -1018,7 +1018,7 @@ message CSIPluginContainerInfo {
  * Describes a CSI plugin.
  */
 message CSIPluginInfo {
-  // The type of the CSI service. This uniquely identifies a CSI
+  // The type of the CSI plugin. This uniquely identifies a CSI
   // implementation. For instance:
   //     org.apache.mesos.csi.test
   //
@@ -1027,11 +1027,17 @@ message CSIPluginInfo {
   // to avoid conflicts on type names.
   required string type = 1;
 
-  // The name of the CSI service. There could be mutliple instances of a
-  // type of CSI service. The name field is used to distinguish these
-  // instances. It should be a legal Java identifier
+  // The name of the CSI plugin. There could be multiple instances of a
+  // type of CSI plugin within a Mesos cluster. The name field is used to
+  // distinguish these instances. It should be a legal Java identifier
   // (https://docs.oracle.com/javase/tutorial/java/nutsandbolts/variables.html)
   // to avoid conflicts on concatenation of type and name.
+  //
+  // The type and name together provide the means to uniquely identify a storage
+  // backend and its resources in the cluster, so the operator should ensure
+  // that the concatenation of type and name is unique in the cluster, and it
+  // remains the same if the instance is migrated to another agent (e.g., there
+  // is a change in the agent ID).
   required string name = 2;
 
   // A list of container configurations to run CSI plugin components.
@@ -1443,12 +1449,18 @@ message Resource {
       optional Path path = 2;
       optional Mount mount = 3;
 
-      // An identifier for this source. This field maps onto CSI
-      // volume IDs and is not expected to be set by frameworks.
+      // The vendor of this source. If present, this field provides the means to
+      // uniquely identify the storage backend of this source in the cluster.
+      optional string vendor = 7; // EXPERIMENTAL.
+
+      // The identifier of this source. This field maps onto CSI volume IDs and
+      // is not expected to be set by frameworks. If both `vendor` and `id` are
+      // present, these two fields together provide the means to uniquely
+      // identify this source in the cluster.
       optional string id = 4; // EXPERIMENTAL.
 
-      // Additional metadata for this source. This field maps onto CSI
-      // volume metadata and is not expected to be set by frameworks.
+      // Additional metadata for this source. This field maps onto CSI volume
+      // attributes and is not expected to be set by frameworks.
       optional Labels metadata = 5; // EXPERIMENTAL.
 
       // This field serves as an indirection to a set of storage
diff --git a/src/common/resources.cpp b/src/common/resources.cpp
index 3989b66..7a77eca 100644
--- a/src/common/resources.cpp
+++ b/src/common/resources.cpp
@@ -192,6 +192,14 @@ bool operator==(
     return false;
   }
 
+  if (left.has_vendor() != right.has_vendor()) {
+    return false;
+  }
+
+  if (left.has_vendor() && left.vendor() != right.vendor()) {
+    return false;
+  }
+
   if (left.has_id() != right.has_id()) {
     return false;
   }
@@ -2381,29 +2389,21 @@ Resources& Resources::operator-=(const Resources& that)
 
 ostream& operator<<(ostream& stream, const Resource::DiskInfo::Source& source)
 {
+  const Option<string> csiSource = source.has_id() || source.has_profile()
+    ? "(" + source.vendor() + "," + source.id() + "," + source.profile() + ")"
+    : Option<string>::none();
+
   switch (source.type()) {
     case Resource::DiskInfo::Source::MOUNT:
-      return stream
-        << "MOUNT"
-        << ((source.has_id() || source.has_profile())
-              ? "(" + source.id() + "," + source.profile() + ")"
-              : (source.mount().has_root() ? ":" + source.mount().root() : ""));
+      return stream << "MOUNT" << csiSource.getOrElse(
+          source.mount().has_root() ? ":" + source.mount().root() : "");
     case Resource::DiskInfo::Source::PATH:
-      return stream
-        << "PATH"
-        << ((source.has_id() || source.has_profile())
-              ? "(" + source.id() + "," + source.profile() + ")"
-              : (source.path().has_root() ? ":" + source.path().root() : ""));
+      return stream << "PATH" << csiSource.getOrElse(
+          source.path().has_root() ? ":" + source.path().root() : "");
     case Resource::DiskInfo::Source::BLOCK:
-      return stream
-        << "BLOCK"
-        << ((source.has_id() || source.has_profile())
-              ? "(" + source.id() + "," + source.profile() + ")" : "");
+      return stream << "BLOCK" << csiSource.getOrElse("");
     case Resource::DiskInfo::Source::RAW:
-      return stream
-        << "RAW"
-        << ((source.has_id() || source.has_profile())
-              ? "(" + source.id() + "," + source.profile() + ")" : "");
+      return stream << "RAW" << csiSource.getOrElse("");
     case Resource::DiskInfo::Source::UNKNOWN:
       return stream << "UNKNOWN";
   }
diff --git a/src/v1/resources.cpp b/src/v1/resources.cpp
index efbc7c34..d953c63 100644
--- a/src/v1/resources.cpp
+++ b/src/v1/resources.cpp
@@ -194,6 +194,14 @@ bool operator==(
     return false;
   }
 
+  if (left.has_vendor() != right.has_vendor()) {
+    return false;
+  }
+
+  if (left.has_vendor() && left.vendor() != right.vendor()) {
+    return false;
+  }
+
   if (left.has_id() != right.has_id()) {
     return false;
   }
@@ -2382,29 +2390,21 @@ Resources& Resources::operator-=(const Resources& that)
 
 ostream& operator<<(ostream& stream, const Resource::DiskInfo::Source& source)
 {
+  const Option<string> csiSource = source.has_id() || source.has_profile()
+    ? "(" + source.vendor() + "," + source.id() + "," + source.profile() + ")"
+    : Option<string>::none();
+
   switch (source.type()) {
     case Resource::DiskInfo::Source::MOUNT:
-      return stream
-        << "MOUNT"
-        << ((source.has_id() || source.has_profile())
-              ? "(" + source.id() + "," + source.profile() + ")"
-              : (source.mount().has_root() ? ":" + source.mount().root() : ""));
+      return stream << "MOUNT" << csiSource.getOrElse(
+          source.mount().has_root() ? ":" + source.mount().root() : "");
     case Resource::DiskInfo::Source::PATH:
-      return stream
-        << "PATH"
-        << ((source.has_id() || source.has_profile())
-              ? "(" + source.id() + "," + source.profile() + ")"
-              : (source.path().has_root() ? ":" + source.path().root() : ""));
+      return stream << "PATH" << csiSource.getOrElse(
+          source.path().has_root() ? ":" + source.path().root() : "");
     case Resource::DiskInfo::Source::BLOCK:
-      return stream
-        << "BLOCK"
-        << ((source.has_id() || source.has_profile())
-              ? "(" + source.id() + "," + source.profile() + ")" : "");
+      return stream << "BLOCK" << csiSource.getOrElse("");
     case Resource::DiskInfo::Source::RAW:
-      return stream
-        << "RAW"
-        << ((source.has_id() || source.has_profile())
-              ? "(" + source.id() + "," + source.profile() + ")" : "");
+      return stream << "RAW" << csiSource.getOrElse("");
     case Resource::DiskInfo::Source::UNKNOWN:
       return stream << "UNKNOWN";
   }