----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55461/#review164823 -----------------------------------------------------------
Fix it, then Ship it! Will make some adjustments based on the comment below. src/master/validation.cpp (lines 1581 - 1593) <https://reviews.apache.org/r/55461/#comment236610> I think we can remove these per my previous comments. For checking against the framework having no roles, this is a special case of the reservation role being absent from the framework's roles, which is caught below. Given that a framework with no roles would not receive offers, there doesn't seem to be a lot of value in special casing the error message for this with an additional check of the special case. For checking against the framework having `{*}` role, it seems odd to check for the framework role's being `{*}` when `{*,foo}` has the same problem: the framework cannot make a reservation for `*`. So, this a special case of the general dynamic reservation check within `resources::validate()` and we can remove this check as well. - Benjamin Mahler On Feb. 8, 2017, 11:41 a.m., Benjamin Bannier wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/55461/ > ----------------------------------------------------------- > > (Updated Feb. 8, 2017, 11:41 a.m.) > > > Review request for mesos, Benjamin Mahler, Jay Guo, and Guangya Liu. > > > Bugs: MESOS-6730 > https://issues.apache.org/jira/browse/MESOS-6730 > > > Repository: mesos > > > Description > ------- > > This updates the resource reservation validation for frameworks which > can have multiple roles. During a deprecation period 'FrameworkInfo' > will have fields for both 'role' and 'roles', however the validation > function works with just an optional set of roles. Here an empty set > captures the previous semantics of either having an empty 'role' field > or 'role' set as '*'. This forces the callers to properly construct a > set of framework roles from the available information. An optional set > is used in order to accommodate callers which have no information > about the framework's roles, and ultimately disables validation taking > that information into account. > > > Diffs > ----- > > src/master/master.cpp 98c39b279e7b9830d02efc8ec6a4469afc15d62a > src/master/validation.hpp fc1eaeebd0643b897f4e14aabc76e7d94f0de263 > src/master/validation.cpp 37d171512ee54f260aabb6e1071739bcc3769fb0 > src/tests/master_validation_tests.cpp > 51185031cb67e64cd69ec6ce1c8f722a0c349970 > > Diff: https://reviews.apache.org/r/55461/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Benjamin Bannier > >
