Repository: mesos Updated Branches: refs/heads/master ae8a51be6 -> 36a26c3ca
Fixed Command Executor when shell is false. Review: https://reviews.apache.org/r/26622 Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/36a26c3c Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/36a26c3c Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/36a26c3c Branch: refs/heads/master Commit: 36a26c3ca7b5a11df0b62540cdd7ad4c58987774 Parents: ae8a51b Author: R.B. Boyer <[email protected]> Authored: Sat Nov 1 00:49:33 2014 -0700 Committer: Timothy Chen <[email protected]> Committed: Sat Nov 1 00:52:45 2014 -0700 ---------------------------------------------------------------------- src/slave/slave.cpp | 28 ++++++++-- src/tests/slave_tests.cpp | 123 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 146 insertions(+), 5 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/36a26c3c/src/slave/slave.cpp ---------------------------------------------------------------------- diff --git a/src/slave/slave.cpp b/src/slave/slave.cpp index 96fb5f7..8aa3524 100644 --- a/src/slave/slave.cpp +++ b/src/slave/slave.cpp @@ -2674,15 +2674,33 @@ ExecutorInfo Slave::getExecutorInfo( executor.set_name("Command Executor " + name); executor.set_source(task.task_id().value()); - // Copy the CommandInfo to get the URIs and environment, but - // update it to invoke 'mesos-executor' (unless we couldn't - // resolve 'mesos-executor' via 'realpath', in which case just - // echo the error and exit). - executor.mutable_command()->MergeFrom(task.command()); + // Copy [uris, environment, container, user] fields from the + // CommandInfo to get the URIs and environment, and use them + // to invoke 'mesos-executor'. + executor.mutable_command()->mutable_uris()->MergeFrom( + task.command().uris()); + + if (task.command().has_environment()) { + executor.mutable_command()->mutable_environment()->MergeFrom( + task.command().environment()); + } + + if (task.command().has_container()) { + executor.mutable_command()->mutable_container()->MergeFrom( + task.command().container()); + } + + if (task.command().has_user()) { + executor.mutable_command()->set_user(task.command().user()); + } Result<string> path = os::realpath( path::join(flags.launcher_dir, "mesos-executor")); + // Strongly enforce a specific shell setting for launching + // command executor. + executor.mutable_command()->set_shell(true); + if (path.isSome()) { executor.mutable_command()->set_value(path.get()); } else { http://git-wip-us.apache.org/repos/asf/mesos/blob/36a26c3c/src/tests/slave_tests.cpp ---------------------------------------------------------------------- diff --git a/src/tests/slave_tests.cpp b/src/tests/slave_tests.cpp index a1bd00c..d2cbaf8 100644 --- a/src/tests/slave_tests.cpp +++ b/src/tests/slave_tests.cpp @@ -393,6 +393,129 @@ TEST_F(SlaveTest, MesosExecutorWithOverride) } +// Test that we don't let task arguments bleed over as +// mesos-executor args. For more details of this see MESOS-1873. +// +// This assumes the ability to execute '/bin/echo --author'. +TEST_F(SlaveTest, MesosExecutorCommandTaskWithArgsList) +{ + Try<PID<Master> > master = StartMaster(); + ASSERT_SOME(master); + + // Need flags for 'executor_registration_timeout'. + slave::Flags flags = CreateSlaveFlags(); + flags.isolation = "posix/cpu,posix/mem"; + + Try<MesosContainerizer*> containerizer = + MesosContainerizer::create(flags, false); + CHECK_SOME(containerizer); + + Try<PID<Slave> > slave = StartSlave(containerizer.get()); + 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()); + + // Launch a task with the command executor. + TaskInfo task; + task.set_name(""); + task.mutable_task_id()->set_value("1"); + task.mutable_slave_id()->MergeFrom(offers.get()[0].slave_id()); + task.mutable_resources()->MergeFrom(offers.get()[0].resources()); + + // Command executor will run as user running test. + CommandInfo command; + command.set_shell(false); + command.set_value("/bin/echo"); + command.add_arguments("/bin/echo"); + command.add_arguments("--author"); + + task.mutable_command()->MergeFrom(command); + + vector<TaskInfo> tasks; + tasks.push_back(task); + + Future<TaskStatus> statusRunning; + Future<TaskStatus> statusFinished; + EXPECT_CALL(sched, statusUpdate(&driver, _)) + .WillOnce(FutureArg<1>(&statusRunning)) + .WillOnce(FutureArg<1>(&statusFinished)); + + driver.launchTasks(offers.get()[0].id(), tasks); + + AWAIT_READY(statusRunning); + EXPECT_EQ(TASK_RUNNING, statusRunning.get().state()); + + AWAIT_READY(statusFinished); + EXPECT_EQ(TASK_FINISHED, statusFinished.get().state()); + + driver.stop(); + driver.join(); + + Shutdown(); // Must shutdown before 'containerizer' gets deallocated. +} + + +// Don't let args from the CommandInfo struct bleed over into +// mesos-executor forking. For more details of this see MESOS-1873. +TEST_F(SlaveTest, GetExecutorInfo) +{ + // Create a thin dummy Slave to access underlying getExecutorInfo(). + // Testing this method should not necessarily require an integration + // test as with most other methods here. + slave::Flags flags = CreateSlaveFlags(); + TestContainerizer containerizer; + StandaloneMasterDetector detector; + Files files; + slave::StatusUpdateManager updateManager(flags); + + slave::GarbageCollector gc; + Slave slave(flags, &detector, &containerizer, &files, &gc, &updateManager); + + FrameworkID frameworkId; + frameworkId.set_value("20141010-221431-251662764-60288-32120-0000"); + + // Launch a task with the command executor. + TaskInfo task; + task.set_name("task"); + task.mutable_task_id()->set_value("1"); + task.mutable_slave_id()->set_value( + "20141010-221431-251662764-60288-32120-0001"); + task.mutable_resources()->MergeFrom( + Resources::parse("cpus:0.1;mem:32").get()); + + CommandInfo command; + command.set_shell(false); + command.set_value("/bin/echo"); + command.add_arguments("/bin/echo"); + command.add_arguments("--author"); + + task.mutable_command()->MergeFrom(command); + + const ExecutorInfo& executor = slave.getExecutorInfo(frameworkId, task); + + // Now assert that it actually is running mesos-executor without any + // bleedover from the command we intend on running. + EXPECT_TRUE(executor.command().shell()); + EXPECT_FALSE(executor.command().has_container()); + EXPECT_EQ(0, executor.command().arguments_size()); + EXPECT_NE(string::npos, executor.command().value().find("mesos-executor")); +} + // This test runs a command without the command user field set. The // command will verify the assumption that the command is run as the // slave user (in this case, root).
