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 2017/05/15 22:06:26 UTC
mesos git commit: Validate DESTROY operation in `Resources::apply()`.
Repository: mesos
Updated Branches:
refs/heads/master e506313cf -> d16e38f4b
Validate DESTROY operation in `Resources::apply()`.
Ensure that applying `DESTROY` operation removes this volume. This is
needed to handle `DESTROY` of shared volumes, i.e. to make sure we
correctly handle resources bookkeeping in the allocator so there is
only a single copy of the shared resource allocated if we allow the
operation to proceed.
Review: https://reviews.apache.org/r/59194/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/d16e38f4
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/d16e38f4
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/d16e38f4
Branch: refs/heads/master
Commit: d16e38f4b82ce6ae052de0d01aecc19c941befa3
Parents: e506313
Author: Anindya Sinha <an...@apple.com>
Authored: Mon May 15 15:00:04 2017 -0700
Committer: Jiang Yan Xu <xu...@apple.com>
Committed: Mon May 15 15:00:04 2017 -0700
----------------------------------------------------------------------
src/common/resources.cpp | 10 +++++++++-
src/tests/resources_tests.cpp | 20 ++++++++++++++++++++
src/v1/resources.cpp | 10 +++++++++-
3 files changed, 38 insertions(+), 2 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/d16e38f4/src/common/resources.cpp
----------------------------------------------------------------------
diff --git a/src/common/resources.cpp b/src/common/resources.cpp
index f6f02eb..1d07f1a 100644
--- a/src/common/resources.cpp
+++ b/src/common/resources.cpp
@@ -1411,6 +1411,15 @@ Try<Resources> Resources::apply(const Offer::Operation& operation) const
"Invalid DESTROY Operation: Persistent volume does not exist");
}
+ result.subtract(volume);
+
+ if (result.contains(volume)) {
+ return Error(
+ "Invalid DESTROY Operation: Persistent volume " +
+ stringify(volume) + " cannot be removed due to additional " +
+ "shared copies");
+ }
+
// Strip persistence and volume from the disk info so that we
// can subtract it from the original resources.
Resource stripped = volume;
@@ -1426,7 +1435,6 @@ Try<Resources> Resources::apply(const Offer::Operation& operation) const
// return the resource to non-shared state after destroy.
stripped.clear_shared();
- result.subtract(volume);
result.add(stripped);
}
break;
http://git-wip-us.apache.org/repos/asf/mesos/blob/d16e38f4/src/tests/resources_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/resources_tests.cpp b/src/tests/resources_tests.cpp
index 4cf320d..5dcbce2 100644
--- a/src/tests/resources_tests.cpp
+++ b/src/tests/resources_tests.cpp
@@ -2563,6 +2563,26 @@ TEST(ResourcesOperationTest, CreateSharedPersistentVolume)
}
+TEST(ResourcesOperationTest, DestroySharedPersistentVolumeMultipleCopies)
+{
+ Resources total = Resources::parse("cpus:1;mem:512;disk(role):800").get();
+ Resource volume1 = createDiskResource(
+ "200", "role", "1", "path", None(), true);
+
+ // Add 2 copies of the shared volume.
+ total += volume1;
+ total += volume1;
+
+ // DESTROY of the shared volume should fail since there are multiple
+ // shared copies in `total`.
+ Offer::Operation destroy1;
+ destroy1.set_type(Offer::Operation::DESTROY);
+ destroy1.mutable_destroy()->add_volumes()->CopyFrom(volume1);
+
+ EXPECT_ERROR(total.apply(destroy1));
+}
+
+
TEST(ResourcesOperationTest, FlattenResources)
{
Resources unreservedCpus = Resources::parse("cpus:1").get();
http://git-wip-us.apache.org/repos/asf/mesos/blob/d16e38f4/src/v1/resources.cpp
----------------------------------------------------------------------
diff --git a/src/v1/resources.cpp b/src/v1/resources.cpp
index cad069d..1bb5d07 100644
--- a/src/v1/resources.cpp
+++ b/src/v1/resources.cpp
@@ -1413,6 +1413,15 @@ Try<Resources> Resources::apply(const Offer::Operation& operation) const
"Invalid DESTROY Operation: Persistent volume does not exist");
}
+ result.subtract(volume);
+
+ if (result.contains(volume)) {
+ return Error(
+ "Invalid DESTROY Operation: Persistent volume " +
+ stringify(volume) + " cannot be removed due to additional " +
+ "shared copies");
+ }
+
// Strip persistence and volume from the disk info so that we
// can subtract it from the original resources.
Resource stripped = volume;
@@ -1428,7 +1437,6 @@ Try<Resources> Resources::apply(const Offer::Operation& operation) const
// return the resource to non-shared state after destroy.
stripped.clear_shared();
- result.subtract(volume);
result.add(stripped);
}
break;