> On Dec. 8, 2015, 4:45 p.m., Alexander Rukletsov wrote:
> > src/master/allocator/mesos/hierarchical.hpp, line 367
> > <https://reviews.apache.org/r/41075/diff/2/?file=1155876#file1155876line367>
> >
> >     s/Frameworks/Roles?

Thanks, fixed.


> On Dec. 8, 2015, 4:45 p.m., Alexander Rukletsov wrote:
> > src/master/master.hpp, lines 1332-1333
> > <https://reviews.apache.org/r/41075/diff/2/?file=1155879#file1155879line1332>
> >
> >     Do you want to add a `TODO` here to remove this hashtable once the 
> > deprecation cycle is over?

I put a TODO (along with a warning) in master.cpp when we parse `--roles`.


> On Dec. 8, 2015, 4:45 p.m., Alexander Rukletsov wrote:
> > src/master/master.cpp, lines 5837-5838
> > <https://reviews.apache.org/r/41075/diff/2/?file=1155880#file1155880line5837>
> >
> >     Fits one line.

You mean

```
  CHECK(roles.contains(role))
    << "Unknown role " << role << " of framework " << *framework;
```

I believe this would violate the "anti-jaggedness" suggestion in the style 
guide.


> On Dec. 8, 2015, 4:45 p.m., Alexander Rukletsov wrote:
> > src/master/http.cpp, lines 1543-1545
> > <https://reviews.apache.org/r/41075/diff/2/?file=1155878#file1155878line1543>
> >
> >     Why don't we consider roles without frameworks but with a non-default 
> > weight active? Or roles with quota but without frameworks?
> >     
> >     Or if you would like to reserve the term "active" for roles with 
> > frameworks, how about expanding the terminology to something like:
> >     ```
> >     Visible = {active} U {weighted} U {quota'ed} U {with-dyn-res}
> >     ```
> >     
> >     Anyway, my point is that we should show all "visible" roles here.

Yeah, this is a good point. Do we think that a user wants to see `{active} U 
{weighted} U {quota'ed} U {with-dyn-res}`?

That does seem reasonable to me, although with the way the data structures are 
organized, it will be somewhat ugly to implement :-\


- Neil


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41075/#review109326
-----------------------------------------------------------


On Dec. 8, 2015, 8:33 a.m., Neil Conway wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41075/
> -----------------------------------------------------------
> 
> (Updated Dec. 8, 2015, 8:33 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Benjamin Hindman, Greg 
> Mann, and Yong Qiao Wang.
> 
> 
> Bugs: MESOS-4085
>     https://issues.apache.org/jira/browse/MESOS-4085
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Changed the behavior of the master when the `--roles` flag is NOT
> specified. Previously, this would allow only the `*` role to be used. Now,
> omitting `--roles` means that any role can be used. This is called "implicit
> roles". Configuring which principals can perform operations as which roles
> should be done using ACLs in the authorization system.
> 
> Note that this changes the behavior of the system when `--roles` is not
> specified. This is likely acceptable: if the operator didn't specify `--roles`
> in prior versions of Mesos, they were likely not using roles or authorization 
> at
> that time.
> 
> Another minor behavioral change is that the "/roles" endpoint will now only
> return results for currently "active" roles (those with one or more registered
> frameworks).
> 
> The `--roles` flag is now considered deprecated and will be removed in a 
> future
> version of Mesos.
> 
> 
> Diffs
> -----
> 
>   include/mesos/master/allocator.hpp 619ef01c3a7d640560653cfc1838dd09046d1da0 
>   include/mesos/master/allocator.proto 
> 702f56f56c3b1331613cecf26522986f6b572f8c 
>   src/master/allocator/mesos/allocator.hpp 
> 97ee80726ad155917811265a983258b0165d3451 
>   src/master/allocator/mesos/hierarchical.hpp 
> 99c742906874c30c39c159e58a65277ade3c07fd 
>   src/master/allocator/mesos/hierarchical.cpp 
> 5da825a1d578a9ee40b4985378fddb3c5fb3b416 
>   src/master/http.cpp 9097eda47558fb5306af5a61b464937d7ab34e83 
>   src/master/master.hpp 4683fa542a740f9a0b80fff7fbe0e63ec66266f2 
>   src/master/master.cpp 953fa4f14929581b226a7e27d30aea7a5aa1fd7c 
>   src/master/quota_handler.cpp b209da42ace752953686eeda9577007a33556d5d 
>   src/tests/allocator.hpp c7670525765491fe931a4ee38446fa7e9d79af42 
>   src/tests/hierarchical_allocator_tests.cpp 
> fb214a829a57529d3f5c49730ae9733f53e622ca 
> 
> Diff: https://reviews.apache.org/r/41075/diff/
> 
> 
> Testing
> -------
> 
> "make check" on OSX 10.10 and Ubuntu 15.10; `--gtest_repeat=1000` for the 
> more likely role-related tests.
> 
> TODOs:
> 
> * Update documentation
> * Add tests for allocation behavior for weights + implicit roles
> * Add tests for quota + implicit roles?
> 
> Notes:
> 
> * There's two places where we use manual `new`/`delete` where a `unique_ptr` 
> would probably be nicer. I'm inclined to leave this as-is for now though 
> (making use of unique_ptr is a broader issue).
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Reply via email to