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:03 UTC
[1/3] mesos git commit: Fixed 'operator==' for
'Resource::DiskInfo::Source'.
Repository: mesos
Updated Branches:
refs/heads/master c0293a6f7 -> 8c4533623
Fixed 'operator==' for 'Resource::DiskInfo::Source'.
When implementing 'operator==' for protobufs as a pattern we typically
first check that two 'optional' fields are set for both the left- and
right-hand side, and only then compare their values, e.g., given a
definition
message Foo {
optional string bar = 1;
}
we would implement 'operator==' similar to the following,
bool operator==(const Foo& lhs, const Foo& rhs)
{
if (lhs.has_bar() != rhs.has_bar()) {
return false;
}
if (lhs.has_bar() && lhs.bar() != rhs.bar()) {
return false;
}
return true;
}
One reason for this is that it allows us to distinguish an unset field
from a set field containing a default constructed value (if e.g.,
above 'lhs.has_bar()' was 'false', 'lhs.bar()' would return an empty
string).
This patch makes sure we use the same pattern when checking
'Resource::DiskInfo::Source' for equality.
Review: https://reviews.apache.org/r/62282/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/591f74d9
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/591f74d9
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/591f74d9
Branch: refs/heads/master
Commit: 591f74d9dded61eee845547d666e1588ad2cc5df
Parents: c0293a6
Author: Benjamin Bannier <bb...@apache.org>
Authored: Thu Sep 21 15:03:03 2017 +0200
Committer: Benjamin Bannier <bb...@apache.org>
Committed: Thu Sep 21 21:15:08 2017 +0200
----------------------------------------------------------------------
src/common/resources.cpp | 8 ++++++++
1 file changed, 8 insertions(+)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/591f74d9/src/common/resources.cpp
----------------------------------------------------------------------
diff --git a/src/common/resources.cpp b/src/common/resources.cpp
index 14b600c..77c655d 100644
--- a/src/common/resources.cpp
+++ b/src/common/resources.cpp
@@ -174,10 +174,18 @@ bool operator==(
return false;
}
+ if (left.has_path() != right.has_path()) {
+ return false;
+ }
+
if (left.has_path() && left.path() != right.path()) {
return false;
}
+ if (left.has_mount() != right.has_mount()) {
+ return false;
+ }
+
if (left.has_mount() && left.mount() != right.mount()) {
return false;
}
[3/3] mesos git commit: Used a namespace alias for 'process::http'.
Posted by bb...@apache.org.
Used a namespace alias for 'process::http'.
This patch introduces 'http' as an alias for 'process::http'.
Review: https://reviews.apache.org/r/62439/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/8c453362
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/8c453362
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/8c453362
Branch: refs/heads/master
Commit: 8c453362356b644ddb5f780eb741ba18997d0e25
Parents: 91e279a
Author: Benjamin Bannier <bb...@apache.org>
Authored: Thu Sep 21 15:03:46 2017 +0200
Committer: Benjamin Bannier <bb...@apache.org>
Committed: Thu Sep 21 21:15:37 2017 +0200
----------------------------------------------------------------------
src/slave/slave.cpp | 24 +++++++++++++-----------
1 file changed, 13 insertions(+), 11 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/8c453362/src/slave/slave.cpp
----------------------------------------------------------------------
diff --git a/src/slave/slave.cpp b/src/slave/slave.cpp
index 01020a6..75e2e25 100644
--- a/src/slave/slave.cpp
+++ b/src/slave/slave.cpp
@@ -108,6 +108,8 @@
#include <slave/posix_signalhandler.hpp>
#endif // __WINDOWS__
+namespace http = process::http;
+
using google::protobuf::RepeatedPtrField;
using mesos::SecretGenerator;
@@ -423,7 +425,7 @@ void Slave::initialize()
}
#endif
- process::http::URL localResourceProviderURL(
+ http::URL localResourceProviderURL(
scheme,
self().address.ip,
self().address.port,
@@ -714,7 +716,7 @@ void Slave::initialize()
// and operators/tooling to use this endpoint?
READWRITE_HTTP_AUTHENTICATION_REALM,
Http::API_HELP(),
- [this](const process::http::Request& request,
+ [this](const http::Request& request,
const Option<Principal>& principal) {
logRequest(request);
return http.api(request, principal);
@@ -724,7 +726,7 @@ void Slave::initialize()
route("/api/v1/executor",
EXECUTOR_HTTP_AUTHENTICATION_REALM,
Http::EXECUTOR_HELP(),
- [this](const process::http::Request& request,
+ [this](const http::Request& request,
const Option<Principal>& principal) {
logRequest(request);
return http.executor(request, principal);
@@ -733,7 +735,7 @@ void Slave::initialize()
route("/api/v1/resource_provider",
READWRITE_HTTP_AUTHENTICATION_REALM,
Http::RESOURCE_PROVIDER_HELP(),
- [this](const process::http::Request& request,
+ [this](const http::Request& request,
const Option<Principal>& principal) {
logRequest(request);
return resourceProviderManager.api(request, principal);
@@ -744,7 +746,7 @@ void Slave::initialize()
route("/state.json",
READONLY_HTTP_AUTHENTICATION_REALM,
Http::STATE_HELP(),
- [this](const process::http::Request& request,
+ [this](const http::Request& request,
const Option<Principal>& principal) {
logRequest(request);
return http.state(request, principal);
@@ -752,7 +754,7 @@ void Slave::initialize()
route("/state",
READONLY_HTTP_AUTHENTICATION_REALM,
Http::STATE_HELP(),
- [this](const process::http::Request& request,
+ [this](const http::Request& request,
const Option<Principal>& principal) {
logRequest(request);
return http.state(request, principal);
@@ -760,20 +762,20 @@ void Slave::initialize()
route("/flags",
READONLY_HTTP_AUTHENTICATION_REALM,
Http::FLAGS_HELP(),
- [this](const process::http::Request& request,
+ [this](const http::Request& request,
const Option<Principal>& principal) {
logRequest(request);
return http.flags(request, principal);
});
route("/health",
Http::HEALTH_HELP(),
- [this](const process::http::Request& request) {
+ [this](const http::Request& request) {
return http.health(request);
});
route("/monitor/statistics",
READONLY_HTTP_AUTHENTICATION_REALM,
Http::STATISTICS_HELP(),
- [this](const process::http::Request& request,
+ [this](const http::Request& request,
const Option<Principal>& principal) {
logRequest(request);
return http.statistics(request, principal);
@@ -783,7 +785,7 @@ void Slave::initialize()
route("/monitor/statistics.json",
READONLY_HTTP_AUTHENTICATION_REALM,
Http::STATISTICS_HELP(),
- [this](const process::http::Request& request,
+ [this](const http::Request& request,
const Option<Principal>& principal) {
logRequest(request);
return http.statistics(request, principal);
@@ -791,7 +793,7 @@ void Slave::initialize()
route("/containers",
READONLY_HTTP_AUTHENTICATION_REALM,
Http::CONTAINERS_HELP(),
- [this](const process::http::Request& request,
+ [this](const http::Request& request,
const Option<Principal>& principal) {
logRequest(request);
return http.containers(request, principal);
[2/3] mesos git commit: Added 'id' and 'metadata' fields to
'Resource.DiskInfo.Source'.
Posted by bb...@apache.org.
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