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 <chhs...@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"; }