Repository: mesos Updated Branches: refs/heads/master 69393f77d -> ae12e7407
Added flag to disable hostname lookup. Under certain circumstances, dynamic lookup of hostname, while successful, provides undesirable results; we would also like, in those circumstances, to be able to just set the hostname to the chosen IP address (possibly set via the --ip_discovery_command method). The flag we add here, --\[no\]-hostname_lookup is \`true\` by default (which is the existing behavior) and will work under most circumstances: however, by disabling lookup (using, for example, \-\-no_hostname_lookup) the hostname used will be the same as the one configured (possibly, via --ip or MESOS_IP) in \`LIBPROCESS_IP\`. This change affects both Master/Agent nodes. WARNING - the \`Address::hostname()\` method always does a dynamic hostname lookup, which may in fact return inconsistent results wrt the Master's configuration (this is \*not\* affected by this change) and should be avoided; use instead \`Master::info()::hostname()\` which is always consistent with the Master's configuration. Review: https://reviews.apache.org/r/38473 Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/ae12e740 Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/ae12e740 Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/ae12e740 Branch: refs/heads/master Commit: ae12e7407d7f385a268f850897debf07a3953dff Parents: 69393f7 Author: Marco Massenzio <[email protected]> Authored: Wed Sep 23 21:20:23 2015 -0700 Committer: Joris Van Remoortere <[email protected]> Committed: Wed Sep 23 21:50:55 2015 -0700 ---------------------------------------------------------------------- docs/configuration.md | 35 +++++++++++++++++++++++++++----- src/master/flags.cpp | 12 ++++++++++- src/master/flags.hpp | 1 + src/master/master.cpp | 16 ++++++++++----- src/slave/flags.cpp | 12 ++++++++++- src/slave/flags.hpp | 1 + src/slave/slave.cpp | 16 ++++++++++----- src/tests/cluster.hpp | 32 ++++++++++++++++++++++++++++++ src/tests/master_tests.cpp | 44 +++++++++++++++++++++++++++++++++++++++++ 9 files changed, 152 insertions(+), 17 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/ae12e740/docs/configuration.md ---------------------------------------------------------------------- diff --git a/docs/configuration.md b/docs/configuration.md index dd7f4aa..dbaadb5 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -407,9 +407,22 @@ file:///path/to/file (where file contains one of the above)</code></pre> --hostname=VALUE </td> <td> - The hostname the master should advertise in ZooKeeper. + The hostname the master should advertise in ZooKeeper.<br> If left unset, the hostname is resolved from the IP address - that the master binds to. + that the slave binds to; unless the user explicitly prevents + that, using --no-hostname_lookup, in which case the IP itself + is used. + </td> + </tr> + <tr> + <td> + --[no-]hostname_lookup + </td> + <td> + Whether we should execute a lookup to find out the server's hostname, + if not explicitly set (via, e.g., `--hostname`). + True by default; if set to 'false' it will cause Mesos + to use the IP address, unless the hostname is explicitly set. </td> </tr> <tr> @@ -1141,10 +1154,22 @@ file:///path/to/file (where file contains one of the above)</code></pre> --hostname=VALUE </td> <td> - The hostname the slave should report. - <p/> + The hostname the agent node should report.<br> If left unset, the hostname is resolved from the IP address - that the slave binds to. + that the slave binds to; unless the user explicitly prevents + that, using --no-hostname_lookup, in which case the IP itself + is used. + </td> + </tr> + <tr> + <td> + --[no-]hostname_lookup + </td> + <td> + Whether we should execute a lookup to find out the server's hostname, + if not explicitly set (via, e.g., `--hostname`). + True by default; if set to 'false' it will cause Mesos + to use the IP address, unless the hostname is explicitly set. </td> </tr> <tr> http://git-wip-us.apache.org/repos/asf/mesos/blob/ae12e740/src/master/flags.cpp ---------------------------------------------------------------------- diff --git a/src/master/flags.cpp b/src/master/flags.cpp index 8087961..0285ce7 100644 --- a/src/master/flags.cpp +++ b/src/master/flags.cpp @@ -36,7 +36,17 @@ mesos::internal::master::Flags::Flags() "hostname", "The hostname the master should advertise in ZooKeeper.\n" "If left unset, the hostname is resolved from the IP address\n" - "that the master binds to."); + "that the slave binds to; unless the user explicitly prevents\n" + "that, using --no-hostname_lookup, in which case the IP itself\n" + "is used."); + + add(&Flags::hostname_lookup, + "hostname_lookup", + "Whether we should execute a lookup to find out the server's hostname,\n" + "if not explicitly set (via, e.g., `--hostname`).\n" + "True by default; if set to 'false' it will cause Mesos\n" + "to use the IP address, unless the hostname is explicitly set.", + true); add(&Flags::root_submissions, "root_submissions", http://git-wip-us.apache.org/repos/asf/mesos/blob/ae12e740/src/master/flags.hpp ---------------------------------------------------------------------- diff --git a/src/master/flags.hpp b/src/master/flags.hpp index e4b1df3..5fd5d50 100644 --- a/src/master/flags.hpp +++ b/src/master/flags.hpp @@ -45,6 +45,7 @@ public: Flags(); bool version; Option<std::string> hostname; + bool hostname_lookup; bool root_submissions; Option<std::string> work_dir; std::string registry; http://git-wip-us.apache.org/repos/asf/mesos/blob/ae12e740/src/master/master.cpp ---------------------------------------------------------------------- diff --git a/src/master/master.cpp b/src/master/master.cpp index 0a40bc3..6bee4f3 100644 --- a/src/master/master.cpp +++ b/src/master/master.cpp @@ -323,13 +323,19 @@ Master::Master( string hostname; if (flags.hostname.isNone()) { - Try<string> result = net::getHostname(self().address.ip); + if (flags.hostname_lookup) { + Try<string> result = net::getHostname(self().address.ip); - if (result.isError()) { - LOG(FATAL) << "Failed to get hostname: " << result.error(); - } + if (result.isError()) { + LOG(FATAL) << "Failed to get hostname: " << result.error(); + } - hostname = result.get(); + hostname = result.get(); + } else { + // We use the IP address for hostname if the user requested us + // NOT to look it up, and it wasn't explicitly set via --hostname: + hostname = stringify(self().address.ip); + } } else { hostname = flags.hostname.get(); } http://git-wip-us.apache.org/repos/asf/mesos/blob/ae12e740/src/slave/flags.cpp ---------------------------------------------------------------------- diff --git a/src/slave/flags.cpp b/src/slave/flags.cpp index ab3b0e1..69976d7 100644 --- a/src/slave/flags.cpp +++ b/src/slave/flags.cpp @@ -36,7 +36,17 @@ mesos::internal::slave::Flags::Flags() "hostname", "The hostname the slave should report.\n" "If left unset, the hostname is resolved from the IP address\n" - "that the slave binds to."); + "that the slave binds to; unless the user explicitly prevents\n" + "that, using --no-hostname_lookup, in which case the IP itself\n" + "is used."); + + add(&Flags::hostname_lookup, + "hostname_lookup", + "Whether we should execute a lookup to find out the server's hostname,\n" + "if not explicitly set (via, e.g., `--hostname`).\n" + "True by default; if set to 'false' it will cause Mesos\n" + "to use the IP address, unless the hostname is explicitly set.", + true); add(&Flags::version, "version", http://git-wip-us.apache.org/repos/asf/mesos/blob/ae12e740/src/slave/flags.hpp ---------------------------------------------------------------------- diff --git a/src/slave/flags.hpp b/src/slave/flags.hpp index bbf8171..3f6601a 100644 --- a/src/slave/flags.hpp +++ b/src/slave/flags.hpp @@ -44,6 +44,7 @@ public: bool version; Option<std::string> hostname; + bool hostname_lookup; Option<std::string> resources; std::string isolation; Option<std::string> launcher; http://git-wip-us.apache.org/repos/asf/mesos/blob/ae12e740/src/slave/slave.cpp ---------------------------------------------------------------------- diff --git a/src/slave/slave.cpp b/src/slave/slave.cpp index 29865ec..d1c9977 100644 --- a/src/slave/slave.cpp +++ b/src/slave/slave.cpp @@ -362,13 +362,19 @@ void Slave::initialize() string hostname; if (flags.hostname.isNone()) { - Try<string> result = net::getHostname(self().address.ip); + if (flags.hostname_lookup) { + Try<string> result = net::getHostname(self().address.ip); - if (result.isError()) { - LOG(FATAL) << "Failed to get hostname: " << result.error(); - } + if (result.isError()) { + LOG(FATAL) << "Failed to get hostname: " << result.error(); + } - hostname = result.get(); + hostname = result.get(); + } else { + // We use the IP address for hostname if the user requested us + // NOT to look it up, and it wasn't explicitly set via --hostname: + hostname = stringify(self().address.ip); + } } else { hostname = flags.hostname.get(); } http://git-wip-us.apache.org/repos/asf/mesos/blob/ae12e740/src/tests/cluster.hpp ---------------------------------------------------------------------- diff --git a/src/tests/cluster.hpp b/src/tests/cluster.hpp index 114583d..032fcdd 100644 --- a/src/tests/cluster.hpp +++ b/src/tests/cluster.hpp @@ -122,6 +122,24 @@ public: // Returns a new master detector for this instance of masters. process::Owned<MasterDetector> detector(); + /** + * The internal map from UPID to Master processes is not available + * externally to test methods; this lookup method helps to expose this to + * `MesosTest` classes. + * + * @param pid the PID for the Master process being looked up. + * @return a pointer to the `master::Master` process, whose UPID corresponds + * to the given value, if any. + */ + Option<master::Master*> find(const process::PID<master::Master>& pid) + { + if (masters.count(pid) != 0) { + return masters[pid].master; + } + + return None(); + } + private: // Not copyable, not assignable. Masters(const Masters&); @@ -226,6 +244,20 @@ public: slaves.shutdown(); } + /** + * Thin wrapper around the internal `Masters::find()` lookup method, which is + * otherwise inaccessible from test methods. + * + * @param pid the PID for the Master process being looked up. + * @return a pointer to the `master::Master` process, whose PID corresponds + * to the given value, if any. + */ + Option<master::Master*> find(const process::PID<master::Master>& pid) + { + return masters.find(pid); + } + + // Cluster wide shared abstractions. Files files; http://git-wip-us.apache.org/repos/asf/mesos/blob/ae12e740/src/tests/master_tests.cpp ---------------------------------------------------------------------- diff --git a/src/tests/master_tests.cpp b/src/tests/master_tests.cpp index f26344d..ee24739 100644 --- a/src/tests/master_tests.cpp +++ b/src/tests/master_tests.cpp @@ -1086,6 +1086,50 @@ TEST_F(WhitelistTest, WhitelistSlave) } +class HostnameTest : public MasterTest {}; + + +TEST_F(HostnameTest, LookupEnabled) +{ + master::Flags flags = CreateMasterFlags(); + EXPECT_TRUE(flags.hostname_lookup); + + Try<PID<Master>> pid = StartMaster(flags); + ASSERT_SOME(pid); + + Option<Master*> master = cluster.find(pid.get()); + ASSERT_SOME(master); + + EXPECT_EQ( + pid.get().address.hostname().get(), + master.get()->info().hostname()); + + Shutdown(); +} + + +TEST_F(HostnameTest, LookupDisabled) +{ + master::Flags flags = CreateMasterFlags(); + EXPECT_TRUE(flags.hostname_lookup); + EXPECT_NONE(flags.hostname); + + flags.hostname_lookup = false; + + Try<PID<Master>> pid = StartMaster(flags); + ASSERT_SOME(pid); + + Option<Master*> master = cluster.find(pid.get()); + ASSERT_SOME(master); + + EXPECT_EQ( + stringify(pid.get().address.ip), + master.get()->info().hostname()); + + Shutdown(); +} + + TEST_F(MasterTest, MasterLost) { Try<PID<Master>> master = StartMaster();
