Repository: mesos
Updated Branches:
  refs/heads/master 81cd023eb -> e65f580bf


Added validation for `ContainerInfo`.

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


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

Branch: refs/heads/master
Commit: e65f580bf0cbea64cedf521cf169b9b4c9f85454
Parents: 81cd023
Author: Alexander Rukletsov <al...@apache.org>
Authored: Wed Sep 14 00:29:19 2016 +0200
Committer: Alexander Rukletsov <al...@apache.org>
Committed: Fri Sep 16 15:06:38 2016 +0200

----------------------------------------------------------------------
 src/master/validation.cpp                       | 64 +++++++++++++++++++-
 src/tests/container_logger_tests.cpp            |  2 +
 src/tests/containerizer/cni_isolator_tests.cpp  |  3 +
 .../docker_containerizer_tests.cpp              |  2 +
 .../docker_volume_isolator_tests.cpp            |  5 ++
 .../containerizer/filesystem_isolator_tests.cpp |  1 +
 src/tests/containerizer/isolator_tests.cpp      |  2 +
 src/tests/master_validation_tests.cpp           |  6 +-
 src/tests/slave_tests.cpp                       |  2 +
 9 files changed, 84 insertions(+), 3 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/e65f580b/src/master/validation.cpp
----------------------------------------------------------------------
diff --git a/src/master/validation.cpp b/src/master/validation.cpp
index aa743d7..1103630 100644
--- a/src/master/validation.cpp
+++ b/src/master/validation.cpp
@@ -28,6 +28,7 @@
 #include <stout/lambda.hpp>
 #include <stout/none.hpp>
 #include <stout/stringify.hpp>
+#include <stout/unreachable.hpp>
 
 #include "health-check/health_checker.hpp"
 
@@ -52,6 +53,34 @@ static bool invalidCharacter(char c)
 }
 
 
+// Validates that `ContainerInfo` is valid.
+Option<Error> validateContainerInfo(const ContainerInfo& container)
+{
+  if (!container.has_type()) {
+    return Error("ContainerInfo must specify 'type'");
+  }
+
+  switch (container.type()) {
+    case ContainerInfo::MESOS:
+      if (!container.has_mesos()) {
+        return Error("Expecting 'mesos' to be set for mesos container");
+      }
+      break;
+
+    case ContainerInfo::DOCKER:
+      if (!container.has_docker()) {
+        return Error("Expecting 'docker' to be set for docker container");
+      }
+      break;
+
+    default:
+      UNREACHABLE();
+  }
+
+  return None();
+}
+
+
 namespace master {
 namespace call {
 
@@ -659,6 +688,21 @@ Option<Error> validateResources(const ExecutorInfo& 
executor)
 }
 
 
+Option<Error> validateContainer(const ExecutorInfo& executor)
+{
+  if (executor.has_container()) {
+    // Do the general validation first.
+    Option<Error> error =
+      validation::validateContainerInfo(executor.container());
+    if (error.isSome()) {
+      return Error("Executor uses invalid container: " + error->message);
+    }
+  }
+
+  return None();
+}
+
+
 Option<Error> validate(
     const ExecutorInfo& executor,
     Framework* framework,
@@ -673,7 +717,8 @@ Option<Error> validate(
     lambda::bind(internal::validateShutdownGracePeriod, executor),
     lambda::bind(internal::validateResources, executor),
     lambda::bind(
-        internal::validateCompatibleExecutorInfo, executor, framework, slave)
+        internal::validateCompatibleExecutorInfo, executor, framework, slave),
+    lambda::bind(internal::validateContainer, executor),
   };
 
   foreach (const lambda::function<Option<Error>()>& validator, validators) {
@@ -818,6 +863,20 @@ Option<Error> validateTaskAndExecutorResources(const 
TaskInfo& task)
 }
 
 
+Option<Error> validateContainer(const TaskInfo& task)
+{
+  if (task.has_container()) {
+    // Do the general validation first.
+    Option<Error> error = validation::validateContainerInfo(task.container());
+    if (error.isSome()) {
+      return Error("Task uses invalid container: " + error->message);
+    }
+  }
+
+  return None();
+}
+
+
 // Validates task specific fields except its executor (if it exists).
 Option<Error> validateTask(
     const TaskInfo& task,
@@ -835,7 +894,8 @@ Option<Error> validateTask(
     lambda::bind(internal::validateSlaveID, task, slave),
     lambda::bind(internal::validateKillPolicy, task),
     lambda::bind(internal::validateHealthCheck, task),
-    lambda::bind(internal::validateResources, task)
+    lambda::bind(internal::validateResources, task),
+    lambda::bind(internal::validateContainer, task)
   };
 
   // TODO(jieyu): Add a validateCommandInfo function.

http://git-wip-us.apache.org/repos/asf/mesos/blob/e65f580b/src/tests/container_logger_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/container_logger_tests.cpp 
b/src/tests/container_logger_tests.cpp
index 1b121a2..c8497c9 100644
--- a/src/tests/container_logger_tests.cpp
+++ b/src/tests/container_logger_tests.cpp
@@ -167,6 +167,8 @@ TEST_F(ContainerLoggerTest, MesosContainerizerRecover)
 
   ExecutorInfo executorInfo;
   executorInfo.mutable_container()->set_type(ContainerInfo::MESOS);
+  executorInfo.mutable_container()->mutable_mesos()->CopyFrom(
+      ContainerInfo::MesosInfo());
 
   ExecutorState executorState;
   executorState.id = executorId;

http://git-wip-us.apache.org/repos/asf/mesos/blob/e65f580b/src/tests/containerizer/cni_isolator_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/containerizer/cni_isolator_tests.cpp 
b/src/tests/containerizer/cni_isolator_tests.cpp
index 0d611c1..d8d8453 100644
--- a/src/tests/containerizer/cni_isolator_tests.cpp
+++ b/src/tests/containerizer/cni_isolator_tests.cpp
@@ -282,6 +282,7 @@ TEST_F(CniIsolatorTest, ROOT_VerifyCheckpointedInfo)
 
   ContainerInfo* container = task.mutable_container();
   container->set_type(ContainerInfo::MESOS);
+  container->mutable_mesos()->CopyFrom(ContainerInfo::MesosInfo());
 
   // Make sure the container join the mock CNI network.
   container->add_network_infos()->set_name("__MESOS_TEST__");
@@ -403,6 +404,7 @@ TEST_F(CniIsolatorTest, ROOT_FailedPlugin)
 
   ContainerInfo* container = task.mutable_container();
   container->set_type(ContainerInfo::MESOS);
+  container->mutable_mesos()->CopyFrom(ContainerInfo::MesosInfo());
   container->add_network_infos()->set_name("__MESOS_TEST__");
 
   Future<TaskStatus> statusFailed;
@@ -472,6 +474,7 @@ TEST_F(CniIsolatorTest, ROOT_SlaveRecovery)
 
   ContainerInfo* container = task.mutable_container();
   container->set_type(ContainerInfo::MESOS);
+  container->mutable_mesos()->CopyFrom(ContainerInfo::MesosInfo());
 
   // Make sure the container join the mock CNI network.
   container->add_network_infos()->set_name("__MESOS_TEST__");

http://git-wip-us.apache.org/repos/asf/mesos/blob/e65f580b/src/tests/containerizer/docker_containerizer_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/containerizer/docker_containerizer_tests.cpp 
b/src/tests/containerizer/docker_containerizer_tests.cpp
index 1671d45..51d9c90 100644
--- a/src/tests/containerizer/docker_containerizer_tests.cpp
+++ b/src/tests/containerizer/docker_containerizer_tests.cpp
@@ -1410,6 +1410,8 @@ TEST_F(DockerContainerizerTest, 
ROOT_DOCKER_SkipRecoverNonDocker)
 
   ExecutorInfo executorInfo;
   executorInfo.mutable_container()->set_type(ContainerInfo::MESOS);
+  executorInfo.mutable_container()->mutable_mesos()->CopyFrom(
+      ContainerInfo::MesosInfo());
 
   ExecutorState executorState;
   executorState.info = executorInfo;

http://git-wip-us.apache.org/repos/asf/mesos/blob/e65f580b/src/tests/containerizer/docker_volume_isolator_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/containerizer/docker_volume_isolator_tests.cpp 
b/src/tests/containerizer/docker_volume_isolator_tests.cpp
index ca7bffd..9607c96 100644
--- a/src/tests/containerizer/docker_volume_isolator_tests.cpp
+++ b/src/tests/containerizer/docker_volume_isolator_tests.cpp
@@ -295,6 +295,7 @@ TEST_F(DockerVolumeIsolatorTest, 
ROOT_CommandTaskNoRootfsWithVolumes)
 
   ContainerInfo containerInfo;
   containerInfo.set_type(ContainerInfo::MESOS);
+  containerInfo.mutable_mesos()->CopyFrom(ContainerInfo::MesosInfo());
   containerInfo.add_volumes()->CopyFrom(volume1);
   containerInfo.add_volumes()->CopyFrom(volume2);
 
@@ -448,6 +449,7 @@ TEST_F(DockerVolumeIsolatorTest, 
ROOT_CommandTaskNoRootfsFailedWithSameVolumes)
 
   ContainerInfo containerInfo;
   containerInfo.set_type(ContainerInfo::MESOS);
+  containerInfo.mutable_mesos()->CopyFrom(ContainerInfo::MesosInfo());
   containerInfo.add_volumes()->CopyFrom(volume1);
   containerInfo.add_volumes()->CopyFrom(volume2);
 
@@ -553,6 +555,7 @@ TEST_F(DockerVolumeIsolatorTest, 
ROOT_CommandTaskNoRootfsSlaveRecovery)
 
   ContainerInfo containerInfo;
   containerInfo.set_type(ContainerInfo::MESOS);
+  containerInfo.mutable_mesos()->CopyFrom(ContainerInfo::MesosInfo());
   containerInfo.add_volumes()->CopyFrom(volume1);
   containerInfo.add_volumes()->CopyFrom(volume2);
 
@@ -731,6 +734,7 @@ TEST_F(DockerVolumeIsolatorTest,
 
   ContainerInfo containerInfo1;
   containerInfo1.set_type(ContainerInfo::MESOS);
+  containerInfo1.mutable_mesos()->CopyFrom(ContainerInfo::MesosInfo());
   containerInfo1.add_volumes()->CopyFrom(volume1);
 
   task1.mutable_container()->CopyFrom(containerInfo1);
@@ -747,6 +751,7 @@ TEST_F(DockerVolumeIsolatorTest,
 
   ContainerInfo containerInfo2;
   containerInfo2.set_type(ContainerInfo::MESOS);
+  containerInfo2.mutable_mesos()->CopyFrom(ContainerInfo::MesosInfo());
   containerInfo2.add_volumes()->CopyFrom(volume1);
 
   task2.mutable_container()->CopyFrom(containerInfo2);

http://git-wip-us.apache.org/repos/asf/mesos/blob/e65f580b/src/tests/containerizer/filesystem_isolator_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/containerizer/filesystem_isolator_tests.cpp 
b/src/tests/containerizer/filesystem_isolator_tests.cpp
index df4642d..92835c8 100644
--- a/src/tests/containerizer/filesystem_isolator_tests.cpp
+++ b/src/tests/containerizer/filesystem_isolator_tests.cpp
@@ -202,6 +202,7 @@ protected:
   {
     ContainerInfo info;
     info.set_type(ContainerInfo::MESOS);
+    info.mutable_mesos()->CopyFrom(ContainerInfo::MesosInfo());
 
     if (imageName.isSome()) {
       Image* image = info.mutable_mesos()->mutable_image();

http://git-wip-us.apache.org/repos/asf/mesos/blob/e65f580b/src/tests/containerizer/isolator_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/containerizer/isolator_tests.cpp 
b/src/tests/containerizer/isolator_tests.cpp
index 9bb1e69..dbe6147 100644
--- a/src/tests/containerizer/isolator_tests.cpp
+++ b/src/tests/containerizer/isolator_tests.cpp
@@ -133,6 +133,7 @@ TEST_F(SharedFilesystemIsolatorTest, 
DISABLED_ROOT_RelativeVolume)
 
   ContainerInfo containerInfo;
   containerInfo.set_type(ContainerInfo::MESOS);
+  containerInfo.mutable_mesos()->CopyFrom(ContainerInfo::MesosInfo());
   containerInfo.add_volumes()->CopyFrom(
       CREATE_VOLUME(containerPath, hostPath, Volume::RW));
 
@@ -239,6 +240,7 @@ TEST_F(SharedFilesystemIsolatorTest, 
DISABLED_ROOT_AbsoluteVolume)
 
   ContainerInfo containerInfo;
   containerInfo.set_type(ContainerInfo::MESOS);
+  containerInfo.mutable_mesos()->CopyFrom(ContainerInfo::MesosInfo());
   containerInfo.add_volumes()->CopyFrom(
       CREATE_VOLUME(containerPath, hostPath, Volume::RW));
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/e65f580b/src/tests/master_validation_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/master_validation_tests.cpp 
b/src/tests/master_validation_tests.cpp
index 16c5773..5cb8013 100644
--- a/src/tests/master_validation_tests.cpp
+++ b/src/tests/master_validation_tests.cpp
@@ -1758,6 +1758,7 @@ TEST_F(TaskGroupValidationTest, 
ExecutorUsesDockerContainerInfo)
   executor.mutable_executor_id()->set_value("E");
   executor.mutable_framework_id()->CopyFrom(frameworkInfo.id());
   executor.mutable_container()->set_type(ContainerInfo::DOCKER);
+  executor.mutable_container()->mutable_docker()->set_image("mesos:forthewin");
 
   TaskInfo task1;
   task1.set_name("1");
@@ -1933,6 +1934,7 @@ TEST_F(TaskGroupValidationTest, 
TaskUsesDockerContainerInfo)
   task1.mutable_slave_id()->MergeFrom(offer.slave_id());
   task1.mutable_resources()->MergeFrom(resources);
   task1.mutable_container()->set_type(ContainerInfo::DOCKER);
+  task1.mutable_container()->mutable_docker()->set_image("mesos:forthewin");
 
   // Create a valid task.
   TaskInfo task2;
@@ -2021,8 +2023,10 @@ TEST_F(TaskGroupValidationTest, TaskUsesNetworkInfo)
   task1.mutable_task_id()->set_value("1");
   task1.mutable_slave_id()->MergeFrom(offer.slave_id());
   task1.mutable_resources()->MergeFrom(resources);
-  task1.mutable_container()->set_type(ContainerInfo::MESOS);
   task1.mutable_container()->add_network_infos();
+  task1.mutable_container()->set_type(ContainerInfo::MESOS);
+  task1.mutable_container()->mutable_mesos()->CopyFrom(
+      ContainerInfo::MesosInfo());
 
   // Create a valid task.
   TaskInfo task2;

http://git-wip-us.apache.org/repos/asf/mesos/blob/e65f580b/src/tests/slave_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/slave_tests.cpp b/src/tests/slave_tests.cpp
index b44b6fc..696e0f7 100644
--- a/src/tests/slave_tests.cpp
+++ b/src/tests/slave_tests.cpp
@@ -829,6 +829,7 @@ TEST_F(SlaveTest, GetExecutorInfoForTaskWithContainer)
 
   ContainerInfo* container = task.mutable_container();
   container->set_type(ContainerInfo::MESOS);
+  container->mutable_mesos()->CopyFrom(ContainerInfo::MesosInfo());
 
   NetworkInfo* network = container->add_network_infos();
   network->add_ip_addresses()->set_ip_address("4.3.2.1");
@@ -906,6 +907,7 @@ TEST_F(SlaveTest, LaunchTaskInfoWithContainerInfo)
 
   ContainerInfo* container = task.mutable_container();
   container->set_type(ContainerInfo::MESOS);
+  container->mutable_mesos()->CopyFrom(ContainerInfo::MesosInfo());
 
   NetworkInfo* network = container->add_network_infos();
   network->add_ip_addresses()->set_ip_address("4.3.2.1");

Reply via email to