You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by jo...@apache.org on 2017/05/26 01:41:01 UTC
[10/16] mesos git commit: Changed naming of Docker containers to the
pre-0.23 scheme.
Changed naming of Docker containers to the pre-0.23 scheme.
For the most part, the choice of naming scheme is cosmetic.
All versions of the Docker containerizer will consider containers
starting with "mesos-" as containers started by Mesos.
The Docker containerizer does not consider the `SlaveID` at all
when recovering.
This naming change also tweaks the recovery logic to store the
name of the recovered container, so that future calls to the Docker
CLI use the actual name of the container, rather than a name generated
based on the ContainerID.
Review: https://reviews.apache.org/r/58905
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/55d7c9f7
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/55d7c9f7
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/55d7c9f7
Branch: refs/heads/master
Commit: 55d7c9f76b61ef574e9d686960b280a05c1afc5d
Parents: 7015097
Author: Joseph Wu <jo...@apache.org>
Authored: Thu Apr 27 13:57:15 2017 -0700
Committer: Joseph Wu <jo...@apache.org>
Committed: Thu May 25 18:37:07 2017 -0700
----------------------------------------------------------------------
src/slave/containerizer/docker.cpp | 66 ++++++++++++++++++++-------------
src/slave/containerizer/docker.hpp | 24 ++++++------
2 files changed, 53 insertions(+), 37 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/55d7c9f7/src/slave/containerizer/docker.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/docker.cpp b/src/slave/containerizer/docker.cpp
index 2480f59..9418e01 100644
--- a/src/slave/containerizer/docker.cpp
+++ b/src/slave/containerizer/docker.cpp
@@ -121,13 +121,15 @@ Option<ContainerID> parse(const Docker::Container& container)
}
if (name.isSome()) {
- // For Mesos version < 0.23.0, the docker container name format
- // was DOCKER_NAME_PREFIX + containerId, and starting with 0.23.0
- // it is changed to DOCKER_NAME_PREFIX + slaveId +
- // DOCKER_NAME_SEPERATOR + containerId.
+ // For Mesos versions 0.23 to 1.3 (inclusive), the docker
+ // container name format was:
+ // DOCKER_NAME_PREFIX + SlaveID + DOCKER_NAME_SEPERATOR + ContainerID.
+ //
+ // In versions <= 0.22 or >= 1.4, the name format is:
+ // DOCKER_NAME_PREFIX + ContainerID.
+ //
// To be backward compatible during upgrade, we still have to
- // support the previous format.
- // TODO(tnachen): Remove this check after deprecation cycle.
+ // support all formats.
if (!strings::contains(name.get(), DOCKER_NAME_SEPERATOR)) {
ContainerID id;
id.set_value(name.get());
@@ -383,7 +385,7 @@ DockerContainerizerProcess::Container::create(
// from a hook. This hook is called after `Container::create`.
::mesos::internal::docker::Flags dockerExecutorFlags = dockerFlags(
flags,
- Container::name(slaveId, stringify(id)),
+ Container::name(id),
containerWorkdir,
None());
@@ -923,16 +925,18 @@ Future<Nothing> DockerContainerizerProcess::_recover(
const list<Docker::Container>& _containers)
{
if (state.isSome()) {
- // Although the slave checkpoints executor pids, before 0.23
- // docker containers without custom executors didn't record the
- // container type in the executor info, therefore the Docker
- // containerizer doesn't know if it should recover that container
- // as it could be launched from another containerizer. The
- // workaround is to reconcile running Docker containers and see
- // if we can find an known container that matches the
- // checkpointed container id.
- // TODO(tnachen): Remove this explicit reconciliation 0.24.
- hashset<ContainerID> existingContainers;
+ // This mapping of ContainerIDs to running Docker container names
+ // is established for two reasons:
+ // * Docker containers launched by Mesos versions prior to 0.23
+ // did not checkpoint the container type, so the Docker
+ // Containerizer does not know if it should recover that
+ // container or not.
+ // * The naming scheme of Docker containers changed in Mesos
+ // versions 0.23 and 1.4. The Docker Containerizer code needs
+ // to use the name of the container when interacting with the
+ // Docker CLI, rather than generating the container name
+ // based on the current version's scheme.
+ hashmap<ContainerID, string> existingContainers;
// Tracks all the task containers that launched an executor in
// a docker container.
@@ -941,7 +945,13 @@ Future<Nothing> DockerContainerizerProcess::_recover(
foreach (const Docker::Container& container, _containers) {
Option<ContainerID> id = parse(container);
if (id.isSome()) {
- existingContainers.insert(id.get());
+ // NOTE: The container name returned by `docker inspect` may
+ // sometimes be prefixed with a forward slash. While this is
+ // technically part of the container name, subsequent calls
+ // to the Docker CLI do not expect the prefix.
+ existingContainers[id.get()] = strings::remove(
+ container.name, "/", strings::PREFIX);
+
if (strings::contains(container.name, ".executor")) {
executorContainers.insert(id.get());
}
@@ -1023,6 +1033,10 @@ Future<Nothing> DockerContainerizerProcess::_recover(
container->launchesExecutorContainer =
executorContainers.contains(containerId);
+ if (existingContainers.contains(containerId)) {
+ container->containerName = existingContainers.at(containerId);
+ }
+
pid_t pid = run.get().forkedPid.get();
container->status.set(process::reap(pid));
@@ -1183,7 +1197,7 @@ Future<bool> DockerContainerizerProcess::launch(
f = HookManager::slavePreLaunchDockerTaskExecutorDecorator(
taskInfo,
executorInfo,
- container.get()->name(),
+ container.get()->containerName,
container.get()->directory,
flags.sandbox_directory,
container.get()->environment)
@@ -1299,7 +1313,7 @@ Future<bool> DockerContainerizerProcess::_launch(
}));
}
- string containerName = container->name();
+ string containerName = container->containerName;
if (container->executorName().isSome()) {
// Launch the container with the executor name as we expect the
@@ -1539,7 +1553,7 @@ Future<pid_t> DockerContainerizerProcess::launchExecutorProcess(
// Prepare the flags to pass to the mesos docker executor process.
::mesos::internal::docker::Flags launchFlags = dockerFlags(
flags,
- container->name(),
+ container->containerName,
container->directory,
container->taskEnvironment);
@@ -1662,7 +1676,7 @@ Future<Nothing> DockerContainerizerProcess::update(
return __update(containerId, _resources, container->pid.get());
}
- return docker->inspect(containers_.at(containerId)->name())
+ return docker->inspect(containers_.at(containerId)->containerName)
.then(defer(self(), &Self::_update, containerId, _resources, lambda::_1));
#else
return Nothing();
@@ -1909,7 +1923,7 @@ Future<ResourceStatistics> DockerContainerizerProcess::usage(
return collectUsage(container->pid.get());
}
- return docker->inspect(container->name())
+ return docker->inspect(container->containerName)
.then(defer(
self(),
[this, containerId, collectUsage]
@@ -2257,7 +2271,7 @@ void DockerContainerizerProcess::_destroy(
// container should be destroyed forcefully.
// The `after` fallback should remain as a precaution against the docker
// stop command hanging.
- docker->stop(container->name(), flags.docker_stop_timeout)
+ docker->stop(container->containerName, flags.docker_stop_timeout)
.after(
flags.docker_stop_timeout + DOCKER_FORCE_KILL_TIMEOUT,
defer(self(), &Self::destroyTimeout, containerId, lambda::_1))
@@ -2303,7 +2317,7 @@ void DockerContainerizerProcess::__destroy(
flags.docker_remove_delay,
self(),
&Self::remove,
- container->name(),
+ container->containerName,
container->executorName());
delete container;
@@ -2374,7 +2388,7 @@ void DockerContainerizerProcess::____destroy(
flags.docker_remove_delay,
self(),
&Self::remove,
- container->name(),
+ container->containerName,
container->executorName());
delete container;
http://git-wip-us.apache.org/repos/asf/mesos/blob/55d7c9f7/src/slave/containerizer/docker.hpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/docker.hpp b/src/slave/containerizer/docker.hpp
index 44efa44..2ed8e1c 100644
--- a/src/slave/containerizer/docker.hpp
+++ b/src/slave/containerizer/docker.hpp
@@ -45,8 +45,9 @@ namespace slave {
// created by Mesos from those created manually.
extern const std::string DOCKER_NAME_PREFIX;
-// Separator used to compose docker container name, which is made up
-// of slave ID and container ID.
+// Separator used to compose docker container name, which consists
+// of the name prefix, ContainerID, and possibly the SlaveID depending
+// on the version of Mesos used to create the container.
extern const std::string DOCKER_NAME_SEPERATOR;
// Directory that stores all the symlinked sandboxes that is mapped
@@ -315,10 +316,9 @@ private:
bool checkpoint,
const Flags& flags);
- static std::string name(const SlaveID& slaveId, const std::string& id)
+ static std::string name(const ContainerID& id)
{
- return DOCKER_NAME_PREFIX + slaveId.value() + DOCKER_NAME_SEPERATOR +
- stringify(id);
+ return DOCKER_NAME_PREFIX + stringify(id);
}
Container(const ContainerID& id)
@@ -348,6 +348,7 @@ private:
checkpoint(checkpoint),
symlinked(symlinked),
flags(flags),
+ containerName(name(id)),
launchesExecutorContainer(launchesExecutorContainer)
{
// NOTE: The task's resources are included in the executor's
@@ -392,15 +393,10 @@ private:
}
}
- std::string name()
- {
- return name(slaveId, stringify(id));
- }
-
Option<std::string> executorName()
{
if (launchesExecutorContainer) {
- return name() + DOCKER_NAME_SEPERATOR + "executor";
+ return containerName + DOCKER_NAME_SEPERATOR + "executor";
} else {
return None();
}
@@ -477,6 +473,12 @@ private:
bool symlinked;
const Flags flags;
+ // The string used to refer to this container via the Docker CLI.
+ // This name is either computed by concatenating the DOCKER_NAME_PREFIX
+ // and the ContainerID; or during recovery, by taking the recovered
+ // container's name.
+ std::string containerName;
+
// Promise for future returned from wait().
process::Promise<mesos::slave::ContainerTermination> termination;