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 2016/07/04 17:01:30 UTC

[09/15] mesos git commit: Removed --sandbox flag from mesos-containerizer launch command.

Removed --sandbox flag from mesos-containerizer launch command.

This flag is unnecessary. The caller should calculate the current
working directory for the command, instead of doing the calculation in
this helper binary. This patch moved the calculation to the caller and
simplified this helper binary.

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


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

Branch: refs/heads/master
Commit: d5247fb34b17faef2c14c6edcbedd1ab8ecfb307
Parents: b3ba22e
Author: Jie Yu <yu...@gmail.com>
Authored: Thu Jun 30 16:12:43 2016 -0700
Committer: Jie Yu <yu...@gmail.com>
Committed: Mon Jul 4 10:01:13 2016 -0700

----------------------------------------------------------------------
 src/slave/containerizer/mesos/containerizer.cpp | 28 ++++++-------
 src/slave/containerizer/mesos/launch.cpp        | 41 ++++++--------------
 src/slave/containerizer/mesos/launch.hpp        |  1 -
 src/tests/containerizer/launch_tests.cpp        |  2 +-
 src/tests/containerizer/port_mapping_tests.cpp  |  2 +-
 5 files changed, 29 insertions(+), 45 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/d5247fb3/src/slave/containerizer/mesos/containerizer.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/containerizer.cpp b/src/slave/containerizer/mesos/containerizer.cpp
index a523799..04daddd 100644
--- a/src/slave/containerizer/mesos/containerizer.cpp
+++ b/src/slave/containerizer/mesos/containerizer.cpp
@@ -1229,20 +1229,22 @@ Future<bool> MesosContainerizerProcess::__launch(
 
     launchFlags.command = JSON::protobuf(executorLaunchCommand.get());
 
-    launchFlags.sandbox = executorRootfs.isSome()
-      ? flags.sandbox_directory
-      : directory;
-
-    // NOTE: If the executor shares the host filesystem, we should not
-    // allow them to 'cd' into an arbitrary directory because that'll
-    // create security issues.
-    if (executorRootfs.isNone() && workingDirectory.isSome()) {
-      LOG(WARNING) << "Ignore working directory '" << workingDirectory.get()
-                   << "' specified in container launch info for container "
-                   << containerId << " since the executor is using the "
-                   << "host filesystem";
+    if (executorRootfs.isNone()) {
+      // NOTE: If the executor shares the host filesystem, we should
+      // not allow them to 'cd' into an arbitrary directory because
+      // that'll create security issues.
+      if (workingDirectory.isSome()) {
+        LOG(WARNING) << "Ignore working directory '" << workingDirectory.get()
+                     << "' specified in container launch info for container "
+                     << containerId << " since the executor is using the "
+                     << "host filesystem";
+      }
+
+      launchFlags.working_directory = directory;
     } else {
-      launchFlags.working_directory = workingDirectory;
+      launchFlags.working_directory = workingDirectory.isSome()
+        ? workingDirectory
+        : flags.sandbox_directory;
     }
 
 #ifdef __WINDOWS__

http://git-wip-us.apache.org/repos/asf/mesos/blob/d5247fb3/src/slave/containerizer/mesos/launch.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/launch.cpp b/src/slave/containerizer/mesos/launch.cpp
index 68f23b3..a08a206 100644
--- a/src/slave/containerizer/mesos/launch.cpp
+++ b/src/slave/containerizer/mesos/launch.cpp
@@ -51,23 +51,16 @@ MesosContainerizerLaunch::Flags::Flags()
       "command",
       "The command to execute.");
 
-  add(&sandbox,
-      "sandbox",
-      "The sandbox for the executor. If rootfs is specified this must\n"
-      "be relative to the new root.");
-
   add(&working_directory,
       "working_directory",
-      "The working directory for the executor. It will be ignored if\n"
-      "container root filesystem is not specified.");
+      "The working directory for the command. It has to be an absolute path \n"
+      "w.r.t. the root filesystem used for the command.");
 
 #ifndef __WINDOWS__
   add(&rootfs,
       "rootfs",
-      "Absolute path to the container root filesystem.\n"
-      "The command and sandbox flags are interpreted relative\n"
-      "to rootfs\n"
-      "Different platforms may implement 'chroot' differently.");
+      "Absolute path to the container root filesystem. The command will be \n"
+      "interpreted relative to this path");
 
   add(&user,
       "user",
@@ -105,11 +98,6 @@ int MesosContainerizerLaunch::execute()
     return 1;
   }
 
-  if (flags.sandbox.isNone()) {
-    cerr << "Flag --sandbox is not specified" << endl;
-    return 1;
-  }
-
   bool controlPipeSpecified =
     flags.pipe_read.isSome() && flags.pipe_write.isSome();
 
@@ -346,19 +334,14 @@ int MesosContainerizerLaunch::execute()
   }
 #endif // __WINDOWS__
 
-  // Determine the current working directory for the executor.
-  string cwd;
-  if (rootfs.isSome() && flags.working_directory.isSome()) {
-    cwd = flags.working_directory.get();
-  } else {
-    cwd = flags.sandbox.get();
-  }
-
-  Try<Nothing> chdir = os::chdir(cwd);
-  if (chdir.isError()) {
-    cerr << "Failed to chdir into current working directory '"
-         << cwd << "': " << chdir.error() << endl;
-    return 1;
+  if (flags.working_directory.isSome()) {
+    Try<Nothing> chdir = os::chdir(flags.working_directory.get());
+    if (chdir.isError()) {
+      cerr << "Failed to chdir into current working directory "
+           << "'" << flags.working_directory.get() << "': "
+           << chdir.error() << endl;
+      return 1;
+    }
   }
 
   // Relay the environment variables.

http://git-wip-us.apache.org/repos/asf/mesos/blob/d5247fb3/src/slave/containerizer/mesos/launch.hpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/launch.hpp b/src/slave/containerizer/mesos/launch.hpp
index c716e03..72b99ad 100644
--- a/src/slave/containerizer/mesos/launch.hpp
+++ b/src/slave/containerizer/mesos/launch.hpp
@@ -35,7 +35,6 @@ public:
     Flags();
 
     Option<JSON::Object> command;
-    Option<std::string> sandbox;
     Option<std::string> working_directory;
 #ifndef __WINDOWS__
     Option<std::string> rootfs;

http://git-wip-us.apache.org/repos/asf/mesos/blob/d5247fb3/src/tests/containerizer/launch_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/containerizer/launch_tests.cpp b/src/tests/containerizer/launch_tests.cpp
index 3e36f2f..ef0c87c 100644
--- a/src/tests/containerizer/launch_tests.cpp
+++ b/src/tests/containerizer/launch_tests.cpp
@@ -63,7 +63,7 @@ public:
     command.set_value(_command);
 
     launchFlags.command = JSON::protobuf(command);
-    launchFlags.sandbox = "/tmp";
+    launchFlags.working_directory = "/tmp";
     launchFlags.pipe_read = open("/dev/zero", O_RDONLY);
     launchFlags.pipe_write = open("/dev/null", O_WRONLY);
     launchFlags.rootfs = rootfs;

http://git-wip-us.apache.org/repos/asf/mesos/blob/d5247fb3/src/tests/containerizer/port_mapping_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/containerizer/port_mapping_tests.cpp b/src/tests/containerizer/port_mapping_tests.cpp
index 1c50cf5..2fbb9ec 100644
--- a/src/tests/containerizer/port_mapping_tests.cpp
+++ b/src/tests/containerizer/port_mapping_tests.cpp
@@ -314,7 +314,7 @@ protected:
     MesosContainerizerLaunch::Flags launchFlags;
 
     launchFlags.command = JSON::protobuf(commandInfo);
-    launchFlags.sandbox = os::getcwd();
+    launchFlags.working_directory = os::getcwd();
 
     CHECK_SOME(os::user());
     launchFlags.user = os::user().get();