[2/3] mesos git commit: Added validation for Resource objects with DiskInfo.

[ji...@apache.org]

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 <yu...@gmail.com>
Authored: Wed Nov 19 12:49:23 2014 -0800
Committer: Jie Yu <yu...@gmail.com>
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")));
+}