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 2016/10/12 02:03:37 UTC
[1/3] mesos git commit: Added MESOS-6371 (deprecation) to Mesos 1.1.0
CHANGELOG.
Repository: mesos
Updated Branches:
refs/heads/master 857d82f64 -> e361aaeda
Added MESOS-6371 (deprecation) to Mesos 1.1.0 CHANGELOG.
Review: https://reviews.apache.org/r/52763/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/e4c410dc
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/e4c410dc
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/e4c410dc
Branch: refs/heads/master
Commit: e4c410dc85679e2b20d88647b7255e51a644be89
Parents: d2e9d1b
Author: Gilbert Song <so...@gmail.com>
Authored: Tue Oct 11 18:55:44 2016 -0700
Committer: Joseph Wu <jo...@apache.org>
Committed: Tue Oct 11 19:02:03 2016 -0700
----------------------------------------------------------------------
CHANGELOG | 2 ++
1 file changed, 2 insertions(+)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/e4c410dc/CHANGELOG
----------------------------------------------------------------------
diff --git a/CHANGELOG b/CHANGELOG
index ab78296..11e361b 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -25,6 +25,8 @@ Deprecations:
* [MESOS-5955] - Health check binary "mesos-health-check" is removed.
+ * [MESOS-6371] - Remove the 'recover()' interface in 'ContainerLogger'.
+
Additional API Changes:
* [MESOS-6220] - HTTP handler failures should result in 500 rather than
503 responses. This means that when using the master or agent endpoints,
[2/3] mesos git commit: Removed 'recover()' interface in
'ContainerLogger'.
Posted by jo...@apache.org.
Removed 'recover()' interface in 'ContainerLogger'.
This change arises from the nested container support in Mesos.
Currently, the container logger interface only contains `recover()` and
`prepare()` methods. The `prepare` is called in the containerizers
prior to launching a container, while `recover` is called during
containerizer recovery. Both methods take two parameters:
an ExecutorInfo and sandbox directory.
Currently, all known implementations of the `ContainerLogger` are
stateless. This means the `recover()` function is a noop in all
implementations. Additionally, recovery needs to be paired with an
associated "cleanup" function to be properly stateful. To avoid
adding tech debt in the containerizers, this removes the `recover()`
function for now. We can add it back together with a "cleanup"
function if needed in future.
Review: https://reviews.apache.org/r/52762/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/d2e9d1b0
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/d2e9d1b0
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/d2e9d1b0
Branch: refs/heads/master
Commit: d2e9d1b0e3ee50309b1cea8a4fd127b8723fd922
Parents: 857d82f
Author: Gilbert Song <so...@gmail.com>
Authored: Tue Oct 11 17:28:32 2016 -0700
Committer: Joseph Wu <jo...@apache.org>
Committed: Tue Oct 11 19:02:03 2016 -0700
----------------------------------------------------------------------
include/mesos/slave/container_logger.hpp | 25 ---
src/slave/container_loggers/lib_logrotate.cpp | 18 --
src/slave/container_loggers/lib_logrotate.hpp | 4 -
src/slave/container_loggers/sandbox.cpp | 19 --
src/slave/container_loggers/sandbox.hpp | 6 -
src/slave/containerizer/docker.cpp | 11 --
src/slave/containerizer/mesos/containerizer.cpp | 14 --
src/tests/container_logger_tests.cpp | 188 -------------------
8 files changed, 285 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/d2e9d1b0/include/mesos/slave/container_logger.hpp
----------------------------------------------------------------------
diff --git a/include/mesos/slave/container_logger.hpp b/include/mesos/slave/container_logger.hpp
index 9623b0c..9399747 100644
--- a/include/mesos/slave/container_logger.hpp
+++ b/include/mesos/slave/container_logger.hpp
@@ -128,31 +128,6 @@ public:
virtual Try<Nothing> initialize() = 0;
/**
- * Called during the agent recovery process as executors re-register with
- * the agent.
- *
- * The container logger should use this method to do any executor-specific
- * setup. This method should have the same side-effects as `prepare`, except
- * that the executor is already running. i.e. If `prepare` associates
- * some metadata with the `ExecutorID` of each executor, that metadata
- * should be restored here.
- *
- * If the container logger's state cannot be recovered, this method should
- * return a `Failure`. This failure is logged by the agent, but has no other
- * effects. The executor will not be killed.
- * TODO(josephw): Consider different behavior when the executor cannot be
- * recovered. For example, we could add a flag to kill non-recovered
- * executors.
- *
- * NOTE: The executor may have been spawned with a different container logger.
- * This can occur if an executor is running and the agent is restarted with
- * a different container logger module.
- */
- virtual process::Future<Nothing> recover(
- const ExecutorInfo& executorInfo,
- const std::string& sandboxDirectory) = 0;
-
- /**
* Called before Mesos creates a container.
*
* The container logger is given some of the arguments which the containerizer
http://git-wip-us.apache.org/repos/asf/mesos/blob/d2e9d1b0/src/slave/container_loggers/lib_logrotate.cpp
----------------------------------------------------------------------
diff --git a/src/slave/container_loggers/lib_logrotate.cpp b/src/slave/container_loggers/lib_logrotate.cpp
index 0ca2b3d..53698d3 100644
--- a/src/slave/container_loggers/lib_logrotate.cpp
+++ b/src/slave/container_loggers/lib_logrotate.cpp
@@ -68,14 +68,6 @@ class LogrotateContainerLoggerProcess :
public:
LogrotateContainerLoggerProcess(const Flags& _flags) : flags(_flags) {}
- Future<Nothing> recover(
- const ExecutorInfo& executorInfo,
- const std::string& sandboxDirectory)
- {
- // No state to recover.
- return Nothing();
- }
-
// Spawns two subprocesses that read from their stdin and write to
// "stdout" and "stderr" files in the sandbox. The subprocesses will rotate
// the files according to the configured maximum size and number of files.
@@ -280,16 +272,6 @@ Try<Nothing> LogrotateContainerLogger::initialize()
return Nothing();
}
-Future<Nothing> LogrotateContainerLogger::recover(
- const ExecutorInfo& executorInfo,
- const std::string& sandboxDirectory)
-{
- return dispatch(
- process.get(),
- &LogrotateContainerLoggerProcess::recover,
- executorInfo,
- sandboxDirectory);
-}
Future<SubprocessInfo> LogrotateContainerLogger::prepare(
const ExecutorInfo& executorInfo,
http://git-wip-us.apache.org/repos/asf/mesos/blob/d2e9d1b0/src/slave/container_loggers/lib_logrotate.hpp
----------------------------------------------------------------------
diff --git a/src/slave/container_loggers/lib_logrotate.hpp b/src/slave/container_loggers/lib_logrotate.hpp
index a8653d7..d6e4f95 100644
--- a/src/slave/container_loggers/lib_logrotate.hpp
+++ b/src/slave/container_loggers/lib_logrotate.hpp
@@ -195,10 +195,6 @@ public:
// This is a noop. The logrotate container logger has nothing to initialize.
virtual Try<Nothing> initialize();
- virtual process::Future<Nothing> recover(
- const ExecutorInfo& executorInfo,
- const std::string& sandboxDirectory);
-
virtual process::Future<mesos::slave::ContainerLogger::SubprocessInfo>
prepare(
const ExecutorInfo& executorInfo,
http://git-wip-us.apache.org/repos/asf/mesos/blob/d2e9d1b0/src/slave/container_loggers/sandbox.cpp
----------------------------------------------------------------------
diff --git a/src/slave/container_loggers/sandbox.cpp b/src/slave/container_loggers/sandbox.cpp
index 807d792..cc263eb 100644
--- a/src/slave/container_loggers/sandbox.cpp
+++ b/src/slave/container_loggers/sandbox.cpp
@@ -55,13 +55,6 @@ public:
SandboxContainerLoggerProcess()
: ProcessBase(process::ID::generate("sandbox-logger")) {}
- Future<Nothing> recover(
- const ExecutorInfo& executorInfo,
- const std::string& sandboxDirectory)
- {
- return Nothing();
- }
-
process::Future<ContainerLogger::SubprocessInfo> prepare(
const ExecutorInfo& executorInfo,
const std::string& sandboxDirectory)
@@ -96,18 +89,6 @@ Try<Nothing> SandboxContainerLogger::initialize()
}
-Future<Nothing> SandboxContainerLogger::recover(
- const ExecutorInfo& executorInfo,
- const std::string& sandboxDirectory)
-{
- return dispatch(
- process.get(),
- &SandboxContainerLoggerProcess::recover,
- executorInfo,
- sandboxDirectory);
-}
-
-
Future<ContainerLogger::SubprocessInfo>
SandboxContainerLogger::prepare(
const ExecutorInfo& executorInfo,
http://git-wip-us.apache.org/repos/asf/mesos/blob/d2e9d1b0/src/slave/container_loggers/sandbox.hpp
----------------------------------------------------------------------
diff --git a/src/slave/container_loggers/sandbox.hpp b/src/slave/container_loggers/sandbox.hpp
index 8b1f8ab..e0aeb32 100644
--- a/src/slave/container_loggers/sandbox.hpp
+++ b/src/slave/container_loggers/sandbox.hpp
@@ -55,12 +55,6 @@ public:
// This is a noop. The sandbox container logger has nothing to initialize.
virtual Try<Nothing> initialize();
- // This is a noop. The agent recovery process already exposes all files
- // in a recovered executor's sandbox.
- virtual process::Future<Nothing> recover(
- const ExecutorInfo& executorInfo,
- const std::string& sandboxDirectory);
-
// Tells the subprocess to redirect the executor/task's stdout and stderr
// to separate "stdout" and "stderr" files in the sandbox.
// The `path`, `argv`, and `environment` are not changed.
http://git-wip-us.apache.org/repos/asf/mesos/blob/d2e9d1b0/src/slave/containerizer/docker.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/docker.cpp b/src/slave/containerizer/docker.cpp
index 1d27761..d713860 100644
--- a/src/slave/containerizer/docker.cpp
+++ b/src/slave/containerizer/docker.cpp
@@ -885,17 +885,6 @@ Future<Nothing> DockerContainerizerProcess::_recover(
containerId);
container->directory = sandboxDirectory;
-
- // Pass recovered containers to the container logger.
- // NOTE: The current implementation of the container logger only
- // outputs a warning and does not have any other consequences.
- // See `ContainerLogger::recover` for more information.
- logger->recover(executorInfo, sandboxDirectory)
- .onFailed(defer(self(), [executorInfo](const string& message) {
- LOG(WARNING) << "Container logger failed to recover executor '"
- << executorInfo.executor_id() << "': "
- << message;
- }));
}
}
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/d2e9d1b0/src/slave/containerizer/mesos/containerizer.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/containerizer.cpp b/src/slave/containerizer/mesos/containerizer.cpp
index 5c4db56..7ec6f78 100644
--- a/src/slave/containerizer/mesos/containerizer.cpp
+++ b/src/slave/containerizer/mesos/containerizer.cpp
@@ -861,20 +861,6 @@ Future<Nothing> MesosContainerizerProcess::__recover(
isolator->watch(containerId)
.onAny(defer(self(), &Self::limited, containerId, lambda::_1));
}
-
- // TODO(gilbert): Make logger nesting aware.
- if (!containerId.has_parent()) {
- // Pass recovered containers to the container logger.
- // NOTE: The current implementation of the container logger only
- // outputs a warning and does not have any other consequences.
- // See `ContainerLogger::recover` for more information.
- logger->recover(run.executor_info(), run.directory())
- .onFailed(defer(self(), [run](const string& message) {
- LOG(WARNING) << "Container logger failed to recover executor '"
- << run.executor_info().executor_id() << "': "
- << message;
- }));
- }
}
// Maintain the children list in the `Container` struct.
http://git-wip-us.apache.org/repos/asf/mesos/blob/d2e9d1b0/src/tests/container_logger_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/container_logger_tests.cpp b/src/tests/container_logger_tests.cpp
index f761172..1bb94a8 100644
--- a/src/tests/container_logger_tests.cpp
+++ b/src/tests/container_logger_tests.cpp
@@ -108,9 +108,6 @@ public:
EXPECT_CALL(*this, initialize())
.WillRepeatedly(Return(Nothing()));
- EXPECT_CALL(*this, recover(_, _))
- .WillRepeatedly(Return(Nothing()));
-
// All output is redirected to STDOUT_FILENO and STDERR_FILENO.
EXPECT_CALL(*this, prepare(_, _))
.WillRepeatedly(Return(mesos::slave::ContainerLogger::SubprocessInfo()));
@@ -121,10 +118,6 @@ public:
MOCK_METHOD0(initialize, Try<Nothing>(void));
MOCK_METHOD2(
- recover,
- Future<Nothing>(const ExecutorInfo&, const string&));
-
- MOCK_METHOD2(
prepare,
Future<mesos::slave::ContainerLogger::SubprocessInfo>(
const ExecutorInfo&, const string&));
@@ -134,187 +127,6 @@ public:
class ContainerLoggerTest : public MesosTest {};
-// Tests that the Mesos Containerizer will pass recovered containers
-// to the container logger for its own bookkeeping.
-TEST_F(ContainerLoggerTest, MesosContainerizerRecover)
-{
- // Prepare a MesosContainerizer with a mocked container logger.
- slave::Flags flags = CreateSlaveFlags();
- flags.launcher = "posix";
-
- Try<Launcher*> launcher = PosixLauncher::create(flags);
- ASSERT_SOME(launcher);
-
- Fetcher fetcher;
-
- MockContainerLogger* logger = new MockContainerLogger();
-
- Try<Owned<Provisioner>> provisioner = Provisioner::create(flags);
- ASSERT_SOME(provisioner);
-
- // Launch a quick task so that we have a valid PID to put in our
- // mock `SlaveState`. This is necessary as the containerizer will
- // try to reap the PID.
- Try<Subprocess> s = subprocess("exit 0");
- ASSERT_SOME(s);
- AWAIT(s->status());
-
- // Construct a mock `SlaveState`.
- ExecutorID executorId;
- executorId.set_value(UUID::random().toString());
-
- ContainerID containerId;
- containerId.set_value(UUID::random().toString());
-
- ExecutorInfo executorInfo;
- executorInfo.mutable_container()->set_type(ContainerInfo::MESOS);
-
- ExecutorState executorState;
- executorState.id = executorId;
- executorState.info = executorInfo;
- executorState.latest = containerId;
-
- RunState runState;
- runState.id = containerId;
- runState.forkedPid = s->pid();
- executorState.runs.put(containerId, runState);
-
- FrameworkState frameworkState;
- frameworkState.executors.put(executorId, executorState);
-
- SlaveState slaveState;
- FrameworkID frameworkId;
- frameworkId.set_value(UUID::random().toString());
- slaveState.frameworks.put(frameworkId, frameworkState);
-
- const string sandboxDirectory = slave::paths::getExecutorRunPath(
- flags.work_dir,
- slaveState.id,
- frameworkState.id,
- executorId,
- containerId);
-
- // This is the crux of the test. The logger's `recover` method
- // should be called with this specific set of arguments when
- // we call `Containerizer::recover`.
- EXPECT_CALL(*logger, recover(executorInfo, sandboxDirectory))
- .WillOnce(Return(Nothing()));
-
- MesosContainerizer containerizer(
- flags,
- true,
- &fetcher,
- Owned<ContainerLogger>(logger),
- Owned<Launcher>(launcher.get()),
- provisioner->share(),
- vector<Owned<Isolator>>());
-
- // Create the container's sandbox to get past a `CHECK` inside
- // the MesosContainerizer's recovery validation logic.
- ASSERT_SOME(os::mkdir(sandboxDirectory));
-
- Future<Nothing> recover = containerizer.recover(slaveState);
- AWAIT_READY(recover);
-}
-
-
-// Tests that the Docker Containerizer will pass recovered containers
-// to the container logger for its own bookkeeping.
-TEST_F(ContainerLoggerTest, ROOT_DOCKER_ContainerizerRecover)
-{
- // Prepare a MockDockerContainerizer with a mocked container logger.
- MockDocker* mockDocker =
- new MockDocker(tests::flags.docker, tests::flags.docker_socket);
-
- Shared<Docker> docker(mockDocker);
-
- slave::Flags flags = CreateSlaveFlags();
-
- Fetcher fetcher;
-
- MockContainerLogger* logger = new MockContainerLogger();
-
- // Launch a quick task so that we have a valid PID to put in our
- // mock `SlaveState`. This is necessary as the containerizer will
- // try to reap the PID.
- Try<Subprocess> s = subprocess("exit 0");
- ASSERT_SOME(s);
- AWAIT(s->status());
-
- // Construct a mock `SlaveState`.
- ExecutorID executorId;
- executorId.set_value(UUID::random().toString());
-
- ContainerID containerId;
- containerId.set_value(UUID::random().toString());
-
- ExecutorInfo executorInfo;
- executorInfo.mutable_container()->set_type(ContainerInfo::DOCKER);
-
- ExecutorState executorState;
- executorState.id = executorId;
- executorState.info = executorInfo;
- executorState.latest = containerId;
-
- RunState runState;
- runState.id = containerId;
- runState.forkedPid = s->pid();
- executorState.runs.put(containerId, runState);
-
- FrameworkState frameworkState;
- frameworkState.executors.put(executorId, executorState);
-
- SlaveState slaveState;
- FrameworkID frameworkId;
- frameworkId.set_value(UUID::random().toString());
- slaveState.frameworks.put(frameworkId, frameworkState);
-
- const string sandboxDirectory = slave::paths::getExecutorRunPath(
- flags.work_dir,
- slaveState.id,
- frameworkState.id,
- executorId,
- containerId);
-
- // This is the crux of the test. The logger's `recover` method
- // should be called with this specific set of arguments when
- // we call `Containerizer::recover`.
- EXPECT_CALL(*logger, recover(executorInfo, sandboxDirectory))
- .WillOnce(Return(Nothing()));
-
- MockDockerContainerizer containerizer(
- flags,
- &fetcher,
- Owned<ContainerLogger>(logger),
- docker);
-
- // Construct a mock response for `Docker::ps` that only has a meaningful
- // ID field set. The other fields are effectively ignored.
- list<Docker::Container> containers;
- Try<Docker::Container> container = Docker::Container::create(
- "[{"
- " \"Id\": \"" + stringify(containerId) + "\","
- " \"Name\": \"mocked\","
- " \"State\": {"
- " \"Pid\": 0,"
- " \"StartedAt\": \"Totally not a time\""
- " },"
- " \"NetworkSettings\": { \"IPAddress\": \"Totally not an IP\" }"
- "}]");
-
- ASSERT_SOME(container);
- containers.push_back(container.get());
-
- // Intercept the `Docker::ps` call made inside `DockerContainerizer::Recover`.
- // We will return a response, pretending that the test container exists.
- EXPECT_CALL(*mockDocker, ps(_, _))
- .WillOnce(Return(containers));
-
- Future<Nothing> recover = containerizer.recover(slaveState);
- AWAIT_READY(recover);
-}
-
-
// Tests that the default container logger writes files into the sandbox.
TEST_F(ContainerLoggerTest, DefaultToSandbox)
{
[3/3] mesos git commit: Updated upgrades.md due to removing
'recover()' in container logger.
Posted by jo...@apache.org.
Updated upgrades.md due to removing 'recover()' in container logger.
Review: https://reviews.apache.org/r/52764/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/e361aaed
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/e361aaed
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/e361aaed
Branch: refs/heads/master
Commit: e361aaedaa55f757b961c19ed37f78eb2babb8fe
Parents: e4c410d
Author: Gilbert Song <so...@gmail.com>
Authored: Tue Oct 11 18:56:23 2016 -0700
Committer: Joseph Wu <jo...@apache.org>
Committed: Tue Oct 11 19:02:04 2016 -0700
----------------------------------------------------------------------
docs/upgrades.md | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/e361aaed/docs/upgrades.md
----------------------------------------------------------------------
diff --git a/docs/upgrades.md b/docs/upgrades.md
index babadf7..7250259 100644
--- a/docs/upgrades.md
+++ b/docs/upgrades.md
@@ -43,6 +43,24 @@ We categorize the changes as follows:
</thead>
<tr>
<td style="word-wrap: break-word; overflow-wrap: break-word;"><!--Version-->
+ 1.1.x
+ </td>
+ <td style="word-wrap: break-word; overflow-wrap: break-word;"><!--Mesos Core-->
+ </td>
+ <td style="word-wrap: break-word; overflow-wrap: break-word;"><!--Flags-->
+ </td>
+ <td style="word-wrap: break-word; overflow-wrap: break-word;"><!--Framework API-->
+ </td>
+ <td style="word-wrap: break-word; overflow-wrap: break-word;"><!--Module API-->
+ <ul style="padding-left:10px;">
+ <li>R <a href="#1-1-x-container-logger-interface">Container Logger recovery method</a></li>
+ </ul>
+ </td>
+ <td style="word-wrap: break-word; overflow-wrap: break-word;"><!--Endpoints-->
+ </td>
+</tr>
+<tr>
+ <td style="word-wrap: break-word; overflow-wrap: break-word;"><!--Version-->
1.0.x
</td>
<td style="word-wrap: break-word; overflow-wrap: break-word;"><!--Mesos Core-->
@@ -184,6 +202,12 @@ We categorize the changes as follows:
</table>
+## Upgrading from 1.0.x to 1.1.x ##
+
+<a name="1-1-x-container-logger-interface"></a>
+
+* Mesos 1.1 removes the `ContainerLogger`'s `recover()` method. The `ContainerLogger` had an incomplete interface for a stateful implementation. This removes the incomplete parts to avoid adding tech debt in the containerizer. Please see [MESOS-6371](https://issues.apache.org/jira/browse/MESOS-6371) for more information.
+
## Upgrading from 0.28.x to 1.0.x ##
<a name="1-0-x-deprecated-ssl-env-variables"></a>