You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by ya...@apache.org on 2016/11/04 17:07:35 UTC
mesos git commit: Allow CREATE of shared persistent volumes based on
capability.
Repository: mesos
Updated Branches:
refs/heads/master e1a3ea11f -> 6a22d4d95
Allow CREATE of shared persistent volumes based on capability.
When a framework issues a CREATE for a shared volume, we allow that
operation only if the framework has opted in for the capability of
SHARED_RESOURCES. However, for HTTP operator (/create-volumes), we
do not check as the operator API is not related to a specific
framework.
This is because it's counterintuitive if a framework can create a
shared persistent volume which cannot be offered to it.
Review: https://reviews.apache.org/r/52587/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/6a22d4d9
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/6a22d4d9
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/6a22d4d9
Branch: refs/heads/master
Commit: 6a22d4d95f664da71cb43f8f6c651cdf05efb308
Parents: e1a3ea1
Author: Anindya Sinha <an...@apple.com>
Authored: Fri Nov 4 09:26:15 2016 -0700
Committer: Jiang Yan Xu <xu...@apple.com>
Committed: Fri Nov 4 10:07:22 2016 -0700
----------------------------------------------------------------------
src/master/master.cpp | 5 ++++-
src/master/validation.cpp | 21 ++++++++++++++++++---
src/master/validation.hpp | 6 +++++-
src/tests/master_validation_tests.cpp | 28 ++++++++++++++++++++++++++++
src/tests/persistent_volume_tests.cpp | 2 ++
5 files changed, 57 insertions(+), 5 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/6a22d4d9/src/master/master.cpp
----------------------------------------------------------------------
diff --git a/src/master/master.cpp b/src/master/master.cpp
index 013bb59..15ad5a6 100644
--- a/src/master/master.cpp
+++ b/src/master/master.cpp
@@ -3975,7 +3975,10 @@ void Master::_accept(
// Make sure this create operation is valid.
Option<Error> error = validation::operation::validate(
- operation.create(), slave->checkpointedResources, principal);
+ operation.create(),
+ slave->checkpointedResources,
+ principal,
+ framework->info);
if (error.isSome()) {
drop(framework, operation, error.get().message);
http://git-wip-us.apache.org/repos/asf/mesos/blob/6a22d4d9/src/master/validation.cpp
----------------------------------------------------------------------
diff --git a/src/master/validation.cpp b/src/master/validation.cpp
index f690a9e..be4b0ac 100644
--- a/src/master/validation.cpp
+++ b/src/master/validation.cpp
@@ -1497,7 +1497,8 @@ Option<Error> validate(const Offer::Operation::Unreserve& unreserve)
Option<Error> validate(
const Offer::Operation::Create& create,
const Resources& checkpointedResources,
- const Option<string>& principal)
+ const Option<string>& principal,
+ const Option<FrameworkInfo>& frameworkInfo)
{
Option<Error> error = resource::validate(create.volumes());
if (error.isSome()) {
@@ -1516,9 +1517,23 @@ Option<Error> validate(
return error;
}
- // Ensure that the provided principals match. If `principal` is `None`, then
- // we allow `volume.disk().persistence().principal()` to take any value.
foreach (const Resource& volume, create.volumes()) {
+ // If the volume being created is a shared persistent volume, we
+ // allow it only if the framework has opted in for SHARED_RESOURCES.
+ if (frameworkInfo.isSome() &&
+ volume.has_shared() &&
+ !protobuf::frameworkHasCapability(
+ frameworkInfo.get(),
+ FrameworkInfo::Capability::SHARED_RESOURCES)) {
+ return Error(
+ "Create volume operation for a shared volume '" + stringify(volume) +
+ "' being attempted by framework '" +
+ stringify(frameworkInfo.get().id()) +
+ "' with no SHARED_RESOURCES capability");
+ }
+
+ // Ensure that the provided principals match. If `principal` is `None`,
+ // we allow `volume.disk().persistence().principal()` to take any value.
if (principal.isSome()) {
if (!volume.disk().persistence().has_principal()) {
return Error(
http://git-wip-us.apache.org/repos/asf/mesos/blob/6a22d4d9/src/master/validation.hpp
----------------------------------------------------------------------
diff --git a/src/master/validation.hpp b/src/master/validation.hpp
index 035f721..ed878ef 100644
--- a/src/master/validation.hpp
+++ b/src/master/validation.hpp
@@ -194,10 +194,14 @@ Option<Error> validate(const Offer::Operation::Unreserve& unreserve);
// Validates the CREATE operation. We need slave's checkpointed resources so
// that we can validate persistence ID uniqueness, and we need the principal to
// verify that it's equal to the one in `DiskInfo.Persistence.principal`.
+// We need the FrameworkInfo (unless the operation is requested by the
+// operator) to ensure shared volumes are created by frameworks with the
+// appropriate capability.
Option<Error> validate(
const Offer::Operation::Create& create,
const Resources& checkpointedResources,
- const Option<std::string>& principal);
+ const Option<std::string>& principal,
+ const Option<FrameworkInfo>& frameworkInfo = None());
// Validates the DESTROY operation. We need slave's checkpointed
http://git-wip-us.apache.org/repos/asf/mesos/blob/6a22d4d9/src/tests/master_validation_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/master_validation_tests.cpp b/src/tests/master_validation_tests.cpp
index a5d8610..8697bb2 100644
--- a/src/tests/master_validation_tests.cpp
+++ b/src/tests/master_validation_tests.cpp
@@ -502,6 +502,34 @@ TEST_F(CreateOperationValidationTest, ReadOnlyPersistentVolume)
}
+TEST_F(CreateOperationValidationTest, SharedVolumeBasedOnCapability)
+{
+ Resource volume = createDiskResource(
+ "128", "role1", "1", "path1", None(), true); // Shared.
+
+ Offer::Operation::Create create;
+ create.add_volumes()->CopyFrom(volume);
+
+ // When no FrameworkInfo is specified, validation is not dependent
+ // on any framework.
+ EXPECT_NONE(operation::validate(create, Resources(), None()));
+
+ // When a FrameworkInfo with no SHARED_RESOURCES capability is
+ // specified, the validation should fail.
+ FrameworkInfo frameworkInfo = DEFAULT_FRAMEWORK_INFO;
+ frameworkInfo.set_role("role1");
+
+ EXPECT_SOME(operation::validate(create, Resources(), None(), frameworkInfo));
+
+ // When a FrameworkInfo with SHARED_RESOURCES capability is specified,
+ // the validation should succeed.
+ frameworkInfo.add_capabilities()->set_type(
+ FrameworkInfo::Capability::SHARED_RESOURCES);
+
+ EXPECT_NONE(operation::validate(create, Resources(), None(), frameworkInfo));
+}
+
+
// This test verifies that creating a persistent volume that is larger
// than the offered disk resource results won't succeed.
TEST_F(CreateOperationValidationTest, InsufficientDiskResource)
http://git-wip-us.apache.org/repos/asf/mesos/blob/6a22d4d9/src/tests/persistent_volume_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/persistent_volume_tests.cpp b/src/tests/persistent_volume_tests.cpp
index 6289009..8651b2c 100644
--- a/src/tests/persistent_volume_tests.cpp
+++ b/src/tests/persistent_volume_tests.cpp
@@ -747,6 +747,8 @@ TEST_P(PersistentVolumeTest, AccessPersistentVolume)
FrameworkInfo frameworkInfo = DEFAULT_FRAMEWORK_INFO;
frameworkInfo.set_role(DEFAULT_TEST_ROLE);
+ frameworkInfo.add_capabilities()->set_type(
+ FrameworkInfo::Capability::SHARED_RESOURCES);
MockScheduler sched;
MesosSchedulerDriver driver(