Save Docker container pid for subsequent containerizer updates. Review: https://reviews.apache.org/r/24768
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/8cbb85c8 Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/8cbb85c8 Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/8cbb85c8 Branch: refs/heads/master Commit: 8cbb85c8af6eae9453a868f67f2fb8dd387e18ba Parents: a7b706a Author: Timothy Chen <[email protected]> Authored: Sat Aug 16 07:53:55 2014 -0700 Committer: Benjamin Hindman <[email protected]> Committed: Sat Aug 16 08:25:40 2014 -0700 ---------------------------------------------------------------------- src/slave/containerizer/docker.cpp | 55 ++++++++++++++++++++++----- src/tests/docker_containerizer_tests.cpp | 20 ++++++++++ 2 files changed, 66 insertions(+), 9 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/8cbb85c8/src/slave/containerizer/docker.cpp ---------------------------------------------------------------------- diff --git a/src/slave/containerizer/docker.cpp b/src/slave/containerizer/docker.cpp index 5fa0275..215c2b4 100644 --- a/src/slave/containerizer/docker.cpp +++ b/src/slave/containerizer/docker.cpp @@ -227,6 +227,11 @@ private: const Resources& resources, const Docker::Container& container); + process::Future<Nothing> __update( + const ContainerID& containerId, + const Resources& resources, + pid_t pid); + Future<ResourceStatistics> _usage( const ContainerID& containerId, const Docker::Container& container); @@ -301,6 +306,10 @@ private: // The docker pull subprocess is stored so we can killtree the // pid when destroy is called while docker is pulling the image. Option<Subprocess> pull; + + // Once the container is running, this saves the pid of the + // running container. + Option<pid_t> pid; }; hashmap<ContainerID, Container*> containers_; @@ -1237,8 +1246,16 @@ Future<Nothing> DockerContainerizerProcess::update( return Nothing(); } + Container* container = containers_[containerId]; + + if (container->state == Container::DESTROYING) { + LOG(INFO) << "Ignoring updating container '" << containerId + << "' that is being destroyed"; + return Nothing(); + } + // Store the resources for usage(). - containers_[containerId]->resources = _resources; + container->resources = _resources; #ifdef __linux__ if (!_resources.cpus().isSome() && !_resources.mem().isSome()) { @@ -1246,6 +1263,11 @@ Future<Nothing> DockerContainerizerProcess::update( return Nothing(); } + // Skip inspecting the docker container if we already have the pid. + if (container->pid.isSome()) { + return __update(containerId, _resources, container->pid.get()); + } + return docker.inspect(containerName(containerId)) .then(defer(self(), &Self::_update, containerId, _resources, lambda::_1)); #else @@ -1259,6 +1281,27 @@ Future<Nothing> DockerContainerizerProcess::_update( const Resources& _resources, const Docker::Container& container) { + if (container.pid.isNone()) { + return Nothing(); + } + + if (!containers_.contains(containerId)) { + LOG(INFO) << "Container has been removed after docker inspect, " + << "skipping update"; + return Nothing(); + } + + containers_[containerId]->pid = container.pid.get(); + + return __update(containerId, _resources, container.pid.get()); +} + + +Future<Nothing> DockerContainerizerProcess::__update( + const ContainerID& containerId, + const Resources& _resources, + pid_t pid) +{ #ifdef __linux__ // Determine the the cgroups hierarchies where the 'cpu' and // 'memory' subsystems are mounted (they may be the same). Note that @@ -1284,15 +1327,9 @@ Future<Nothing> DockerContainerizerProcess::_update( // the hierarchy with the 'memory' subsystem attached so we can // update the proper cgroup control files. - // First check that this container still appears to be running. - Option<pid_t> pid = container.pid; - if (pid.isNone()) { - return Nothing(); - } - // Determine the cgroup for the 'cpu' subsystem (based on the // container's pid). - Result<string> cpuCgroup = cgroups::cpu::cgroup(pid.get()); + Result<string> cpuCgroup = cgroups::cpu::cgroup(pid); if (cpuCgroup.isError()) { return Failure("Failed to determine cgroup for the 'cpu' subsystem: " + @@ -1325,7 +1362,7 @@ Future<Nothing> DockerContainerizerProcess::_update( } // Now determine the cgroup for the 'memory' subsystem. - Result<string> memoryCgroup = cgroups::memory::cgroup(pid.get()); + Result<string> memoryCgroup = cgroups::memory::cgroup(pid); if (memoryCgroup.isError()) { return Failure("Failed to determine cgroup for the 'memory' subsystem: " + http://git-wip-us.apache.org/repos/asf/mesos/blob/8cbb85c8/src/tests/docker_containerizer_tests.cpp ---------------------------------------------------------------------- diff --git a/src/tests/docker_containerizer_tests.cpp b/src/tests/docker_containerizer_tests.cpp index 3a55f5e..c37bc52 100644 --- a/src/tests/docker_containerizer_tests.cpp +++ b/src/tests/docker_containerizer_tests.cpp @@ -734,6 +734,26 @@ TEST_F(DockerContainerizerTest, ROOT_DOCKER_Update) EXPECT_EQ(1024u, cpu.get()); EXPECT_EQ(128u, mem.get().megabytes()); + newResources = Resources::parse("cpus:1;mem:144"); + + // Issue second update that uses the cached pid instead of inspect. + update = dockerContainerizer.update(containerId.get(), newResources.get()); + + AWAIT_READY(update); + + cpu = cgroups::cpu::shares(cpuHierarchy.get(), cpuCgroup.get()); + + ASSERT_SOME(cpu); + + mem = cgroups::memory::soft_limit_in_bytes( + memoryHierarchy.get(), + memoryCgroup.get()); + + ASSERT_SOME(mem); + + EXPECT_EQ(1024u, cpu.get()); + EXPECT_EQ(144u, mem.get().megabytes()); + driver.stop(); driver.join();
