Added validation for task's kill policy. Review: https://reviews.apache.org/r/44707/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/1fe6221a Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/1fe6221a Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/1fe6221a Branch: refs/heads/master Commit: 1fe6221aa30f35f31378433412d8cb725009bd47 Parents: 7ab6a47 Author: Alexander Rukletsov <ruklet...@gmail.com> Authored: Thu Mar 24 17:30:42 2016 +0100 Committer: Alexander Rukletsov <al...@apache.org> Committed: Thu Mar 24 18:21:03 2016 +0100 ---------------------------------------------------------------------- src/master/validation.cpp | 15 ++++++++ src/master/validation.hpp | 3 ++ src/tests/master_validation_tests.cpp | 56 ++++++++++++++++++++++++++++++ 3 files changed, 74 insertions(+) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/1fe6221a/src/master/validation.cpp ---------------------------------------------------------------------- diff --git a/src/master/validation.cpp b/src/master/validation.cpp index d4fe568..701a5c4 100644 --- a/src/master/validation.cpp +++ b/src/master/validation.cpp @@ -487,6 +487,20 @@ Option<Error> validateResources(const TaskInfo& task) return None(); } + +Option<Error> validateKillPolicy(const TaskInfo& task) +{ + if (task.has_kill_policy() && + task.kill_policy().has_grace_period() && + Nanoseconds(task.kill_policy().grace_period().nanoseconds()) < + Duration::zero()) { + return Error("Task's 'kill_policy.grace_period' must be non-negative"); + } + + return None(); +} + + } // namespace internal { @@ -509,6 +523,7 @@ Option<Error> validate( lambda::bind(internal::validateSlaveID, task, slave), lambda::bind(internal::validateExecutorInfo, task, framework, slave), lambda::bind(internal::validateResources, task), + lambda::bind(internal::validateKillPolicy, task), lambda::bind( internal::validateResourceUsage, task, framework, slave, offered) }; http://git-wip-us.apache.org/repos/asf/mesos/blob/1fe6221a/src/master/validation.hpp ---------------------------------------------------------------------- diff --git a/src/master/validation.hpp b/src/master/validation.hpp index 29dbdf1..d1f2323 100644 --- a/src/master/validation.hpp +++ b/src/master/validation.hpp @@ -78,6 +78,9 @@ namespace internal { // Validates resources of the task and executor (if present). Option<Error> validateResources(const TaskInfo& task); +// Validates the kill policy of the task. +Option<Error> validateKillPolicy(const TaskInfo& task); + } // namespace internal { } // namespace task { http://git-wip-us.apache.org/repos/asf/mesos/blob/1fe6221a/src/tests/master_validation_tests.cpp ---------------------------------------------------------------------- diff --git a/src/tests/master_validation_tests.cpp b/src/tests/master_validation_tests.cpp index d531fc1..010479c 100644 --- a/src/tests/master_validation_tests.cpp +++ b/src/tests/master_validation_tests.cpp @@ -1224,6 +1224,62 @@ TEST_F(TaskValidationTest, ExecutorShutdownGracePeriodIsNonNegative) } +// Ensures that negative grace period in `KillPolicy` +// is rejected during `TaskInfo` validation. +TEST_F(TaskValidationTest, KillPolicyGracePeriodIsNonNegative) +{ + Try<Owned<cluster::Master>> master = StartMaster(); + ASSERT_SOME(master); + + Owned<MasterDetector> detector = master.get()->createDetector(); + Try<Owned<cluster::Slave>> slave = StartSlave(detector.get()); + ASSERT_SOME(slave); + + MockScheduler sched; + MesosSchedulerDriver driver( + &sched, DEFAULT_FRAMEWORK_INFO, master.get()->pid, DEFAULT_CREDENTIAL); + + EXPECT_CALL(sched, registered(&driver, _, _)) + .Times(1); + + Future<vector<Offer>> offers; + EXPECT_CALL(sched, resourceOffers(&driver, _)) + .WillOnce(FutureArg<1>(&offers)) + .WillRepeatedly(Return()); // Ignore subsequent offers. + + driver.start(); + + AWAIT_READY(offers); + EXPECT_NE(0u, offers->size()); + Offer offer = offers.get()[0]; + + TaskInfo task; + task.set_name(""); + task.mutable_task_id()->set_value("1"); + task.mutable_slave_id()->MergeFrom(offer.slave_id()); + task.mutable_resources()->MergeFrom(offer.resources()); + task.mutable_executor()->MergeFrom(DEFAULT_EXECUTOR_INFO); + task.mutable_kill_policy()->mutable_grace_period()->set_nanoseconds( + Seconds(-1).ns()); + + Future<TaskStatus> status; + EXPECT_CALL(sched, statusUpdate(&driver, _)) + .WillOnce(FutureArg<1>(&status)); + + driver.launchTasks(offer.id(), {task}); + + AWAIT_READY(status); + EXPECT_EQ(task.task_id(), status->task_id()); + EXPECT_EQ(TASK_ERROR, status->state()); + EXPECT_EQ(TaskStatus::REASON_TASK_INVALID, status->reason()); + EXPECT_TRUE(status->has_message()); + EXPECT_EQ("Task's 'kill_policy.grace_period' must be non-negative", + status->message()); + + driver.stop(); + driver.join(); +} + // TODO(jieyu): Add tests for checking duplicated persistence ID // against offered resources.
