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 <[email protected]> Authored: Wed Feb 4 15:35:29 2015 -0800 Committer: Jie Yu <[email protected]> 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())); +}
