Allow override without CommandInfo::value to run in command executor. Review: https://reviews.apache.org/r/24730
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/9ce30e35 Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/9ce30e35 Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/9ce30e35 Branch: refs/heads/master Commit: 9ce30e350b2e94c1047ccec64eb06b17a2e6537b Parents: 0a57d25 Author: Timothy Chen <[email protected]> Authored: Fri Aug 15 15:12:48 2014 -0700 Committer: Benjamin Hindman <[email protected]> Committed: Fri Aug 15 18:22:02 2014 -0700 ---------------------------------------------------------------------- src/launcher/executor.cpp | 61 ++--- src/tests/docker_containerizer_tests.cpp | 335 ++++++++++++++++++++++++++ 2 files changed, 367 insertions(+), 29 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/9ce30e35/src/launcher/executor.cpp ---------------------------------------------------------------------- diff --git a/src/launcher/executor.cpp b/src/launcher/executor.cpp index 1aa2c99..12ac14b 100644 --- a/src/launcher/executor.cpp +++ b/src/launcher/executor.cpp @@ -118,23 +118,27 @@ public: return; } - // Sanity checks. - CHECK(task.has_command()) << "Expecting task " << task.task_id() - << " to have a command!"; - - // TODO(jieyu): For now, we just fail the executor if the task's - // CommandInfo is not valid. The framework will receive - // TASK_FAILED for the task, and will most likely find out the - // cause with some debugging. This is a temporary solution. A more - // correct solution is to perform this validation at master side. - if (task.command().shell()) { - CHECK(task.command().has_value()) - << "Shell command of task " << task.task_id() - << " is not specified!"; - } else { - CHECK(task.command().has_value()) - << "Executable of task " << task.task_id() - << " is not specified!"; + // Skip sanity checks for TaskInfo if override is provided since + // the executor will be running the override command. + if (override.isNone()) { + // Sanity checks. + CHECK(task.has_command()) << "Expecting task " << task.task_id() + << " to have a command!"; + + // TODO(jieyu): For now, we just fail the executor if the task's + // CommandInfo is not valid. The framework will receive + // TASK_FAILED for the task, and will most likely find out the + // cause with some debugging. This is a temporary solution. A more + // correct solution is to perform this validation at master side. + if (task.command().shell()) { + CHECK(task.command().has_value()) + << "Shell command of task " << task.task_id() + << " is not specified!"; + } else { + CHECK(task.command().has_value()) + << "Executable of task " << task.task_id() + << " is not specified!"; + } } cout << "Starting task " << task.task_id() << endl; @@ -169,9 +173,16 @@ public: } argv[task.command().arguments().size()] = NULL; - // Prepare the messages before fork as it's not async signal safe. + // Prepare the command log message. string command; - if (task.command().shell()) { + if (override.isSome()) { + char** argv = override.get(); + // argv is guaranteed to be NULL terminated and we rely on + // that fact to print command to be executed. + for (int i = 0; argv[i] != NULL; i++) { + command += string(argv[i]) + " "; + } + } else if (task.command().shell()) { command = "sh -c '" + task.command().value() + "'"; } else { command = @@ -217,10 +228,10 @@ public: os::close(pipes[1]); + cout << command << endl; + // The child has successfully setsid, now run the command. if (override.isNone()) { - cout << command << endl; - if (task.command().shell()) { execl( "/bin/sh", @@ -233,14 +244,6 @@ public: } } else { char** argv = override.get(); - - // argv is guaranteed to be NULL terminated and we rely on - // that fact to print command to be executed. - for (int i = 0; argv[i] != NULL; i++) { - cout << argv[i] << " "; - } - cout << endl; - execvp(argv[0], argv); } http://git-wip-us.apache.org/repos/asf/mesos/blob/9ce30e35/src/tests/docker_containerizer_tests.cpp ---------------------------------------------------------------------- diff --git a/src/tests/docker_containerizer_tests.cpp b/src/tests/docker_containerizer_tests.cpp index 60d9b2d..0d7c3b1 100644 --- a/src/tests/docker_containerizer_tests.cpp +++ b/src/tests/docker_containerizer_tests.cpp @@ -947,3 +947,338 @@ TEST_F(DockerContainerizerTest, ROOT_DOCKER_Logs) Shutdown(); } + + +// The following test uses a Docker image (mesosphere/inky) that has +// an entrypoint "echo" and a default command "inky". +TEST_F(DockerContainerizerTest, ROOT_DOCKER_Default_CMD) +{ + Try<PID<Master> > master = StartMaster(); + ASSERT_SOME(master); + + slave::Flags flags = CreateSlaveFlags(); + + Docker docker = Docker::create(tests::flags.docker, false).get(); + + MockDockerContainerizer dockerContainerizer(flags, docker); + + Try<PID<Slave> > slave = StartSlave(&dockerContainerizer); + ASSERT_SOME(slave); + + MockScheduler sched; + MesosSchedulerDriver driver( + &sched, DEFAULT_FRAMEWORK_INFO, master.get(), DEFAULT_CREDENTIAL); + + Future<FrameworkID> frameworkId; + EXPECT_CALL(sched, registered(&driver, _, _)) + .WillOnce(FutureArg<1>(&frameworkId)); + + Future<vector<Offer> > offers; + EXPECT_CALL(sched, resourceOffers(&driver, _)) + .WillOnce(FutureArg<1>(&offers)) + .WillRepeatedly(Return()); // Ignore subsequent offers. + + driver.start(); + + AWAIT_READY(frameworkId); + + AWAIT_READY(offers); + EXPECT_NE(0u, offers.get().size()); + + const Offer& offer = offers.get()[0]; + + TaskInfo task; + task.set_name(""); + task.mutable_task_id()->set_value("1"); + task.mutable_slave_id()->CopyFrom(offer.slave_id()); + task.mutable_resources()->CopyFrom(offer.resources()); + + CommandInfo command; + command.set_shell(false); + + // NOTE: By not setting CommandInfo::value we're testing that we + // will still be able to run the container because it has a default + // entrypoint! + + ContainerInfo containerInfo; + containerInfo.set_type(ContainerInfo::DOCKER); + + ContainerInfo::DockerInfo dockerInfo; + dockerInfo.set_image("mesosphere/inky"); + containerInfo.mutable_docker()->CopyFrom(dockerInfo); + + task.mutable_command()->CopyFrom(command); + task.mutable_container()->CopyFrom(containerInfo); + + vector<TaskInfo> tasks; + tasks.push_back(task); + + Future<ContainerID> containerId; + Future<string> directory; + EXPECT_CALL(dockerContainerizer, launch(_, _, _, _, _, _, _, _)) + .WillOnce(DoAll(FutureArg<0>(&containerId), + FutureArg<3>(&directory), + Invoke(&dockerContainerizer, + &MockDockerContainerizer::_launch))); + + Future<TaskStatus> statusRunning; + Future<TaskStatus> statusFinished; + EXPECT_CALL(sched, statusUpdate(&driver, _)) + .WillOnce(FutureArg<1>(&statusRunning)) + .WillOnce(FutureArg<1>(&statusFinished)) + .WillRepeatedly(DoDefault()); + + driver.launchTasks(offers.get()[0].id(), tasks); + + AWAIT_READY_FOR(containerId, Seconds(60)); + AWAIT_READY(directory); + AWAIT_READY_FOR(statusRunning, Seconds(60)); + EXPECT_EQ(TASK_RUNNING, statusRunning.get().state()); + AWAIT_READY_FOR(statusFinished, Seconds(60)); + EXPECT_EQ(TASK_FINISHED, statusFinished.get().state()); + + Try<string> read = os::read(path::join(directory.get(), "stdout")); + + ASSERT_SOME(read); + + // Since we're not passing any command value, we're expecting the + // default entry point to be run which is 'echo' with the default + // command from the image which is 'inky'. + EXPECT_TRUE(strings::contains(read.get(), "inky")); + + read = os::read(path::join(directory.get(), "stderr")); + ASSERT_SOME(read); + EXPECT_FALSE(strings::contains(read.get(), "inky")); + + driver.stop(); + driver.join(); + + Shutdown(); +} + + +// The following test uses a docker image (mesosphere/inky) that has +// an entrypoint "echo" and a default command "inky". +TEST_F(DockerContainerizerTest, ROOT_DOCKER_Default_CMD_Override) +{ + Try<PID<Master> > master = StartMaster(); + ASSERT_SOME(master); + + slave::Flags flags = CreateSlaveFlags(); + + Docker docker = Docker::create(tests::flags.docker, false).get(); + + MockDockerContainerizer dockerContainerizer(flags, docker); + + Try<PID<Slave> > slave = StartSlave(&dockerContainerizer); + ASSERT_SOME(slave); + + MockScheduler sched; + MesosSchedulerDriver driver( + &sched, DEFAULT_FRAMEWORK_INFO, master.get(), DEFAULT_CREDENTIAL); + + Future<FrameworkID> frameworkId; + EXPECT_CALL(sched, registered(&driver, _, _)) + .WillOnce(FutureArg<1>(&frameworkId)); + + Future<vector<Offer> > offers; + EXPECT_CALL(sched, resourceOffers(&driver, _)) + .WillOnce(FutureArg<1>(&offers)) + .WillRepeatedly(Return()); // Ignore subsequent offers. + + driver.start(); + + AWAIT_READY(frameworkId); + + AWAIT_READY(offers); + EXPECT_NE(0u, offers.get().size()); + + const Offer& offer = offers.get()[0]; + + TaskInfo task; + task.set_name(""); + task.mutable_task_id()->set_value("1"); + task.mutable_slave_id()->CopyFrom(offer.slave_id()); + task.mutable_resources()->CopyFrom(offer.resources()); + + string uuid = UUID::random().toString(); + + CommandInfo command; + command.set_shell(false); + + // We can set the value to just the 'uuid' since it should get + // passed as an argument to the entrypoint, i.e., 'echo uuid'. + command.set_value(uuid); + + ContainerInfo containerInfo; + containerInfo.set_type(ContainerInfo::DOCKER); + + ContainerInfo::DockerInfo dockerInfo; + dockerInfo.set_image("mesosphere/inky"); + containerInfo.mutable_docker()->CopyFrom(dockerInfo); + + task.mutable_command()->CopyFrom(command); + task.mutable_container()->CopyFrom(containerInfo); + + vector<TaskInfo> tasks; + tasks.push_back(task); + + Future<ContainerID> containerId; + Future<string> directory; + EXPECT_CALL(dockerContainerizer, launch(_, _, _, _, _, _, _, _)) + .WillOnce(DoAll(FutureArg<0>(&containerId), + FutureArg<3>(&directory), + Invoke(&dockerContainerizer, + &MockDockerContainerizer::_launch))); + + Future<TaskStatus> statusRunning; + Future<TaskStatus> statusFinished; + EXPECT_CALL(sched, statusUpdate(&driver, _)) + .WillOnce(FutureArg<1>(&statusRunning)) + .WillOnce(FutureArg<1>(&statusFinished)) + .WillRepeatedly(DoDefault()); + + driver.launchTasks(offers.get()[0].id(), tasks); + + AWAIT_READY_FOR(containerId, Seconds(60)); + AWAIT_READY(directory); + AWAIT_READY_FOR(statusRunning, Seconds(60)); + EXPECT_EQ(TASK_RUNNING, statusRunning.get().state()); + AWAIT_READY_FOR(statusFinished, Seconds(60)); + EXPECT_EQ(TASK_FINISHED, statusFinished.get().state()); + + // Now check that the proper output is in stderr and stdout. + Try<string> read = os::read(path::join(directory.get(), "stdout")); + + ASSERT_SOME(read); + + // We expect the passed in command value to override the image's + // default command, thus we should see the value of 'uuid' in the + // output instead of the default command which is 'inky'. + EXPECT_TRUE(strings::contains(read.get(), uuid)); + EXPECT_FALSE(strings::contains(read.get(), "inky")); + + read = os::read(path::join(directory.get(), "stderr")); + ASSERT_SOME(read); + EXPECT_FALSE(strings::contains(read.get(), "inky")); + EXPECT_FALSE(strings::contains(read.get(), uuid)); + + driver.stop(); + driver.join(); + + Shutdown(); +} + + +// The following test uses a docker image (mesosphere/inky) that has +// an entrypoint "echo" and a default command "inky". +TEST_F(DockerContainerizerTest, ROOT_DOCKER_Default_CMD_Args) +{ + Try<PID<Master> > master = StartMaster(); + ASSERT_SOME(master); + + slave::Flags flags = CreateSlaveFlags(); + + Docker docker = Docker::create(tests::flags.docker, false).get(); + + MockDockerContainerizer dockerContainerizer(flags, docker); + + Try<PID<Slave> > slave = StartSlave(&dockerContainerizer); + ASSERT_SOME(slave); + + MockScheduler sched; + MesosSchedulerDriver driver( + &sched, DEFAULT_FRAMEWORK_INFO, master.get(), DEFAULT_CREDENTIAL); + + Future<FrameworkID> frameworkId; + EXPECT_CALL(sched, registered(&driver, _, _)) + .WillOnce(FutureArg<1>(&frameworkId)); + + Future<vector<Offer> > offers; + EXPECT_CALL(sched, resourceOffers(&driver, _)) + .WillOnce(FutureArg<1>(&offers)) + .WillRepeatedly(Return()); // Ignore subsequent offers. + + driver.start(); + + AWAIT_READY(frameworkId); + + AWAIT_READY(offers); + EXPECT_NE(0u, offers.get().size()); + + const Offer& offer = offers.get()[0]; + + TaskInfo task; + task.set_name(""); + task.mutable_task_id()->set_value("1"); + task.mutable_slave_id()->CopyFrom(offer.slave_id()); + task.mutable_resources()->CopyFrom(offer.resources()); + + string uuid = UUID::random().toString(); + + CommandInfo command; + command.set_shell(false); + + // We should also be able to skip setting the comamnd value and just + // set the arguments and those should also get passed through to the + // entrypoint! + command.add_arguments(uuid); + + ContainerInfo containerInfo; + containerInfo.set_type(ContainerInfo::DOCKER); + + ContainerInfo::DockerInfo dockerInfo; + dockerInfo.set_image("mesosphere/inky"); + containerInfo.mutable_docker()->CopyFrom(dockerInfo); + + task.mutable_command()->CopyFrom(command); + task.mutable_container()->CopyFrom(containerInfo); + + vector<TaskInfo> tasks; + tasks.push_back(task); + + Future<ContainerID> containerId; + Future<string> directory; + EXPECT_CALL(dockerContainerizer, launch(_, _, _, _, _, _, _, _)) + .WillOnce(DoAll(FutureArg<0>(&containerId), + FutureArg<3>(&directory), + Invoke(&dockerContainerizer, + &MockDockerContainerizer::_launch))); + + Future<TaskStatus> statusRunning; + Future<TaskStatus> statusFinished; + EXPECT_CALL(sched, statusUpdate(&driver, _)) + .WillOnce(FutureArg<1>(&statusRunning)) + .WillOnce(FutureArg<1>(&statusFinished)) + .WillRepeatedly(DoDefault()); + + driver.launchTasks(offers.get()[0].id(), tasks); + + AWAIT_READY_FOR(containerId, Seconds(60)); + AWAIT_READY(directory); + AWAIT_READY_FOR(statusRunning, Seconds(60)); + EXPECT_EQ(TASK_RUNNING, statusRunning.get().state()); + AWAIT_READY_FOR(statusFinished, Seconds(60)); + EXPECT_EQ(TASK_FINISHED, statusFinished.get().state()); + + // Now check that the proper output is in stderr and stdout. + Try<string> read = os::read(path::join(directory.get(), "stdout")); + + ASSERT_SOME(read); + + // We expect the passed in command arguments to override the image's + // default command, thus we should see the value of 'uuid' in the + // output instead of the default command which is 'inky'. + EXPECT_TRUE(strings::contains(read.get(), uuid)); + EXPECT_FALSE(strings::contains(read.get(), "inky")); + + read = os::read(path::join(directory.get(), "stderr")); + ASSERT_SOME(read); + EXPECT_FALSE(strings::contains(read.get(), "inky")); + EXPECT_FALSE(strings::contains(read.get(), uuid)); + + driver.stop(); + driver.join(); + + Shutdown(); +}
