Stopped mutating executor info with default container info.

Review: https://reviews.apache.org/r/37091


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/210dadf0
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/210dadf0
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/210dadf0

Branch: refs/heads/master
Commit: 210dadf002dcee838e4a6fd245aea6f4601103d8
Parents: d944393
Author: Jie Yu <[email protected]>
Authored: Tue Aug 4 12:00:02 2015 -0700
Committer: Jie Yu <[email protected]>
Committed: Fri Aug 7 16:54:14 2015 -0700

----------------------------------------------------------------------
 src/slave/containerizer/mesos/containerizer.cpp | 36 ++++++++++++++------
 src/slave/slave.cpp                             | 15 +-------
 2 files changed, 27 insertions(+), 24 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/210dadf0/src/slave/containerizer/mesos/containerizer.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/containerizer.cpp 
b/src/slave/containerizer/mesos/containerizer.cpp
index 0222245..e9fc4d7 100644
--- a/src/slave/containerizer/mesos/containerizer.cpp
+++ b/src/slave/containerizer/mesos/containerizer.cpp
@@ -482,7 +482,17 @@ Future<Nothing> MesosContainerizerProcess::__recover(
     // containers should be running after recover.
     container->state = RUNNING;
 
-    container->executorInfo = run.executor_info();
+    // Add the default container info to the executor info.
+    // TODO(jieyu): Checkpoint the default container info in case the
+    // slave changes the default container info flag.
+    ExecutorInfo executorInfo = run.executor_info();
+    if (!executorInfo.has_container() &&
+        flags.default_container_info.isSome()) {
+      executorInfo.mutable_container()->CopyFrom(
+          flags.default_container_info.get());
+    }
+
+    container->executorInfo = executorInfo;
 
     containers_[containerId] = Owned<Container>(container);
 
@@ -552,7 +562,7 @@ void MesosContainerizerProcess::___recover(
 //    executor.
 Future<bool> MesosContainerizerProcess::launch(
     const ContainerID& containerId,
-    const ExecutorInfo& executorInfo,
+    const ExecutorInfo& _executorInfo,
     const string& directory,
     const Option<string>& user,
     const SlaveID& slaveId,
@@ -560,22 +570,28 @@ Future<bool> MesosContainerizerProcess::launch(
     bool checkpoint)
 {
   if (containers_.contains(containerId)) {
-    LOG(ERROR) << "Cannot start already running container '"
-               << containerId << "'";
     return Failure("Container already started");
   }
 
-  // We support MESOS containers or ExecutorInfos with no
-  // ContainerInfo given.
+  // NOTE: We make a copy of the executor info because we may mutate
+  // it with default container info.
+  ExecutorInfo executorInfo = _executorInfo;
+
   if (executorInfo.has_container() &&
       executorInfo.container().type() != ContainerInfo::MESOS) {
     return false;
   }
 
-  const CommandInfo& command = executorInfo.command();
-  if (command.has_container()) {
-    // We return false as this containerizer does not support
-    // handling ContainerInfo.
+  // 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());
+  }
+
+  // MesosContainerizer does not support ContainerInfo in CommandInfo.
+  if (executorInfo.command().has_container()) {
     return false;
   }
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/210dadf0/src/slave/slave.cpp
----------------------------------------------------------------------
diff --git a/src/slave/slave.cpp b/src/slave/slave.cpp
index f181b1b..9061e67 100644
--- a/src/slave/slave.cpp
+++ b/src/slave/slave.cpp
@@ -3269,23 +3269,10 @@ ExecutorInfo Slave::getExecutorInfo(
           "cpus:" + stringify(DEFAULT_EXECUTOR_CPUS) + ";" +
           "mem:" + stringify(DEFAULT_EXECUTOR_MEM.megabytes())).get());
 
-    // Add in any default ContainerInfo.
-    if (!executor.has_container() && flags.default_container_info.isSome()) {
-      executor.mutable_container()->CopyFrom(
-          flags.default_container_info.get());
-    }
-
     return executor;
   }
 
-  ExecutorInfo executor = task.executor();
-
-  // Add in any default ContainerInfo.
-  if (!executor.has_container() && flags.default_container_info.isSome()) {
-    executor.mutable_container()->CopyFrom(flags.default_container_info.get());
-  }
-
-  return executor;
+  return task.executor();
 }
 
 

Reply via email to