You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by tn...@apache.org on 2015/04/03 21:58:07 UTC
[1/3] mesos git commit: Only update docker container when resources
differs.
Repository: mesos
Updated Branches:
refs/heads/master eb4b19ba4 -> 00318fc1b
Only update docker container when resources differs.
Review: https://reviews.apache.org/r/32796
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/591d1090
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/591d1090
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/591d1090
Branch: refs/heads/master
Commit: 591d1090f69a7d9197aea355488c973c41881be7
Parents: eb4b19b
Author: Timothy Chen <tn...@apache.org>
Authored: Wed Apr 1 16:03:43 2015 -0700
Committer: Timothy Chen <tn...@apache.org>
Committed: Fri Apr 3 12:49:35 2015 -0700
----------------------------------------------------------------------
src/slave/containerizer/docker.cpp | 8 ++++++++
src/slave/containerizer/docker.hpp | 2 +-
2 files changed, 9 insertions(+), 1 deletion(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/591d1090/src/slave/containerizer/docker.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/docker.cpp b/src/slave/containerizer/docker.cpp
index e83b912..76e47cc 100644
--- a/src/slave/containerizer/docker.cpp
+++ b/src/slave/containerizer/docker.cpp
@@ -851,6 +851,14 @@ Future<Nothing> DockerContainerizerProcess::update(
return Nothing();
}
+ if (container->resources == _resources) {
+ LOG(INFO) << "Ignoring updating container '" << containerId
+ << "' with resources passed to update is identical to "
+ << "existing resources";
+ return Nothing();
+ }
+
+
// Store the resources for usage().
container->resources = _resources;
http://git-wip-us.apache.org/repos/asf/mesos/blob/591d1090/src/slave/containerizer/docker.hpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/docker.hpp b/src/slave/containerizer/docker.hpp
index b7bf54a..5ae3a9d 100644
--- a/src/slave/containerizer/docker.hpp
+++ b/src/slave/containerizer/docker.hpp
@@ -290,7 +290,7 @@ private:
flags(flags)
{
if (task.isSome()) {
- resources = task.get().resources();
+ resources = task.get().resources() + executor.resources();
} else {
resources = executor.resources();
}
[3/3] mesos git commit: Add test to verify executor clean up in
docker containerizer.
Posted by tn...@apache.org.
Add test to verify executor clean up in docker containerizer.
Review: https://reviews.apache.org/r/32798
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/00318fc1
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/00318fc1
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/00318fc1
Branch: refs/heads/master
Commit: 00318fc1b30fc0961c2dfa4d934c37866577d801
Parents: 8790440
Author: Timothy Chen <tn...@apache.org>
Authored: Wed Apr 1 16:56:53 2015 -0700
Committer: Timothy Chen <tn...@apache.org>
Committed: Fri Apr 3 12:49:43 2015 -0700
----------------------------------------------------------------------
src/tests/docker_containerizer_tests.cpp | 138 ++++++++++++++++++++++++++
1 file changed, 138 insertions(+)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/00318fc1/src/tests/docker_containerizer_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/docker_containerizer_tests.cpp b/src/tests/docker_containerizer_tests.cpp
index fdd706a..c772d4c 100644
--- a/src/tests/docker_containerizer_tests.cpp
+++ b/src/tests/docker_containerizer_tests.cpp
@@ -83,8 +83,22 @@ public:
EXPECT_CALL(*this, stop(_, _, _))
.WillRepeatedly(Invoke(this, &MockDocker::_stop));
+
+ EXPECT_CALL(*this, run(_, _, _, _, _, _, _))
+ .WillRepeatedly(Invoke(this, &MockDocker::_run));
}
+ MOCK_CONST_METHOD7(
+ run,
+ process::Future<Nothing>(
+ const mesos::ContainerInfo&,
+ const mesos::CommandInfo&,
+ const std::string&,
+ const std::string&,
+ const std::string&,
+ const Option<mesos::Resources>& resources,
+ const Option<std::map<std::string, std::string>>&));
+
MOCK_CONST_METHOD2(
logs,
process::Future<Nothing>(
@@ -97,6 +111,25 @@ public:
const string&,
const Duration&, bool));
+ process::Future<Nothing> _run(
+ const mesos::ContainerInfo& containerInfo,
+ const mesos::CommandInfo& commandInfo,
+ const std::string& name,
+ const std::string& sandboxDirectory,
+ const std::string& mappedDirectory,
+ const Option<mesos::Resources>& resources = None(),
+ const Option<std::map<std::string, std::string> >& env = None()) const
+ {
+ return Docker::run(
+ containerInfo,
+ commandInfo,
+ name,
+ sandboxDirectory,
+ mappedDirectory,
+ resources,
+ env);
+ }
+
process::Future<Nothing> _logs(
const string& container,
const string& directory) const
@@ -2588,6 +2621,111 @@ TEST_F(DockerContainerizerTest, ROOT_DOCKER_DestroyWhilePulling)
Shutdown();
}
+
+// This test checks that when a docker containerizer update failed
+// and the container failed before the executor started, the executor
+// is properly killed and cleaned up.
+TEST_F(DockerContainerizerTest, ROOT_DOCKER_ExecutorCleanupWhenLaunchFailed)
+{
+ Try<PID<Master>> master = StartMaster();
+ ASSERT_SOME(master);
+
+ slave::Flags flags = CreateSlaveFlags();
+
+ MockDocker* mockDocker = new MockDocker(tests::flags.docker);
+ Shared<Docker> docker(mockDocker);
+
+ // Skip logging so we can avoid waiting for it to terminate.
+ EXPECT_CALL(*mockDocker, logs(_, _))
+ .WillOnce(Return(Nothing()));
+
+ Fetcher fetcher;
+
+ // The docker containerizer will free the process, so we must
+ // allocate on the heap.
+ MockDockerContainerizerProcess* process =
+ new MockDockerContainerizerProcess(flags, &fetcher, docker);
+
+ MockDockerContainerizer dockerContainerizer(
+ (Owned<DockerContainerizerProcess>(process)));
+
+ Try<PID<Slave>> slave = StartSlave(&dockerContainerizer);
+ 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];
+
+ TaskInfo task;
+ task.set_name("");
+ task.mutable_task_id()->set_value("1");
+ task.mutable_slave_id()->CopyFrom(offer.slave_id());
+ task.mutable_resources()->CopyFrom(offer.resources());
+
+ CommandInfo command;
+ command.set_value("ls");
+
+ 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);
+
+ Future<TaskStatus> statusFailed;
+ EXPECT_CALL(sched, statusUpdate(&driver, _))
+ .WillOnce(FutureArg<1>(&statusFailed));
+
+ Future<ContainerID> containerId;
+ EXPECT_CALL(dockerContainerizer, launch(_, _, _, _, _, _, _, _))
+ .WillOnce(DoAll(FutureArg<0>(&containerId),
+ Invoke(&dockerContainerizer,
+ &MockDockerContainerizer::_launch)));
+
+ // Fail the update so we don't proceed to send run task to the executor.
+ EXPECT_CALL(dockerContainerizer, update(_, _))
+ .WillRepeatedly(Return(Failure("Fail resource update")));
+
+ vector<TaskInfo> tasks;
+ tasks.push_back(task);
+
+ driver.launchTasks(offers.get()[0].id(), tasks);
+
+ AWAIT_READY_FOR(containerId, Seconds(60));
+
+ AWAIT_READY(statusFailed);
+
+ EXPECT_EQ(TASK_FAILED, statusFailed.get().state());
+
+ driver.stop();
+ driver.join();
+
+ // We expect the executor to have exited, and if not in Shutdown
+ // the test will fail because of the executor process still running.
+ Shutdown();
+}
+
} // namespace tests {
} // namespace internal {
} // namespace mesos {
[2/3] mesos git commit: Kill the executor when docker container is
destroyed.
Posted by tn...@apache.org.
Kill the executor when docker container is destroyed.
Review: https://reviews.apache.org/r/32797
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/87904409
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/87904409
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/87904409
Branch: refs/heads/master
Commit: 879044096f80adb1799ce43acc1fc5ae58dfac69
Parents: 591d109
Author: Timothy Chen <tn...@apache.org>
Authored: Wed Apr 1 16:29:54 2015 -0700
Committer: Timothy Chen <tn...@apache.org>
Committed: Fri Apr 3 12:49:39 2015 -0700
----------------------------------------------------------------------
src/slave/containerizer/docker.cpp | 22 +++++++++++++++++++++-
src/slave/containerizer/docker.hpp | 5 +++++
2 files changed, 26 insertions(+), 1 deletion(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/87904409/src/slave/containerizer/docker.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/docker.cpp b/src/slave/containerizer/docker.cpp
index 76e47cc..f9fb078 100644
--- a/src/slave/containerizer/docker.cpp
+++ b/src/slave/containerizer/docker.cpp
@@ -263,6 +263,8 @@ Try<Nothing> DockerContainerizerProcess::checkpoint(
Container* container = containers_[containerId];
+ container->executorPid = pid;
+
if (container->checkpoint) {
const string& path =
slave::paths::getForkedPidPath(
@@ -1225,11 +1227,29 @@ void DockerContainerizerProcess::destroy(
container->state = Container::DESTROYING;
+ if (container->executorPid.isSome()) {
+ LOG(INFO) << "Sending SIGTERM to executor with pid: "
+ << container->executorPid.get();
+ // We need to clean up the executor as the executor might not have
+ // received run task due to a failed containerizer update.
+ // We also kill the executor first since container->status below
+ // is waiting for the executor to finish.
+ Try<std::list<os::ProcessTree>> kill =
+ os::killtree(container->executorPid.get(), SIGTERM);
+
+ if (kill.isError()) {
+ // Ignoring the error from killing executor as it can already
+ // have exited.
+ VLOG(1) << "Ignoring error when killing executor pid "
+ << container->executorPid.get() << " in destroy, error: "
+ << kill.error();
+ }
+ }
+
// Otherwise, wait for Docker::run to succeed, in which case we'll
// continue in _destroy (calling Docker::kill) or for Docker::run to
// fail, in which case we'll re-execute this function and cleanup
// above.
-
container->status.future()
.onAny(defer(self(), &Self::_destroy, containerId, killed));
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/87904409/src/slave/containerizer/docker.hpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/docker.hpp b/src/slave/containerizer/docker.hpp
index 5ae3a9d..6893684 100644
--- a/src/slave/containerizer/docker.hpp
+++ b/src/slave/containerizer/docker.hpp
@@ -432,6 +432,11 @@ private:
// Once the container is running, this saves the pid of the
// running container.
Option<pid_t> pid;
+
+ // The executor pid that was forked to wait on the running
+ // container. This is stored so we can clean up the executor
+ // on destroy.
+ Option<pid_t> executorPid;
};
hashmap<ContainerID, Container*> containers_;