----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/43083/#review117522 -----------------------------------------------------------
src/slave/containerizer/mesos/containerizer.cpp (lines 1029 - 1034) <https://reviews.apache.org/r/43083/#comment178716> We shouldn't allow executor to cd into an arbitrary directory if filesystem isolation is not used (because that'll create security issue). I would do the following: ``` if (rootfs.isSome()) { launchFlags.directory = workingDir.isSome() ? workingDir.get() : flags.sandbox_directory; } else { // 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 (workingDir.isSome()) { LOG(WARNING) << "Ignore working directory '" << workingDir.get() << "' specified in container launch info for container " << containerId << " since the executor is using the " << "host filesystem"; } launchFlags.directory = directory; } ``` src/slave/containerizer/mesos/isolators/docker/runtime.cpp (lines 112 - 113) <https://reviews.apache.org/r/43083/#comment178719> I think we should distinguish between sandbox_direcotry and working_directory. For instance, for the marathon docker image, sandbox_directory is still /mnt/mesos/sandbox while working_directory is /marathon. So please add another flag to command executor. src/slave/containerizer/mesos/isolators/docker/runtime.cpp (lines 114 - 121) <https://reviews.apache.org/r/43083/#comment178722> if (!containerConfig.has_task_info()) { // Custom executor case. Option<string> dir = getWorkingDirectory(containerConfig); if (dir.isSome()) { launchInfo.set_working_directory(dir.get()); } } src/slave/containerizer/mesos/isolators/docker/runtime.cpp (lines 357 - 359) <https://reviews.apache.org/r/43083/#comment178721> I would make this function very simple: get the working directory specified in the docker image. I'll move the logics of setting --working_directory to getExecutorLaunchCommand. src/slave/containerizer/mesos/isolators/docker/runtime.cpp (lines 361 - 363) <https://reviews.apache.org/r/43083/#comment178718> Ditto on container_config? - Jie Yu On Feb. 2, 2016, 5:18 p.m., Gilbert Song wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/43083/ > ----------------------------------------------------------- > > (Updated Feb. 2, 2016, 5:18 p.m.) > > > Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen. > > > Bugs: MESOS-4005 > https://issues.apache.org/jira/browse/MESOS-4005 > > > Repository: mesos > > > Description > ------- > > Supported working dir in docker runtime isolator. > > > Diffs > ----- > > src/slave/containerizer/mesos/containerizer.cpp > 4b504dbb58823ce7675f1d2048dcc7a27c05663d > src/slave/containerizer/mesos/isolators/docker/runtime.hpp PRE-CREATION > src/slave/containerizer/mesos/isolators/docker/runtime.cpp PRE-CREATION > > Diff: https://reviews.apache.org/r/43083/diff/ > > > Testing > ------- > > make check (ubuntu14.04 + clang-3.6) > > > Thanks, > > Gilbert Song > >
