> On May 5, 2014, 2:48 p.m., Niklas Nielsen wrote: > > include/mesos/mesos.proto, line 137 > > <https://reviews.apache.org/r/21052/diff/2/?file=574065#file574065line137> > > > > Shouldn't this be a hostname? > > Benjamin Hindman wrote: > I wasn't convinced that a hostname would be sufficient. For example, in > the event that we have internal containers that we're trying to query that > have private IP addresses (i.e., via Docker) it seemed like it was possible > we'd want to run mesos-health-checker both outside of such a container (in > which case the NAT'ing might be sufficient) or inside of a container in which > case it seemed like having an IP might be nice. I could have both with some > semantics about what to do if both are present. > > Niklas Nielsen wrote: > I was thinking along the lines of an full URL instead of ip + port + > path. That could accommodate hostname _and_ ip's too, would capture > http/https, port, login and so on. Are their downsides I am missing? > > Connor Doyle wrote: > +1 Niklas. A full URL would be the most flexible here, and schedulers > could readily derive this value at launchTasks() time. > > Benjamin Hindman wrote: > First, just so everyone is on the same page, the 'ip' field is optional > and is meant as an escape hatch for internal usage with the > mesos-health-checker command line program that Niklas is working on. The > intended behavior here is to make things simple for a scheduler writer by > just saying: to check the health of this command do a GET on this path at > this port, with these timeouts and expected response statuses (but you figure > out the resulting IP/hostname of the command since you're the one running > it). I'm almost inclined to remove the 'ip' field and pass it explicitly to > mesos-health-checker. > > Taking a step back, IMHO requiring that a scheduler constructs a complete > URL is less flexible as the containerizer/executor can't "late bind" > necessary components of the URL (like the IP/hostname) depending on the > execution environment. > > I do like that a URL can accommodate a lot of the other fields (like user > authentication, https, etc), but I'm not convinced that we won't end up > breaking the URL apart anyway in order to (a) do validation or (b) rewrite > the URL in order to late-bind things that might have changed based on the > environment the mesos-health-checker command line program is run. > > So, current proposals: > > (1) Use a full URL, validate on the master, and "rewrite" as necessary > based on late-binding for mesos-health-checker. > (2) Drop 'ip' completely, add other fields in protobuf (like user > authentication) as necessary, let containerizer/executor "late-bind" as > necessary, and expect mesos-health-checker to take an argument for the > IP/hostname to use for the HTTP(S) request.
I missed the point that the ip wasn't supposed to be set by the framework - that changes the situation. What the framework developer sees is only path + accepted status codes? How about HTTP services that listens to specific ips / hostnames? Isn't that pretty common? I think we should be able to accommodate both scenarios. The URL can encode much more that we would want to add protobuf fields for. I am up for adding a first simple one though. How about having two HTTP checks? HTTPSimple / HTTP, or HTTPRelative / HTTPAbsolute ? - Niklas ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21052/#review42207 ----------------------------------------------------------- On May 2, 2014, 11:38 p.m., Benjamin Hindman wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/21052/ > ----------------------------------------------------------- > > (Updated May 2, 2014, 11:38 p.m.) > > > Review request for mesos, Niklas Nielsen and Vinod Kone. > > > Bugs: MESOS-741 > https://issues.apache.org/jira/browse/MESOS-741 > > > Repository: mesos-git > > > Description > ------- > > See summary. > > > Diffs > ----- > > include/mesos/mesos.proto e48e50aae248bd9d5289dbaa36753be53b2e592a > src/master/master.cpp c79fdafde6b08d74e6fea71afa79aded41b3a9ab > > Diff: https://reviews.apache.org/r/21052/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Benjamin Hindman > >