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 2015/02/05 01:04:47 UTC
[1/2] mesos git commit: Added implementation for DESTROY operation.
Repository: mesos
Updated Branches:
refs/heads/master e95dda149 -> b05e2f0bb
Added implementation for DESTROY operation.
Review: https://reviews.apache.org/r/30644
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/b05e2f0b
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/b05e2f0b
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/b05e2f0b
Branch: refs/heads/master
Commit: b05e2f0bbd3447df21ad183a061a73c5d481464e
Parents: 9ef2e50
Author: Jie Yu <yu...@gmail.com>
Authored: Wed Feb 4 15:45:24 2015 -0800
Committer: Jie Yu <yu...@gmail.com>
Committed: Wed Feb 4 16:04:17 2015 -0800
----------------------------------------------------------------------
src/master/master.cpp | 34 +++++++++++++++++++++++++++++-----
1 file changed, 29 insertions(+), 5 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/b05e2f0b/src/master/master.cpp
----------------------------------------------------------------------
diff --git a/src/master/master.cpp b/src/master/master.cpp
index f4b6463..46caa55 100644
--- a/src/master/master.cpp
+++ b/src/master/master.cpp
@@ -2292,8 +2292,29 @@ void Master::_accept(
}
case Offer::Operation::DESTROY: {
- // TODO(jieyu): Provide implementation for DESTROY.
- drop(framework, operation, "Unimplemented");
+ Option<Error> error = validation::operation::validate(
+ operation.destroy(),
+ slave->checkpointedResources);
+
+ if (error.isSome()) {
+ drop(framework, operation, error.get().message);
+ continue;
+ }
+
+ Try<Resources> resources = _offeredResources.apply(operation);
+ if (resources.isError()) {
+ drop(framework, operation, error.get().message);
+ continue;
+ }
+
+ _offeredResources = resources.get();
+
+ allocator->updateAllocation(
+ frameworkId,
+ slaveId,
+ {operation});
+
+ updateCheckpointedResources(slave, operation);
break;
}
@@ -4640,9 +4661,12 @@ void Master::updateCheckpointedResources(
}
case Offer::Operation::DESTROY: {
- // TODO(jieyu): Provide implementation.
- LOG(ERROR) << "Failed to update checkpointed resources for slave "
- << *slave << ": Unimplemented DESTROY operation";
+ Resources volumes = operation.destroy().volumes();
+
+ CHECK(slave->checkpointedResources.contains(volumes))
+ << "Not expecting unknown persistent volumes";
+
+ slave->checkpointedResources -= volumes;
break;
}
[2/2] mesos git commit: Added validation for DESTROY operation.
Posted by ji...@apache.org.
Added validation for DESTROY operation.
Review: https://reviews.apache.org/r/30642
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/9ef2e504
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/9ef2e504
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/9ef2e504
Branch: refs/heads/master
Commit: 9ef2e504fbe0610808c26ade322c992f6912748c
Parents: e95dda1
Author: Jie Yu <yu...@gmail.com>
Authored: Wed Feb 4 15:35:29 2015 -0800
Committer: Jie Yu <yu...@gmail.com>
Committed: Wed Feb 4 16:04:17 2015 -0800
----------------------------------------------------------------------
src/master/validation.cpp | 22 +++++++++++++++
src/master/validation.hpp | 7 +++++
src/tests/master_validation_tests.cpp | 43 ++++++++++++++++++++++++++++++
3 files changed, 72 insertions(+)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/9ef2e504/src/master/validation.cpp
----------------------------------------------------------------------
diff --git a/src/master/validation.cpp b/src/master/validation.cpp
index 550782e..2c4b16d 100644
--- a/src/master/validation.cpp
+++ b/src/master/validation.cpp
@@ -556,6 +556,28 @@ Option<Error> validate(
return None();
}
+
+Option<Error> validate(
+ const Offer::Operation::Destroy& destroy,
+ const Resources& checkpointedResources)
+{
+ Option<Error> error = resource::validate(destroy.volumes());
+ if (error.isSome()) {
+ return Error("Invalid resources: " + error.get().message);
+ }
+
+ error = resource::validatePersistentVolume(destroy.volumes());
+ if (error.isSome()) {
+ return Error("Not a persistent volume: " + error.get().message);
+ }
+
+ if (!checkpointedResources.contains(destroy.volumes())) {
+ return Error("Persistent volumes not found");
+ }
+
+ return None();
+}
+
} // namespace operation {
} // namespace validation {
http://git-wip-us.apache.org/repos/asf/mesos/blob/9ef2e504/src/master/validation.hpp
----------------------------------------------------------------------
diff --git a/src/master/validation.hpp b/src/master/validation.hpp
index 81dc7ee..2d7416c 100644
--- a/src/master/validation.hpp
+++ b/src/master/validation.hpp
@@ -87,6 +87,13 @@ Option<Error> validate(
const Offer::Operation::Create& create,
const Resources& checkpointedResources);
+
+// Validates the DESTROY operation. We need slave's checkpointed
+// resources to validate that the volumes to destroy actually exist.
+Option<Error> validate(
+ const Offer::Operation::Destroy& destroy,
+ const Resources& checkpointedResources);
+
} // namespace operation {
} // namespace validation {
http://git-wip-us.apache.org/repos/asf/mesos/blob/9ef2e504/src/tests/master_validation_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/master_validation_tests.cpp b/src/tests/master_validation_tests.cpp
index 1277311..81c635f 100644
--- a/src/tests/master_validation_tests.cpp
+++ b/src/tests/master_validation_tests.cpp
@@ -154,3 +154,46 @@ TEST_F(CreateOperationValidationTest, DuplicatedPersistenceID)
EXPECT_SOME(operation::validate(create, Resources()));
}
+
+
+class DestroyOperationValidationTest : public ::testing::Test {};
+
+
+// This test verifies that all resources specified in the DESTROY
+// operation are persistent volumes.
+TEST_F(DestroyOperationValidationTest, PersistentVolumes)
+{
+ Resource volume1 = Resources::parse("disk", "128", "role1").get();
+ volume1.mutable_disk()->CopyFrom(createDiskInfo("id1", "path1"));
+
+ Resource volume2 = Resources::parse("disk", "64", "role1").get();
+ volume2.mutable_disk()->CopyFrom(createDiskInfo("id2", "path2"));
+
+ Resources volumes;
+ volumes += volume1;
+ volumes += volume2;
+
+ Offer::Operation::Destroy destroy;
+ destroy.add_volumes()->CopyFrom(volume1);
+
+ EXPECT_NONE(operation::validate(destroy, volumes));
+
+ Resource cpus = Resources::parse("cpus", "2", "*").get();
+
+ destroy.add_volumes()->CopyFrom(cpus);
+
+ EXPECT_SOME(operation::validate(destroy, volumes));
+}
+
+
+TEST_F(DestroyOperationValidationTest, UnknownPersistentVolume)
+{
+ Resource volume = Resources::parse("disk", "128", "role1").get();
+ volume.mutable_disk()->CopyFrom(createDiskInfo("id1", "path1"));
+
+ Offer::Operation::Destroy destroy;
+ destroy.add_volumes()->CopyFrom(volume);
+
+ EXPECT_NONE(operation::validate(destroy, volume));
+ EXPECT_SOME(operation::validate(destroy, Resources()));
+}