[jira] [Comment Edited] (MESOS-2315) Deprecate / Remove CommandInfo::ContainerInfo

["Vaibhav Khanduja (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/MESOS-2315?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14995519#comment-14995519 ] 

Vaibhav Khanduja edited comment on MESOS-2315 at 11/8/15 4:49 AM:
------------------------------------------------------------------

I have started to put fix for issue (diff attached). All the test case (sudo make check) pass through except few related to LinuxFileSystemIsolator .. I believe, they make use of ContainerInfo::MESOS. [~adam-mesos] Any pointers? I am trying to generate verbose outputs of these ..

[  FAILED  ] LinuxFilesystemIsolatorTest.ROOT_PersistentVolumeWithRootFilesystem
[  FAILED  ] LinuxFilesystemIsolatorTest.ROOT_PersistentVolumeWithoutRootFilesystem
[  FAILED  ] LinuxFilesystemIsolatorTest.ROOT_ImageInVolumeWithRootFilesystem
[  FAILED  ] LinuxFilesystemIsolatorTest.ROOT_MultipleContainers
[  FAILED  ] LinuxFilesystemIsolatorTest.ROOT_SandboxEnvironmentVariable



was (Author: vaibhavkhanduja):
I have started to put fix for issue (diff below). All the test case (sudo make check) pass through except few related to LinuxFileSystemIsolator .. I believe, they make use of ContainerInfo::MESOS. [~adam-mesos] Any pointers? I am trying to generate verbose outputs of these ..

[  FAILED  ] LinuxFilesystemIsolatorTest.ROOT_PersistentVolumeWithRootFilesystem
[  FAILED  ] LinuxFilesystemIsolatorTest.ROOT_PersistentVolumeWithoutRootFilesystem
[  FAILED  ] LinuxFilesystemIsolatorTest.ROOT_ImageInVolumeWithRootFilesystem
[  FAILED  ] LinuxFilesystemIsolatorTest.ROOT_MultipleContainers
[  FAILED  ] LinuxFilesystemIsolatorTest.ROOT_SandboxEnvironmentVariable

diff:

diff --git a/include/mesos/mesos.proto b/include/mesos/mesos.proto
index 1b36e66..c14c0db 100644
--- a/include/mesos/mesos.proto
+++ b/include/mesos/mesos.proto
@@ -369,27 +369,6 @@ message CommandInfo {
     optional bool cache = 4;
   }
 
-  // Describes a container.
-  // Not all containerizers currently implement ContainerInfo, so it
-  // is possible that a launched task will fail due to supplying this
-  // attribute.
-  // NOTE: The containerizer API is currently in an early beta or
-  // even alpha state. Some details, like the exact semantics of an
-  // "image" or "options" are not yet hardened.
-  // TODO(tillt): Describe the exact scheme and semantics of "image"
-  // and "options".
-  message ContainerInfo {
-    // URI describing the container image name.
-    required string image = 1;
-
-    // Describes additional options passed to the containerizer.
-    repeated string options = 2;
-  }
-
-  // NOTE: MesosContainerizer does currently not support this
-  // attribute and tasks supplying a 'container' will fail.
-  optional ContainerInfo container = 4;
-
   repeated URI uris = 1;
 
   optional Environment environment = 2;
diff --git a/src/slave/containerizer/mesos/containerizer.cpp b/src/slave/containerizer/mesos/containerizer.cpp
index a1c06b1..9222ea2 100644
--- a/src/slave/containerizer/mesos/containerizer.cpp
+++ b/src/slave/containerizer/mesos/containerizer.cpp
@@ -610,11 +610,6 @@ Future<bool> MesosContainerizerProcess::launch(
         flags.default_container_info.get());
   }
 
-  // MesosContainerizer does not support ContainerInfo in CommandInfo.
-  if (executorInfo.command().has_container()) {
-    return false;
-  }
-
   LOG(INFO) << "Starting container '" << containerId
             << "' for executor '" << executorInfo.executor_id()
             << "' of framework '" << executorInfo.framework_id() << "'";
diff --git a/src/slave/slave.cpp b/src/slave/slave.cpp
index 7d70e86..476ada0 100644
--- a/src/slave/slave.cpp
+++ b/src/slave/slave.cpp
@@ -3253,7 +3253,7 @@ ExecutorInfo Slave::getExecutorInfo(
     executor.mutable_framework_id()->CopyFrom(frameworkInfo.id());
 
     if (task.has_container() &&
-        task.container().type() != ContainerInfo::MESOS) {
+         task.container().type() != ContainerInfo::MESOS) {
       // Store the container info in the executor info so it will
       // be checkpointed. This allows the correct containerizer to
       // recover this task on restart.
@@ -3324,9 +3324,9 @@ ExecutorInfo Slave::getExecutorInfo(
           task.command().environment());
     }
 
-    if (task.command().has_container()) {
-      executor.mutable_command()->mutable_container()->MergeFrom(
-          task.command().container());
+    if (task.has_container()) {
+      executor.mutable_container()->MergeFrom(
+          task.container());
     }
 
     // We skip setting the user for the command executor that has
diff --git a/src/tests/slave_tests.cpp b/src/tests/slave_tests.cpp
index 6b89eaf..cf72248 100644
--- a/src/tests/slave_tests.cpp
+++ b/src/tests/slave_tests.cpp
@@ -568,7 +568,7 @@ TEST_F(SlaveTest, GetExecutorInfo)
   // Now assert that it actually is running mesos-executor without any
   // bleedover from the command we intend on running.
   EXPECT_TRUE(executor.command().shell());
-  EXPECT_FALSE(executor.command().has_container());
+  EXPECT_FALSE(executor.has_container());
   EXPECT_EQ(0, executor.command().arguments_size());
   EXPECT_NE(string::npos, executor.command().value().find("mesos-executor"));
 }


> Deprecate / Remove CommandInfo::ContainerInfo
> ---------------------------------------------
>
>                 Key: MESOS-2315
>                 URL: https://issues.apache.org/jira/browse/MESOS-2315
>             Project: Mesos
>          Issue Type: Task
>            Reporter: Ian Downes
>            Assignee: Vaibhav Khanduja
>            Priority: Minor
>              Labels: mesosphere, newbie
>         Attachments: diff.txt
>
>
> IIUC this has been deprecated and all current code (except examples/docker_no_executor_framework.cpp) uses the top-level ContainerInfo?



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)