Symlink sandbox directories in docker containerizer Review: https://reviews.apache.org/r/26517
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/23be6c83 Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/23be6c83 Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/23be6c83 Branch: refs/heads/docker_symlink Commit: 23be6c832beb6f89f4cfcbb1bebadbeef20ababa Parents: 196ad01 Author: Timothy Chen <[email protected]> Authored: Tue Oct 7 21:36:32 2014 -0700 Committer: Timothy Chen <[email protected]> Committed: Tue Oct 28 16:53:38 2014 -0700 ---------------------------------------------------------------------- src/slave/containerizer/docker.cpp | 80 +++++++++++++---- src/slave/containerizer/docker.hpp | 6 ++ src/tests/docker_containerizer_tests.cpp | 121 ++++++++++++++++++++++++++ 3 files changed, 192 insertions(+), 15 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/23be6c83/src/slave/containerizer/docker.cpp ---------------------------------------------------------------------- diff --git a/src/slave/containerizer/docker.cpp b/src/slave/containerizer/docker.cpp index 12ca9f1..e29f004 100644 --- a/src/slave/containerizer/docker.cpp +++ b/src/slave/containerizer/docker.cpp @@ -29,6 +29,7 @@ #include <process/reap.hpp> #include <process/subprocess.hpp> +#include <stout/fs.hpp> #include <stout/hashmap.hpp> #include <stout/hashset.hpp> #include <stout/os.hpp> @@ -74,6 +75,9 @@ using state::RunState; // the Docker container name creation include the slave ID. string DOCKER_NAME_PREFIX = "mesos-"; +// Declared in header, see explanation there. +string DOCKER_SYMLINK_DIR = "docker/links"; + class DockerContainerizerProcess : public process::Process<DockerContainerizerProcess> @@ -125,10 +129,7 @@ public: private: // Continuations and helpers. - process::Future<Nothing> fetch( - const ContainerID& containerId, - const CommandInfo& commandInfo, - const std::string& directory); + process::Future<Nothing> fetch(const ContainerID& containerId); process::Future<Nothing> _fetch( const ContainerID& containerId, @@ -237,6 +238,7 @@ private: const SlaveID& slaveId, const PID<Slave>& slavePid, bool checkpoint, + bool symlinked, const Flags& flags) : state(FETCHING), id(id), @@ -247,6 +249,7 @@ private: slaveId(slaveId), slavePid(slavePid), checkpoint(checkpoint), + symlinked(symlinked), flags(flags) { if (task.isSome()) { @@ -256,6 +259,15 @@ private: } } + ~Container() + { + if (symlinked) { + // The sandbox directory is a symlink, remove it at container + // destroy. + os::rm(directory); + } + } + std::string name() { return DOCKER_NAME_PREFIX + stringify(id); @@ -337,11 +349,16 @@ private: ContainerID id; Option<TaskInfo> task; ExecutorInfo executor; + + // The sandbox directory for the container. This holds the + // symlinked path if symlinked boolean is true. std::string directory; + Option<std::string> user; SlaveID slaveId; PID<Slave> slavePid; bool checkpoint; + bool symlinked; Flags flags; // Promise for future returned from wait(). @@ -467,24 +484,57 @@ DockerContainerizerProcess::Container::create( } } + string dockerSymlinkPath = path::join( + paths::getSlavePath(flags.work_dir, slaveId), + DOCKER_SYMLINK_DIR); + + if (!os::exists(dockerSymlinkPath)) { + Try<Nothing> mkdir = os::mkdir(dockerSymlinkPath); + if (mkdir.isError()) { + return Error("Unable to create symlink folder for docker " + + dockerSymlinkPath + ": " + mkdir.error()); + } + } + + bool symlinked = false; + string containerWorkdir = directory; + // We need to symlink the sandbox directory if the directory + // path has a colon, as Docker CLI uses the colon as a seperator. + if (strings::contains(directory, ":")) { + containerWorkdir = path::join(dockerSymlinkPath, id.value()); + + Try<Nothing> symlink = ::fs::symlink(directory, containerWorkdir); + + if (symlink.isError()) { + return Error("Failed to symlink directory '" + directory + + "' to '" + containerWorkdir + "': " + symlink.error()); + } + + symlinked = true; + } + return new Container( id, taskInfo, executorInfo, - directory, + containerWorkdir, user, slaveId, slavePid, checkpoint, + symlinked, flags); } Future<Nothing> DockerContainerizerProcess::fetch( - const ContainerID& containerId, - const CommandInfo& commandInfo, - const string& directory) + const ContainerID& containerId) { + CHECK(containers_.contains(containerId)); + Container* container = containers_[containerId]; + + CommandInfo commandInfo = container->command(); + if (commandInfo.uris().size() == 0) { return Nothing(); } @@ -504,25 +554,25 @@ Future<Nothing> DockerContainerizerProcess::fetch( map<string, string> fetcherEnv = fetcherEnvironment( commandInfo, - directory, + container->directory, None(), flags); VLOG(1) << "Starting to fetch URIs for container: " << containerId - << ", directory: " << directory; + << ", directory: " << container->directory; Try<Subprocess> fetcher = subprocess( realpath.get(), Subprocess::PIPE(), - Subprocess::PATH(path::join(directory, "stdout")), - Subprocess::PATH(path::join(directory, "stderr")), + Subprocess::PATH(path::join(container->directory, "stdout")), + Subprocess::PATH(path::join(container->directory, "stderr")), fetcherEnv); if (fetcher.isError()) { return Failure("Failed to execute mesos-fetcher: " + fetcher.error()); } - containers_[containerId]->fetcher = fetcher.get(); + container->fetcher = fetcher.get(); return fetcher.get().status() .then(defer(self(), &Self::_fetch, containerId, lambda::_1)); @@ -878,7 +928,7 @@ Future<bool> DockerContainerizerProcess::launch( << "' (and executor '" << executorInfo.executor_id() << "') of framework '" << executorInfo.framework_id() << "'"; - return fetch(containerId, taskInfo.command(), directory) + return fetch(containerId) .then(defer(self(), &Self::_launch, containerId)) .then(defer(self(), &Self::__launch, containerId)) .then(defer(self(), &Self::___launch, containerId)) @@ -1047,7 +1097,7 @@ Future<bool> DockerContainerizerProcess::launch( << "' for executor '" << executorInfo.executor_id() << "' and framework '" << executorInfo.framework_id() << "'"; - return fetch(containerId, executorInfo.command(), directory) + return fetch(containerId) .then(defer(self(), &Self::_launch, containerId)) .then(defer(self(), &Self::__launch, containerId)) .then(defer(self(), &Self::____launch, containerId)) http://git-wip-us.apache.org/repos/asf/mesos/blob/23be6c83/src/slave/containerizer/docker.hpp ---------------------------------------------------------------------- diff --git a/src/slave/containerizer/docker.hpp b/src/slave/containerizer/docker.hpp index 1aae84f..3603690 100644 --- a/src/slave/containerizer/docker.hpp +++ b/src/slave/containerizer/docker.hpp @@ -33,6 +33,12 @@ namespace slave { // created by Mesos from those created manually. extern std::string DOCKER_NAME_PREFIX; +// Directory that stores all the symlinked sandboxes that is mapped +// into Docker containers. This is a relative directory that will +// joined with the slave path. Only sandbox paths that contains a +// colon will be symlinked due to the limiitation of the Docker cli. +extern std::string DOCKER_SYMLINK_DIR; + // Forward declaration. class DockerContainerizerProcess; http://git-wip-us.apache.org/repos/asf/mesos/blob/23be6c83/src/tests/docker_containerizer_tests.cpp ---------------------------------------------------------------------- diff --git a/src/tests/docker_containerizer_tests.cpp b/src/tests/docker_containerizer_tests.cpp index ba24307..4a66c92 100644 --- a/src/tests/docker_containerizer_tests.cpp +++ b/src/tests/docker_containerizer_tests.cpp @@ -39,6 +39,7 @@ using namespace mesos; using namespace mesos::internal; +using namespace mesos::internal::slave::paths; using namespace mesos::internal::slave::state; using namespace mesos::internal::tests; @@ -2148,3 +2149,123 @@ TEST_F(DockerContainerizerTest, ROOT_DOCKER_PortMapping) Shutdown(); } + + +// This test verifies that sandbox with ':' in the path can still +// run successfully. This a limitation of the Docker CLI where +// the volume map parameter treats colons (:) as seperators, +// and incorrectly seperates the sandbox directory. +TEST_F(DockerContainerizerTest, ROOT_DOCKER_LaunchSandboxWithColon) +{ + Try<PID<Master>> master = StartMaster(); + ASSERT_SOME(master); + + slave::Flags flags = CreateSlaveFlags(); + + MockDocker* mockDocker = new MockDocker(tests::flags.docker); + memory::shared_ptr<Docker> docker(mockDocker); + + // We need to capture and await on the logs process's future so that + // we can ensure there is no child process at the end of the test. + // The logs future is being awaited at teardown. + Future<Nothing> logs; + EXPECT_CALL(*mockDocker, logs(_, _)) + .WillRepeatedly(FutureResult( + &logs, Invoke((MockDocker*)docker.get(), &MockDocker::_logs))); + + MockDockerContainerizer dockerContainerizer(flags, docker); + + Try<PID<Slave> > slave = StartSlave(&dockerContainerizer, flags); + ASSERT_SOME(slave); + + MockScheduler sched; + MesosSchedulerDriver driver( + &sched, DEFAULT_FRAMEWORK_INFO, master.get(), DEFAULT_CREDENTIAL); + + Future<FrameworkID> frameworkId; + EXPECT_CALL(sched, registered(&driver, _, _)) + .WillOnce(FutureArg<1>(&frameworkId)); + + Future<vector<Offer> > offers; + EXPECT_CALL(sched, resourceOffers(&driver, _)) + .WillOnce(FutureArg<1>(&offers)) + .WillRepeatedly(Return()); // Ignore subsequent offers. + + driver.start(); + + AWAIT_READY(frameworkId); + + AWAIT_READY(offers); + EXPECT_NE(0u, offers.get().size()); + + const Offer& offer = offers.get()[0]; + + string slaveWorkDir = getSlavePath(flags.work_dir, offer.slave_id()); + string dockerSymlinkDir = path::join(slaveWorkDir, slave::DOCKER_SYMLINK_DIR); + if (!os::exists(dockerSymlinkDir)) { + Try<Nothing> mkdir = os::mkdir(dockerSymlinkDir); + ASSERT_SOME(mkdir) + << "Unable to create symlink dir for docker: " << mkdir.error(); + } + + TaskInfo task; + task.set_name(""); + task.mutable_task_id()->set_value("test:colon"); + task.mutable_slave_id()->CopyFrom(offer.slave_id()); + task.mutable_resources()->CopyFrom(offer.resources()); + + CommandInfo command; + command.set_value("sleep 1000"); + + ContainerInfo containerInfo; + containerInfo.set_type(ContainerInfo::DOCKER); + + ContainerInfo::DockerInfo dockerInfo; + dockerInfo.set_image("busybox"); + containerInfo.mutable_docker()->CopyFrom(dockerInfo); + + task.mutable_command()->CopyFrom(command); + task.mutable_container()->CopyFrom(containerInfo); + + vector<TaskInfo> tasks; + tasks.push_back(task); + + Future<ContainerID> containerId; + EXPECT_CALL(dockerContainerizer, launch(_, _, _, _, _, _, _, _)) + .WillOnce(DoAll(FutureArg<0>(&containerId), + Invoke(&dockerContainerizer, + &MockDockerContainerizer::_launch))); + + Future<TaskStatus> statusRunning; + EXPECT_CALL(sched, statusUpdate(&driver, _)) + .WillOnce(FutureArg<1>(&statusRunning)) + .WillRepeatedly(DoDefault()); + + driver.launchTasks(offers.get()[0].id(), tasks); + + AWAIT_READY_FOR(containerId, Seconds(60)); + AWAIT_READY_FOR(statusRunning, Seconds(60)); + EXPECT_EQ(TASK_RUNNING, statusRunning.get().state()); + + Future<list<Docker::Container> > containers = + docker->ps(true, slave::DOCKER_NAME_PREFIX); + + AWAIT_READY(containers); + + ASSERT_TRUE(containers.get().size() > 0); + + ASSERT_TRUE(exists(containers.get(), containerId.get())); + + Future<containerizer::Termination> termination = + dockerContainerizer.wait(containerId.get()); + + driver.stop(); + driver.join(); + + AWAIT_READY(termination); + + // See above where we assign logs future for more comments. + AWAIT_READY_FOR(logs, Seconds(30)); + + Shutdown(); +}
