----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40995/#review110460 -----------------------------------------------------------
Thanks for validating the expected `--roles` behavior with tests, but I think you're mixing up the scheduler reservation/volume API (where only the framework's role is valid) with the operator API (where any whitelisted role is valid). src/tests/role_tests.cpp (line 44) <https://reviews.apache.org/r/40995/#comment170379> A little counterintuitive that the master whitelist includes "role-bad" while the framework is trying to register with "role". How about you make it explicit that `frameworkInfo.set_role("invalid")` and `masterFlags.roles = "foo, bar"`? src/tests/role_tests.cpp (line 124) <https://reviews.apache.org/r/40995/#comment170380> This should fail because the framework is reserving resources for a role other than its own, not because the role is not on the whitelist. I'm not sure this is a relevant test. src/tests/role_tests.cpp (lines 141 - 142) <https://reviews.apache.org/r/40995/#comment170381> Again, using the scheduler `driver.acceptOffers` API means that the framework already shouldn't be able to create a volume with a role other than the role with which the framework registered. Instead, maybe you're thinking of the `/reserve` and `/create` operator endpoints, which would want to verify the operation against the role whitelist. src/tests/role_tests.cpp (lines 267 - 268) <https://reviews.apache.org/r/40995/#comment170382> I'd like to see one role explicitly configured, but without a weight, and another explicitly configured but also with a weight. - Adam B On Dec. 14, 2015, 3:15 p.m., Neil Conway wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/40995/ > ----------------------------------------------------------- > > (Updated Dec. 14, 2015, 3:15 p.m.) > > > Review request for mesos, Adam B, Alexander Rukletsov, Greg Mann, and > Yongqiao Wang. > > > Repository: mesos > > > Description > ------- > > Added test cases for role behavior. > > > Diffs > ----- > > src/Makefile.am acd17de04bce81f1d0550abfa0f43dec1a25fe7c > src/tests/reservation_tests.cpp f429472e6b93a5d6d8fe6a5f7d5b94fc331f7295 > src/tests/role_tests.cpp PRE-CREATION > > Diff: https://reviews.apache.org/r/40995/diff/ > > > Testing > ------- > > make check. > > > Thanks, > > Neil Conway > >
