Added validation for Resource objects with DiskInfo. Review: https://reviews.apache.org/r/28258
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/c1940a90 Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/c1940a90 Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/c1940a90 Branch: refs/heads/master Commit: c1940a9049d5c2a7afe5200597e49654829e38d2 Parents: f42d250 Author: Jie Yu <[email protected]> Authored: Wed Nov 19 12:49:23 2014 -0800 Committer: Jie Yu <[email protected]> Committed: Thu Nov 20 16:10:31 2014 -0800 ---------------------------------------------------------------------- src/common/resources.cpp | 28 +++++++++++++++ src/tests/resources_tests.cpp | 74 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 102 insertions(+) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/c1940a90/src/common/resources.cpp ---------------------------------------------------------------------- diff --git a/src/common/resources.cpp b/src/common/resources.cpp index 61d16a8..5cd64ff 100644 --- a/src/common/resources.cpp +++ b/src/common/resources.cpp @@ -308,6 +308,34 @@ Option<Error> Resources::validate(const Resource& resource) return Error("Unsupported resource type"); } + // Checks for 'disk' resource. + if (resource.has_disk()) { + if (resource.name() != "disk") { + return Error( + "DiskInfo should not be set for " + resource.name() + " resource"); + } + + if (resource.disk().has_persistence()) { + if (resource.role() == "*") { + return Error("Persistent disk volume is disallowed for '*' role"); + } + + if (!resource.disk().has_volume()) { + return Error("Persistent disk should specify a volume"); + } + } + + if (resource.disk().has_volume()) { + if (resource.disk().volume().mode() == Volume::RO) { + return Error("Do not support RO volume in DiskInfo"); + } + + if (resource.disk().volume().has_host_path()) { + return Error("Volume in DiskInfo should not have 'host_path' set"); + } + } + } + return None(); } http://git-wip-us.apache.org/repos/asf/mesos/blob/c1940a90/src/tests/resources_tests.cpp ---------------------------------------------------------------------- diff --git a/src/tests/resources_tests.cpp b/src/tests/resources_tests.cpp index 73f50ba..3857f8b 100644 --- a/src/tests/resources_tests.cpp +++ b/src/tests/resources_tests.cpp @@ -798,3 +798,77 @@ TEST(ResourcesTest, Find) EXPECT_NONE(resources4.find(targets4)); } + + +class DiskResourcesTest : public ::testing::Test +{ +public: + Resource::DiskInfo createDiskInfo( + const Option<string>& persistenceID, + const Option<string>& containerPath) + { + Resource::DiskInfo info; + + if (persistenceID.isSome()) { + Resource::DiskInfo::Persistence persistence; + persistence.set_id(persistenceID.get()); + info.mutable_persistence()->CopyFrom(persistence); + } + + if (containerPath.isSome()) { + Volume volume; + volume.set_container_path(containerPath.get()); + volume.set_mode(Volume::RW); + info.mutable_volume()->CopyFrom(volume); + } + + return info; + } + + Resource createDiskResource( + const string& value, + const string& role, + const Option<string>& persistenceID, + const Option<string>& containerPath) + { + Resource resource = Resources::parse("disk", value, role).get(); + + if (persistenceID.isSome() || containerPath.isSome()) { + resource.mutable_disk()->CopyFrom( + createDiskInfo(persistenceID, containerPath)); + } + + return resource; + } +}; + + +TEST_F(DiskResourcesTest, Validation) +{ + Resource cpus = Resources::parse("cpus", "2", "*").get(); + cpus.mutable_disk()->CopyFrom(createDiskInfo("1", "path")); + + Option<Error> error = Resources::validate(cpus); + ASSERT_SOME(error); + EXPECT_EQ( + "Resource with DiskInfo does not have the name 'disk'", + error.get().message); + + error = Resources::validate(createDiskResource("10", "*", "1", "path")); + ASSERT_SOME(error); + EXPECT_EQ( + "Do not allow a persistent disk volume without reservation", + error.get().message); + + error = Resources::validate(createDiskResource("10", "role", "1", None())); + ASSERT_SOME(error); + EXPECT_EQ( + "Persistent disk should specify a volume", + error.get().message); + + EXPECT_NONE( + Resources::validate(createDiskResource("10", "role", "1", "path"))); + + EXPECT_NONE( + Resources::validate(createDiskResource("10", "*", None(), "path"))); +}
