----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/72956/#review222061 -----------------------------------------------------------
Ship it! src/master/master.cpp Line 2622 (original), 2622-2624 (patched) <https://reviews.apache.org/r/72956/#comment311130> ``` Make sure the offer constraints don't use roles other than the framework's roles. ``` That seems sufficient for the reader? I'm not sure they need to be explained here that you could construct a "valid" OfferConstraintsFilter with roles outside of the framework's roles src/master/validation.hpp Lines 137-139 (patched) <https://reviews.apache.org/r/72956/#comment311129> Similar to the last review, just want to comment on how these are unit testable. In this case, I'm not sure how we could avoid the integration test though since it's not a sub-case of validation where we can just ensure in the integration test that overall validation is called. Still, might want to just add the simple unit test for this? src/master/validation.cpp Lines 706 (patched) <https://reviews.apache.org/r/72956/#comment311128> could just use `foreachkey (` ``` foreachkey (const string& role, offerConstraints.role_constraints()) { } ``` - Benjamin Mahler On Oct. 14, 2020, 6:18 p.m., Andrei Sekretenko wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/72956/ > ----------------------------------------------------------- > > (Updated Oct. 14, 2020, 6:18 p.m.) > > > Review request for mesos and Benjamin Mahler. > > > Bugs: MESOS-10176 > https://issues.apache.org/jira/browse/MESOS-10176 > > > Repository: mesos > > > Description > ------- > > This patch makes SUBSCRIBE/UPDATE_FRAMEWORK calls validate that > the framework does not specify offer constraints for roles to which > it is not going to be subscribed. > > > Diffs > ----- > > src/master/master.cpp d6d3ea7e27539a65b601783eca2ac4e3b688f188 > src/master/validation.hpp 7fe8f087b322f8e77218f4dd9f9f75707afa3208 > src/master/validation.cpp 5b1bcb5207aac0553bf9c1a0d89525c835393ccb > src/tests/master/update_framework_tests.cpp > 3f86573e8dfeea63fe99064f2137c61d901f4fc7 > > > Diff: https://reviews.apache.org/r/72956/diff/2/ > > > Testing > ------- > > `make check` > `src/mesos-tests > --gtest_filter='*UpdateFrameworkTest.OfferConstraintsForUnsubscribedRole*' > --gtest_break_on_failure --gtest_repeat=1000` > > > Thanks, > > Andrei Sekretenko > >
