Repository: mesos Updated Branches: refs/heads/master 1ab190a07 -> cf25ea38f
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/57994/ Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/cf25ea38 Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/cf25ea38 Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/cf25ea38 Branch: refs/heads/master Commit: cf25ea38fb770dd1772b3c40e880b936aa11c4f2 Parents: 1ab190a Author: Benjamin Bannier <[email protected]> Authored: Fri Mar 31 00:10:05 2017 -0700 Committer: Michael Park <[email protected]> Committed: Fri Mar 31 11:54:32 2017 -0700 ---------------------------------------------------------------------- src/master/flags.cpp | 35 +++++++++++++++++++++++++++ src/master/flags.hpp | 16 +++++++++++++ src/master/main.cpp | 59 +-------------------------------------------- src/slave/flags.cpp | 35 +++++++++++++++++++++++++++ src/slave/flags.hpp | 15 ++++++++++++ src/slave/main.cpp | 61 ++--------------------------------------------- 6 files changed, 104 insertions(+), 117 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/cf25ea38/src/master/flags.cpp ---------------------------------------------------------------------- diff --git a/src/master/flags.cpp b/src/master/flags.cpp index 496ca31..b5660e4 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/cf25ea38/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/cf25ea38/src/master/main.cpp ---------------------------------------------------------------------- diff --git a/src/master/main.cpp b/src/master/main.cpp index 3d2176c..90d159e 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/cf25ea38/src/slave/flags.cpp ---------------------------------------------------------------------- diff --git a/src/slave/flags.cpp b/src/slave/flags.cpp index a7b23b5..d3ac66d 100644 --- a/src/slave/flags.cpp +++ b/src/slave/flags.cpp @@ -968,4 +968,39 @@ 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/cf25ea38/src/slave/flags.hpp ---------------------------------------------------------------------- diff --git a/src/slave/flags.hpp b/src/slave/flags.hpp index 224fac1..171f67e 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> @@ -161,6 +162,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/cf25ea38/src/slave/main.cpp ---------------------------------------------------------------------- diff --git a/src/slave/main.cpp b/src/slave/main.cpp index 81d61b1..72b141c 100644 --- a/src/slave/main.cpp +++ b/src/slave/main.cpp @@ -99,63 +99,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; -}; - - #ifdef __linux__ // Move the slave into its own cgroup for each of the specified // subsystems. @@ -167,7 +110,7 @@ public: // TODO(jieyu): Make sure the corresponding cgroup isolator is // enabled so that the container processes are moved to different // cgroups than the agent cgroup. -static Try<Nothing> assignCgroups(const ::Flags& flags) +static Try<Nothing> assignCgroups(const slave::Flags& flags) { CHECK_SOME(flags.agent_subsystems); @@ -303,7 +246,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);
