----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50298/#review143147 -----------------------------------------------------------
Fix it, then Ship it! Thanks! Looks like our validation for this field has the potential to crash the master in light of the HTTP API (see below). This patch can stand on its own, just need to pull out the 0.22.0 compatibility removal into a separate patch. src/master/master.cpp <https://reviews.apache.org/r/50298/#comment208886> This looks unrelated to the summary / description, did it slip in? This looks ok, but how about a separate patch? src/master/master.cpp (line 6139) <https://reviews.apache.org/r/50298/#comment208889> Given that we also have the new HTTP API, have you audited whether we validate that executorinfo has the framework id set in that path? Looks to me like we need to update the validation function to return an Error rather than CHECK, since HTTP frameworks can send this to us: https://github.com/apache/mesos/blob/f69a27c43eaf9ade386819dd707ce33041ee1f20/src/master/validation.cpp#L573-L577 Feel free to send in a separate patch with a test (it should be easy to test since you can just unit test the validation function). - Benjamin Mahler On July 21, 2016, 10:34 p.m., Neil Conway wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/50298/ > ----------------------------------------------------------- > > (Updated July 21, 2016, 10:34 p.m.) > > > Review request for mesos and Benjamin Mahler. > > > Repository: mesos > > > Description > ------- > > Removed backward compat code for old (< 0.15) Mesos agents. > > > Diffs > ----- > > src/master/master.cpp b87d18f457927114bebf6068d08ff0c4f210aece > > Diff: https://reviews.apache.org/r/50298/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Neil Conway > >
