detect: prefer ip address over hostname

[s-urbaniak]

Fixes #289, #276

[s-urbaniak]

@tsenart PTAL

[s-urbaniak]

/cc @jdef

[tsenart]

LGTM

[jdef]

Probably worth adding a TODO in here to upgrade to MasterInfo.Address as soon as mesos-go makes that available via updated protos. That way you don't need to be in the business of big-vs-little endian decoding (which there were some plans to change at some point).

[jdef]

It sounds like little vs. big endian encoding depends on the arch that the remote UPID process is running on: https://issues.apache.org/jira/browse/MESOS-1201

[jdef]

The "trivial" fix uses ntohl for the ordering problem (copied from the xref'd MESOS issue above):

 // you get the minfo from … magic!
  MasterInfo minfo = getMyMasterInfo(...);

  // then convert the bytes into host-order:
  net::IP ip{ ntohl(minfo.ip()) };
  // you can then generate back the original string ("192.168.1.10")
  std::ostringstream out;
  out << ip;
  return out.str();

[s-urbaniak]

@jdef PTAL (just added a TODO)

[tsenart]

@s-urbaniak: TODOs in code get easily lost. Please create an issue to track this instead.

[s-urbaniak]

@tsenart you are very right, I just created #291

[jdef]

actually wondering if it makes sense to extract this packing logic into a
set of separate go files controlled by build flags to target different
arch's since each may have different endian-ness:
https://golang.org/doc/install/source

On Tue, Sep 29, 2015 at 10:25 AM, Sergiusz Urbaniak <
notifications@github.com> wrote:

@tsenart https://github.com/tsenart you are very right, I just created
#291 #291

—
Reply to this email directly or view it on GitHub
#290 (comment).

[tsenart]

@jdef: No need to do that after implementing #291 since it is a string, not an int.

[jdef]

lgtm. we're probably not going to make things any worse for people since
it's already broken. there's a (slight) chance that by fixing this for 99%
of people maybe it breaks for 1% of others. this issue, in general, isn't
fully resolvable without upgrading to the v0.24 bindings. which I don't
think mesos-go has generated protos for yet.

On Tue, Sep 29, 2015 at 10:41 AM, Tomás Senart notifications@github.com
wrote:

@jdef https://github.com/jdef: No need to do that after implementing
#291 #291 since it is a
string, not an int.

—
Reply to this email directly or view it on GitHub
#290 (comment).

[tsenart]

Even if we update the bindings, this new address field would only be set in more recent versions of Mesos, so older clusters would still be affected.

[jdef]

true, but given the current deprecation cycle systems need to upgrade
quickly to stay compat. @nqn sent something to the mesos-dev list recently
about the fast-pace of the current deprecation process and how it might
make sense to slow it down.

On Tue, Sep 29, 2015 at 12:23 PM, Tomás Senart notifications@github.com
wrote:

Even if we update the bindings, this new address field would only be set
in more recent versions of Mesos, so older clusters would still be affected.

—
Reply to this email directly or view it on GitHub
#290 (comment).

[jdef]

like the byte order detection

[s-urbaniak]

small nit, otherwise I really like the refactorings, LGTM