You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by bb...@apache.org on 2017/09/21 19:18:04 UTC
[2/3] mesos git commit: Added 'id' and 'metadata' fields to
'Resource.DiskInfo.Source'.
Added 'id' and 'metadata' fields to 'Resource.DiskInfo.Source'.
IDs will allow to create distinguishable resources, e.g., of RAW or
BLOCK type. We also add a metadata field which can be used to expose
additional disk information.
Review: https://reviews.apache.org/r/58048/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/91e279ad
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/91e279ad
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/91e279ad
Branch: refs/heads/master
Commit: 91e279ad1855ac7f1ae628778731173aa603d5e3
Parents: 591f74d
Author: Benjamin Bannier <bb...@apache.org>
Authored: Thu Sep 21 15:03:22 2017 +0200
Committer: Benjamin Bannier <bb...@apache.org>
Committed: Thu Sep 21 21:15:37 2017 +0200
----------------------------------------------------------------------
include/mesos/mesos.proto | 3 ++
include/mesos/v1/mesos.proto | 3 ++
src/common/resources.cpp | 78 ++++++++++++++++++++++++++++-----
src/tests/resources_tests.cpp | 90 ++++++++++++++++++++++++++++++++++++++
src/v1/resources.cpp | 70 ++++++++++++++++++++++++-----
5 files changed, 220 insertions(+), 24 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/91e279ad/include/mesos/mesos.proto
----------------------------------------------------------------------
diff --git a/include/mesos/mesos.proto b/include/mesos/mesos.proto
index fa475dc..3b2d6bb 100644
--- a/include/mesos/mesos.proto
+++ b/include/mesos/mesos.proto
@@ -1340,6 +1340,9 @@ message Resource {
required Type type = 1;
optional Path path = 2;
optional Mount mount = 3;
+
+ optional string id = 4; // EXPERIMENTAL.
+ optional Labels metadata = 5; // EXPERIMENTAL.
}
optional Source source = 3;
http://git-wip-us.apache.org/repos/asf/mesos/blob/91e279ad/include/mesos/v1/mesos.proto
----------------------------------------------------------------------
diff --git a/include/mesos/v1/mesos.proto b/include/mesos/v1/mesos.proto
index dfba418..4ca4886 100644
--- a/include/mesos/v1/mesos.proto
+++ b/include/mesos/v1/mesos.proto
@@ -1323,6 +1323,9 @@ message Resource {
required Type type = 1;
optional Path path = 2;
optional Mount mount = 3;
+
+ optional string id = 4; // EXPERIMENTAL.
+ optional Labels metadata = 5; // EXPERIMENTAL.
}
optional Source source = 3;
http://git-wip-us.apache.org/repos/asf/mesos/blob/91e279ad/src/common/resources.cpp
----------------------------------------------------------------------
diff --git a/src/common/resources.cpp b/src/common/resources.cpp
index 77c655d..ea0f3a2 100644
--- a/src/common/resources.cpp
+++ b/src/common/resources.cpp
@@ -190,6 +190,22 @@ bool operator==(
return false;
}
+ if (left.has_id() != right.has_id()) {
+ return false;
+ }
+
+ if (left.has_id() && left.id() != right.id()) {
+ return false;
+ }
+
+ if (left.has_metadata() != right.has_metadata()) {
+ return false;
+ }
+
+ if (left.has_metadata() && left.metadata() != right.metadata()) {
+ return false;
+ }
+
return true;
}
@@ -358,11 +374,29 @@ static bool addable(const Resource& left, const Resource& right)
if (left.has_disk()) {
if (left.disk() != right.disk()) { return false; }
- // Two non-shared resources that represent exclusive 'MOUNT' disks
- // cannot be added together; this would defeat the exclusivity.
- if (left.disk().has_source() &&
- left.disk().source().type() == Resource::DiskInfo::Source::MOUNT) {
- return false;
+ if (left.disk().has_source()) {
+ switch (left.disk().source().type()) {
+ case Resource::DiskInfo::Source::PATH: {
+ // Two PATH resources can be added if their disks are identical.
+ break;
+ }
+ case Resource::DiskInfo::Source::BLOCK:
+ case Resource::DiskInfo::Source::MOUNT: {
+ // Two resources that represent exclusive 'MOUNT' or 'RAW' disks
+ // cannot be added together; this would defeat the exclusivity.
+ return false;
+ }
+ case Resource::DiskInfo::Source::RAW: {
+ // We can only add resources representing 'RAW' disks if
+ // they have no identity or are identical.
+ if (left.disk().source().has_id()) {
+ return false;
+ }
+ break;
+ }
+ case Resource::DiskInfo::Source::UNKNOWN:
+ UNREACHABLE();
+ }
}
// TODO(jieyu): Even if two Resource objects with DiskInfo have
@@ -445,13 +479,33 @@ static bool subtractable(const Resource& left, const Resource& right)
if (left.has_disk()) {
if (left.disk() != right.disk()) { return false; }
- // Two resources that represent exclusive 'MOUNT' disks cannot be
- // subtracted from each other if they are not the exact same mount;
- // this would defeat the exclusivity.
- if (left.disk().has_source() &&
- left.disk().source().type() == Resource::DiskInfo::Source::MOUNT &&
- left != right) {
- return false;
+ if (left.disk().has_source()) {
+ switch (left.disk().source().type()) {
+ case Resource::DiskInfo::Source::PATH: {
+ // Two PATH resources can be subtracted if their disks are identical.
+ break;
+ }
+ case Resource::DiskInfo::Source::BLOCK:
+ case Resource::DiskInfo::Source::MOUNT: {
+ // Two resources that represent exclusive 'MOUNT' or 'BLOCK' disks
+ // cannot be subtracted from each other if they are not the exact same
+ // mount; this would defeat the exclusivity.
+ if (left != right) {
+ return false;
+ }
+ break;
+ }
+ case Resource::DiskInfo::Source::RAW: {
+ // We can only subtract resources representing 'RAW' disks
+ // if they have no identity, or refer to the same disk.
+ if (left.disk().source().has_id() && left != right) {
+ return false;
+ }
+ break;
+ }
+ case Resource::DiskInfo::Source::UNKNOWN:
+ UNREACHABLE();
+ }
}
// NOTE: For Resource objects that have DiskInfo, we can only subtract
http://git-wip-us.apache.org/repos/asf/mesos/blob/91e279ad/src/tests/resources_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/resources_tests.cpp b/src/tests/resources_tests.cpp
index 8a86efe..7f0150f 100644
--- a/src/tests/resources_tests.cpp
+++ b/src/tests/resources_tests.cpp
@@ -2248,6 +2248,96 @@ TEST(DiskResourcesTest, DiskSourceEquals)
}
+class DiskResourcesSourceTest
+ : public ::testing::Test,
+ public ::testing::WithParamInterface<
+ std::tr1::tuple<Resource::DiskInfo::Source::Type, bool>> {};
+
+
+INSTANTIATE_TEST_CASE_P(
+ TypeAndIdentity,
+ DiskResourcesSourceTest,
+ ::testing::Combine(
+ // We test all source types.
+ ::testing::Values(
+ Resource::DiskInfo::Source::RAW,
+ Resource::DiskInfo::Source::PATH,
+ Resource::DiskInfo::Source::BLOCK,
+ Resource::DiskInfo::Source::MOUNT),
+ // We test the case where the source has identity (i.e., has
+ // an `id` set) and where not.
+ ::testing::Bool()));
+
+
+TEST_P(DiskResourcesSourceTest, SourceIdentity)
+{
+ const std::tr1::tuple<Resource::DiskInfo::Source::Type, bool>& parameters =
+ GetParam();
+
+ Resource::DiskInfo::Source::Type type = std::tr1::get<0>(parameters);
+ bool hasIdentity = std::tr1::get<1>(parameters);
+
+ // Create a disk, possibly with an id to signify identiy.
+ Resource::DiskInfo::Source source;
+ source.set_type(type);
+
+ if (hasIdentity) {
+ source.set_id("id");
+ }
+
+ // Create two disk resources with the created source.
+ Resource disk1 = Resources::parse("disk", "1", "*").get();
+ disk1.mutable_disk()->mutable_source()->CopyFrom(source);
+ const Resources r1 = disk1;
+
+ EXPECT_TRUE(r1.contains(r1));
+
+ Resource disk2 = Resources::parse("disk", "2", "*").get();
+ disk2.mutable_disk()->mutable_source()->CopyFrom(source);
+ const Resources r2 = disk2;
+
+ // We perform three checks here: checks involving `r1` and `r2`
+ // test subtraction semantics while tests of the size of the
+ // resources test addition semantics.
+ switch (type) {
+ case Resource::DiskInfo::Source::RAW: {
+ if (hasIdentity) {
+ // `RAW` resources with source identity cannot be added or split.
+ EXPECT_FALSE(r2.contains(r1));
+ EXPECT_NE(r2, r1 + r1);
+ EXPECT_EQ(2u, (r1 + r1).size());
+ } else {
+ // `RAW` resources without source identity can be added and split.
+ EXPECT_TRUE(r2.contains(r1));
+ EXPECT_EQ(r2, r1 + r1);
+ EXPECT_EQ(1u, (r1 + r1).size());
+ }
+ break;
+ }
+ case Resource::DiskInfo::Source::BLOCK:
+ case Resource::DiskInfo::Source::MOUNT: {
+ // `BLOCK` or `MOUNT` resources cannot be added or split,
+ // regardless of identity.
+ EXPECT_FALSE(r2.contains(r1));
+ EXPECT_NE(r2, r1 + r1);
+ EXPECT_EQ(2u, (r1 + r1).size());
+ break;
+ }
+ case Resource::DiskInfo::Source::PATH: {
+ // `PATH` resources can be added and split, regardless of identity.
+ EXPECT_TRUE(r2.contains(r1));
+ EXPECT_EQ(r2, r1 + r1);
+ EXPECT_EQ(1u, (r1 + r1).size());
+ break;
+ }
+ case Resource::DiskInfo::Source::UNKNOWN: {
+ FAIL() << "Unexpected source type";
+ break;
+ }
+ }
+}
+
+
TEST(DiskResourcesTest, Addition)
{
Resources r1 = createDiskResource("10", "role", None(), "path");
http://git-wip-us.apache.org/repos/asf/mesos/blob/91e279ad/src/v1/resources.cpp
----------------------------------------------------------------------
diff --git a/src/v1/resources.cpp b/src/v1/resources.cpp
index a5cc155..a64180b 100644
--- a/src/v1/resources.cpp
+++ b/src/v1/resources.cpp
@@ -182,6 +182,14 @@ bool operator==(
return false;
}
+ if (left.has_id() && left.id() != right.id()) {
+ return false;
+ }
+
+ if (left.has_metadata() && left.metadata() != right.metadata()) {
+ return false;
+ }
+
return true;
}
@@ -350,11 +358,29 @@ static bool addable(const Resource& left, const Resource& right)
if (left.has_disk()) {
if (left.disk() != right.disk()) { return false; }
- // Two non-shared resources that represent exclusive 'MOUNT' disks
- // cannot be added together; this would defeat the exclusivity.
- if (left.disk().has_source() &&
- left.disk().source().type() == Resource::DiskInfo::Source::MOUNT) {
- return false;
+ if (left.disk().has_source()) {
+ switch (left.disk().source().type()) {
+ case Resource::DiskInfo::Source::PATH: {
+ // Two PATH resources can be added if their disks are identical.
+ break;
+ }
+ case Resource::DiskInfo::Source::BLOCK:
+ case Resource::DiskInfo::Source::MOUNT: {
+ // Two resources that represent exclusive 'MOUNT' or 'BLOCK' disks
+ // cannot be added together; this would defeat the exclusivity.
+ return false;
+ }
+ case Resource::DiskInfo::Source::RAW: {
+ // We can only add resources representing 'RAW' disks if
+ // they have no identity or are identical.
+ if (left.disk().source().has_id()) {
+ return false;
+ }
+ break;
+ }
+ case Resource::DiskInfo::Source::UNKNOWN:
+ UNREACHABLE();
+ }
}
// TODO(jieyu): Even if two Resource objects with DiskInfo have
@@ -437,13 +463,33 @@ static bool subtractable(const Resource& left, const Resource& right)
if (left.has_disk()) {
if (left.disk() != right.disk()) { return false; }
- // Two resources that represent exclusive 'MOUNT' disks cannot be
- // subtracted from each other if they are not the exact same mount;
- // this would defeat the exclusivity.
- if (left.disk().has_source() &&
- left.disk().source().type() == Resource::DiskInfo::Source::MOUNT &&
- left != right) {
- return false;
+ if (left.disk().has_source()) {
+ switch (left.disk().source().type()) {
+ case Resource::DiskInfo::Source::PATH: {
+ // Two PATH resources can be subtracted if their disks are identical.
+ break;
+ }
+ case Resource::DiskInfo::Source::BLOCK:
+ case Resource::DiskInfo::Source::MOUNT: {
+ // Two resources that represent exclusive 'MOUNT' or 'BLOCK' disks
+ // cannot be subtracted from each other if they are not the exact same
+ // mount; this would defeat the exclusivity.
+ if (left != right) {
+ return false;
+ }
+ break;
+ }
+ case Resource::DiskInfo::Source::RAW: {
+ // We can only subtract resources representing 'RAW' disks
+ // if they have no identity.
+ if (left.disk().source().has_id() && left != right) {
+ return false;
+ }
+ break;
+ }
+ case Resource::DiskInfo::Source::UNKNOWN:
+ UNREACHABLE();
+ }
}
// NOTE: For Resource objects that have DiskInfo, we can only subtract