You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by ti...@apache.org on 2016/11/29 21:44:17 UTC
[2/4] mesos git commit: Removed superseded
`slavePreLaunchDockerEnvironmentDecorator` hook.
Removed superseded `slavePreLaunchDockerEnvironmentDecorator` hook.
Review: https://reviews.apache.org/r/54129/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/1fe9876c
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/1fe9876c
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/1fe9876c
Branch: refs/heads/master
Commit: 1fe9876ca069162e46cc2436384e8d5da2b9d551
Parents: f765828
Author: Till Toenshoff <to...@me.com>
Authored: Tue Nov 29 22:05:30 2016 +0100
Committer: Till Toenshoff <to...@me.com>
Committed: Tue Nov 29 22:43:55 2016 +0100
----------------------------------------------------------------------
include/mesos/hook.hpp | 36 -----------
src/examples/test_hook_module.cpp | 38 +++---------
src/hook/manager.cpp | 49 ---------------
src/hook/manager.hpp | 9 ---
src/slave/containerizer/docker.cpp | 42 -------------
src/tests/hook_tests.cpp | 105 --------------------------------
6 files changed, 8 insertions(+), 271 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/1fe9876c/include/mesos/hook.hpp
----------------------------------------------------------------------
diff --git a/include/mesos/hook.hpp b/include/mesos/hook.hpp
index 134951c..8087817 100644
--- a/include/mesos/hook.hpp
+++ b/include/mesos/hook.hpp
@@ -89,39 +89,6 @@ public:
return None();
}
- // This environment decorator is called from within the slave after
- // receiving a run task request from the master but before the docker
- // containerizer launches the task. A module implementing the hook can
- // inspect the arguments and return a `Failure` if the task should be
- // rejected with a `TASK_FAILED`.
- // The hook can return a set of environment variables. For command tasks
- // the environment variables will become part of the task's environment.
- // For custom executors, the environment variables will be part of the
- // custom executor's environment.
- //
- // NOTE: The order of hooks matters for environment variables.
- // If there is a conflict, the hook loaded last will take priority.
- //
- // NOTE: Unlike `slaveExecutorEnvironmentDecorator`, environment variables
- // returned from this hook, in case of conflicts, *will* overwrite
- // environment variables inside the `ExecutorInfo`.
- //
- // NOTE: This hook is designed to be an asynchronous replacement for
- // `slavePreLaunchDockerHook`. This hook is called first.
- //
- // NOTE: Superseded by `slavePreLaunchDockerTaskExecutorDecorator`.
- virtual process::Future<Option<Environment>>
- slavePreLaunchDockerEnvironmentDecorator(
- const Option<TaskInfo>& taskInfo,
- const ExecutorInfo& executorInfo,
- const std::string& containerName,
- const std::string& containerWorkDirectory,
- const std::string& mappedSandboxDirectory,
- const Option<std::map<std::string, std::string>>& env)
- {
- return None();
- }
-
// This task and executor decorator is called from within the slave after
// receiving a run task request from the master but before the docker
// containerizer launches the task. A module implementing the hook can
@@ -138,9 +105,6 @@ public:
// NOTE: Unlike `slaveExecutorEnvironmentDecorator`, environment variables
// returned from this hook, in case of conflicts, *will* overwrite
// environment variables inside the `ExecutorInfo`.
- //
- // NOTE: This hook is designed to be a replacement for
- // `slavePreLaunchDockerEnvironmentDecorator`.
virtual process::Future<Option<DockerTaskExecutorPrepareInfo>>
slavePreLaunchDockerTaskExecutorDecorator(
const Option<TaskInfo>& taskInfo,
http://git-wip-us.apache.org/repos/asf/mesos/blob/1fe9876c/src/examples/test_hook_module.cpp
----------------------------------------------------------------------
diff --git a/src/examples/test_hook_module.cpp b/src/examples/test_hook_module.cpp
index 993c010..3a7ffa4 100644
--- a/src/examples/test_hook_module.cpp
+++ b/src/examples/test_hook_module.cpp
@@ -187,36 +187,6 @@ public:
}
- // In this hook, look for the existence of a specific label.
- // If found, return a `Failure`.
- // Otherwise, add an environment variable to the task.
- virtual Future<Option<Environment>> slavePreLaunchDockerEnvironmentDecorator(
- const Option<TaskInfo>& taskInfo,
- const ExecutorInfo& executorInfo,
- const string& containerName,
- const string& containerWorkDirectory,
- const string& mappedSandboxDirectory,
- const Option<map<string, string>>& env)
- {
- LOG(INFO) << "Executing 'slavePreLaunchDockerEnvironmentDecorator' hook";
-
- if (taskInfo.isSome()) {
- foreach (const Label& label, taskInfo->labels().labels()) {
- if (label.key() == testErrorLabelKey) {
- return Failure("Spotted error label");
- }
- }
- }
-
- Environment environment;
- Environment::Variable* variable = environment.add_variables();
- variable->set_name("FOO_DOCKER");
- variable->set_value("docker_bar");
-
- return environment;
- }
-
-
// In this hook, add an environment variable to the executor and task.
virtual Future<Option<DockerTaskExecutorPrepareInfo>>
slavePreLaunchDockerTaskExecutorDecorator(
@@ -229,6 +199,14 @@ public:
{
LOG(INFO) << "Executing 'slavePreLaunchDockerTaskExecutorDecorator' hook";
+ if (taskInfo.isSome()) {
+ foreach (const Label& label, taskInfo->labels().labels()) {
+ if (label.key() == testErrorLabelKey) {
+ return Failure("Spotted error label");
+ }
+ }
+ }
+
DockerTaskExecutorPrepareInfo prepareInfo;
Environment* taskEnvironment =
http://git-wip-us.apache.org/repos/asf/mesos/blob/1fe9876c/src/hook/manager.cpp
----------------------------------------------------------------------
diff --git a/src/hook/manager.cpp b/src/hook/manager.cpp
index 59f402b..a3dc1c1 100644
--- a/src/hook/manager.cpp
+++ b/src/hook/manager.cpp
@@ -252,55 +252,6 @@ Future<DockerTaskExecutorPrepareInfo>
}
-Future<map<string, string>>
- HookManager::slavePreLaunchDockerEnvironmentDecorator(
- const Option<TaskInfo>& taskInfo,
- const ExecutorInfo& executorInfo,
- const string& containerName,
- const string& containerWorkDirectory,
- const string& mappedSandboxDirectory,
- const Option<map<string, string>>& env)
-{
- // We execute these hooks according to their ordering so any conflicting
- // environment variables can be deterministically resolved
- // (the last hook takes priority).
- list<Future<Option<Environment>>> futures;
-
- foreach (const string& name, availableHooks.keys()) {
- Hook* hook = availableHooks[name];
-
- // Chain together each hook.
- futures.push_back(
- hook->slavePreLaunchDockerEnvironmentDecorator(
- taskInfo,
- executorInfo,
- containerName,
- containerWorkDirectory,
- mappedSandboxDirectory,
- env));
- }
-
- return collect(futures)
- .then([](const list<Option<Environment>>& results)
- -> Future<map<string, string>> {
- // Combine all the Environments.
- map<string, string> environment;
-
- foreach (const Option<Environment>& result, results) {
- if (result.isNone()) {
- continue;
- }
-
- foreach (const Environment::Variable& variable, result->variables()) {
- environment[variable.name()] = variable.value();
- }
- }
-
- return environment;
- });
-}
-
-
void HookManager::slavePreLaunchDockerHook(
const ContainerInfo& containerInfo,
const CommandInfo& commandInfo,
http://git-wip-us.apache.org/repos/asf/mesos/blob/1fe9876c/src/hook/manager.hpp
----------------------------------------------------------------------
diff --git a/src/hook/manager.hpp b/src/hook/manager.hpp
index ad52717..fa23987 100644
--- a/src/hook/manager.hpp
+++ b/src/hook/manager.hpp
@@ -56,15 +56,6 @@ public:
static Environment slaveExecutorEnvironmentDecorator(
ExecutorInfo executorInfo);
- static process::Future<std::map<std::string, std::string>>
- slavePreLaunchDockerEnvironmentDecorator(
- const Option<TaskInfo>& taskInfo,
- const ExecutorInfo& executorInfo,
- const std::string& containerName,
- const std::string& containerWorkDirectory,
- const std::string& mappedSandboxDirectory,
- const Option<std::map<std::string, std::string>>& env);
-
static process::Future<DockerTaskExecutorPrepareInfo>
slavePreLaunchDockerTaskExecutorDecorator(
const Option<TaskInfo>& taskInfo,
http://git-wip-us.apache.org/repos/asf/mesos/blob/1fe9876c/src/slave/containerizer/docker.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/docker.cpp b/src/slave/containerizer/docker.cpp
index 28b487a..d6818a6 100644
--- a/src/slave/containerizer/docker.cpp
+++ b/src/slave/containerizer/docker.cpp
@@ -1128,48 +1128,6 @@ Future<bool> DockerContainerizerProcess::launch(
Future<Nothing> f = Nothing();
if (HookManager::hooksAvailable()) {
- // TODO(tillt): `slavePreLaunchDockerEnvironmentDecorator` is
- // deprecated, remove this entire block after Mesos 1.2's
- // deprecation cycle ends.
- f = HookManager::slavePreLaunchDockerEnvironmentDecorator(
- taskInfo,
- executorInfo,
- container.get()->name(),
- container.get()->directory,
- flags.sandbox_directory,
- container.get()->environment)
- .then(defer(self(), [this, taskInfo, containerId](
- const map<string, string>& environment) -> Future<Nothing> {
- if (!containers_.contains(containerId)) {
- return Failure("Container is already destroyed");
- }
-
- Container* container = containers_.at(containerId);
-
- if (taskInfo.isSome()) {
- // The built-in command executors explicitly support passing
- // environment variables from a hook into a task.
- container->taskEnvironment = environment;
-
- // For dockerized command executors, the flags have already been
- // serialized into the command, albeit without these environment
- // variables. Append the last flag to the overridden command.
- if (container->launchesExecutorContainer) {
- container->command.add_arguments(
- "--task_environment=" + string(jsonify(environment)));
- }
- } else {
- // For custom executors, the environment variables from a hook
- // are passed directly into the executor. It is up to the custom
- // executor whether individual tasks should inherit these variables.
- foreachpair (const string& key, const string& value, environment) {
- container->environment[key] = value;
- }
- }
-
- return Nothing();
- }));
-
f = HookManager::slavePreLaunchDockerTaskExecutorDecorator(
taskInfo,
executorInfo,
http://git-wip-us.apache.org/repos/asf/mesos/blob/1fe9876c/src/tests/hook_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/hook_tests.cpp b/src/tests/hook_tests.cpp
index 96d8956..379d418 100644
--- a/src/tests/hook_tests.cpp
+++ b/src/tests/hook_tests.cpp
@@ -633,111 +633,6 @@ TEST_F(HookTest, VerifySlaveTaskStatusDecorator)
// This test verifies that the slave pre-launch docker environment
-// decorator can attach environment variables to a task.
-TEST_F(HookTest, ROOT_DOCKER_VerifySlavePreLaunchDockerEnvironmentDecorator)
-{
- Try<Owned<cluster::Master>> master = StartMaster();
- ASSERT_SOME(master);
-
- MockDocker* mockDocker =
- new MockDocker(tests::flags.docker, tests::flags.docker_socket);
-
- Shared<Docker> docker(mockDocker);
-
- slave::Flags flags = CreateSlaveFlags();
-
- Fetcher fetcher;
-
- Try<ContainerLogger*> logger =
- ContainerLogger::create(flags.container_logger);
-
- ASSERT_SOME(logger);
-
- MockDockerContainerizer containerizer(
- flags,
- &fetcher,
- Owned<ContainerLogger>(logger.get()),
- docker);
-
- Owned<MasterDetector> detector = master.get()->createDetector();
-
- Try<Owned<cluster::Slave>> slave =
- StartSlave(detector.get(), &containerizer, flags);
- ASSERT_SOME(slave);
-
- MockScheduler sched;
- MesosSchedulerDriver driver(
- &sched, DEFAULT_FRAMEWORK_INFO, master.get()->pid, DEFAULT_CREDENTIAL);
-
- EXPECT_CALL(sched, registered(&driver, _, _));
-
- Future<vector<Offer>> offers;
- EXPECT_CALL(sched, resourceOffers(&driver, _))
- .WillOnce(FutureArg<1>(&offers))
- .WillRepeatedly(Return()); // Ignore subsequent offers.
-
- driver.start();
-
- AWAIT_READY(offers);
- ASSERT_EQ(1u, offers.get().size());
-
- TaskInfo task = createTask(
- offers.get()[0],
- "test \"$FOO_DOCKER\" = 'docker_bar'");
-
- ContainerInfo containerInfo;
- containerInfo.set_type(ContainerInfo::DOCKER);
-
- // TODO(tnachen): Use local image to test if possible.
- ContainerInfo::DockerInfo dockerInfo;
- dockerInfo.set_image("alpine");
- containerInfo.mutable_docker()->CopyFrom(dockerInfo);
-
- task.mutable_container()->CopyFrom(containerInfo);
-
- Future<ContainerID> containerId;
- EXPECT_CALL(containerizer, launch(_, _, _, _, _, _, _, _))
- .WillOnce(DoAll(FutureArg<0>(&containerId),
- Invoke(&containerizer,
- &MockDockerContainerizer::_launch)));
-
- Future<TaskStatus> statusRunning;
- Future<TaskStatus> statusFinished;
- EXPECT_CALL(sched, statusUpdate(&driver, _))
- .WillOnce(FutureArg<1>(&statusRunning))
- .WillOnce(FutureArg<1>(&statusFinished))
- .WillRepeatedly(DoDefault());
-
- driver.launchTasks(offers.get()[0].id(), {task});
-
- AWAIT_READY_FOR(containerId, Seconds(60));
- AWAIT_READY_FOR(statusRunning, Seconds(60));
- EXPECT_EQ(TASK_RUNNING, statusRunning.get().state());
- AWAIT_READY_FOR(statusFinished, Seconds(60));
- EXPECT_EQ(TASK_FINISHED, statusFinished.get().state());
-
- Future<Option<ContainerTermination>> termination =
- containerizer.wait(containerId.get());
-
- driver.stop();
- driver.join();
-
- AWAIT_READY(termination);
- EXPECT_SOME(termination.get());
-
- Future<list<Docker::Container>> containers =
- docker.get()->ps(true, slave::DOCKER_NAME_PREFIX);
-
- AWAIT_READY(containers);
-
- // Cleanup all mesos launched containers.
- foreach (const Docker::Container& container, containers.get()) {
- AWAIT_READY_FOR(docker.get()->rm(container.id, true), Seconds(30));
- }
-}
-
-
-// This test verifies that the slave pre-launch docker environment
// decorator can attach environment variables to a task exclusively.
TEST_F(HookTest, ROOT_DOCKER_VerifySlavePreLaunchDockerTaskExecutorDecorator)
{