[09/15] mesos git commit: Added validation for disk related new operations.

[ji...@apache.org]

Added validation for disk related new operations.

(This is based on https://reviews.apache.org/r/61946)

Review: https://reviews.apache.org/r/63355


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/6c8ae681
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/6c8ae681
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/6c8ae681

Branch: refs/heads/master
Commit: 6c8ae68180f92bba0dbdf0516cc833c04f958f5b
Parents: ae8619d
Author: Jan Schlicht <ja...@mesosphere.io>
Authored: Tue Oct 24 00:37:36 2017 -0700
Committer: Jie Yu <yu...@gmail.com>
Committed: Mon Oct 30 16:18:31 2017 +0100

----------------------------------------------------------------------
 src/master/validation.cpp             |  90 ++++++++++++++++
 src/master/validation.hpp             |   6 ++
 src/tests/master_validation_tests.cpp | 164 +++++++++++++++++++++++++++++
 3 files changed, 260 insertions(+)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/6c8ae681/src/master/validation.cpp
----------------------------------------------------------------------
diff --git a/src/master/validation.cpp b/src/master/validation.cpp
index d8ff1fd..d6d6f21 100644
--- a/src/master/validation.cpp
+++ b/src/master/validation.cpp
@@ -2268,6 +2268,96 @@ Option<Error> validate(
   return None();
 }
 
+
+Option<Error> validate(const Offer::Operation::CreateVolume& createVolume)
+{
+  const Resource& source = createVolume.source();
+
+  Option<Error> error = resource::validate(Resources(source));
+  if (error.isSome()) {
+    return Error("Invalid resource: " + error->message);
+  }
+
+  if (!Resources::hasResourceProvider(source)) {
+    return Error("Does not have a resource provider");
+  }
+
+  if (!Resources::isDisk(source, Resource::DiskInfo::Source::RAW)) {
+    return Error("'source' is not a RAW disk resource");
+  }
+
+  if (createVolume.target_type() != Resource::DiskInfo::Source::MOUNT &&
+      createVolume.target_type() != Resource::DiskInfo::Source::PATH) {
+    return Error("'target_type' is neither MOUNT or PATH");
+  }
+
+  return None();
+}
+
+
+Option<Error> validate(const Offer::Operation::DestroyVolume& destroyVolume)
+{
+  const Resource& volume = destroyVolume.volume();
+
+  Option<Error> error = resource::validate(Resources(volume));
+  if (error.isSome()) {
+    return Error("Invalid resource: " + error->message);
+  }
+
+  if (!Resources::hasResourceProvider(volume)) {
+    return Error("Does not have a resource provider");
+  }
+
+  if (!Resources::isDisk(volume, Resource::DiskInfo::Source::MOUNT) &&
+      !Resources::isDisk(volume, Resource::DiskInfo::Source::PATH)) {
+    return Error("'volume' is neither a MOUTN or PATH disk resource");
+  }
+
+  return None();
+}
+
+
+Option<Error> validate(const Offer::Operation::CreateBlock& createBlock)
+{
+  const Resource& source = createBlock.source();
+
+  Option<Error> error = resource::validate(Resources(source));
+  if (error.isSome()) {
+    return Error("Invalid resource: " + error.get().message);
+  }
+
+  if (!Resources::hasResourceProvider(source)) {
+    return Error("Does not have a resource provider");
+  }
+
+  if (!Resources::isDisk(source, Resource::DiskInfo::Source::RAW)) {
+    return Error("'source' is not a RAW disk resource");
+  }
+
+  return None();
+}
+
+
+Option<Error> validate(const Offer::Operation::DestroyBlock& destroyBlock)
+{
+  const Resource& block = destroyBlock.block();
+
+  Option<Error> error = resource::validate(Resources(block));
+  if (error.isSome()) {
+    return Error("Invalid resource: " + error.get().message);
+  }
+
+  if (!Resources::hasResourceProvider(block)) {
+    return Error("Does not have a resource provider");
+  }
+
+  if (!Resources::isDisk(block, Resource::DiskInfo::Source::BLOCK)) {
+    return Error("'block' is not a BLOCK disk resource");
+  }
+
+  return None();
+}
+
 } // namespace operation {
 
 } // namespace validation {

http://git-wip-us.apache.org/repos/asf/mesos/blob/6c8ae681/src/master/validation.hpp
----------------------------------------------------------------------
diff --git a/src/master/validation.hpp b/src/master/validation.hpp
index ccad352..ac54062 100644
--- a/src/master/validation.hpp
+++ b/src/master/validation.hpp
@@ -306,6 +306,12 @@ Option<Error> validate(
     const hashmap<FrameworkID, hashmap<TaskID, TaskInfo>>& pendingTasks,
     const Option<FrameworkInfo>& frameworkInfo = None());
 
+
+Option<Error> validate(const Offer::Operation::CreateVolume& createVolume);
+Option<Error> validate(const Offer::Operation::DestroyVolume& destroyVolume);
+Option<Error> validate(const Offer::Operation::CreateBlock& createBlock);
+Option<Error> validate(const Offer::Operation::DestroyBlock& destroyBlock);
+
 } // namespace operation {
 
 } // namespace validation {

http://git-wip-us.apache.org/repos/asf/mesos/blob/6c8ae681/src/tests/master_validation_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/master_validation_tests.cpp b/src/tests/master_validation_tests.cpp
index 4cf3ba7..be8c892 100644
--- a/src/tests/master_validation_tests.cpp
+++ b/src/tests/master_validation_tests.cpp
@@ -1219,6 +1219,170 @@ TEST_F(DestroyOperationValidationTest, UnknownPersistentVolume)
 }
 
 
+TEST(OperationValidationTest, CreateVolume)
+{
+  Resource disk1 = createDiskResource(
+      "10", "*", None(), None(), createDiskSourceRaw());
+
+  Resource disk2 = createDiskResource(
+      "20", "*", None(), None(), createDiskSourceMount());
+
+  Resource disk3 = createDiskResource(
+      "30", "*", None(), None(), createDiskSourceRaw());
+
+  disk1.mutable_provider_id()->set_value("provider1");
+  disk2.mutable_provider_id()->set_value("provider2");
+
+  Offer::Operation::CreateVolume createVolume;
+  createVolume.mutable_source()->CopyFrom(disk1);
+  createVolume.set_target_type(Resource::DiskInfo::Source::MOUNT);
+
+  Option<Error> error = operation::validate(createVolume);
+  EXPECT_NONE(error);
+
+  createVolume.mutable_source()->CopyFrom(disk2);
+  createVolume.set_target_type(Resource::DiskInfo::Source::MOUNT);
+
+  error = operation::validate(createVolume);
+  ASSERT_SOME(error);
+  EXPECT_TRUE(strings::contains(
+      error->message,
+      "'source' is not a RAW disk resource"));
+
+  createVolume.mutable_source()->CopyFrom(disk3);
+  createVolume.set_target_type(Resource::DiskInfo::Source::PATH);
+
+  error = operation::validate(createVolume);
+  ASSERT_SOME(error);
+  EXPECT_TRUE(strings::contains(
+      error->message,
+      "Does not have a resource provider"));
+
+  createVolume.mutable_source()->CopyFrom(disk1);
+  createVolume.set_target_type(Resource::DiskInfo::Source::BLOCK);
+
+  error = operation::validate(createVolume);
+  ASSERT_SOME(error);
+  EXPECT_TRUE(strings::contains(
+      error->message,
+      "'target_type' is neither MOUNT or PATH"));
+}
+
+
+TEST(OperationValidationTest, DestroyVolume)
+{
+  Resource disk1 = createDiskResource(
+      "10", "*", None(), None(), createDiskSourceMount());
+
+  Resource disk2 = createDiskResource(
+      "20", "*", None(), None(), createDiskSourcePath());
+
+  Resource disk3 = createDiskResource(
+      "30", "*", None(), None(), createDiskSourceRaw());
+
+  disk1.mutable_provider_id()->set_value("provider1");
+  disk3.mutable_provider_id()->set_value("provider3");
+
+  Offer::Operation::DestroyVolume destroyVolume;
+  destroyVolume.mutable_volume()->CopyFrom(disk1);
+
+  Option<Error> error = operation::validate(destroyVolume);
+  EXPECT_NONE(error);
+
+  destroyVolume.mutable_volume()->CopyFrom(disk2);
+
+  error = operation::validate(destroyVolume);
+  ASSERT_SOME(error);
+  EXPECT_TRUE(strings::contains(
+      error->message,
+      "Does not have a resource provider"));
+
+  destroyVolume.mutable_volume()->CopyFrom(disk3);
+
+  error = operation::validate(destroyVolume);
+  ASSERT_SOME(error);
+  EXPECT_TRUE(strings::contains(
+      error->message,
+      "'volume' is neither a MOUTN or PATH disk resource"));
+}
+
+
+TEST(OperationValidationTest, CreateBlock)
+{
+  Resource disk1 = createDiskResource(
+      "10", "*", None(), None(), createDiskSourceRaw());
+
+  Resource disk2 = createDiskResource(
+      "20", "*", None(), None(), createDiskSourceMount());
+
+  Resource disk3 = createDiskResource(
+      "30", "*", None(), None(), createDiskSourceRaw());
+
+  disk1.mutable_provider_id()->set_value("provider1");
+  disk2.mutable_provider_id()->set_value("provider2");
+
+  Offer::Operation::CreateBlock createBlock;
+  createBlock.mutable_source()->CopyFrom(disk1);
+
+  Option<Error> error = operation::validate(createBlock);
+  EXPECT_NONE(error);
+
+  createBlock.mutable_source()->CopyFrom(disk2);
+
+  error = operation::validate(createBlock);
+  ASSERT_SOME(error);
+  EXPECT_TRUE(strings::contains(
+      error->message,
+      "'source' is not a RAW disk resource"));
+
+  createBlock.mutable_source()->CopyFrom(disk3);
+
+  error = operation::validate(createBlock);
+  ASSERT_SOME(error);
+  EXPECT_TRUE(strings::contains(
+      error->message,
+      "Does not have a resource provider"));
+}
+
+
+TEST(OperationValidationTest, DestroyBlock)
+{
+  Resource disk1 = createDiskResource(
+      "10", "*", None(), None(), createDiskSourceBlock());
+
+  Resource disk2 = createDiskResource(
+      "20", "*", None(), None(), createDiskSourceBlock());
+
+  Resource disk3 = createDiskResource(
+      "30", "*", None(), None(), createDiskSourceMount());
+
+  disk1.mutable_provider_id()->set_value("provider1");
+  disk3.mutable_provider_id()->set_value("provider3");
+
+  Offer::Operation::DestroyBlock destroyBlock;
+  destroyBlock.mutable_block()->CopyFrom(disk1);
+
+  Option<Error> error = operation::validate(destroyBlock);
+  EXPECT_NONE(error);
+
+  destroyBlock.mutable_block()->CopyFrom(disk2);
+
+  error = operation::validate(destroyBlock);
+  ASSERT_SOME(error);
+  EXPECT_TRUE(strings::contains(
+      error->message,
+      "Does not have a resource provider"));
+
+  destroyBlock.mutable_block()->CopyFrom(disk3);
+
+  error = operation::validate(destroyBlock);
+  ASSERT_SOME(error);
+  EXPECT_TRUE(strings::contains(
+      error->message,
+      "'block' is not a BLOCK disk resource"));
+}
+
+
 // TODO(jieyu): All of the task validation tests have the same flow:
 // launch a task, expect an update of a particular format (invalid w/
 // message). Consider providing common functionalities in the test