You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Marco Massenzio <ma...@mesosphere.io> on 2015/09/18 23:53:56 UTC

Re: Review Request 38473: Add flag to disable hostname lookup.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38473/
-----------------------------------------------------------

(Updated Sept. 18, 2015, 9:53 p.m.)


Review request for mesos, Benjamin Hindman, Cody Maloney, and Neil Conway.


Changes
-------

Addressed comments + minor cosmetic changes.


Bugs: MESOS-3457
    https://issues.apache.org/jira/browse/MESOS-3457


Repository: mesos


Description
-------

Under certain circumstances, dynamic lookup of hostname, while
successful, provides undesirable results; we would also like, in
those circumstances, 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.


Diffs (updated)
-----

  docs/configuration.md dd7f4aa806a3c1a8653a0fda9a326a3707308e7c 
  src/master/flags.hpp e4b1df3f5a33049defff4688463274067f1f1ebf 
  src/master/flags.cpp 80879611fbcfd764c9fc8f60a31613a9c8fc2364 
  src/master/master.cpp ca4d5876dcd427964111428edc22d567ddaede0b 
  src/slave/flags.hpp e31a4183170c3442ac4a15365c229391e7e91480 
  src/slave/flags.cpp add4196dfd06c0f602ff5ebd39960dc05c4cd11f 
  src/slave/slave.cpp ad710d7b930a2f115d503ceb8f8fd7421ad30287 
  src/tests/cluster.hpp 114583de8c867495a2b7a953e6826708838e5d23 
  src/tests/master_tests.cpp a477794df37c658b80d79dea8555b001415f7b6a 
  src/tests/mesos.hpp 3db97aca921c9216d90384e1eb17030849516454 

Diff: https://reviews.apache.org/r/38473/diff/


Testing
-------

make check


Thanks,

Marco Massenzio


Re: Review Request 38473: Add flag to disable hostname lookup.

Posted by Cong Wang <cw...@twopensource.com>.

> On Sept. 18, 2015, 10:57 p.m., Cong Wang wrote:
> > Why not just set --host_name=$LIBPROCESS_IP for your case since you anyway need to provide a flag?
> 
> Guangya Liu wrote:
>     I also have the same question with Cong, @Marco, can you please show more detail for why not using the solution as above? What is the benefit of this compared to using --host_name=$LIBPROCESS_IP? Thanks.
> 
> Marco Massenzio wrote:
>     `$LIBPROCESS_IP` may not be set by the time we run the binary - it is actually set in the `main()` method (of both Master/Agent) after parsing the `--ip` (and `--ip_discovery_command`) flags.
>     
>     The reason I use `LIBPROCESS_IP` here is to ensure that the IP used by `libprocess` (which is initialized before creating the `Master` or `Slave` object) is consistent with what the `Master/Agent` will be configured with.
>     
>     Please also refer to the conversation about the introduction of `--ip_discovery_command` as to why we need a "dynamic" way of configuring hostname/IP - it all boils down to make Mesos play nice in various Cloud (and Enterprise data centers) where the DNS resolution (and hostname configuration of VMs) is vastly different and may not necessarily conform to the generally assumed scenario, where IP/hostname are kept in sync / resolved by the OS and/or external services (DHCP/DNS).
> 
> Qian Zhang wrote:
>     If you want to set hostname to the chosen IP, then I think you can just use the existing flag "--hostname" (set it to the chosen IP manually). And if "--hostname" is set, then I think the hostname lookup will be bypassed. So why do we need to introduce a new flag "--no_hostname_lookup"?
> 
> Marco Massenzio wrote:
>     Please refer to the conversation on https://issues.apache.org/jira/browse/MESOS-3154 for why this may not be possible in the above-mentioned environments.
>     (and, yes, we do know about the `--hostname` flag - unfortunately, under certain scenarios, it's not possible to set it to a "static" value)

Makes sense for me, can you document it in configuration.md too? Thanks.


- Cong


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38473/#review99628
-----------------------------------------------------------


On Sept. 19, 2015, 1:25 a.m., Marco Massenzio wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38473/
> -----------------------------------------------------------
> 
> (Updated Sept. 19, 2015, 1:25 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Cody Maloney, and Neil Conway.
> 
> 
> Bugs: MESOS-3457
>     https://issues.apache.org/jira/browse/MESOS-3457
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Under certain circumstances, dynamic lookup of hostname, while
> successful, provides undesirable results; we would also like, in
> those circumstances, 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.
> 
> 
> Diffs
> -----
> 
>   docs/configuration.md dd7f4aa806a3c1a8653a0fda9a326a3707308e7c 
>   src/master/flags.hpp e4b1df3f5a33049defff4688463274067f1f1ebf 
>   src/master/flags.cpp 80879611fbcfd764c9fc8f60a31613a9c8fc2364 
>   src/master/master.cpp ca4d5876dcd427964111428edc22d567ddaede0b 
>   src/slave/flags.hpp e31a4183170c3442ac4a15365c229391e7e91480 
>   src/slave/flags.cpp add4196dfd06c0f602ff5ebd39960dc05c4cd11f 
>   src/slave/slave.cpp ad710d7b930a2f115d503ceb8f8fd7421ad30287 
>   src/tests/cluster.hpp 114583de8c867495a2b7a953e6826708838e5d23 
>   src/tests/master_tests.cpp a477794df37c658b80d79dea8555b001415f7b6a 
>   src/tests/mesos.hpp 3db97aca921c9216d90384e1eb17030849516454 
> 
> Diff: https://reviews.apache.org/r/38473/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Marco Massenzio
> 
>


Re: Review Request 38473: Add flag to disable hostname lookup.

Posted by Marco Massenzio <ma...@mesosphere.io>.

> On Sept. 18, 2015, 10:57 p.m., Cong Wang wrote:
> > Why not just set --host_name=$LIBPROCESS_IP for your case since you anyway need to provide a flag?
> 
> Guangya Liu wrote:
>     I also have the same question with Cong, @Marco, can you please show more detail for why not using the solution as above? What is the benefit of this compared to using --host_name=$LIBPROCESS_IP? Thanks.

`$LIBPROCESS_IP` may not be set by the time we run the binary - it is actually set in the `main()` method (of both Master/Agent) after parsing the `--ip` (and `--ip_discovery_command`) flags.

The reason I use `LIBPROCESS_IP` here is to ensure that the IP used by `libprocess` (which is initialized before creating the `Master` or `Slave` object) is consistent with what the `Master/Agent` will be configured with.

Please also refer to the conversation about the introduction of `--ip_discovery_command` as to why we need a "dynamic" way of configuring hostname/IP - it all boils down to make Mesos play nice in various Cloud (and Enterprise data centers) where the DNS resolution (and hostname configuration of VMs) is vastly different and may not necessarily conform to the generally assumed scenario, where IP/hostname are kept in sync / resolved by the OS and/or external services (DHCP/DNS).


- Marco


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38473/#review99628
-----------------------------------------------------------


On Sept. 19, 2015, 1:25 a.m., Marco Massenzio wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38473/
> -----------------------------------------------------------
> 
> (Updated Sept. 19, 2015, 1:25 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Cody Maloney, and Neil Conway.
> 
> 
> Bugs: MESOS-3457
>     https://issues.apache.org/jira/browse/MESOS-3457
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Under certain circumstances, dynamic lookup of hostname, while
> successful, provides undesirable results; we would also like, in
> those circumstances, 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.
> 
> 
> Diffs
> -----
> 
>   docs/configuration.md dd7f4aa806a3c1a8653a0fda9a326a3707308e7c 
>   src/master/flags.hpp e4b1df3f5a33049defff4688463274067f1f1ebf 
>   src/master/flags.cpp 80879611fbcfd764c9fc8f60a31613a9c8fc2364 
>   src/master/master.cpp ca4d5876dcd427964111428edc22d567ddaede0b 
>   src/slave/flags.hpp e31a4183170c3442ac4a15365c229391e7e91480 
>   src/slave/flags.cpp add4196dfd06c0f602ff5ebd39960dc05c4cd11f 
>   src/slave/slave.cpp ad710d7b930a2f115d503ceb8f8fd7421ad30287 
>   src/tests/cluster.hpp 114583de8c867495a2b7a953e6826708838e5d23 
>   src/tests/master_tests.cpp a477794df37c658b80d79dea8555b001415f7b6a 
>   src/tests/mesos.hpp 3db97aca921c9216d90384e1eb17030849516454 
> 
> Diff: https://reviews.apache.org/r/38473/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Marco Massenzio
> 
>


Re: Review Request 38473: Add flag to disable hostname lookup.

Posted by Guangya Liu <gy...@gmail.com>.

> On 九月 18, 2015, 10:57 p.m., Cong Wang wrote:
> > Why not just set --host_name=$LIBPROCESS_IP for your case since you anyway need to provide a flag?

I also have the same question with Cong, @Marco, can you please show more detail for why not using the solution as above? What is the benefit of this compared to using --host_name=$LIBPROCESS_IP? Thanks.


- Guangya


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38473/#review99628
-----------------------------------------------------------


On 九月 19, 2015, 1:25 a.m., Marco Massenzio wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38473/
> -----------------------------------------------------------
> 
> (Updated 九月 19, 2015, 1:25 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Cody Maloney, and Neil Conway.
> 
> 
> Bugs: MESOS-3457
>     https://issues.apache.org/jira/browse/MESOS-3457
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Under certain circumstances, dynamic lookup of hostname, while
> successful, provides undesirable results; we would also like, in
> those circumstances, 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.
> 
> 
> Diffs
> -----
> 
>   docs/configuration.md dd7f4aa806a3c1a8653a0fda9a326a3707308e7c 
>   src/master/flags.hpp e4b1df3f5a33049defff4688463274067f1f1ebf 
>   src/master/flags.cpp 80879611fbcfd764c9fc8f60a31613a9c8fc2364 
>   src/master/master.cpp ca4d5876dcd427964111428edc22d567ddaede0b 
>   src/slave/flags.hpp e31a4183170c3442ac4a15365c229391e7e91480 
>   src/slave/flags.cpp add4196dfd06c0f602ff5ebd39960dc05c4cd11f 
>   src/slave/slave.cpp ad710d7b930a2f115d503ceb8f8fd7421ad30287 
>   src/tests/cluster.hpp 114583de8c867495a2b7a953e6826708838e5d23 
>   src/tests/master_tests.cpp a477794df37c658b80d79dea8555b001415f7b6a 
>   src/tests/mesos.hpp 3db97aca921c9216d90384e1eb17030849516454 
> 
> Diff: https://reviews.apache.org/r/38473/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Marco Massenzio
> 
>


Re: Review Request 38473: Add flag to disable hostname lookup.

Posted by Qian Zhang <zh...@cn.ibm.com>.

> On Sept. 19, 2015, 6:57 a.m., Cong Wang wrote:
> > Why not just set --host_name=$LIBPROCESS_IP for your case since you anyway need to provide a flag?
> 
> Guangya Liu wrote:
>     I also have the same question with Cong, @Marco, can you please show more detail for why not using the solution as above? What is the benefit of this compared to using --host_name=$LIBPROCESS_IP? Thanks.
> 
> Marco Massenzio wrote:
>     `$LIBPROCESS_IP` may not be set by the time we run the binary - it is actually set in the `main()` method (of both Master/Agent) after parsing the `--ip` (and `--ip_discovery_command`) flags.
>     
>     The reason I use `LIBPROCESS_IP` here is to ensure that the IP used by `libprocess` (which is initialized before creating the `Master` or `Slave` object) is consistent with what the `Master/Agent` will be configured with.
>     
>     Please also refer to the conversation about the introduction of `--ip_discovery_command` as to why we need a "dynamic" way of configuring hostname/IP - it all boils down to make Mesos play nice in various Cloud (and Enterprise data centers) where the DNS resolution (and hostname configuration of VMs) is vastly different and may not necessarily conform to the generally assumed scenario, where IP/hostname are kept in sync / resolved by the OS and/or external services (DHCP/DNS).

If you want to set hostname to the chosen IP, then I think you can just use the existing flag "--hostname" (set it to the chosen IP manually). And if "--hostname" is set, then I think the hostname lookup will be bypassed. So why do we need to introduce a new flag "--no_hostname_lookup"?


- Qian


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38473/#review99628
-----------------------------------------------------------


On Sept. 19, 2015, 9:25 a.m., Marco Massenzio wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38473/
> -----------------------------------------------------------
> 
> (Updated Sept. 19, 2015, 9:25 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Cody Maloney, and Neil Conway.
> 
> 
> Bugs: MESOS-3457
>     https://issues.apache.org/jira/browse/MESOS-3457
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Under certain circumstances, dynamic lookup of hostname, while
> successful, provides undesirable results; we would also like, in
> those circumstances, 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.
> 
> 
> Diffs
> -----
> 
>   docs/configuration.md dd7f4aa806a3c1a8653a0fda9a326a3707308e7c 
>   src/master/flags.hpp e4b1df3f5a33049defff4688463274067f1f1ebf 
>   src/master/flags.cpp 80879611fbcfd764c9fc8f60a31613a9c8fc2364 
>   src/master/master.cpp ca4d5876dcd427964111428edc22d567ddaede0b 
>   src/slave/flags.hpp e31a4183170c3442ac4a15365c229391e7e91480 
>   src/slave/flags.cpp add4196dfd06c0f602ff5ebd39960dc05c4cd11f 
>   src/slave/slave.cpp ad710d7b930a2f115d503ceb8f8fd7421ad30287 
>   src/tests/cluster.hpp 114583de8c867495a2b7a953e6826708838e5d23 
>   src/tests/master_tests.cpp a477794df37c658b80d79dea8555b001415f7b6a 
>   src/tests/mesos.hpp 3db97aca921c9216d90384e1eb17030849516454 
> 
> Diff: https://reviews.apache.org/r/38473/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Marco Massenzio
> 
>


Re: Review Request 38473: Add flag to disable hostname lookup.

Posted by Marco Massenzio <ma...@mesosphere.io>.

> On Sept. 18, 2015, 10:57 p.m., Cong Wang wrote:
> > Why not just set --host_name=$LIBPROCESS_IP for your case since you anyway need to provide a flag?
> 
> Guangya Liu wrote:
>     I also have the same question with Cong, @Marco, can you please show more detail for why not using the solution as above? What is the benefit of this compared to using --host_name=$LIBPROCESS_IP? Thanks.
> 
> Marco Massenzio wrote:
>     `$LIBPROCESS_IP` may not be set by the time we run the binary - it is actually set in the `main()` method (of both Master/Agent) after parsing the `--ip` (and `--ip_discovery_command`) flags.
>     
>     The reason I use `LIBPROCESS_IP` here is to ensure that the IP used by `libprocess` (which is initialized before creating the `Master` or `Slave` object) is consistent with what the `Master/Agent` will be configured with.
>     
>     Please also refer to the conversation about the introduction of `--ip_discovery_command` as to why we need a "dynamic" way of configuring hostname/IP - it all boils down to make Mesos play nice in various Cloud (and Enterprise data centers) where the DNS resolution (and hostname configuration of VMs) is vastly different and may not necessarily conform to the generally assumed scenario, where IP/hostname are kept in sync / resolved by the OS and/or external services (DHCP/DNS).
> 
> Qian Zhang wrote:
>     If you want to set hostname to the chosen IP, then I think you can just use the existing flag "--hostname" (set it to the chosen IP manually). And if "--hostname" is set, then I think the hostname lookup will be bypassed. So why do we need to introduce a new flag "--no_hostname_lookup"?

Please refer to the conversation on https://issues.apache.org/jira/browse/MESOS-3154 for why this may not be possible in the above-mentioned environments.
(and, yes, we do know about the `--hostname` flag - unfortunately, under certain scenarios, it's not possible to set it to a "static" value)


- Marco


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38473/#review99628
-----------------------------------------------------------


On Sept. 19, 2015, 1:25 a.m., Marco Massenzio wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38473/
> -----------------------------------------------------------
> 
> (Updated Sept. 19, 2015, 1:25 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Cody Maloney, and Neil Conway.
> 
> 
> Bugs: MESOS-3457
>     https://issues.apache.org/jira/browse/MESOS-3457
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Under certain circumstances, dynamic lookup of hostname, while
> successful, provides undesirable results; we would also like, in
> those circumstances, 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.
> 
> 
> Diffs
> -----
> 
>   docs/configuration.md dd7f4aa806a3c1a8653a0fda9a326a3707308e7c 
>   src/master/flags.hpp e4b1df3f5a33049defff4688463274067f1f1ebf 
>   src/master/flags.cpp 80879611fbcfd764c9fc8f60a31613a9c8fc2364 
>   src/master/master.cpp ca4d5876dcd427964111428edc22d567ddaede0b 
>   src/slave/flags.hpp e31a4183170c3442ac4a15365c229391e7e91480 
>   src/slave/flags.cpp add4196dfd06c0f602ff5ebd39960dc05c4cd11f 
>   src/slave/slave.cpp ad710d7b930a2f115d503ceb8f8fd7421ad30287 
>   src/tests/cluster.hpp 114583de8c867495a2b7a953e6826708838e5d23 
>   src/tests/master_tests.cpp a477794df37c658b80d79dea8555b001415f7b6a 
>   src/tests/mesos.hpp 3db97aca921c9216d90384e1eb17030849516454 
> 
> Diff: https://reviews.apache.org/r/38473/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Marco Massenzio
> 
>


Re: Review Request 38473: Add flag to disable hostname lookup.

Posted by Marco Massenzio <ma...@mesosphere.io>.

> On Sept. 18, 2015, 10:57 p.m., Cong Wang wrote:
> > Why not just set --host_name=$LIBPROCESS_IP for your case since you anyway need to provide a flag?
> 
> Guangya Liu wrote:
>     I also have the same question with Cong, @Marco, can you please show more detail for why not using the solution as above? What is the benefit of this compared to using --host_name=$LIBPROCESS_IP? Thanks.
> 
> Marco Massenzio wrote:
>     `$LIBPROCESS_IP` may not be set by the time we run the binary - it is actually set in the `main()` method (of both Master/Agent) after parsing the `--ip` (and `--ip_discovery_command`) flags.
>     
>     The reason I use `LIBPROCESS_IP` here is to ensure that the IP used by `libprocess` (which is initialized before creating the `Master` or `Slave` object) is consistent with what the `Master/Agent` will be configured with.
>     
>     Please also refer to the conversation about the introduction of `--ip_discovery_command` as to why we need a "dynamic" way of configuring hostname/IP - it all boils down to make Mesos play nice in various Cloud (and Enterprise data centers) where the DNS resolution (and hostname configuration of VMs) is vastly different and may not necessarily conform to the generally assumed scenario, where IP/hostname are kept in sync / resolved by the OS and/or external services (DHCP/DNS).
> 
> Qian Zhang wrote:
>     If you want to set hostname to the chosen IP, then I think you can just use the existing flag "--hostname" (set it to the chosen IP manually). And if "--hostname" is set, then I think the hostname lookup will be bypassed. So why do we need to introduce a new flag "--no_hostname_lookup"?
> 
> Marco Massenzio wrote:
>     Please refer to the conversation on https://issues.apache.org/jira/browse/MESOS-3154 for why this may not be possible in the above-mentioned environments.
>     (and, yes, we do know about the `--hostname` flag - unfortunately, under certain scenarios, it's not possible to set it to a "static" value)
> 
> Cong Wang wrote:
>     Makes sense for me, can you document it in configuration.md too? Thanks.

>From what I've seen, configuration.md is essentially a verbatim list of what is emitted by --help, not sure what more to add there; the `--no-hostname_lookup` flag will be a no-op for anyone who doesn't know how to use - and completely obvious to those (such our SRE team) who need it :)

But I will definitely add this to the Jira (which will be referenced to by this commit) so that folks can understand where did this come from, no problem!


- Marco


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38473/#review99628
-----------------------------------------------------------


On Sept. 19, 2015, 1:25 a.m., Marco Massenzio wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38473/
> -----------------------------------------------------------
> 
> (Updated Sept. 19, 2015, 1:25 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Cody Maloney, and Neil Conway.
> 
> 
> Bugs: MESOS-3457
>     https://issues.apache.org/jira/browse/MESOS-3457
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Under certain circumstances, dynamic lookup of hostname, while
> successful, provides undesirable results; we would also like, in
> those circumstances, 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.
> 
> 
> Diffs
> -----
> 
>   docs/configuration.md dd7f4aa806a3c1a8653a0fda9a326a3707308e7c 
>   src/master/flags.hpp e4b1df3f5a33049defff4688463274067f1f1ebf 
>   src/master/flags.cpp 80879611fbcfd764c9fc8f60a31613a9c8fc2364 
>   src/master/master.cpp ca4d5876dcd427964111428edc22d567ddaede0b 
>   src/slave/flags.hpp e31a4183170c3442ac4a15365c229391e7e91480 
>   src/slave/flags.cpp add4196dfd06c0f602ff5ebd39960dc05c4cd11f 
>   src/slave/slave.cpp ad710d7b930a2f115d503ceb8f8fd7421ad30287 
>   src/tests/cluster.hpp 114583de8c867495a2b7a953e6826708838e5d23 
>   src/tests/master_tests.cpp a477794df37c658b80d79dea8555b001415f7b6a 
>   src/tests/mesos.hpp 3db97aca921c9216d90384e1eb17030849516454 
> 
> Diff: https://reviews.apache.org/r/38473/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Marco Massenzio
> 
>


Re: Review Request 38473: Add flag to disable hostname lookup.

Posted by Cong Wang <cw...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38473/#review99628
-----------------------------------------------------------


Why not just set --host_name=$LIBPROCESS_IP for your case since you anyway need to provide a flag?

- Cong Wang


On Sept. 18, 2015, 9:53 p.m., Marco Massenzio wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38473/
> -----------------------------------------------------------
> 
> (Updated Sept. 18, 2015, 9:53 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Cody Maloney, and Neil Conway.
> 
> 
> Bugs: MESOS-3457
>     https://issues.apache.org/jira/browse/MESOS-3457
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Under certain circumstances, dynamic lookup of hostname, while
> successful, provides undesirable results; we would also like, in
> those circumstances, 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.
> 
> 
> Diffs
> -----
> 
>   docs/configuration.md dd7f4aa806a3c1a8653a0fda9a326a3707308e7c 
>   src/master/flags.hpp e4b1df3f5a33049defff4688463274067f1f1ebf 
>   src/master/flags.cpp 80879611fbcfd764c9fc8f60a31613a9c8fc2364 
>   src/master/master.cpp ca4d5876dcd427964111428edc22d567ddaede0b 
>   src/slave/flags.hpp e31a4183170c3442ac4a15365c229391e7e91480 
>   src/slave/flags.cpp add4196dfd06c0f602ff5ebd39960dc05c4cd11f 
>   src/slave/slave.cpp ad710d7b930a2f115d503ceb8f8fd7421ad30287 
>   src/tests/cluster.hpp 114583de8c867495a2b7a953e6826708838e5d23 
>   src/tests/master_tests.cpp a477794df37c658b80d79dea8555b001415f7b6a 
>   src/tests/mesos.hpp 3db97aca921c9216d90384e1eb17030849516454 
> 
> Diff: https://reviews.apache.org/r/38473/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Marco Massenzio
> 
>


Re: Review Request 38473: Add flag to disable hostname lookup.

Posted by Neil Conway <ne...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38473/#review99616
-----------------------------------------------------------

Ship it!


Ship It!

- Neil Conway


On Sept. 18, 2015, 9:53 p.m., Marco Massenzio wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38473/
> -----------------------------------------------------------
> 
> (Updated Sept. 18, 2015, 9:53 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Cody Maloney, and Neil Conway.
> 
> 
> Bugs: MESOS-3457
>     https://issues.apache.org/jira/browse/MESOS-3457
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Under certain circumstances, dynamic lookup of hostname, while
> successful, provides undesirable results; we would also like, in
> those circumstances, 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.
> 
> 
> Diffs
> -----
> 
>   docs/configuration.md dd7f4aa806a3c1a8653a0fda9a326a3707308e7c 
>   src/master/flags.hpp e4b1df3f5a33049defff4688463274067f1f1ebf 
>   src/master/flags.cpp 80879611fbcfd764c9fc8f60a31613a9c8fc2364 
>   src/master/master.cpp ca4d5876dcd427964111428edc22d567ddaede0b 
>   src/slave/flags.hpp e31a4183170c3442ac4a15365c229391e7e91480 
>   src/slave/flags.cpp add4196dfd06c0f602ff5ebd39960dc05c4cd11f 
>   src/slave/slave.cpp ad710d7b930a2f115d503ceb8f8fd7421ad30287 
>   src/tests/cluster.hpp 114583de8c867495a2b7a953e6826708838e5d23 
>   src/tests/master_tests.cpp a477794df37c658b80d79dea8555b001415f7b6a 
>   src/tests/mesos.hpp 3db97aca921c9216d90384e1eb17030849516454 
> 
> Diff: https://reviews.apache.org/r/38473/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Marco Massenzio
> 
>


Re: Review Request 38473: Add flag to disable hostname lookup.

Posted by Marco Massenzio <ma...@mesosphere.io>.

> On Sept. 21, 2015, 7:39 p.m., Benjamin Hindman wrote:
> > src/master/master.cpp, line 345
> > <https://reviews.apache.org/r/38473/diff/3/?file=1077335#file1077335line345>
> >
> >     Why are you reading LIBPROCESS_IP here? Why not `flags.ip`? There appears to be a very non-obvious global invariant and I'd like to capture the dependency here as a comment (for posterity!) as well as where we're setting LIBPROCESS_IP in the main.cpp files.

I've added the following comment, please let me know if (a) I'm getting this wrong and (b) whether it requires a more detailed explanation (but the underlying rationale is - libprocess will use that (and not the flags) and we need to be consistent with it).
```
      // We need to use LIBPROCESS_IP here, instead of 'flags.ip' because the
      // latter may not have been set, and the IP may have been set by other
      // means (eg, auto-discovery; the --ip_discovery_command) and we need to
      // be consistent with what 'libprocess' is initialized with.
```


> On Sept. 21, 2015, 7:39 p.m., Benjamin Hindman wrote:
> > src/master/master.cpp, line 349
> > <https://reviews.apache.org/r/38473/diff/3/?file=1077335#file1077335line349>
> >
> >     In circumstances where a user can easily avoid the master from crashing we prefer NOT to use LOG(FATAL) because it prints a stack trace which can hide the actual error message. Instead, an EXIT(EXIT_FAILURE) is a good thing to use here.
> >     
> >     Same for the LOG(FATAL) in the slave below.

ah! I was trying to be "consistent" with the (existing) code above.
Would you like me to change that one too?


> On Sept. 21, 2015, 7:39 p.m., Benjamin Hindman wrote:
> > src/tests/master_tests.cpp, line 1121
> > <https://reviews.apache.org/r/38473/diff/3/?file=1077340#file1077340line1121>
> >
> >     Why not just do `cluster.find`? Not sure I understand the need for this alias.

You are right, no need - I started out adding this, then eventually gone down the rabbit hole of same-named classes and hadn't realized that I could actually get rid of the alias.

It's gone.


> On Sept. 21, 2015, 7:39 p.m., Benjamin Hindman wrote:
> > src/tests/master_tests.cpp, lines 1122-1123
> > <https://reviews.apache.org/r/38473/diff/3/?file=1077340#file1077340line1122>
> >
> >     What does someone running the test get from this extra output information?

Mostly the hostname that was set up and then not found via the UPID lookup - it may help debug weird network configuration issues and/or situations where we have broken the hostname lookup/configuration.
(I think? I like to add extra info when tests go wrong, so as to take out (some of) the guesswork from the poor soul who'll have to fix my code many months/years in the future... I believe in karma :) )

Anyways, figured out that (a) this needs to be an `ASSERT_EQ` (as we `master.get()` in the following EXPECT) and (b) the comment here may actually be unnecessary, so I've removed it.


- Marco


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38473/#review99812
-----------------------------------------------------------


On Sept. 19, 2015, 1:25 a.m., Marco Massenzio wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38473/
> -----------------------------------------------------------
> 
> (Updated Sept. 19, 2015, 1:25 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Cody Maloney, and Neil Conway.
> 
> 
> Bugs: MESOS-3457
>     https://issues.apache.org/jira/browse/MESOS-3457
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Under certain circumstances, dynamic lookup of hostname, while
> successful, provides undesirable results; we would also like, in
> those circumstances, 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.
> 
> 
> Diffs
> -----
> 
>   docs/configuration.md dd7f4aa806a3c1a8653a0fda9a326a3707308e7c 
>   src/master/flags.hpp e4b1df3f5a33049defff4688463274067f1f1ebf 
>   src/master/flags.cpp 80879611fbcfd764c9fc8f60a31613a9c8fc2364 
>   src/master/master.cpp ca4d5876dcd427964111428edc22d567ddaede0b 
>   src/slave/flags.hpp e31a4183170c3442ac4a15365c229391e7e91480 
>   src/slave/flags.cpp add4196dfd06c0f602ff5ebd39960dc05c4cd11f 
>   src/slave/slave.cpp ad710d7b930a2f115d503ceb8f8fd7421ad30287 
>   src/tests/cluster.hpp 114583de8c867495a2b7a953e6826708838e5d23 
>   src/tests/master_tests.cpp a477794df37c658b80d79dea8555b001415f7b6a 
>   src/tests/mesos.hpp 3db97aca921c9216d90384e1eb17030849516454 
> 
> Diff: https://reviews.apache.org/r/38473/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Marco Massenzio
> 
>


Re: Review Request 38473: Add flag to disable hostname lookup.

Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38473/#review99812
-----------------------------------------------------------



src/master/master.cpp (line 345)
<https://reviews.apache.org/r/38473/#comment156781>

    Why are you reading LIBPROCESS_IP here? Why not `flags.ip`? There appears to be a very non-obvious global invariant and I'd like to capture the dependency here as a comment (for posterity!) as well as where we're setting LIBPROCESS_IP in the main.cpp files.



src/master/master.cpp (line 349)
<https://reviews.apache.org/r/38473/#comment156780>

    In circumstances where a user can easily avoid the master from crashing we prefer NOT to use LOG(FATAL) because it prints a stack trace which can hide the actual error message. Instead, an EXIT(EXIT_FAILURE) is a good thing to use here.
    
    Same for the LOG(FATAL) in the slave below.



src/tests/cluster.hpp (line 126)
<https://reviews.apache.org/r/38473/#comment156782>

    Thank you for the doxygen comments!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!



src/tests/master_tests.cpp (line 1098)
<https://reviews.apache.org/r/38473/#comment156777>

    This should be virtual because you're extending `MasterTest`.



src/tests/master_tests.cpp (line 1121)
<https://reviews.apache.org/r/38473/#comment156783>

    Why not just do `cluster.find`? Not sure I understand the need for this alias.



src/tests/master_tests.cpp (lines 1122 - 1123)
<https://reviews.apache.org/r/38473/#comment156784>

    What does someone running the test get from this extra output information?



src/tests/master_tests.cpp (line 1126)
<https://reviews.apache.org/r/38473/#comment156785>

    Indention.


- Benjamin Hindman


On Sept. 19, 2015, 1:25 a.m., Marco Massenzio wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38473/
> -----------------------------------------------------------
> 
> (Updated Sept. 19, 2015, 1:25 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Cody Maloney, and Neil Conway.
> 
> 
> Bugs: MESOS-3457
>     https://issues.apache.org/jira/browse/MESOS-3457
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Under certain circumstances, dynamic lookup of hostname, while
> successful, provides undesirable results; we would also like, in
> those circumstances, 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.
> 
> 
> Diffs
> -----
> 
>   docs/configuration.md dd7f4aa806a3c1a8653a0fda9a326a3707308e7c 
>   src/master/flags.hpp e4b1df3f5a33049defff4688463274067f1f1ebf 
>   src/master/flags.cpp 80879611fbcfd764c9fc8f60a31613a9c8fc2364 
>   src/master/master.cpp ca4d5876dcd427964111428edc22d567ddaede0b 
>   src/slave/flags.hpp e31a4183170c3442ac4a15365c229391e7e91480 
>   src/slave/flags.cpp add4196dfd06c0f602ff5ebd39960dc05c4cd11f 
>   src/slave/slave.cpp ad710d7b930a2f115d503ceb8f8fd7421ad30287 
>   src/tests/cluster.hpp 114583de8c867495a2b7a953e6826708838e5d23 
>   src/tests/master_tests.cpp a477794df37c658b80d79dea8555b001415f7b6a 
>   src/tests/mesos.hpp 3db97aca921c9216d90384e1eb17030849516454 
> 
> Diff: https://reviews.apache.org/r/38473/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Marco Massenzio
> 
>


Re: Review Request 38473: Add flag to disable hostname lookup.

Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38473/#review99658
-----------------------------------------------------------


Patch looks great!

Reviews applied: [38473]

All tests passed.

- Mesos ReviewBot


On Sept. 19, 2015, 1:25 a.m., Marco Massenzio wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38473/
> -----------------------------------------------------------
> 
> (Updated Sept. 19, 2015, 1:25 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Cody Maloney, and Neil Conway.
> 
> 
> Bugs: MESOS-3457
>     https://issues.apache.org/jira/browse/MESOS-3457
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Under certain circumstances, dynamic lookup of hostname, while
> successful, provides undesirable results; we would also like, in
> those circumstances, 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.
> 
> 
> Diffs
> -----
> 
>   docs/configuration.md dd7f4aa806a3c1a8653a0fda9a326a3707308e7c 
>   src/master/flags.hpp e4b1df3f5a33049defff4688463274067f1f1ebf 
>   src/master/flags.cpp 80879611fbcfd764c9fc8f60a31613a9c8fc2364 
>   src/master/master.cpp ca4d5876dcd427964111428edc22d567ddaede0b 
>   src/slave/flags.hpp e31a4183170c3442ac4a15365c229391e7e91480 
>   src/slave/flags.cpp add4196dfd06c0f602ff5ebd39960dc05c4cd11f 
>   src/slave/slave.cpp ad710d7b930a2f115d503ceb8f8fd7421ad30287 
>   src/tests/cluster.hpp 114583de8c867495a2b7a953e6826708838e5d23 
>   src/tests/master_tests.cpp a477794df37c658b80d79dea8555b001415f7b6a 
>   src/tests/mesos.hpp 3db97aca921c9216d90384e1eb17030849516454 
> 
> Diff: https://reviews.apache.org/r/38473/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Marco Massenzio
> 
>


Re: Review Request 38473: Add flag to disable hostname lookup.

Posted by Marco Massenzio <ma...@mesosphere.io>.

> On Sept. 23, 2015, 10:01 p.m., Benjamin Hindman wrote:
> > src/tests/master_tests.cpp, line 1127
> > <https://reviews.apache.org/r/38473/diff/5/?file=1079528#file1079528line1127>
> >
> >     I'm not sure what this tests for and if it doesn't actually introduce a potential source of flakiness? If we keep this, it needs a comment for why we are doing it.

removed - it was there because I was doing a `get()` on it.


> On Sept. 23, 2015, 10:01 p.m., Benjamin Hindman wrote:
> > src/tests/master_tests.cpp, line 1137
> > <https://reviews.apache.org/r/38473/diff/5/?file=1079528#file1079528line1137>
> >
> >     This shouldn't be here right?

this test has been entirely removed - this code path will never be exercised in the new code.


> On Sept. 23, 2015, 10:01 p.m., Benjamin Hindman wrote:
> > src/tests/master_tests.cpp, line 1143
> > <https://reviews.apache.org/r/38473/diff/5/?file=1079528#file1079528line1143>
> >
> >     This test shouldn't fail should it? I think this entire test is no longer valid.

I have a personal ToDO to discover more about EXPECT_EXIT() and its semantics.


- Marco


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38473/#review100296
-----------------------------------------------------------


On Sept. 22, 2015, 12:22 a.m., Marco Massenzio wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38473/
> -----------------------------------------------------------
> 
> (Updated Sept. 22, 2015, 12:22 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Cody Maloney, and Neil Conway.
> 
> 
> Bugs: MESOS-3457
>     https://issues.apache.org/jira/browse/MESOS-3457
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Under certain circumstances, dynamic lookup of hostname, while
> successful, provides undesirable results; we would also like, in
> those circumstances, 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.
> 
> 
> Diffs
> -----
> 
>   docs/configuration.md dd7f4aa806a3c1a8653a0fda9a326a3707308e7c 
>   src/master/flags.hpp e4b1df3f5a33049defff4688463274067f1f1ebf 
>   src/master/flags.cpp 80879611fbcfd764c9fc8f60a31613a9c8fc2364 
>   src/master/master.cpp 90ef8c663c90ffbdcb4aa2377bfba65ea5d3fda9 
>   src/slave/flags.hpp e31a4183170c3442ac4a15365c229391e7e91480 
>   src/slave/flags.cpp add4196dfd06c0f602ff5ebd39960dc05c4cd11f 
>   src/slave/slave.cpp ad710d7b930a2f115d503ceb8f8fd7421ad30287 
>   src/tests/cluster.hpp 114583de8c867495a2b7a953e6826708838e5d23 
>   src/tests/master_tests.cpp f26344d39543f65f2b0a94b8ff566836c8256bf7 
> 
> Diff: https://reviews.apache.org/r/38473/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Marco Massenzio
> 
>


Re: Review Request 38473: Add flag to disable hostname lookup.

Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38473/#review100296
-----------------------------------------------------------

Ship it!



src/slave/slave.cpp (line 375)
<https://reviews.apache.org/r/38473/#comment157469>

    Shouldn't this be doing what the master is doing as well and just setting the hostname to the stringified IP?



src/tests/cluster.hpp (line 134)
<https://reviews.apache.org/r/38473/#comment157460>

    Why is the variable called 'upid' instead of just 'pid'? This is not actually a `UPID`. Also, why not just call this 'find' like you did for the function below? It seems weird to ever need to do `masters.findMaster(...)`, you'd just want `master.find(...)`.



src/tests/master_tests.cpp (lines 1091 - 1093)
<https://reviews.apache.org/r/38473/#comment157459>

    Unnecessary code.



src/tests/master_tests.cpp (line 1102)
<https://reviews.apache.org/r/38473/#comment157461>

    s/upid/pid/g



src/tests/master_tests.cpp (lines 1108 - 1109)
<https://reviews.apache.org/r/38473/#comment157464>

    Aren't you checking 'master.get()->info().hostname()' and expecting 'pid.get().address.hostname()'? In otherwords, aren't these in the wrong ordering?



src/tests/master_tests.cpp (line 1124)
<https://reviews.apache.org/r/38473/#comment157463>

    Newline here like you did in test above please.



src/tests/master_tests.cpp (line 1127)
<https://reviews.apache.org/r/38473/#comment157465>

    I'm not sure what this tests for and if it doesn't actually introduce a potential source of flakiness? If we keep this, it needs a comment for why we are doing it.



src/tests/master_tests.cpp (line 1137)
<https://reviews.apache.org/r/38473/#comment157466>

    This shouldn't be here right?



src/tests/master_tests.cpp (line 1143)
<https://reviews.apache.org/r/38473/#comment157467>

    This test shouldn't fail should it? I think this entire test is no longer valid.


- Benjamin Hindman


On Sept. 22, 2015, 12:22 a.m., Marco Massenzio wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38473/
> -----------------------------------------------------------
> 
> (Updated Sept. 22, 2015, 12:22 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Cody Maloney, and Neil Conway.
> 
> 
> Bugs: MESOS-3457
>     https://issues.apache.org/jira/browse/MESOS-3457
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Under certain circumstances, dynamic lookup of hostname, while
> successful, provides undesirable results; we would also like, in
> those circumstances, 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.
> 
> 
> Diffs
> -----
> 
>   docs/configuration.md dd7f4aa806a3c1a8653a0fda9a326a3707308e7c 
>   src/master/flags.hpp e4b1df3f5a33049defff4688463274067f1f1ebf 
>   src/master/flags.cpp 80879611fbcfd764c9fc8f60a31613a9c8fc2364 
>   src/master/master.cpp 90ef8c663c90ffbdcb4aa2377bfba65ea5d3fda9 
>   src/slave/flags.hpp e31a4183170c3442ac4a15365c229391e7e91480 
>   src/slave/flags.cpp add4196dfd06c0f602ff5ebd39960dc05c4cd11f 
>   src/slave/slave.cpp ad710d7b930a2f115d503ceb8f8fd7421ad30287 
>   src/tests/cluster.hpp 114583de8c867495a2b7a953e6826708838e5d23 
>   src/tests/master_tests.cpp f26344d39543f65f2b0a94b8ff566836c8256bf7 
> 
> Diff: https://reviews.apache.org/r/38473/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Marco Massenzio
> 
>


Re: Review Request 38473: Add flag to disable hostname lookup.

Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38473/#review99912
-----------------------------------------------------------


Patch looks great!

Reviews applied: [38473]

All tests passed.

- Mesos ReviewBot


On Sept. 22, 2015, 12:22 a.m., Marco Massenzio wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38473/
> -----------------------------------------------------------
> 
> (Updated Sept. 22, 2015, 12:22 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Cody Maloney, and Neil Conway.
> 
> 
> Bugs: MESOS-3457
>     https://issues.apache.org/jira/browse/MESOS-3457
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Under certain circumstances, dynamic lookup of hostname, while
> successful, provides undesirable results; we would also like, in
> those circumstances, 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.
> 
> 
> Diffs
> -----
> 
>   docs/configuration.md dd7f4aa806a3c1a8653a0fda9a326a3707308e7c 
>   src/master/flags.hpp e4b1df3f5a33049defff4688463274067f1f1ebf 
>   src/master/flags.cpp 80879611fbcfd764c9fc8f60a31613a9c8fc2364 
>   src/master/master.cpp 90ef8c663c90ffbdcb4aa2377bfba65ea5d3fda9 
>   src/slave/flags.hpp e31a4183170c3442ac4a15365c229391e7e91480 
>   src/slave/flags.cpp add4196dfd06c0f602ff5ebd39960dc05c4cd11f 
>   src/slave/slave.cpp ad710d7b930a2f115d503ceb8f8fd7421ad30287 
>   src/tests/cluster.hpp 114583de8c867495a2b7a953e6826708838e5d23 
>   src/tests/master_tests.cpp f26344d39543f65f2b0a94b8ff566836c8256bf7 
> 
> Diff: https://reviews.apache.org/r/38473/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Marco Massenzio
> 
>


Re: Review Request 38473: Add flag to disable hostname lookup.

Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38473/#review100340
-----------------------------------------------------------


Patch looks great!

Reviews applied: [38473]

All tests passed.

- Mesos ReviewBot


On Sept. 23, 2015, 11:10 p.m., Marco Massenzio wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38473/
> -----------------------------------------------------------
> 
> (Updated Sept. 23, 2015, 11:10 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Cody Maloney, and Neil Conway.
> 
> 
> Bugs: MESOS-3457
>     https://issues.apache.org/jira/browse/MESOS-3457
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Under certain circumstances, dynamic lookup of hostname, while
> successful, provides undesirable results; we would also like, in
> those circumstances, 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.
> 
> 
> Diffs
> -----
> 
>   docs/configuration.md dd7f4aa806a3c1a8653a0fda9a326a3707308e7c 
>   src/master/flags.hpp e4b1df3f5a33049defff4688463274067f1f1ebf 
>   src/master/flags.cpp 80879611fbcfd764c9fc8f60a31613a9c8fc2364 
>   src/master/master.cpp 0a40bc3509d56cd8824a1eb74989f11c7f7c038b 
>   src/slave/flags.hpp b8d6bb4df4bbe0bac72d4b218ec38ad23fa56214 
>   src/slave/flags.cpp 6164b4bae3f1b74da87f01a6db934f265e1a0117 
>   src/slave/slave.cpp 29865ece00fa8bff3054a7f8c87cbf93403405db 
>   src/tests/cluster.hpp 114583de8c867495a2b7a953e6826708838e5d23 
>   src/tests/master_tests.cpp f26344d39543f65f2b0a94b8ff566836c8256bf7 
> 
> Diff: https://reviews.apache.org/r/38473/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Marco Massenzio
> 
>


Re: Review Request 38473: Add flag to disable hostname lookup.

Posted by Marco Massenzio <ma...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38473/
-----------------------------------------------------------

(Updated Sept. 23, 2015, 11:10 p.m.)


Review request for mesos, Benjamin Hindman, Cody Maloney, and Neil Conway.


Changes
-------

Addressed benh comments


Bugs: MESOS-3457
    https://issues.apache.org/jira/browse/MESOS-3457


Repository: mesos


Description
-------

Under certain circumstances, dynamic lookup of hostname, while
successful, provides undesirable results; we would also like, in
those circumstances, 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.


Diffs (updated)
-----

  docs/configuration.md dd7f4aa806a3c1a8653a0fda9a326a3707308e7c 
  src/master/flags.hpp e4b1df3f5a33049defff4688463274067f1f1ebf 
  src/master/flags.cpp 80879611fbcfd764c9fc8f60a31613a9c8fc2364 
  src/master/master.cpp 0a40bc3509d56cd8824a1eb74989f11c7f7c038b 
  src/slave/flags.hpp b8d6bb4df4bbe0bac72d4b218ec38ad23fa56214 
  src/slave/flags.cpp 6164b4bae3f1b74da87f01a6db934f265e1a0117 
  src/slave/slave.cpp 29865ece00fa8bff3054a7f8c87cbf93403405db 
  src/tests/cluster.hpp 114583de8c867495a2b7a953e6826708838e5d23 
  src/tests/master_tests.cpp f26344d39543f65f2b0a94b8ff566836c8256bf7 

Diff: https://reviews.apache.org/r/38473/diff/


Testing
-------

make check


Thanks,

Marco Massenzio


Re: Review Request 38473: Add flag to disable hostname lookup.

Posted by Marco Massenzio <ma...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38473/
-----------------------------------------------------------

(Updated Sept. 22, 2015, 12:22 a.m.)


Review request for mesos, Benjamin Hindman, Cody Maloney, and Neil Conway.


Changes
-------

Removed dependency from LIBPROCESS_IP global invariant, after conversation with benh.
Using instead the Master::info::ip


Bugs: MESOS-3457
    https://issues.apache.org/jira/browse/MESOS-3457


Repository: mesos


Description
-------

Under certain circumstances, dynamic lookup of hostname, while
successful, provides undesirable results; we would also like, in
those circumstances, 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.


Diffs (updated)
-----

  docs/configuration.md dd7f4aa806a3c1a8653a0fda9a326a3707308e7c 
  src/master/flags.hpp e4b1df3f5a33049defff4688463274067f1f1ebf 
  src/master/flags.cpp 80879611fbcfd764c9fc8f60a31613a9c8fc2364 
  src/master/master.cpp 90ef8c663c90ffbdcb4aa2377bfba65ea5d3fda9 
  src/slave/flags.hpp e31a4183170c3442ac4a15365c229391e7e91480 
  src/slave/flags.cpp add4196dfd06c0f602ff5ebd39960dc05c4cd11f 
  src/slave/slave.cpp ad710d7b930a2f115d503ceb8f8fd7421ad30287 
  src/tests/cluster.hpp 114583de8c867495a2b7a953e6826708838e5d23 
  src/tests/master_tests.cpp f26344d39543f65f2b0a94b8ff566836c8256bf7 

Diff: https://reviews.apache.org/r/38473/diff/


Testing
-------

make check


Thanks,

Marco Massenzio


Re: Review Request 38473: Add flag to disable hostname lookup.

Posted by Marco Massenzio <ma...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38473/
-----------------------------------------------------------

(Updated Sept. 21, 2015, 8:05 p.m.)


Review request for mesos, Benjamin Hindman, Cody Maloney, and Neil Conway.


Changes
-------

benh comments


Bugs: MESOS-3457
    https://issues.apache.org/jira/browse/MESOS-3457


Repository: mesos


Description
-------

Under certain circumstances, dynamic lookup of hostname, while
successful, provides undesirable results; we would also like, in
those circumstances, 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.


Diffs (updated)
-----

  docs/configuration.md dd7f4aa806a3c1a8653a0fda9a326a3707308e7c 
  src/master/flags.hpp e4b1df3f5a33049defff4688463274067f1f1ebf 
  src/master/flags.cpp 80879611fbcfd764c9fc8f60a31613a9c8fc2364 
  src/master/master.cpp 90ef8c663c90ffbdcb4aa2377bfba65ea5d3fda9 
  src/slave/flags.hpp e31a4183170c3442ac4a15365c229391e7e91480 
  src/slave/flags.cpp add4196dfd06c0f602ff5ebd39960dc05c4cd11f 
  src/slave/slave.cpp ad710d7b930a2f115d503ceb8f8fd7421ad30287 
  src/tests/cluster.hpp 114583de8c867495a2b7a953e6826708838e5d23 
  src/tests/master_tests.cpp f26344d39543f65f2b0a94b8ff566836c8256bf7 

Diff: https://reviews.apache.org/r/38473/diff/


Testing
-------

make check


Thanks,

Marco Massenzio


Re: Review Request 38473: Add flag to disable hostname lookup.

Posted by Marco Massenzio <ma...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38473/
-----------------------------------------------------------

(Updated Sept. 19, 2015, 1:25 a.m.)


Review request for mesos, Benjamin Hindman, Cody Maloney, and Neil Conway.


Changes
-------

Fixed test failure


Bugs: MESOS-3457
    https://issues.apache.org/jira/browse/MESOS-3457


Repository: mesos


Description
-------

Under certain circumstances, dynamic lookup of hostname, while
successful, provides undesirable results; we would also like, in
those circumstances, 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.


Diffs (updated)
-----

  docs/configuration.md dd7f4aa806a3c1a8653a0fda9a326a3707308e7c 
  src/master/flags.hpp e4b1df3f5a33049defff4688463274067f1f1ebf 
  src/master/flags.cpp 80879611fbcfd764c9fc8f60a31613a9c8fc2364 
  src/master/master.cpp ca4d5876dcd427964111428edc22d567ddaede0b 
  src/slave/flags.hpp e31a4183170c3442ac4a15365c229391e7e91480 
  src/slave/flags.cpp add4196dfd06c0f602ff5ebd39960dc05c4cd11f 
  src/slave/slave.cpp ad710d7b930a2f115d503ceb8f8fd7421ad30287 
  src/tests/cluster.hpp 114583de8c867495a2b7a953e6826708838e5d23 
  src/tests/master_tests.cpp a477794df37c658b80d79dea8555b001415f7b6a 
  src/tests/mesos.hpp 3db97aca921c9216d90384e1eb17030849516454 

Diff: https://reviews.apache.org/r/38473/diff/


Testing
-------

make check


Thanks,

Marco Massenzio