Repository: mesos
Updated Branches:
  refs/heads/1.2.x f575c586b -> 9f035e1d1


Fixed command task with container image 'root' user issue.

This issue is command task with container image provided specific.
We used to set user as 'root' explicitly for command task with
container image. However, this would break operators who set 'user'
for FrameworkInfo/CommandInfo to any user other than 'root' because
the task cannot access all other contents owned by 'root', e.g.,
persistent volumes, stdout/stderr or any other directories/files
written by modules.

Instead of relying on each isolator/module to explicitly chown,
Mesos should set user to 'root' right before launching the command
executor, because the root privilege is only necessary for 'chroot'
in command executor launch, which should not impact on other
components.

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


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

Branch: refs/heads/1.2.x
Commit: 0f3a68d10f21cb2d16b2bd51214dc86c393f7d1b
Parents: 9c0bbf1
Author: Gilbert Song <[email protected]>
Authored: Thu Mar 9 12:42:44 2017 -0800
Committer: Jie Yu <[email protected]>
Committed: Thu Mar 9 13:50:22 2017 -0800

----------------------------------------------------------------------
 src/slave/containerizer/mesos/containerizer.cpp |  9 +++++++++
 src/slave/slave.cpp                             | 10 +---------
 2 files changed, 10 insertions(+), 9 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/0f3a68d1/src/slave/containerizer/mesos/containerizer.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/containerizer.cpp 
b/src/slave/containerizer/mesos/containerizer.cpp
index a23a6fa..7676a4d 100644
--- a/src/slave/containerizer/mesos/containerizer.cpp
+++ b/src/slave/containerizer/mesos/containerizer.cpp
@@ -1449,6 +1449,15 @@ Future<bool> MesosContainerizerProcess::_launch(
     launchInfo.set_user(container->config.user());
   }
 
+  // TODO(gilbert): Remove this once we no longer support command
+  // task in favor of default executor.
+  if (container->config.has_task_info() &&
+      container->config.has_rootfs()) {
+    // We need to set the executor user as root as it needs to
+    // perform chroot (even when switch_user is set to false).
+    launchInfo.set_user("root");
+  }
+
   // Use a pipe to block the child until it's been isolated.
   // The `pipes` array is captured later in a lambda.
   Try<std::array<int_fd, 2>> pipes_ = os::pipe();

http://git-wip-us.apache.org/repos/asf/mesos/blob/0f3a68d1/src/slave/slave.cpp
----------------------------------------------------------------------
diff --git a/src/slave/slave.cpp b/src/slave/slave.cpp
index 7564e8d..db48726 100644
--- a/src/slave/slave.cpp
+++ b/src/slave/slave.cpp
@@ -4462,10 +4462,6 @@ ExecutorInfo Slave::getExecutorInfo(
     // task. For this reason, we need to strip the image in
     // `executor.container.mesos`.
     container->mutable_mesos()->clear_image();
-
-    // We need to set the executor user as root as it needs to
-    // perform chroot (even when switch_user is set to false).
-    executor.mutable_command()->set_user("root");
   }
 
   // Prepare an executor name which includes information on the
@@ -4544,11 +4540,7 @@ ExecutorInfo Slave::getExecutorInfo(
         gracePeriod.ns());
   }
 
-  // We skip setting the user for the command executor that has
-  // a rootfs image since we need root permissions to chroot.
-  // We assume command executor will change to the correct user
-  // later on.
-  if (!hasRootfs && task.command().has_user()) {
+  if (task.command().has_user()) {
     executor.mutable_command()->set_user(task.command().user());
   }
 

Reply via email to