Updated an agent test to use the mock garbage collector. The test `RemoveExecutorUponFailedTaskGroupLaunch` currently depends on catching and modifying the input arguments of the `Slave::__run()` function to mock the garbage collector behavior. Using the mock garbage collector gives better readability and more functionality.
Review: https://reviews.apache.org/r/66120/ Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/aa2b4959 Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/aa2b4959 Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/aa2b4959 Branch: refs/heads/master Commit: aa2b49591c7ef2898c7ed07170783c782ee783d2 Parents: 397cdd5 Author: Meng Zhu <[email protected]> Authored: Thu Apr 5 17:44:00 2018 -0700 Committer: Greg Mann <[email protected]> Committed: Thu Apr 5 17:51:46 2018 -0700 ---------------------------------------------------------------------- src/tests/mesos.cpp | 13 ++++++--- src/tests/mesos.hpp | 3 ++- src/tests/slave_tests.cpp | 61 +++++++++++++++--------------------------- 3 files changed, 34 insertions(+), 43 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/aa2b4959/src/tests/mesos.cpp ---------------------------------------------------------------------- diff --git a/src/tests/mesos.cpp b/src/tests/mesos.cpp index 6d09df2..6a924d7 100644 --- a/src/tests/mesos.cpp +++ b/src/tests/mesos.cpp @@ -430,16 +430,23 @@ Try<Owned<cluster::Slave>> MesosTest::StartSlave( Try<Owned<cluster::Slave>> MesosTest::StartSlave( MasterDetector* detector, slave::GarbageCollector* gc, - const Option<slave::Flags>& flags) + const Option<slave::Flags>& flags, + bool mock) { Try<Owned<cluster::Slave>> slave = cluster::Slave::create( detector, flags.isNone() ? CreateSlaveFlags() : flags.get(), None(), None(), - gc); + gc, + None(), + None(), + None(), + None(), + None(), + mock); - if (slave.isSome()) { + if (slave.isSome() && !mock) { slave.get()->start(); } http://git-wip-us.apache.org/repos/asf/mesos/blob/aa2b4959/src/tests/mesos.hpp ---------------------------------------------------------------------- diff --git a/src/tests/mesos.hpp b/src/tests/mesos.hpp index bddce3d..b3b1817 100644 --- a/src/tests/mesos.hpp +++ b/src/tests/mesos.hpp @@ -204,7 +204,8 @@ protected: virtual Try<process::Owned<cluster::Slave>> StartSlave( mesos::master::detector::MasterDetector* detector, slave::GarbageCollector* gc, - const Option<slave::Flags>& flags = None()); + const Option<slave::Flags>& flags = None(), + bool mock = false); // Starts a slave with the specified detector, resource estimator, and flags. virtual Try<process::Owned<cluster::Slave>> StartSlave( http://git-wip-us.apache.org/repos/asf/mesos/blob/aa2b4959/src/tests/slave_tests.cpp ---------------------------------------------------------------------- diff --git a/src/tests/slave_tests.cpp b/src/tests/slave_tests.cpp index 242cbaf..ae8eeba 100644 --- a/src/tests/slave_tests.cpp +++ b/src/tests/slave_tests.cpp @@ -4752,9 +4752,13 @@ TEST_F(SlaveTest, RemoveExecutorUponFailedTaskGroupLaunch) Owned<MasterDetector> detector = master.get()->createDetector(); + MockGarbageCollector mockGarbageCollector; + + slave::Flags slaveFlags = CreateSlaveFlags(); + // Start a mock slave. Try<Owned<cluster::Slave>> slave = - StartSlave(detector.get(), CreateSlaveFlags(), true); + StartSlave(detector.get(), &mockGarbageCollector, slaveFlags, true); ASSERT_SOME(slave); ASSERT_NE(nullptr, slave.get()->mock()); @@ -4816,50 +4820,29 @@ TEST_F(SlaveTest, RemoveExecutorUponFailedTaskGroupLaunch) v1::Offer::Operation launchGroup = v1::LAUNCH_GROUP(executorInfo, taskGroup); - // Saved arguments from `Slave::_run()`. - Future<list<bool>> _unschedules; - FrameworkInfo _frameworkInfo; - ExecutorInfo _executorInfo; - Option<TaskGroupInfo> _taskGroup; - Option<TaskInfo> _task; - vector<ResourceVersionUUID> _resourceVersionUuids; - Option<bool> _launchExecutor; - - // Capture `_run` arguments. - Future<Nothing> _run; - EXPECT_CALL(*slave.get()->mock(), _run(_, _, _, _, _, _, _)) - .WillOnce(DoAll(FutureSatisfy(&_run), - SaveArg<0>(&_unschedules), - SaveArg<1>(&_frameworkInfo), - SaveArg<2>(&_executorInfo), - SaveArg<3>(&_task), - SaveArg<4>(&_taskGroup), - SaveArg<5>(&_resourceVersionUuids), - SaveArg<6>(&_launchExecutor))); + // The `unschedule()` function is used to prevent premature garbage + // collection when the executor directory already exists due to a + // previously-launched task. Simulate this scenario by creating the + // executor directory manually. + string path = paths::getExecutorPath( + slaveFlags.work_dir, + devolve(agentId), + devolve(frameworkId), + devolve(executorInfo.executor_id())); + + Try<Nothing> mkdir = os::mkdir(path, true); + CHECK_SOME(mkdir); + + // Induce agent unschedule GC failure. This will result in + // task launch failure before the executor launch. + EXPECT_CALL(mockGarbageCollector, unschedule(_)) + .WillRepeatedly(Return(Failure(""))); Future<ExitedExecutorMessage> exitedExecutorMessage = FUTURE_PROTOBUF(ExitedExecutorMessage(), _, _); mesos.send(v1::createCallAccept(frameworkId, offer, {launchGroup})); - AWAIT_READY(_run); - - // Induce a failed GC unschedule. - Promise<list<bool>> promise; - Future<list<bool>> failedFuture = promise.future(); - promise.fail(""); - - process::dispatch(slave.get()->pid, [&] { - slave.get()->mock()->unmocked__run( - failedFuture, - _frameworkInfo, - _executorInfo, - _task, - _taskGroup, - _resourceVersionUuids, - _launchExecutor); - }); - AWAIT_READY(exitedExecutorMessage); // Helper function to post a request to '/api/v1' master endpoint
