You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by ji...@apache.org on 2017/08/24 04:10:36 UTC

mesos git commit: Adjusted the ContainerLaunchInfo.command merge logic.

Repository: mesos
Updated Branches:
  refs/heads/master 5d7fd3cfe -> fe696c7d5


Adjusted the ContainerLaunchInfo.command merge logic.

This patches addressed MESOS-7909 by eliminating the ordering
dependency between the `linux/capabilities` isolator and the
`docker/runtime` or `appc/runtime` isolator.

Now, for command tasks, we always merge with the command executor
command in MesosContainerizer. Previously, this is done in the
`docker/runtime` or `appc/runtime` isolator, which introduced this
unintentional dependency on the isolator ordering.

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


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

Branch: refs/heads/master
Commit: fe696c7d541c85ae9832e385f80ec14a39c021e2
Parents: 5d7fd3c
Author: Jie Yu <yu...@gmail.com>
Authored: Wed Aug 23 15:55:13 2017 -0700
Committer: Jie Yu <yu...@gmail.com>
Committed: Wed Aug 23 21:10:29 2017 -0700

----------------------------------------------------------------------
 src/slave/containerizer/mesos/containerizer.cpp | 24 ++++++++++++++++++++
 .../mesos/isolators/appc/runtime.cpp            | 11 +++------
 .../mesos/isolators/docker/runtime.cpp          | 11 +++------
 3 files changed, 30 insertions(+), 16 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/fe696c7d/src/slave/containerizer/mesos/containerizer.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/containerizer.cpp b/src/slave/containerizer/mesos/containerizer.cpp
index 5805dfb..f91eef2 100644
--- a/src/slave/containerizer/mesos/containerizer.cpp
+++ b/src/slave/containerizer/mesos/containerizer.cpp
@@ -1409,6 +1409,30 @@ Future<bool> MesosContainerizerProcess::_launch(
   // Determine the launch command for the container.
   if (!launchInfo.has_command()) {
     launchInfo.mutable_command()->CopyFrom(container->config.command_info());
+  } else {
+    // For command tasks, merge the launch commands with the executor
+    // launch command.
+    if (container->config.has_task_info()) {
+      // Isolators are not supposed to set any other fields in the
+      // command except the arguments for the command executor.
+      CHECK(launchInfo.command().uris().empty())
+        << "Isolators mutate 'uris' in container launch command";
+      CHECK(!launchInfo.command().has_environment())
+        << "Isolators mutate 'environment' in container launch command";
+      CHECK(!launchInfo.command().has_shell())
+        << "Isolators mutate 'shell' in container launch command";
+      CHECK(!launchInfo.command().has_value())
+        << "Isolators mutate 'value' in container launch command";
+      CHECK(!launchInfo.command().has_user())
+        << "Isolators mutate 'user' in container launch command";
+
+      // NOTE: The ordering here is important because we want the
+      // command executor arguments to be in front of the arguments
+      // set by isolators. See details in MESOS-7909.
+      CommandInfo launchCommand = container->config.command_info();
+      launchCommand.MergeFrom(launchInfo.command());
+      launchInfo.mutable_command()->CopyFrom(launchCommand);
+    }
   }
 
   // For command tasks specifically, we should add the task_environment

http://git-wip-us.apache.org/repos/asf/mesos/blob/fe696c7d/src/slave/containerizer/mesos/isolators/appc/runtime.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/isolators/appc/runtime.cpp b/src/slave/containerizer/mesos/isolators/appc/runtime.cpp
index f76b22b..535ea1a 100644
--- a/src/slave/containerizer/mesos/isolators/appc/runtime.cpp
+++ b/src/slave/containerizer/mesos/isolators/appc/runtime.cpp
@@ -111,29 +111,24 @@ Future<Option<ContainerLaunchInfo>> AppcRuntimeIsolatorProcess::prepare(
   // be included in 'ContainerLaunchInfo', and will be passed back
   // to containerizer.
   if (containerConfig.has_task_info()) {
-    // Command task case. The 'executorCommand' below is the
-    // command with value as 'mesos-executor'.
-    CommandInfo executorCommand = containerConfig.executor_info().command();
-
+    // Command task case.
     if (environment.isSome()) {
       launchInfo.mutable_task_environment()->CopyFrom(environment.get());
     }
 
     // Pass working directory to command executor as a flag.
     if (workingDirectory.isSome()) {
-      executorCommand.add_arguments(
+      launchInfo.mutable_command()->add_arguments(
           "--working_directory=" + workingDirectory.get());
     }
 
     // Pass task command as a flag, which will be loaded by
     // command executor.
     if (command.isSome()) {
-      executorCommand.add_arguments(
+      launchInfo.mutable_command()->add_arguments(
           "--task_command=" +
           stringify(JSON::protobuf(command.get())));
     }
-
-    launchInfo.mutable_command()->CopyFrom(executorCommand);
   } else {
     // The custom executor, default executor and nested container cases.
     if (environment.isSome()) {

http://git-wip-us.apache.org/repos/asf/mesos/blob/fe696c7d/src/slave/containerizer/mesos/isolators/docker/runtime.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/isolators/docker/runtime.cpp b/src/slave/containerizer/mesos/isolators/docker/runtime.cpp
index 2a6e0b1..93394f8 100644
--- a/src/slave/containerizer/mesos/isolators/docker/runtime.cpp
+++ b/src/slave/containerizer/mesos/isolators/docker/runtime.cpp
@@ -131,29 +131,24 @@ Future<Option<ContainerLaunchInfo>> DockerRuntimeIsolatorProcess::prepare(
   // be included in 'ContainerLaunchInfo', and will be passed
   // back to containerizer.
   if (containerConfig.has_task_info()) {
-    // Command task case. The 'executorCommand' below is the
-    // command with value as 'mesos-executor'.
-    CommandInfo executorCommand = containerConfig.executor_info().command();
-
+    // Command task case.
     if (environment.isSome()) {
       launchInfo.mutable_task_environment()->CopyFrom(environment.get());
     }
 
     // Pass working directory to command executor as a flag.
     if (workingDirectory.isSome()) {
-      executorCommand.add_arguments(
+      launchInfo.mutable_command()->add_arguments(
           "--working_directory=" + workingDirectory.get());
     }
 
     // Pass task command as a flag, which will be loaded by
     // command executor.
     if (command.isSome()) {
-      executorCommand.add_arguments(
+      launchInfo.mutable_command()->add_arguments(
           "--task_command=" +
           stringify(JSON::protobuf(command.get())));
     }
-
-    launchInfo.mutable_command()->CopyFrom(executorCommand);
   } else {
     // The custom executor, default executor and nested container cases.
     if (environment.isSome()) {