Changed containerizer->launch callsites to new interface. As part of combining the two `containerizer->launch` code paths, callers now need to construct the `ContainerConfig` (instead of passing some fields that are then copied into a `ContainerConfig`).
Review: https://reviews.apache.org/r/58902 Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/af21bb71 Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/af21bb71 Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/af21bb71 Branch: refs/heads/master Commit: af21bb71655053d223eec1595a0384fe9e26f8c7 Parents: a6a7a38 Author: Joseph Wu <josep...@apache.org> Authored: Mon Apr 10 16:04:55 2017 -0700 Committer: Joseph Wu <josep...@apache.org> Committed: Thu May 25 18:37:06 2017 -0700 ---------------------------------------------------------------------- src/slave/http.cpp | 26 +++++++++--- src/slave/slave.cpp | 101 +++++++++++++++++++++++++++++------------------ 2 files changed, 83 insertions(+), 44 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/af21bb71/src/slave/http.cpp ---------------------------------------------------------------------- diff --git a/src/slave/http.cpp b/src/slave/http.cpp index 4444a38..3160407 100644 --- a/src/slave/http.cpp +++ b/src/slave/http.cpp @@ -15,6 +15,7 @@ // limitations under the License. #include <list> +#include <map> #include <memory> #include <sstream> #include <string> @@ -78,6 +79,7 @@ using mesos::authorization::createSubject; using mesos::internal::recordio::Reader; using mesos::slave::ContainerClass; +using mesos::slave::ContainerConfig; using mesos::slave::ContainerTermination; using process::AUTHENTICATION; @@ -116,6 +118,7 @@ using process::metrics::internal::MetricsProcess; using ::recordio::Decoder; using std::list; +using std::map; using std::string; using std::tie; using std::tuple; @@ -2327,13 +2330,26 @@ Future<Response> Slave::Http::_launchNestedContainer( } #endif + ContainerConfig containerConfig; + containerConfig.mutable_command_info()->CopyFrom(commandInfo); + + if (user.isSome()) { + containerConfig.set_user(user.get()); + } + + if (containerInfo.isSome()) { + containerConfig.mutable_container_info()->CopyFrom(containerInfo.get()); + } + + if (containerClass.isSome()) { + containerConfig.set_container_class(containerClass.get()); + } + Future<bool> launched = slave->containerizer->launch( containerId, - commandInfo, - containerInfo, - user, - slave->info.id(), - containerClass); + containerConfig, + map<string, string>(), + None()); // TODO(bmahler): The containerizers currently require that // the caller calls destroy if the launch fails. See MESOS-6214. http://git-wip-us.apache.org/repos/asf/mesos/blob/af21bb71/src/slave/slave.cpp ---------------------------------------------------------------------- diff --git a/src/slave/slave.cpp b/src/slave/slave.cpp index 0f21cf8..c578c52 100644 --- a/src/slave/slave.cpp +++ b/src/slave/slave.cpp @@ -117,6 +117,7 @@ using mesos::executor::Call; using mesos::master::detector::MasterDetector; +using mesos::slave::ContainerConfig; using mesos::slave::ContainerTermination; using mesos::slave::QoSController; using mesos::slave::QoSCorrection; @@ -2686,6 +2687,8 @@ void Slave::launchExecutor( } // Tell the containerizer to launch the executor. + // NOTE: We make a copy of the executor info because we may mutate + // it with some default fields and resources. ExecutorInfo executorInfo_ = executor->info; // Populate the command info for default executor. We modify the ExecutorInfo @@ -2711,6 +2714,41 @@ void Slave::launchExecutor( executorInfo_.mutable_resources()->CopyFrom(resources); + // Add the default container info to the executor info. + // TODO(jieyu): Rename the flag to be default_mesos_container_info. + if (!executorInfo_.has_container() && + flags.default_container_info.isSome()) { + executorInfo_.mutable_container()->CopyFrom( + flags.default_container_info.get()); + } + + // Bundle all the container launch fields together. + ContainerConfig containerConfig; + containerConfig.mutable_executor_info()->CopyFrom(executorInfo_); + containerConfig.mutable_command_info()->CopyFrom(executorInfo_.command()); + containerConfig.mutable_resources()->CopyFrom(executorInfo_.resources()); + containerConfig.set_directory(executor->directory); + + if (executor->user.isSome()) { + containerConfig.set_user(executor->user.get()); + } + + if (executor->isCommandExecutor()) { + if (taskInfo.isSome()) { + containerConfig.mutable_task_info()->CopyFrom(taskInfo.get()); + + if (taskInfo.get().has_container()) { + containerConfig.mutable_container_info() + ->CopyFrom(taskInfo.get().container()); + } + } + } else { + if (executorInfo_.has_container()) { + containerConfig.mutable_container_info() + ->CopyFrom(executorInfo_.container()); + } + } + // Prepare environment variables for the executor. map<string, string> environment = executorEnvironment( flags, @@ -2721,48 +2759,33 @@ void Slave::launchExecutor( authenticationToken, framework->info.checkpoint()); - // Launch the container. - Future<bool> launch; - if (!executor->isCommandExecutor()) { - // If the executor is _not_ a command executor, this means that - // the task will include the executor to run. The actual task to - // run will be enqueued and subsequently handled by the executor - // when it has registered to the slave. - launch = containerizer->launch( - executor->containerId, - None(), - executorInfo_, - executor->directory, - executor->user, - info.id(), - environment, - framework->info.checkpoint()); - } else { - // An executor has _not_ been provided by the task and will - // instead define a command and/or container to run. Right now, - // these tasks will require an executor anyway and the slave - // creates a command executor. However, it is up to the - // containerizer how to execute those tasks and the generated - // executor info works as a placeholder. - // TODO(nnielsen): Obsolete the requirement for executors to run - // one-off tasks. - launch = containerizer->launch( - executor->containerId, - taskInfo, - executorInfo_, - executor->directory, - executor->user, + // Prepare the filename of the pidfile, for checkpoint-enabled frameworks. + Option<string> pidCheckpointPath = None(); + if (framework->info.checkpoint()){ + pidCheckpointPath = slave::paths::getForkedPidPath( + slave::paths::getMetaRootDir(flags.work_dir), info.id(), - environment, - framework->info.checkpoint()); + framework->id(), + executor->id, + executor->containerId); } - launch.onAny(defer(self(), - &Self::executorLaunched, - frameworkId, - executor->id, - executor->containerId, - lambda::_1)); + LOG(INFO) << "Launching container " << executor->containerId + << " for executor '" << executor->id + << "' of framework " << framework->id(); + + // Launch the container. + containerizer->launch( + executor->containerId, + containerConfig, + environment, + pidCheckpointPath) + .onAny(defer(self(), + &Self::executorLaunched, + frameworkId, + executor->id, + executor->containerId, + lambda::_1)); // Make sure the executor registers within the given timeout. delay(flags.executor_registration_timeout,
