You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by ji...@apache.org on 2017/04/14 00:00:50 UTC

mesos git commit: Made Path and Mount root optional in Resource.DiskInfo.Source.

Repository: mesos
Updated Branches:
  refs/heads/master 6cf2536c2 -> 5660d7633


Made Path and Mount root optional in Resource.DiskInfo.Source.

This commit is preparation for future changes where path or mount
roots might be calculated lazily, i.e., not known early enough
anymore.

Review: https://reviews.apache.org/r/57999/


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/5660d763
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/5660d763
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/5660d763

Branch: refs/heads/master
Commit: 5660d7633280bf592b12eeed88c4ebadb33f44c5
Parents: 6cf2536
Author: Benjamin Bannier <be...@mesosphere.io>
Authored: Thu Apr 13 14:35:34 2017 -0700
Committer: Jie Yu <yu...@gmail.com>
Committed: Thu Apr 13 17:00:44 2017 -0700

----------------------------------------------------------------------
 include/mesos/mesos.proto    |  4 ++--
 include/mesos/v1/mesos.proto |  4 ++--
 src/common/resources.cpp     | 38 ++++++++++++++++++++++++--------------
 src/slave/paths.cpp          |  2 ++
 src/slave/slave.cpp          | 12 ++++++++++++
 src/v1/resources.cpp         | 38 ++++++++++++++++++++++++--------------
 6 files changed, 66 insertions(+), 32 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/5660d763/include/mesos/mesos.proto
----------------------------------------------------------------------
diff --git a/include/mesos/mesos.proto b/include/mesos/mesos.proto
index a7689a1..eaa2d2a 100644
--- a/include/mesos/mesos.proto
+++ b/include/mesos/mesos.proto
@@ -1003,7 +1003,7 @@ message Resource {
       // can be shared and carved up as necessary between frameworks.
       message Path {
         // Path to the folder (e.g., /mnt/raid/disk0).
-        required string root = 1;
+        optional string root = 1;
       }
 
       // A mounted file-system set up by the Agent administrator. This
@@ -1011,7 +1011,7 @@ message Resource {
       // partial amount of this disk.
       message Mount {
         // Path to mount point (e.g., /mnt/raid/disk0).
-        required string root = 1;
+        optional string root = 1;
       }
 
       required Type type = 1;

http://git-wip-us.apache.org/repos/asf/mesos/blob/5660d763/include/mesos/v1/mesos.proto
----------------------------------------------------------------------
diff --git a/include/mesos/v1/mesos.proto b/include/mesos/v1/mesos.proto
index 2a09b92..1a32a7b 100644
--- a/include/mesos/v1/mesos.proto
+++ b/include/mesos/v1/mesos.proto
@@ -997,7 +997,7 @@ message Resource {
       // can be shared and carved up as necessary between frameworks.
       message Path {
         // Path to the folder (e.g., /mnt/raid/disk0).
-        required string root = 1;
+        optional string root = 1;
       }
 
       // A mounted file-system set up by the Agent administrator. This
@@ -1005,7 +1005,7 @@ message Resource {
       // partial amount of this disk.
       message Mount {
         // Path to mount point (e.g., /mnt/raid/disk0).
-        required string root = 1;
+        optional string root = 1;
       }
 
       required Type type = 1;

http://git-wip-us.apache.org/repos/asf/mesos/blob/5660d763/src/common/resources.cpp
----------------------------------------------------------------------
diff --git a/src/common/resources.cpp b/src/common/resources.cpp
index 289df9e..77bac0c 100644
--- a/src/common/resources.cpp
+++ b/src/common/resources.cpp
@@ -112,7 +112,15 @@ bool operator==(
     const Resource::DiskInfo::Source::Path& left,
     const Resource::DiskInfo::Source::Path& right)
 {
-  return left.root() == right.root();
+  if (left.has_root() != right.has_root()) {
+    return false;
+  }
+
+  if (left.has_root() && left.root() != right.root()) {
+    return false;
+  }
+
+  return true;
 }
 
 
@@ -120,6 +128,14 @@ bool operator==(
     const Resource::DiskInfo::Source::Mount& left,
     const Resource::DiskInfo::Source::Mount& right)
 {
+  if (left.has_root() != right.has_root()) {
+    return false;
+  }
+
+  if (left.has_root() && left.root() != right.root()) {
+    return false;
+  }
+
   return left.root() == right.root();
 }
 
@@ -797,18 +813,8 @@ Option<Error> Resources::validate(const Resource& resource)
 
       switch (source.type()) {
         case Resource::DiskInfo::Source::PATH:
-          if (!source.has_path()) {
-            return Error(
-                "DiskInfo::Source 'type' set to 'PATH' but missing 'path' "
-                "data");
-          }
-          break;
         case Resource::DiskInfo::Source::MOUNT:
-          if (!source.has_mount()) {
-            return Error(
-                "DiskInfo::Source 'type' set to 'MOUNT' but missing 'mount' "
-                "data");
-          }
+          // `PATH` and `MOUNT` contain only `optional` members.
           break;
         case Resource::DiskInfo::Source::UNKNOWN:
           return Error(
@@ -1873,9 +1879,13 @@ ostream& operator<<(ostream& stream, const Resource::DiskInfo::Source& source)
 {
   switch (source.type()) {
     case Resource::DiskInfo::Source::MOUNT:
-      return stream << "MOUNT:" + source.mount().root();
+      return stream << "MOUNT"
+                    << (source.mount().has_root() ? ":" + source.mount().root()
+                                                  : "");
     case Resource::DiskInfo::Source::PATH:
-      return stream << "PATH:" + source.path().root();
+      return stream << "PATH"
+                    << (source.path().has_root() ? ":" + source.path().root()
+                                                 : "");
     case Resource::DiskInfo::Source::UNKNOWN:
       return stream << "UNKNOWN";
   }

http://git-wip-us.apache.org/repos/asf/mesos/blob/5660d763/src/slave/paths.cpp
----------------------------------------------------------------------
diff --git a/src/slave/paths.cpp b/src/slave/paths.cpp
index ef22ee1..d5903b8 100644
--- a/src/slave/paths.cpp
+++ b/src/slave/paths.cpp
@@ -483,6 +483,7 @@ string getPersistentVolumePath(
     case Resource::DiskInfo::Source::PATH: {
       // For `PATH` we mount a directory inside the `root`.
       CHECK(volume.disk().source().has_path());
+      CHECK(volume.disk().source().path().has_root());
       return getPersistentVolumePath(
           volume.disk().source().path().root(),
           volume.role(),
@@ -491,6 +492,7 @@ string getPersistentVolumePath(
     case Resource::DiskInfo::Source::MOUNT: {
       // For `MOUNT` we map straight onto the root of the mount.
       CHECK(volume.disk().source().has_mount());
+      CHECK(volume.disk().source().mount().has_root());
       return volume.disk().source().mount().root();
     }
     case Resource::DiskInfo::Source::UNKNOWN:

http://git-wip-us.apache.org/repos/asf/mesos/blob/5660d763/src/slave/slave.cpp
----------------------------------------------------------------------
diff --git a/src/slave/slave.cpp b/src/slave/slave.cpp
index 3ad4ce4..4ff522e 100644
--- a/src/slave/slave.cpp
+++ b/src/slave/slave.cpp
@@ -402,6 +402,12 @@ void Slave::initialize()
         // For `PATH` sources we create them if they do not exist.
         CHECK(source.has_path());
 
+        if (!source.path().has_root()) {
+          EXIT(EXIT_FAILURE)
+            << "PATH disk root directory is not specified "
+            << "'" << resource << "'";
+        }
+
         Try<Nothing> mkdir = os::mkdir(source.path().root(), true);
 
         if (mkdir.isError()) {
@@ -414,6 +420,12 @@ void Slave::initialize()
       case Resource::DiskInfo::Source::MOUNT: {
         CHECK(source.has_mount());
 
+        if (!source.mount().has_root()) {
+          EXIT(EXIT_FAILURE)
+            << "MOUNT disk root directory is not specified "
+            << "'" << resource << "'";
+        }
+
         // For `MOUNT` sources we fail if they don't exist.
         // On Linux we test the mount table for existence.
 #ifdef __linux__

http://git-wip-us.apache.org/repos/asf/mesos/blob/5660d763/src/v1/resources.cpp
----------------------------------------------------------------------
diff --git a/src/v1/resources.cpp b/src/v1/resources.cpp
index c6cb585..2e6b298 100644
--- a/src/v1/resources.cpp
+++ b/src/v1/resources.cpp
@@ -114,7 +114,15 @@ bool operator==(
     const Resource::DiskInfo::Source::Path& left,
     const Resource::DiskInfo::Source::Path& right)
 {
-  return left.root() == right.root();
+  if (left.has_root() != right.has_root()) {
+    return false;
+  }
+
+  if (left.has_root() && left.root() != right.root()) {
+    return false;
+  }
+
+  return true;
 }
 
 
@@ -122,6 +130,14 @@ bool operator==(
     const Resource::DiskInfo::Source::Mount& left,
     const Resource::DiskInfo::Source::Mount& right)
 {
+  if (left.has_root() != right.has_root()) {
+    return false;
+  }
+
+  if (left.has_root() && left.root() != right.root()) {
+    return false;
+  }
+
   return left.root() == right.root();
 }
 
@@ -799,18 +815,8 @@ Option<Error> Resources::validate(const Resource& resource)
 
       switch (source.type()) {
         case Resource::DiskInfo::Source::PATH:
-          if (!source.has_path()) {
-            return Error(
-                "DiskInfo::Source 'type' set to 'PATH' but missing 'path' "
-                "data");
-          }
-          break;
         case Resource::DiskInfo::Source::MOUNT:
-          if (!source.has_mount()) {
-            return Error(
-                "DiskInfo::Source 'type' set to 'MOUNT' but missing 'mount' "
-                "data");
-          }
+          // `PATH` and `MOUNT` contain only `optional` members.
           break;
         case Resource::DiskInfo::Source::UNKNOWN:
           return Error(
@@ -1875,9 +1881,13 @@ ostream& operator<<(ostream& stream, const Resource::DiskInfo::Source& source)
 {
   switch (source.type()) {
     case Resource::DiskInfo::Source::MOUNT:
-      return stream << "MOUNT:" + source.mount().root();
+      return stream << "MOUNT"
+                    << (source.mount().has_root() ? ":" + source.mount().root()
+                                                  : "");
     case Resource::DiskInfo::Source::PATH:
-      return stream << "PATH:" + source.path().root();
+      return stream << "PATH"
+                    << (source.path().has_root() ? ":" + source.path().root()
+                                                 : "");
     case Resource::DiskInfo::Source::UNKNOWN:
       return stream << "UNKNOWN";
   }