Introduced DockerContainerizerProcess::Container::create. Simplifies the generic setup of a new container that currently consists of making sure stdout/stderr exists and the directory is properly chown'ed.
Review: https://reviews.apache.org/r/26612 Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/c3e7f2a8 Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/c3e7f2a8 Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/c3e7f2a8 Branch: refs/heads/master Commit: c3e7f2a8176f7ae333986fb226dc6bf8952f4d84 Parents: 05211c5 Author: Benjamin Hindman <[email protected]> Authored: Sat Oct 11 12:51:04 2014 -0700 Committer: Benjamin Hindman <[email protected]> Committed: Fri Oct 31 15:05:38 2014 -0700 ---------------------------------------------------------------------- src/slave/containerizer/docker.cpp | 86 +++++++++++++++++---------------- 1 file changed, 44 insertions(+), 42 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/c3e7f2a8/src/slave/containerizer/docker.cpp ---------------------------------------------------------------------- diff --git a/src/slave/containerizer/docker.cpp b/src/slave/containerizer/docker.cpp index 312c8d8..8873022 100644 --- a/src/slave/containerizer/docker.cpp +++ b/src/slave/containerizer/docker.cpp @@ -240,6 +240,11 @@ private: struct Container { + static Try<Container*> create( + const ContainerID& id, + const std::string& directory, + const Option<std::string>& user); + Container(const ContainerID& id) : state(FETCHING), id(id) {} @@ -365,6 +370,37 @@ DockerContainerizer::~DockerContainerizer() } +Future<Nothing> DockerContainerizerProcess::Container::create( + const ContainerID& containerId, + const string& directory, + const Option<string>& user) +{ + // Before we do anything else we first make sure the stdout/stderr + // files exist and have the right file ownership. + Try<Nothing> touch = os::touch(path::join(directory, "stdout")); + + if (touch.isError()) { + return Failure("Failed to touch 'stdout': " + touch.error()); + } + + touch = os::touch(path::join(directory, "stderr")); + + if (touch.isError()) { + return Failure("Failed to touch 'stderr': " + touch.error()); + } + + if (user.isSome()) { + Try<Nothing> chown = os::chown(user.get(), directory); + + if (chown.isError()) { + return Failure("Failed to chown: " + chown.error()); + } + } + + return new Container(containerId); +} + + Future<Nothing> DockerContainerizerProcess::fetch( const ContainerID& containerId, const CommandInfo& commandInfo, @@ -715,36 +751,19 @@ Future<bool> DockerContainerizerProcess::launch( return false; } - // Before we do anything else we first make sure the stdout/stderr - // files exist and have the right file ownership. - Try<Nothing> touch = os::touch(path::join(directory, "stdout")); + Try<Container*> container = Container::create(containerId, directory, user); - if (touch.isError()) { - return Failure("Failed to touch 'stdout': " + touch.error()); + if (container.isError()) { + return Failure("Failed to create container: " + container.error()); } - touch = os::touch(path::join(directory, "stderr")); - - if (touch.isError()) { - return Failure("Failed to touch 'stderr': " + touch.error()); - } - - if (user.isSome()) { - Try<Nothing> chown = os::chown(user.get(), directory); - - if (chown.isError()) { - return Failure("Failed to chown: " + chown.error()); - } - } + containers_[containerId] = container.get(); LOG(INFO) << "Starting container '" << containerId << "' for task '" << taskInfo.task_id() << "' (and executor '" << executorInfo.executor_id() << "') of framework '" << executorInfo.framework_id() << "'"; - Container* container = new Container(containerId); - containers_[containerId] = container; - return fetch(containerId, taskInfo.command(), directory) .then(defer(self(), &Self::_launch, @@ -956,35 +975,18 @@ Future<bool> DockerContainerizerProcess::launch( return false; } - // Before we do anything else we first make sure the stdout/stderr - // files exist and have the right file ownership. - Try<Nothing> touch = os::touch(path::join(directory, "stdout")); + Try<Container*> container = Container::create(containerId, directory, user); - if (touch.isError()) { - return Failure("Failed to touch 'stdout': " + touch.error()); - } - - touch = os::touch(path::join(directory, "stderr")); - - if (touch.isError()) { - return Failure("Failed to touch 'stderr': " + touch.error()); + if (container.isError()) { + return Failure("Failed to create container: " + container.error()); } - if (user.isSome()) { - Try<Nothing> chown = os::chown(user.get(), directory); - - if (chown.isError()) { - return Failure("Failed to chown: " + chown.error()); - } - } + containers_[containerId] = container.get(); LOG(INFO) << "Starting container '" << containerId << "' for executor '" << executorInfo.executor_id() << "' and framework '" << executorInfo.framework_id() << "'"; - Container* container = new Container(containerId); - containers_[containerId] = container; - return fetch(containerId, executorInfo.command(), directory) .then(defer(self(), &Self::_launch,
