You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by mp...@apache.org on 2017/04/17 23:49:09 UTC

mesos git commit: Fixed a regression hiding previously exposed master and agent flags.

Repository: mesos
Updated Branches:
  refs/heads/1.2.x 990e58498 -> 78cf56e9e


Fixed a regression hiding previously exposed master and agent flags.

In f441eb9 we in a number of places changed  how 'Flag's were added to
'Flags' by moving from ad-hoc invocations of 'FlagsBase::add' on
particular instances to proper 'Flags' member variables. This was needed
to ensure 'Flags' instances could always safely be copied. For that we
introduced local derived 'Flags' classes to support localized parsing
needs. At the same time, this implementation strategy led to these these
local variables not being accessible through instances of the original
class anymore (this was inevitable when making 'Flags' classes properly
copyable), which e.g., causes a regression in the flags displayed in a
master's '/flags' endpoint.

This commit moves the flags into the respective base class removing the
local classes so that all passed flags are exposed to users.

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


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

Branch: refs/heads/1.2.x
Commit: 78cf56e9e5d42d9055013251f01c152f6fb9882f
Parents: 990e584
Author: Benjamin Bannier <be...@mesosphere.io>
Authored: Mon Apr 17 16:38:17 2017 -0700
Committer: Michael Park <mp...@apache.org>
Committed: Mon Apr 17 16:38:17 2017 -0700

----------------------------------------------------------------------
 src/master/flags.cpp | 35 ++++++++++++++++++++++++++++
 src/master/flags.hpp | 16 +++++++++++++
 src/master/main.cpp  | 59 +----------------------------------------------
 src/slave/flags.cpp  | 34 +++++++++++++++++++++++++++
 src/slave/flags.hpp  | 15 ++++++++++++
 src/slave/main.cpp   | 59 +----------------------------------------------
 6 files changed, 102 insertions(+), 116 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/78cf56e9/src/master/flags.cpp
----------------------------------------------------------------------
diff --git a/src/master/flags.cpp b/src/master/flags.cpp
index d25cfdd..3a89304 100644
--- a/src/master/flags.cpp
+++ b/src/master/flags.cpp
@@ -592,4 +592,39 @@ mesos::internal::master::Flags::Flags()
       "information about all connected agents. See also the\n"
       "`registry_max_agent_age` flag.",
       DEFAULT_REGISTRY_MAX_AGENT_COUNT);
+
+  add(&Flags::ip,
+      "ip",
+      "IP address to listen on. This cannot be used in conjunction\n"
+      "with `--ip_discovery_command`.");
+
+  add(&Flags::port, "port", "Port to listen on.", MasterInfo().port());
+
+  add(&Flags::advertise_ip,
+      "advertise_ip",
+      "IP address advertised to reach this Mesos master.\n"
+      "The master does not bind using this IP address.\n"
+      "However, this IP address may be used to access this master.");
+
+  add(&Flags::advertise_port,
+      "advertise_port",
+      "Port advertised to reach Mesos master (along with\n"
+      "`advertise_ip`). The master does not bind to this port.\n"
+      "However, this port (along with `advertise_ip`) may be used to\n"
+      "access this master.");
+
+  add(&Flags::zk,
+      "zk",
+      "ZooKeeper URL (used for leader election amongst masters)\n"
+      "May be one of:\n"
+      "  `zk://host1:port1,host2:port2,.../path`\n"
+      "  `zk://username:password@host1:port1,host2:port2,.../path`\n"
+      "  `file:///path/to/file` (where file contains one of the above)\n"
+      "NOTE: Not required if master is run in standalone mode (non-HA).");
+
+  add(&Flags::ip_discovery_command,
+      "ip_discovery_command",
+      "Optional IP discovery binary: if set, it is expected to emit\n"
+      "the IP address which the master will try to bind to.\n"
+      "Cannot be used in conjunction with `--ip`.");
 }

http://git-wip-us.apache.org/repos/asf/mesos/blob/78cf56e9/src/master/flags.hpp
----------------------------------------------------------------------
diff --git a/src/master/flags.hpp b/src/master/flags.hpp
index 41a0edf..9336a50 100644
--- a/src/master/flags.hpp
+++ b/src/master/flags.hpp
@@ -17,6 +17,8 @@
 #ifndef __MASTER_FLAGS_HPP__
 #define __MASTER_FLAGS_HPP__
 
+#include <cstdint>
+
 #include <string>
 
 #include <stout/duration.hpp>
@@ -94,6 +96,20 @@ public:
   Duration registry_max_agent_age;
   size_t registry_max_agent_count;
 
+  // The following flags are executable specific (e.g., since we only
+  // have one instance of libprocess per execution, we only want to
+  // advertise the IP and port option once, here).
+
+  Option<std::string> ip;
+  uint16_t port;
+  Option<std::string> advertise_ip;
+  Option<std::string> advertise_port;
+  Option<std::string> zk;
+
+  // Optional IP discover script that will set the Master IP.
+  // If set, its output is expected to be a valid parseable IP string.
+  Option<std::string> ip_discovery_command;
+
 #ifdef WITH_NETWORK_ISOLATOR
   Option<size_t> max_executors_per_agent;
 #endif  // WITH_NETWORK_ISOLATOR

http://git-wip-us.apache.org/repos/asf/mesos/blob/78cf56e9/src/master/main.cpp
----------------------------------------------------------------------
diff --git a/src/master/main.cpp b/src/master/main.cpp
index fa7ba13..da75fe9 100644
--- a/src/master/main.cpp
+++ b/src/master/main.cpp
@@ -128,63 +128,6 @@ using std::string;
 using std::vector;
 
 
-class Flags : public virtual master::Flags
-{
-public:
-  Flags()
-  {
-    add(&Flags::ip,
-        "ip",
-        "IP address to listen on. This cannot be used in conjunction\n"
-        "with `--ip_discovery_command`.");
-
-    add(&Flags::port, "port", "Port to listen on.", MasterInfo().port());
-
-    add(&Flags::advertise_ip,
-        "advertise_ip",
-        "IP address advertised to reach this Mesos master.\n"
-        "The master does not bind using this IP address.\n"
-        "However, this IP address may be used to access this master.");
-
-    add(&Flags::advertise_port,
-        "advertise_port",
-        "Port advertised to reach Mesos master (along with\n"
-        "`advertise_ip`). The master does not bind to this port.\n"
-        "However, this port (along with `advertise_ip`) may be used to\n"
-        "access this master.");
-
-    add(&Flags::zk,
-        "zk",
-        "ZooKeeper URL (used for leader election amongst masters)\n"
-        "May be one of:\n"
-        "  `zk://host1:port1,host2:port2,.../path`\n"
-        "  `zk://username:password@host1:port1,host2:port2,.../path`\n"
-        "  `file:///path/to/file` (where file contains one of the above)\n"
-        "NOTE: Not required if master is run in standalone mode (non-HA).");
-
-    add(&Flags::ip_discovery_command,
-        "ip_discovery_command",
-        "Optional IP discovery binary: if set, it is expected to emit\n"
-        "the IP address which the master will try to bind to.\n"
-        "Cannot be used in conjunction with `--ip`.");
-    }
-
-    // The following flags are executable specific (e.g., since we only
-    // have one instance of libprocess per execution, we only want to
-    // advertise the IP and port option once, here).
-
-    Option<string> ip;
-    uint16_t port;
-    Option<string> advertise_ip;
-    Option<string> advertise_port;
-    Option<string> zk;
-
-    // Optional IP discover script that will set the Master IP.
-    // If set, its output is expected to be a valid parseable IP string.
-    Option<string> ip_discovery_command;
-};
-
-
 int main(int argc, char** argv)
 {
   // The order of initialization of various master components is as follows:
@@ -213,7 +156,7 @@ int main(int argc, char** argv)
 
   GOOGLE_PROTOBUF_VERIFY_VERSION;
 
-  ::Flags flags;
+  master::Flags flags;
 
   Try<flags::Warnings> load = flags.load("MESOS_", argc, argv);
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/78cf56e9/src/slave/flags.cpp
----------------------------------------------------------------------
diff --git a/src/slave/flags.cpp b/src/slave/flags.cpp
index 71935de..7010114 100644
--- a/src/slave/flags.cpp
+++ b/src/slave/flags.cpp
@@ -953,4 +953,38 @@ mesos::internal::slave::Flags::Flags()
       "NOTE: This flag is *experimental* and should not be used in\n"
       "production yet.",
       false);
+
+  add(&Flags::ip,
+      "ip",
+      "IP address to listen on. This cannot be used in conjunction\n"
+      "with `--ip_discovery_command`.");
+
+  add(&Flags::port, "port", "Port to listen on.", SlaveInfo().port());
+
+  add(&Flags::advertise_ip,
+      "advertise_ip",
+      "IP address advertised to reach this Mesos slave.\n"
+      "The slave does not bind to this IP address.\n"
+      "However, this IP address may be used to access this slave.");
+
+  add(&Flags::advertise_port,
+      "advertise_port",
+      "Port advertised to reach this Mesos slave (along with\n"
+      "`advertise_ip`). The slave does not bind to this port.\n"
+      "However, this port (along with `advertise_ip`) may be used to\n"
+      "access this slave.");
+
+  add(&Flags::master,
+      "master",
+      "May be one of:\n"
+      "  `host:port`\n"
+      "  `zk://host1:port1,host2:port2,.../path`\n"
+      "  `zk://username:password@host1:port1,host2:port2,.../path`\n"
+      "  `file:///path/to/file` (where file contains one of the above)");
+
+  add(&Flags::ip_discovery_command,
+      "ip_discovery_command",
+      "Optional IP discovery binary: if set, it is expected to emit\n"
+      "the IP address which the slave will try to bind to.\n"
+      "Cannot be used in conjunction with `--ip`.");
 }

http://git-wip-us.apache.org/repos/asf/mesos/blob/78cf56e9/src/slave/flags.hpp
----------------------------------------------------------------------
diff --git a/src/slave/flags.hpp b/src/slave/flags.hpp
index 2c4bd6a..1e5494c 100644
--- a/src/slave/flags.hpp
+++ b/src/slave/flags.hpp
@@ -17,6 +17,7 @@
 #ifndef __SLAVE_FLAGS_HPP__
 #define __SLAVE_FLAGS_HPP__
 
+#include <cstdint>
 #include <string>
 
 #include <stout/bytes.hpp>
@@ -155,6 +156,20 @@ public:
   std::string xfs_project_range;
 #endif
   bool http_command_executor;
+
+  // The following flags are executable specific (e.g., since we only
+  // have one instance of libprocess per execution, we only want to
+  // advertise the IP and port option once, here).
+
+  Option<std::string> ip;
+  uint16_t port;
+  Option<std::string> advertise_ip;
+  Option<std::string> advertise_port;
+  Option<std::string> master;
+
+  // Optional IP discover script that will set the slave's IP.
+  // If set, its output is expected to be a valid parseable IP string.
+  Option<std::string> ip_discovery_command;
 };
 
 } // namespace slave {

http://git-wip-us.apache.org/repos/asf/mesos/blob/78cf56e9/src/slave/main.cpp
----------------------------------------------------------------------
diff --git a/src/slave/main.cpp b/src/slave/main.cpp
index a124d2e..31f2b4f 100644
--- a/src/slave/main.cpp
+++ b/src/slave/main.cpp
@@ -91,63 +91,6 @@ using std::string;
 using std::vector;
 
 
-class Flags : public virtual slave::Flags
-{
-public:
-  Flags()
-  {
-    add(&Flags::ip,
-        "ip",
-        "IP address to listen on. This cannot be used in conjunction\n"
-        "with `--ip_discovery_command`.");
-
-    add(&Flags::port, "port", "Port to listen on.", SlaveInfo().port());
-
-    add(&Flags::advertise_ip,
-        "advertise_ip",
-        "IP address advertised to reach this Mesos slave.\n"
-        "The slave does not bind to this IP address.\n"
-        "However, this IP address may be used to access this slave.");
-
-    add(&Flags::advertise_port,
-        "advertise_port",
-        "Port advertised to reach this Mesos slave (along with\n"
-        "`advertise_ip`). The slave does not bind to this port.\n"
-        "However, this port (along with `advertise_ip`) may be used to\n"
-        "access this slave.");
-
-    add(&Flags::master,
-        "master",
-        "May be one of:\n"
-        "  `host:port`\n"
-        "  `zk://host1:port1,host2:port2,.../path`\n"
-        "  `zk://username:password@host1:port1,host2:port2,.../path`\n"
-        "  `file:///path/to/file` (where file contains one of the above)");
-
-
-    add(&Flags::ip_discovery_command,
-        "ip_discovery_command",
-        "Optional IP discovery binary: if set, it is expected to emit\n"
-        "the IP address which the slave will try to bind to.\n"
-        "Cannot be used in conjunction with `--ip`.");
-  }
-
-  // The following flags are executable specific (e.g., since we only
-  // have one instance of libprocess per execution, we only want to
-  // advertise the IP and port option once, here).
-
-  Option<string> ip;
-  uint16_t port;
-  Option<string> advertise_ip;
-  Option<string> advertise_port;
-  Option<string> master;
-
-  // Optional IP discover script that will set the slave's IP.
-  // If set, its output is expected to be a valid parseable IP string.
-  Option<string> ip_discovery_command;
-};
-
-
 int main(int argc, char** argv)
 {
   // The order of initialization is as follows:
@@ -178,7 +121,7 @@ int main(int argc, char** argv)
 
   GOOGLE_PROTOBUF_VERIFY_VERSION;
 
-  ::Flags flags;
+  slave::Flags flags;
 
   Try<flags::Warnings> load = flags.load("MESOS_", argc, argv);