Repository: mesos
Updated Branches:
  refs/heads/master 8b27afc9b -> 1c2ee5c97


Refactored the agent 'launcher' flag to always have a value.

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


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

Branch: refs/heads/master
Commit: 1c2ee5c97f89d96a9a3f006ba11303c8980315f4
Parents: 8b27afc
Author: Benjamin Hindman <b...@berkeley.edu>
Authored: Fri Sep 16 14:36:47 2016 -0700
Committer: Benjamin Hindman <benjamin.hind...@gmail.com>
Committed: Fri Sep 16 15:26:06 2016 -0700

----------------------------------------------------------------------
 src/local/local.cpp                             | 84 +++++++++++---------
 src/slave/containerizer/mesos/containerizer.cpp | 30 +++----
 src/slave/flags.cpp                             | 14 +++-
 src/slave/flags.hpp                             |  2 +-
 4 files changed, 72 insertions(+), 58 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/1c2ee5c9/src/local/local.cpp
----------------------------------------------------------------------
diff --git a/src/local/local.cpp b/src/local/local.cpp
index 450e2c1..2be5bcf 100644
--- a/src/local/local.cpp
+++ b/src/local/local.cpp
@@ -161,8 +161,8 @@ PID<Master> launch(const Flags& flags, Allocator* 
_allocator)
     // Save the instance for deleting later.
     allocator = defaultAllocator.get();
   } else {
-    // TODO(benh): Figure out the behavior of allocator pointer and remove the
-    // else block.
+    // TODO(benh): Figure out the behavior of allocator pointer and
+    // remove the else block.
     allocator = nullptr;
   }
 
@@ -176,12 +176,13 @@ PID<Master> launch(const Flags& flags, Allocator* 
_allocator)
       << flags.work_dir << "': " << mkdir.error();
   }
 
-  map<string, string> propagated_flags;
-  propagated_flags["work_dir"] = flags.work_dir;
-
   {
-    master::Flags flags;
-    Try<flags::Warnings> load = flags.load(propagated_flags, false, "MESOS_");
+    master::Flags masterFlags;
+
+    Try<flags::Warnings> load = masterFlags.load("MESOS_");
+
+    masterFlags.work_dir = flags.work_dir;
+
     if (load.isError()) {
       EXIT(EXIT_FAILURE)
         << "Failed to start a local cluster while loading"
@@ -195,40 +196,42 @@ PID<Master> launch(const Flags& flags, Allocator* 
_allocator)
 
     // Load modules. Note that this covers both, master and slave
     // specific modules as both use the same flag (--modules).
-    if (flags.modules.isSome()) {
-      Try<Nothing> result = ModuleManager::load(flags.modules.get());
+    if (masterFlags.modules.isSome()) {
+      Try<Nothing> result = ModuleManager::load(masterFlags.modules.get());
       if (result.isError()) {
         EXIT(EXIT_FAILURE) << "Error loading modules: " << result.error();
       }
     }
 
-    if (flags.registry == "in_memory") {
+    if (masterFlags.registry == "in_memory") {
       storage = new mesos::state::InMemoryStorage();
-    } else if (flags.registry == "replicated_log") {
+    } else if (masterFlags.registry == "replicated_log") {
       // TODO(vinod): Add support for replicated log with ZooKeeper.
       log = new Log(
           1,
-          path::join(flags.work_dir.get(), "replicated_log"),
+          path::join(masterFlags.work_dir.get(), "replicated_log"),
           set<UPID>(),
-          flags.log_auto_initialize,
+          masterFlags.log_auto_initialize,
           "registrar/");
       storage = new mesos::state::LogStorage(log);
     } else {
       EXIT(EXIT_FAILURE)
-        << "'" << flags.registry << "' is not a supported"
+        << "'" << masterFlags.registry << "' is not a supported"
         << " option for registry persistence";
     }
 
     CHECK_NOTNULL(storage);
 
     state = new mesos::state::protobuf::State(storage);
-    registrar =
-      new Registrar(flags, state, master::READONLY_HTTP_AUTHENTICATION_REALM);
+    registrar = new Registrar(
+        masterFlags,
+        state,
+        master::READONLY_HTTP_AUTHENTICATION_REALM);
 
     contender = new StandaloneMasterContender();
     detector = new StandaloneMasterDetector();
 
-    auto authorizerNames = strings::split(flags.authorizers, ",");
+    auto authorizerNames = strings::split(masterFlags.authorizers, ",");
     if (authorizerNames.empty()) {
       EXIT(EXIT_FAILURE) << "No authorizer specified";
     }
@@ -248,10 +251,10 @@ PID<Master> launch(const Flags& flags, Allocator* 
_allocator)
       authorizer = Authorizer::create(authorizerName);
     } else {
       // `authorizerName` is `DEFAULT_AUTHORIZER` at this point.
-      if (flags.acls.isSome()) {
+      if (masterFlags.acls.isSome()) {
         LOG(INFO) << "Creating default '" << authorizerName << "' authorizer";
 
-        authorizer = Authorizer::create(flags.acls.get());
+        authorizer = Authorizer::create(masterFlags.acls.get());
       }
     }
 
@@ -263,17 +266,17 @@ PID<Master> launch(const Flags& flags, Allocator* 
_allocator)
     }
 
     Option<shared_ptr<RateLimiter>> slaveRemovalLimiter = None();
-    if (flags.agent_removal_rate_limit.isSome()) {
+    if (masterFlags.agent_removal_rate_limit.isSome()) {
       // Parse the flag value.
       // TODO(vinod): Move this parsing logic to flags once we have a
       // 'Rate' abstraction in stout.
       vector<string> tokens =
-        strings::tokenize(flags.agent_removal_rate_limit.get(), "/");
+        strings::tokenize(masterFlags.agent_removal_rate_limit.get(), "/");
 
       if (tokens.size() != 2) {
         EXIT(EXIT_FAILURE)
           << "Invalid agent_removal_rate_limit: "
-          << flags.agent_removal_rate_limit.get()
+          << masterFlags.agent_removal_rate_limit.get()
           << ". Format is <Number of agents>/<Duration>";
       }
 
@@ -281,7 +284,7 @@ PID<Master> launch(const Flags& flags, Allocator* 
_allocator)
       if (permits.isError()) {
         EXIT(EXIT_FAILURE)
           << "Invalid agent_removal_rate_limit: "
-          << flags.agent_removal_rate_limit.get()
+          << masterFlags.agent_removal_rate_limit.get()
           << ". Format is <Number of agents>/<Duration>"
           << ": " << permits.error();
       }
@@ -290,7 +293,7 @@ PID<Master> launch(const Flags& flags, Allocator* 
_allocator)
       if (duration.isError()) {
         EXIT(EXIT_FAILURE)
           << "Invalid agent_removal_rate_limit: "
-          << flags.agent_removal_rate_limit.get()
+          << masterFlags.agent_removal_rate_limit.get()
           << ". Format is <Number of agents>/<Duration>"
           << ": " << duration.error();
       }
@@ -323,7 +326,7 @@ PID<Master> launch(const Flags& flags, Allocator* 
_allocator)
         detector,
         authorizer_,
         slaveRemovalLimiter,
-        flags);
+        masterFlags);
 
     detector->appoint(master->info());
   }
@@ -339,9 +342,11 @@ PID<Master> launch(const Flags& flags, Allocator* 
_allocator)
   vector<UPID> pids;
 
   for (int i = 0; i < flags.num_slaves; i++) {
-    slave::Flags flags;
+    slave::Flags slaveFlags;
+
+    Try<flags::Warnings> load = slaveFlags.load("MESOS_");
 
-    Try<flags::Warnings> load = flags.load(propagated_flags, false, "MESOS_");
+    slaveFlags.work_dir = flags.work_dir;
 
     if (load.isError()) {
       EXIT(EXIT_FAILURE)
@@ -355,14 +360,14 @@ PID<Master> launch(const Flags& flags, Allocator* 
_allocator)
     }
 
     // Use a different work directory for each slave.
-    flags.work_dir = path::join(flags.work_dir, stringify(i));
+    slaveFlags.work_dir = path::join(slaveFlags.work_dir, stringify(i));
 
     garbageCollectors->push_back(new GarbageCollector());
-    statusUpdateManagers->push_back(new StatusUpdateManager(flags));
+    statusUpdateManagers->push_back(new StatusUpdateManager(slaveFlags));
     fetchers->push_back(new Fetcher());
 
     Try<ResourceEstimator*> resourceEstimator =
-      ResourceEstimator::create(flags.resource_estimator);
+      ResourceEstimator::create(slaveFlags.resource_estimator);
 
     if (resourceEstimator.isError()) {
       EXIT(EXIT_FAILURE)
@@ -372,7 +377,7 @@ PID<Master> launch(const Flags& flags, Allocator* 
_allocator)
     resourceEstimators->push_back(resourceEstimator.get());
 
     Try<QoSController*> qosController =
-      QoSController::create(flags.qos_controller);
+      QoSController::create(slaveFlags.qos_controller);
 
     if (qosController.isError()) {
       EXIT(EXIT_FAILURE)
@@ -381,13 +386,20 @@ PID<Master> launch(const Flags& flags, Allocator* 
_allocator)
 
     qosControllers->push_back(qosController.get());
 
-    // Set default launcher to 'posix'(see MESOS-3793).
-    if (flags.launcher.isNone()) {
-      flags.launcher = "posix";
+    // Override the default launcher that gets created per agent to
+    // 'posix' if we're creating multiple agents because the
+    // LinuxLauncher does not support multiple agents on the same host
+    // (see MESOS-3793).
+    if (flags.num_slaves > 1 && slaveFlags.launcher != "posix") {
+      // TODO(benh): This log line will print out for each slave!
+      LOG(WARNING) << "Using the 'posix' launcher instead of '"
+                   << slaveFlags.launcher << "' since currently only the "
+                   << "'posix' launcher supports multiple agents per host";
+      slaveFlags.launcher = "posix";
     }
 
     Try<Containerizer*> containerizer =
-      Containerizer::create(flags, true, fetchers->back());
+      Containerizer::create(slaveFlags, true, fetchers->back());
 
     if (containerizer.isError()) {
       EXIT(EXIT_FAILURE)
@@ -398,7 +410,7 @@ PID<Master> launch(const Flags& flags, Allocator* 
_allocator)
     // Master.
     Slave* slave = new Slave(
         process::ID::generate("slave"),
-        flags,
+        slaveFlags,
         detector,
         containerizer.get(),
         files,

http://git-wip-us.apache.org/repos/asf/mesos/blob/1c2ee5c9/src/slave/containerizer/mesos/containerizer.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/containerizer.cpp 
b/src/slave/containerizer/mesos/containerizer.cpp
index dc18e4e..e54169b 100644
--- a/src/slave/containerizer/mesos/containerizer.cpp
+++ b/src/slave/containerizer/mesos/containerizer.cpp
@@ -227,34 +227,24 @@ Try<MesosContainerizer*> MesosContainerizer::create(
   // Create the launcher for the MesosContainerizer.
   Try<Launcher*> launcher = [&flags_]() -> Try<Launcher*> {
 #ifdef __linux__
-    if (flags_.launcher.isSome()) {
-      // If the user has specified the launcher, use it.
-      if (flags_.launcher.get() == "linux") {
-        return LinuxLauncher::create(flags_);
-      } else if (flags_.launcher.get() == "posix") {
-        return PosixLauncher::create(flags_);
-      } else {
-        return Error(
-            "Unknown or unsupported launcher: " + flags_.launcher.get());
-      }
+    if (flags_.launcher == "linux") {
+      return LinuxLauncher::create(flags_);
+    } else if (flags_.launcher == "posix") {
+      return PosixLauncher::create(flags_);
+    } else {
+      return Error("Unknown or unsupported launcher: " + flags_.launcher);
     }
-
-    // Use Linux launcher if it is available, POSIX otherwise.
-    return LinuxLauncher::available()
-      ? LinuxLauncher::create(flags_)
-      : PosixLauncher::create(flags_);
 #elif __WINDOWS__
     // NOTE: Because the most basic launcher historically has been "posix", we
     // accept this flag on Windows, but map it to the `WindowsLauncher`.
-    if (flags_.launcher.isSome() && !(flags_.launcher.get() == "posix" ||
-        flags_.launcher.get() == "windows")) {
-      return Error("Unsupported launcher: " + flags_.launcher.get());
+    if (flags_.launcher != "posix" && flags_.launcher != "windows") {
+      return Error("Unsupported launcher: " + flags_.launcher);
     }
 
     return WindowsLauncher::create(flags_);
 #else
-    if (flags_.launcher.isSome() && flags_.launcher.get() != "posix") {
-      return Error("Unsupported launcher: " + flags_.launcher.get());
+    if (flags_.launcher != "posix") {
+      return Error("Unsupported launcher: " + flags_.launcher);
     }
 
     return PosixLauncher::create(flags_);

http://git-wip-us.apache.org/repos/asf/mesos/blob/1c2ee5c9/src/slave/flags.cpp
----------------------------------------------------------------------
diff --git a/src/slave/flags.cpp b/src/slave/flags.cpp
index f6cd6f7..491d10f 100644
--- a/src/slave/flags.cpp
+++ b/src/slave/flags.cpp
@@ -30,6 +30,10 @@
 
 #include "slave/constants.hpp"
 
+#ifdef __linux__
+#include "slave/containerizer/mesos/linux_launcher.hpp"
+#endif // __linux__
+
 using std::string;
 
 mesos::internal::slave::Flags::Flags()
@@ -105,7 +109,15 @@ mesos::internal::slave::Flags::Flags()
       "`linux` or `posix`. The Linux launcher is required for cgroups\n"
       "isolation and for any isolators that require Linux namespaces such as\n"
       "network, pid, etc. If unspecified, the agent will choose the Linux\n"
-      "launcher if it's running as root on Linux.");
+      "launcher if it's running as root on Linux.",
+#ifdef __linux__
+      LinuxLauncher::available() ? "linux" : "posix"
+#elif __WINDOWS__
+      "windows"
+#else
+      "posix"
+#endif // __linux__
+      );
 
   add(&Flags::image_providers,
       "image_providers",

http://git-wip-us.apache.org/repos/asf/mesos/blob/1c2ee5c9/src/slave/flags.hpp
----------------------------------------------------------------------
diff --git a/src/slave/flags.hpp b/src/slave/flags.hpp
index 897f3f8..3952d04 100644
--- a/src/slave/flags.hpp
+++ b/src/slave/flags.hpp
@@ -45,7 +45,7 @@ public:
   bool hostname_lookup;
   Option<std::string> resources;
   std::string isolation;
-  Option<std::string> launcher;
+  std::string launcher;
 
   Option<std::string> image_providers;
   std::string image_provisioner_backend;

Reply via email to