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/03/29 21:53:15 UTC
mesos git commit: Added UNKNOWN DiskInfo.Source type.
Repository: mesos
Updated Branches:
refs/heads/master 8c4e5f190 -> 75c6dfaa9
Added UNKNOWN DiskInfo.Source type.
We introduce an explicit UNKNOWN enum kind to allow explicit handling
of unknown enum values (e.g., when the sending and receiving end use
different versions of a message using the enum).
This commit also migrates pattern matching of values of this enum from
if statements to switch statements so that compiler diagnostics can be
used to identify unhandled cases when other types are added in the
future.
Review: https://reviews.apache.org/r/57911/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/75c6dfaa
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/75c6dfaa
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/75c6dfaa
Branch: refs/heads/master
Commit: 75c6dfaa9b816c61eb7a3e155f990d96276dcaa3
Parents: 8c4e5f1
Author: Benjamin Bannier <be...@mesosphere.io>
Authored: Wed Mar 29 14:28:38 2017 -0700
Committer: Jie Yu <yu...@gmail.com>
Committed: Wed Mar 29 14:28:38 2017 -0700
----------------------------------------------------------------------
include/mesos/mesos.proto | 1 +
include/mesos/v1/mesos.proto | 1 +
src/common/resources.cpp | 31 +++++++----
src/slave/paths.cpp | 31 ++++++-----
src/slave/slave.cpp | 110 ++++++++++++++++++++------------------
src/v1/resources.cpp | 31 +++++++----
6 files changed, 121 insertions(+), 84 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/75c6dfaa/include/mesos/mesos.proto
----------------------------------------------------------------------
diff --git a/include/mesos/mesos.proto b/include/mesos/mesos.proto
index 9a66967..4c1fd9f 100644
--- a/include/mesos/mesos.proto
+++ b/include/mesos/mesos.proto
@@ -983,6 +983,7 @@ message Resource {
// TODO(jmlvanre): Add support for BLOCK devices.
message Source {
enum Type {
+ UNKNOWN = 0;
PATH = 1;
MOUNT = 2;
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/75c6dfaa/include/mesos/v1/mesos.proto
----------------------------------------------------------------------
diff --git a/include/mesos/v1/mesos.proto b/include/mesos/v1/mesos.proto
index 115f1b4..3324f0e 100644
--- a/include/mesos/v1/mesos.proto
+++ b/include/mesos/v1/mesos.proto
@@ -982,6 +982,7 @@ message Resource {
// TODO(jmlvanre): Add support for BLOCK devices.
message Source {
enum Type {
+ UNKNOWN = 0;
PATH = 1;
MOUNT = 2;
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/75c6dfaa/src/common/resources.cpp
----------------------------------------------------------------------
diff --git a/src/common/resources.cpp b/src/common/resources.cpp
index ca1add1..c26e0f9 100644
--- a/src/common/resources.cpp
+++ b/src/common/resources.cpp
@@ -777,16 +777,25 @@ Option<Error> Resources::validate(const Resource& resource)
if (disk.has_source()) {
const Resource::DiskInfo::Source& source = disk.source();
- if (source.type() == Resource::DiskInfo::Source::PATH &&
- !source.has_path()) {
- return Error(
- "DiskInfo::Source 'type' set to 'PATH' but missing 'path' data");
- }
-
- if (source.type() == Resource::DiskInfo::Source::MOUNT &&
- !source.has_mount()) {
- return Error(
- "DiskInfo::Source 'type' set to 'MOUNT' but missing 'mount' data");
+ 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");
+ }
+ break;
+ case Resource::DiskInfo::Source::UNKNOWN:
+ return Error(
+ "Unsupported 'DiskInfo.Source.Type' in "
+ "'" + stringify(source) + "'");
}
}
}
@@ -1849,6 +1858,8 @@ ostream& operator<<(ostream& stream, const Resource::DiskInfo::Source& source)
return stream << "MOUNT:" + source.mount().root();
case Resource::DiskInfo::Source::PATH:
return stream << "PATH:" + source.path().root();
+ case Resource::DiskInfo::Source::UNKNOWN:
+ return stream << "UNKNOWN";
}
UNREACHABLE();
http://git-wip-us.apache.org/repos/asf/mesos/blob/75c6dfaa/src/slave/paths.cpp
----------------------------------------------------------------------
diff --git a/src/slave/paths.cpp b/src/slave/paths.cpp
index 38ad199..ef22ee1 100644
--- a/src/slave/paths.cpp
+++ b/src/slave/paths.cpp
@@ -31,6 +31,8 @@
#include <stout/try.hpp>
#include <stout/unreachable.hpp>
+#include <glog/logging.h>
+
#include "common/validation.hpp"
#include "messages/messages.hpp"
@@ -477,18 +479,23 @@ string getPersistentVolumePath(
// If a `source` was provided for the volume, we map it according
// to the `type` of disk. Currently only the `PATH` and 'MOUNT'
// types are supported.
- if (volume.disk().source().type() == Resource::DiskInfo::Source::PATH) {
- // For `PATH` we mount a directory inside the `root`.
- CHECK(volume.disk().source().has_path());
- return getPersistentVolumePath(
- volume.disk().source().path().root(),
- volume.role(),
- volume.disk().persistence().id());
- } else if (
- volume.disk().source().type() == Resource::DiskInfo::Source::MOUNT) {
- // For `MOUNT` we map straight onto the root of the mount.
- CHECK(volume.disk().source().has_mount());
- return volume.disk().source().mount().root();
+ switch (volume.disk().source().type()) {
+ case Resource::DiskInfo::Source::PATH: {
+ // For `PATH` we mount a directory inside the `root`.
+ CHECK(volume.disk().source().has_path());
+ return getPersistentVolumePath(
+ volume.disk().source().path().root(),
+ volume.role(),
+ volume.disk().persistence().id());
+ }
+ case Resource::DiskInfo::Source::MOUNT: {
+ // For `MOUNT` we map straight onto the root of the mount.
+ CHECK(volume.disk().source().has_mount());
+ return volume.disk().source().mount().root();
+ }
+ case Resource::DiskInfo::Source::UNKNOWN:
+ LOG(FATAL) << "Unsupported DiskInfo.Source.type";
+ break;
}
UNREACHABLE();
http://git-wip-us.apache.org/repos/asf/mesos/blob/75c6dfaa/src/slave/slave.cpp
----------------------------------------------------------------------
diff --git a/src/slave/slave.cpp b/src/slave/slave.cpp
index c8479d7..1c164c9 100644
--- a/src/slave/slave.cpp
+++ b/src/slave/slave.cpp
@@ -375,71 +375,77 @@ void Slave::initialize()
resources->filter([](const Resource& _resource) {
return _resource.has_disk() && _resource.disk().has_source();
})) {
- // For `PATH` sources we create them if they do not exist.
const Resource::DiskInfo::Source& source = resource.disk().source();
- if (source.type() == Resource::DiskInfo::Source::PATH) {
- CHECK(source.has_path());
+ switch (source.type()) {
+ case Resource::DiskInfo::Source::PATH: {
+ // For `PATH` sources we create them if they do not exist.
+ CHECK(source.has_path());
- Try<Nothing> mkdir =
- os::mkdir(source.path().root(), true);
+ Try<Nothing> mkdir = os::mkdir(source.path().root(), true);
- if (mkdir.isError()) {
- EXIT(EXIT_FAILURE)
- << "Failed to create DiskInfo path directory '"
- << source.path().root() << "': " << mkdir.error();
+ if (mkdir.isError()) {
+ EXIT(EXIT_FAILURE)
+ << "Failed to create DiskInfo path directory "
+ << "'" << source.path().root() << "': " << mkdir.error();
+ }
+ break;
}
- } else if (source.type() == Resource::DiskInfo::Source::MOUNT) {
- CHECK(source.has_mount());
+ case Resource::DiskInfo::Source::MOUNT: {
+ CHECK(source.has_mount());
- // For `MOUNT` sources we fail if they don't exist.
- // On Linux we test the mount table for existence.
+ // For `MOUNT` sources we fail if they don't exist.
+ // On Linux we test the mount table for existence.
#ifdef __linux__
- // Get the `realpath` of the `root` to verify it against the
- // mount table entries.
- // TODO(jmlvanre): Consider enforcing allowing only real paths
- // as opposed to symlinks. This would prevent the ability for
- // an operator to change the underlying data while the slave
- // checkpointed `root` had the same value. We could also check
- // the UUID of the underlying block device to catch this case.
- Result<string> realpath = os::realpath(source.mount().root());
-
- if (!realpath.isSome()) {
- EXIT(EXIT_FAILURE)
- << "Failed to determine `realpath` for DiskInfo mount in resource '"
- << resource << "' with path '" << source.mount().root() << "': "
- << (realpath.isError() ? realpath.error() : "no such path");
- }
+ // Get the `realpath` of the `root` to verify it against the
+ // mount table entries.
+ // TODO(jmlvanre): Consider enforcing allowing only real paths
+ // as opposed to symlinks. This would prevent the ability for
+ // an operator to change the underlying data while the slave
+ // checkpointed `root` had the same value. We could also check
+ // the UUID of the underlying block device to catch this case.
+ Result<string> realpath = os::realpath(source.mount().root());
+
+ if (!realpath.isSome()) {
+ EXIT(EXIT_FAILURE)
+ << "Failed to determine `realpath` for DiskInfo mount in resource '"
+ << resource << "' with path '" << source.mount().root() << "': "
+ << (realpath.isError() ? realpath.error() : "no such path");
+ }
- // TODO(jmlvanre): Consider moving this out of the for loop.
- Try<fs::MountTable> mountTable = fs::MountTable::read("/proc/mounts");
- if (mountTable.isError()) {
- EXIT(EXIT_FAILURE)
- << "Failed to open mount table to verify mounts: "
- << mountTable.error();
- }
+ // TODO(jmlvanre): Consider moving this out of the for loop.
+ Try<fs::MountTable> mountTable = fs::MountTable::read("/proc/mounts");
+ if (mountTable.isError()) {
+ EXIT(EXIT_FAILURE)
+ << "Failed to open mount table to verify mounts: "
+ << mountTable.error();
+ }
- bool foundEntry = false;
- foreach (const fs::MountTable::Entry& entry, mountTable.get().entries) {
- if (entry.dir == realpath.get()) {
- foundEntry = true;
- break;
+ bool foundEntry = false;
+ foreach (const fs::MountTable::Entry& entry, mountTable.get().entries) {
+ if (entry.dir == realpath.get()) {
+ foundEntry = true;
+ break;
+ }
}
- }
- if (!foundEntry) {
- EXIT(EXIT_FAILURE)
- << "Failed to found mount '" << realpath.get() << "' in /proc/mounts";
- }
+ if (!foundEntry) {
+ EXIT(EXIT_FAILURE)
+ << "Failed to found mount '" << realpath.get()
+ << "' in /proc/mounts";
+ }
#else // __linux__
- // On other platforms we test whether that provided `root` exists.
- if (!os::exists(source.mount().root())) {
+ // On other platforms we test whether that provided `root` exists.
+ if (!os::exists(source.mount().root())) {
+ EXIT(EXIT_FAILURE)
+ << "Failed to find mount point '" << source.mount().root() << "'";
+ }
+#endif // __linux__
+ break;
+ }
+ case Resource::DiskInfo::Source::UNKNOWN: {
EXIT(EXIT_FAILURE)
- << "Failed to find mount point '" << source.mount().root() << "'";
+ << "Unsupported 'DiskInfo.Source.Type' in '" << resource << "'";
}
-#endif // __linux__
- } else {
- EXIT(EXIT_FAILURE)
- << "Unsupported 'DiskInfo.Source.Type' in '" << resource << "'";
}
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/75c6dfaa/src/v1/resources.cpp
----------------------------------------------------------------------
diff --git a/src/v1/resources.cpp b/src/v1/resources.cpp
index 6008a9d..a53deaf 100644
--- a/src/v1/resources.cpp
+++ b/src/v1/resources.cpp
@@ -779,16 +779,25 @@ Option<Error> Resources::validate(const Resource& resource)
if (disk.has_source()) {
const Resource::DiskInfo::Source& source = disk.source();
- if (source.type() == Resource::DiskInfo::Source::PATH &&
- !source.has_path()) {
- return Error(
- "DiskInfo::Source 'type' set to 'PATH' but missing 'path' data");
- }
-
- if (source.type() == Resource::DiskInfo::Source::MOUNT &&
- !source.has_mount()) {
- return Error(
- "DiskInfo::Source 'type' set to 'MOUNT' but missing 'mount' data");
+ 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");
+ }
+ break;
+ case Resource::DiskInfo::Source::UNKNOWN:
+ return Error(
+ "Unsupported 'DiskInfo.Source.Type' in "
+ "'" + stringify(source) + "'");
}
}
}
@@ -1851,6 +1860,8 @@ ostream& operator<<(ostream& stream, const Resource::DiskInfo::Source& source)
return stream << "MOUNT:" + source.mount().root();
case Resource::DiskInfo::Source::PATH:
return stream << "PATH:" + source.path().root();
+ case Resource::DiskInfo::Source::UNKNOWN:
+ return stream << "UNKNOWN";
}
UNREACHABLE();