Added tests for basic DiskInfo checker. Review: https://reviews.apache.org/r/28627
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/3f0f275a Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/3f0f275a Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/3f0f275a Branch: refs/heads/master Commit: 3f0f275ad1f0ac52c0927740b33eb4c52e2b964a Parents: 34bb637 Author: Jie Yu <[email protected]> Authored: Tue Dec 2 19:02:14 2014 -0800 Committer: Jie Yu <[email protected]> Committed: Wed Dec 3 16:12:07 2014 -0800 ---------------------------------------------------------------------- src/common/protobuf_utils.cpp | 7 +- src/common/protobuf_utils.hpp | 3 + src/tests/mesos.hpp | 31 +++ src/tests/resource_offers_tests.cpp | 377 +++++++++++++++++++++++++++++++ 4 files changed, 415 insertions(+), 3 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/3f0f275a/src/common/protobuf_utils.cpp ---------------------------------------------------------------------- diff --git a/src/common/protobuf_utils.cpp b/src/common/protobuf_utils.cpp index baf04a6..8ab5cdd 100644 --- a/src/common/protobuf_utils.cpp +++ b/src/common/protobuf_utils.cpp @@ -27,6 +27,8 @@ #include "messages/messages.hpp" +using std::string; + namespace mesos { namespace internal { namespace protobuf { @@ -48,7 +50,7 @@ StatusUpdate createStatusUpdate( const TaskID& taskId, const TaskState& state, const TaskStatus::Source& source, - const std::string& message = "", + const string& message = "", const Option<TaskStatus::Reason>& reason = None(), const Option<ExecutorID>& executorId = None()) { @@ -117,7 +119,7 @@ MasterInfo createMasterInfo(const process::UPID& pid) info.set_port(pid.node.port); info.set_pid(pid); - Try<std::string> hostname = net::getHostname(pid.node.ip); + Try<string> hostname = net::getHostname(pid.node.ip); if (hostname.isSome()) { info.set_hostname(hostname.get()); } @@ -125,7 +127,6 @@ MasterInfo createMasterInfo(const process::UPID& pid) return info; } - } // namespace protobuf { } // namespace internal { } // namespace mesos { http://git-wip-us.apache.org/repos/asf/mesos/blob/3f0f275a/src/common/protobuf_utils.hpp ---------------------------------------------------------------------- diff --git a/src/common/protobuf_utils.hpp b/src/common/protobuf_utils.hpp index bc3ef2a..e42aaa5 100644 --- a/src/common/protobuf_utils.hpp +++ b/src/common/protobuf_utils.hpp @@ -36,6 +36,7 @@ namespace protobuf { bool isTerminalState(const TaskState& state); + StatusUpdate createStatusUpdate( const FrameworkID& frameworkId, const Option<SlaveID>& slaveId, @@ -46,11 +47,13 @@ StatusUpdate createStatusUpdate( const Option<TaskStatus::Reason>& reason = None(), const Option<ExecutorID>& executorId = None()); + Task createTask( const TaskInfo& task, const TaskState& state, const FrameworkID& frameworkId); + // Helper function that creates a MasterInfo from UPID. MasterInfo createMasterInfo(const process::UPID& pid); http://git-wip-us.apache.org/repos/asf/mesos/blob/3f0f275a/src/tests/mesos.hpp ---------------------------------------------------------------------- diff --git a/src/tests/mesos.hpp b/src/tests/mesos.hpp index aa10343..90c575e 100644 --- a/src/tests/mesos.hpp +++ b/src/tests/mesos.hpp @@ -359,6 +359,37 @@ inline TaskInfo createTask( } +// NOTE: We only set the volume in DiskInfo if 'containerPath' is set. +// If volume mode is not specified, Volume::RW will be used (assuming +// 'containerPath' is set). +inline Resource::DiskInfo createDiskInfo( + const Option<std::string>& persistenceId, + const Option<std::string>& containerPath, + const Option<Volume::Mode>& mode = None(), + const Option<std::string>& hostPath = None()) +{ + Resource::DiskInfo info; + + if (persistenceId.isSome()) { + info.mutable_persistence()->set_id(persistenceId.get()); + } + + if (containerPath.isSome()) { + Volume volume; + volume.set_container_path(containerPath.get()); + volume.set_mode(mode.isSome() ? mode.get() : Volume::RW); + + if (hostPath.isSome()) { + volume.set_host_path(hostPath.get()); + } + + info.mutable_volume()->CopyFrom(volume); + } + + return info; +} + + // Definition of a mock Scheduler to be used in tests with gmock. class MockScheduler : public Scheduler { http://git-wip-us.apache.org/repos/asf/mesos/blob/3f0f275a/src/tests/resource_offers_tests.cpp ---------------------------------------------------------------------- diff --git a/src/tests/resource_offers_tests.cpp b/src/tests/resource_offers_tests.cpp index c4afc38..467c7e5 100644 --- a/src/tests/resource_offers_tests.cpp +++ b/src/tests/resource_offers_tests.cpp @@ -53,6 +53,11 @@ using testing::AtMost; using testing::Return; +// TODO(jieyu): All of the task validation tests have the same flow: +// launch a task, expect an update of a particular format (invalid w/ +// message). Consider providing common functionalities in the test +// fixture to avoid code bloat. Ultimately, we should make task or +// offer validation unit testable. class TaskValidationTest : public MesosTest {}; @@ -549,6 +554,378 @@ TEST_F(TaskValidationTest, ExecutorInfoDiffersOnDifferentSlaves) } +TEST_F(TaskValidationTest, UnreservedDiskInfo) +{ + Try<PID<Master>> master = StartMaster(); + ASSERT_SOME(master); + + Try<PID<Slave>> slave = StartSlave(); + ASSERT_SOME(slave); + + MockScheduler sched; + MesosSchedulerDriver driver( + &sched, DEFAULT_FRAMEWORK_INFO, master.get(), 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.get().size()); + + // Create a persistent disk resource with "*" role. + Resource diskResource = Resources::parse("disk", "128", "*").get(); + diskResource.mutable_disk()->CopyFrom(createDiskInfo("1", "1")); + + // Include persistent disk resource in task resources. + Resources taskResources = + Resources::parse("cpus:1;mem:128").get() + diskResource; + + Offer offer = offers.get()[0]; + TaskInfo task = + createTask(offer.slave_id(), taskResources, "", DEFAULT_EXECUTOR_ID); + + vector<TaskInfo> tasks; + tasks.push_back(task); + + Future<TaskStatus> status; + EXPECT_CALL(sched, statusUpdate(&driver, _)) + .WillOnce(FutureArg<1>(&status)); + + driver.launchTasks(offer.id(), tasks); + + AWAIT_READY(status); + EXPECT_EQ(task.task_id(), status.get().task_id()); + EXPECT_EQ(TASK_ERROR, status.get().state()); + EXPECT_EQ(TaskStatus::REASON_TASK_INVALID, status.get().reason()); + EXPECT_TRUE(status.get().has_message()); + EXPECT_TRUE(strings::contains( + status.get().message(), + "Persistent disk volume is disallowed for '*' role")); + + driver.stop(); + driver.join(); + + Shutdown(); +} + + +TEST_F(TaskValidationTest, InvalidPersistenceID) +{ + Try<PID<Master>> master = StartMaster(); + ASSERT_SOME(master); + + Try<PID<Slave>> slave = StartSlave(); + ASSERT_SOME(slave); + + MockScheduler sched; + MesosSchedulerDriver driver( + &sched, DEFAULT_FRAMEWORK_INFO, master.get(), 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.get().size()); + + // Create a persistent disk resource with an invalid persistence id. + Resource diskResource = Resources::parse("disk", "128", "role1").get(); + diskResource.mutable_disk()->CopyFrom(createDiskInfo("1/", "1")); + + // Include persistent disk resource in task resources. + Resources taskResources = + Resources::parse("cpus:1;mem:128").get() + diskResource; + + Offer offer = offers.get()[0]; + TaskInfo task = + createTask(offer.slave_id(), taskResources, "", DEFAULT_EXECUTOR_ID); + + vector<TaskInfo> tasks; + tasks.push_back(task); + + Future<TaskStatus> status; + EXPECT_CALL(sched, statusUpdate(&driver, _)) + .WillOnce(FutureArg<1>(&status)); + + driver.launchTasks(offer.id(), tasks); + + AWAIT_READY(status); + EXPECT_EQ(task.task_id(), status.get().task_id()); + EXPECT_EQ(TASK_ERROR, status.get().state()); + EXPECT_EQ(TaskStatus::REASON_TASK_INVALID, status.get().reason()); + EXPECT_TRUE(status.get().has_message()); + EXPECT_TRUE(strings::contains( + status.get().message(), + "Persistence ID '1/' contains invalid characters")); + + driver.stop(); + driver.join(); + + Shutdown(); +} + + +TEST_F(TaskValidationTest, PersistentDiskInfoWithoutVolume) +{ + Try<PID<Master>> master = StartMaster(); + ASSERT_SOME(master); + + Try<PID<Slave>> slave = StartSlave(); + ASSERT_SOME(slave); + + MockScheduler sched; + MesosSchedulerDriver driver( + &sched, DEFAULT_FRAMEWORK_INFO, master.get(), 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.get().size()); + + // Create a persistent disk resource without a volume. + Resource diskResource = Resources::parse("disk", "128", "role1").get(); + diskResource.mutable_disk()->CopyFrom(createDiskInfo("1", None())); + + // Include persistent disk resource in task resources. + Resources taskResources = + Resources::parse("cpus:1;mem:128").get() + diskResource; + + Offer offer = offers.get()[0]; + TaskInfo task = + createTask(offer.slave_id(), taskResources, "", DEFAULT_EXECUTOR_ID); + + vector<TaskInfo> tasks; + tasks.push_back(task); + + Future<TaskStatus> status; + EXPECT_CALL(sched, statusUpdate(&driver, _)) + .WillOnce(FutureArg<1>(&status)); + + driver.launchTasks(offer.id(), tasks); + + AWAIT_READY(status); + EXPECT_EQ(task.task_id(), status.get().task_id()); + EXPECT_EQ(TASK_ERROR, status.get().state()); + EXPECT_EQ(TaskStatus::REASON_TASK_INVALID, status.get().reason()); + EXPECT_TRUE(status.get().has_message()); + EXPECT_TRUE(strings::contains( + status.get().message(), + "Persistent disk should specify a volume")); + + driver.stop(); + driver.join(); + + Shutdown(); +} + + +TEST_F(TaskValidationTest, PersistentDiskInfoWithReadOnlyVolume) +{ + Try<PID<Master>> master = StartMaster(); + ASSERT_SOME(master); + + Try<PID<Slave>> slave = StartSlave(); + ASSERT_SOME(slave); + + MockScheduler sched; + MesosSchedulerDriver driver( + &sched, DEFAULT_FRAMEWORK_INFO, master.get(), 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.get().size()); + + // Create a persistent disk resource with read-only volume. + Resource diskResource = Resources::parse("disk", "128", "role1").get(); + diskResource.mutable_disk()->CopyFrom(createDiskInfo("1", "1", Volume::RO)); + + // Include persistent disk resource in task resources. + Resources taskResources = + Resources::parse("cpus:1;mem:128").get() + diskResource; + + Offer offer = offers.get()[0]; + TaskInfo task = + createTask(offer.slave_id(), taskResources, "", DEFAULT_EXECUTOR_ID); + + vector<TaskInfo> tasks; + tasks.push_back(task); + + Future<TaskStatus> status; + EXPECT_CALL(sched, statusUpdate(&driver, _)) + .WillOnce(FutureArg<1>(&status)); + + driver.launchTasks(offer.id(), tasks); + + AWAIT_READY(status); + EXPECT_EQ(task.task_id(), status.get().task_id()); + EXPECT_EQ(TASK_ERROR, status.get().state()); + EXPECT_EQ(TaskStatus::REASON_TASK_INVALID, status.get().reason()); + EXPECT_TRUE(status.get().has_message()); + EXPECT_TRUE(strings::contains( + status.get().message(), + "Read-only volume is not supported for DiskInfo")); + + driver.stop(); + driver.join(); + + Shutdown(); +} + + +TEST_F(TaskValidationTest, PersistentDiskInfoWithHostPath) +{ + Try<PID<Master>> master = StartMaster(); + ASSERT_SOME(master); + + Try<PID<Slave>> slave = StartSlave(); + ASSERT_SOME(slave); + + MockScheduler sched; + MesosSchedulerDriver driver( + &sched, DEFAULT_FRAMEWORK_INFO, master.get(), 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.get().size()); + + // Create a persistent disk resource with host path in the volume. + Resource diskResource = Resources::parse("disk", "128", "role1").get(); + diskResource.mutable_disk()->CopyFrom( + createDiskInfo("1", "1", Volume::RW, "foo")); + + // Include persistent disk resource in task resources. + Resources taskResources = + Resources::parse("cpus:1;mem:128").get() + diskResource; + + Offer offer = offers.get()[0]; + TaskInfo task = + createTask(offer.slave_id(), taskResources, "", DEFAULT_EXECUTOR_ID); + + vector<TaskInfo> tasks; + tasks.push_back(task); + + Future<TaskStatus> status; + EXPECT_CALL(sched, statusUpdate(&driver, _)) + .WillOnce(FutureArg<1>(&status)); + + driver.launchTasks(offer.id(), tasks); + + AWAIT_READY(status); + EXPECT_EQ(task.task_id(), status.get().task_id()); + EXPECT_EQ(TASK_ERROR, status.get().state()); + EXPECT_EQ(TaskStatus::REASON_TASK_INVALID, status.get().reason()); + EXPECT_TRUE(status.get().has_message()); + EXPECT_TRUE(strings::contains( + status.get().message(), + "Volume in DiskInfo should not have 'host_path' set")); + + driver.stop(); + driver.join(); + + Shutdown(); +} + + +TEST_F(TaskValidationTest, NonPersistentDiskInfoWithVolume) +{ + Try<PID<Master>> master = StartMaster(); + ASSERT_SOME(master); + + Try<PID<Slave>> slave = StartSlave(); + ASSERT_SOME(slave); + + MockScheduler sched; + MesosSchedulerDriver driver( + &sched, DEFAULT_FRAMEWORK_INFO, master.get(), 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.get().size()); + + // Create a non-persistent disk resource with volume. + Resource diskResource = Resources::parse("disk", "128", "role1").get(); + diskResource.mutable_disk()->CopyFrom(createDiskInfo(None(), "1")); + + // Include non-persistent disk resource in task resources. + Resources taskResources = + Resources::parse("cpus:1;mem:128").get() + diskResource; + + Offer offer = offers.get()[0]; + TaskInfo task = + createTask(offer.slave_id(), taskResources, "", DEFAULT_EXECUTOR_ID); + + vector<TaskInfo> tasks; + tasks.push_back(task); + + Future<TaskStatus> status; + EXPECT_CALL(sched, statusUpdate(&driver, _)) + .WillOnce(FutureArg<1>(&status)); + + driver.launchTasks(offer.id(), tasks); + + AWAIT_READY(status); + EXPECT_EQ(task.task_id(), status.get().task_id()); + EXPECT_EQ(TASK_ERROR, status.get().state()); + EXPECT_EQ(TaskStatus::REASON_TASK_INVALID, status.get().reason()); + EXPECT_TRUE(status.get().has_message()); + EXPECT_TRUE(strings::contains( + status.get().message(), + "Non-persistent disk volume is not supported")); + + driver.stop(); + driver.join(); + + Shutdown(); +} + // TODO(benh): Add tests for checking correct slave IDs. // TODO(benh): Add tests for checking executor resource usage.
