Re: Review Request 41075: Added support for implicit roles.

> On Dec. 18, 2015, 6:04 a.m., Yongqiao Wang wrote: > > src/master/master.cpp, line 597 > > > > > > If the value of the specified weight is default value 1.0, do we still > > need to put it in weights hashmap and

Re: Review Request 41075: Added support for implicit roles.

> On Dec. 19, 2015, 5:23 a.m., Guangya Liu wrote: > > src/master/master.cpp, line 2675 > > > > > > Can we rename this as _isValidRole_ or some others which is more clear? > > The current _isWhitelistedRole_ can

Re: Review Request 41075: Added support for implicit roles.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41075/#review111360 --- src/master/master.cpp (line 2658)

Re: Review Request 41075: Added support for implicit roles.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41075/#review21 --- src/master/master.cpp (line 588)

Re: Review Request 41075: Added support for implicit roles.

> On Dec. 18, 2015, 1:24 a.m., Adam B wrote: > > src/master/master.cpp, lines 2677-2681 > > > > > > Or how about `return roleWhitelist.isNone() || > > roleWhitelist.get().contains(name);`? Personally, I'd rather

Re: Review Request 41075: Added support for implicit roles.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41075/#review111096 --- Ship it! Looks great. Worst thing I could find was an unnecessary

Re: Review Request 41075: Added support for implicit roles.

> On Dec. 15, 2015, 9:56 a.m., Adam B wrote: > > src/master/allocator/mesos/hierarchical.cpp, lines 1016-1031 > > > > > > If the allocator's roleSorter doesn't know about the role (i.e. no > > frameworks are

Re: Review Request 41075: Added support for implicit roles.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41075/ --- (Updated Dec. 16, 2015, 9:08 p.m.) Review request for mesos, Adam B, Alexander

Re: Review Request 41075: Added support for implicit roles.

> On Dec. 15, 2015, 9:56 a.m., Adam B wrote: > > src/master/allocator/mesos/hierarchical.cpp, lines 1016-1031 > > > > > > If the allocator's roleSorter doesn't know about the role (i.e. no > > frameworks are

Re: Review Request 41075: Added support for implicit roles.

> On Dec. 15, 2015, 1:56 a.m., Adam B wrote: > > src/master/allocator/mesos/hierarchical.cpp, lines 1016-1031 > > > > > > If the allocator's roleSorter doesn't know about the role (i.e. no > > frameworks are

Re: Review Request 41075: Added support for implicit roles.

> On Dec. 15, 2015, 9:56 a.m., Adam B wrote: > > src/master/http.cpp, line 316 > > > > > > I'm in agreement with Yong. This form of `model(role, name, weight)` > > isn't used anywhere, and is confusing alongside

Re: Review Request 41075: Added support for implicit roles.

> On Dec. 17, 2015, 2:54 a.m., Yongqiao Wang wrote: > > src/master/http.cpp, line 1543 > > > > > > I am not sure /roles will return the reserved resources of role, as far > > as I know, it will return the used

Re: Review Request 41075: Added support for implicit roles.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41075/ --- (Updated Dec. 17, 2015, 2:58 a.m.) Review request for mesos, Adam B, Alexander

Re: Review Request 41075: Added support for implicit roles.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41075/ --- (Updated Dec. 17, 2015, 2:01 a.m.) Review request for mesos, Adam B, Alexander

Re: Review Request 41075: Added support for implicit roles.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41075/#review110867 --- src/master/http.cpp (line 1521)

Re: Review Request 41075: Added support for implicit roles.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41075/#review110879 --- Ship it! Ship It! - Yongqiao Wang On Dec. 17, 2015, 2:58 a.m.,

Re: Review Request 41075: Added support for implicit roles.

> On Dec. 15, 2015, 3:10 a.m., Yongqiao Wang wrote: > > src/master/http.cpp, line 1556 > > > > > > Roles with a non-default quota are shown in /roles endpoint, but their > > quota infromation does not be shown, is

Re: Review Request 41075: Added support for implicit roles.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41075/ --- (Updated Dec. 15, 2015, 8:56 p.m.) Review request for mesos, Adam B, Alexander

Re: Review Request 41075: Added support for implicit roles.

> On Dec. 15, 2015, 9:56 a.m., Adam B wrote: > > Good progress. I'm more confident now in the removal of RoleInfo in favor > > of a weights map. > > - I'm liking the idea of naming this a role "whitelist" rather than > > "validRoles". > > - We need to decide if /roles should care about quota.

Re: Review Request 41075: Added support for implicit roles.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41075/ --- (Updated Dec. 16, 2015, 4:18 a.m.) Review request for mesos, Adam B, Alexander

Re: Review Request 41075: Added support for implicit roles.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41075/#review110604 --- Just a reminder, no any changes are found between revision 6 and

Re: Review Request 41075: Added support for implicit roles.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41075/ --- (Updated Dec. 16, 2015, 4:19 a.m.) Review request for mesos, Adam B, Alexander

Re: Review Request 41075: Added support for implicit roles.

> On Dec. 16, 2015, 2:41 a.m., Yongqiao Wang wrote: > > Just a reminder, no any changes are found between revision 6 and 7, please > > check if something is wrong. Weird -- I sent the update when connected via unreliable wifi, so maybe RB got confused somehow. I'll post a revised version

Re: Review Request 41075: Added support for implicit roles.

> On Dec. 15, 2015, 2:10 a.m., Yongqiao Wang wrote: > > src/master/http.cpp, line 1578 > > > > > > In Implicit Roles, Quota configuration and Weight configuration will > > not create a role in Mesos master, and a

Re: Review Request 41075: Added support for implicit roles.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41075/#review110381 --- src/master/http.cpp (line 1556)

Re: Review Request 41075: Added support for implicit roles.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41075/#review110374 --- src/master/http.cpp (line 316)

Re: Review Request 41075: Added support for implicit roles.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41075/#review110371 --- src/master/http.cpp (line 1578)

Re: Review Request 41075: Added support for implicit roles.

> On Dec. 10, 2015, 10:32 p.m., Neil Conway wrote: > > src/master/http.cpp, line 1588 > > > > > > AlexR suggested including roles that have any reserved resources here. > > That makes sense, but AFAIK there isn't

Re: Review Request 41075: Added support for implicit roles.

> On Dec. 10, 2015, 10:32 p.m., Neil Conway wrote: > > src/master/http.cpp, line 1588 > > > > > > AlexR suggested including roles that have any reserved resources here. > > That makes sense, but AFAIK there isn't

Re: Review Request 41075: Added support for implicit roles.

> On Dec. 10, 2015, 10:32 p.m., Neil Conway wrote: > > src/master/http.cpp, line 1588 > > > > > > AlexR suggested including roles that have any reserved resources here. > > That makes sense, but AFAIK there isn't

Re: Review Request 41075: Added support for implicit roles.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41075/ --- (Updated Dec. 14, 2015, 11:16 p.m.) Review request for mesos, Adam B,

Re: Review Request 41075: Added support for implicit roles.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41075/#review109878 --- src/master/http.cpp (line 1588)

Re: Review Request 41075: Added support for implicit roles.

> On Dec. 10, 2015, 10:13 a.m., Adam B wrote: > > src/master/allocator/mesos/hierarchical.hpp, lines 367-368 > > > > > > I'm not sure how badly we need to clean up this map. What happens if we > > leave a role in

Re: Review Request 41075: Added support for implicit roles.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41075/#review109708 --- Seems like the right overall approach, but I've realized that we

Re: Review Request 41075: Added support for implicit roles.

> On Dec. 8, 2015, 4:45 p.m., Alexander Rukletsov wrote: > > src/master/http.cpp, lines 1543-1545 > > > > > > Why don't we consider roles without frameworks but with a non-default > > weight active? Or roles with

Re: Review Request 41075: Added support for implicit roles.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41075/ --- (Updated Dec. 8, 2015, 8:29 a.m.) Review request for mesos, Adam B, Alexander

Re: Review Request 41075: Added support for implicit roles.

> On Dec. 8, 2015, 7:02 a.m., Yong Qiao Wang wrote: > > include/mesos/master/allocator.hpp, line 101 > > > > > > When allocator initialize, their should be no active frameworks in > > mesos, can we consider to

Re: Review Request 41075: Added support for implicit roles.

> On Dec. 7, 2015, 11:02 p.m., Yong Qiao Wang wrote: > > > > Yong Qiao Wang wrote: > I have talked with our shepherd Adam B yesterday, and we all agree to > improve /roles endpoint to update/remove/list active roles, and Implicit > Roles will focus on removing the static role

Re: Review Request 41075: Added support for implicit roles.

--- 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

Re: Review Request 41075: Added support for implicit roles.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41075/#review109308 --- src/master/allocator/mesos/hierarchical.cpp (line 301)

Re: Review Request 41075: Added support for implicit roles.

> On Dec. 8, 2015, 7:02 a.m., Yong Qiao Wang wrote: > > > > Yong Qiao Wang wrote: > I have talked with our shepherd Adam B yesterday, and we all agree to > improve /roles endpoint to update/remove/list active roles, and Implicit > Roles will focus on removing the static role

Re: Review Request 41075: Added support for implicit roles.

> On Dec. 8, 2015, 3:02 p.m., Yong Qiao Wang wrote: > > include/mesos/master/allocator.hpp, line 101 > > > > > > When allocator initialize, their should be no active frameworks in > > mesos, can we consider to

Re: Review Request 41075: Added support for implicit roles.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41075/#review109321 --- src/master/allocator/mesos/hierarchical.hpp (line 367)

Re: Review Request 41075: Added support for implicit roles.

> On Dec. 8, 2015, 7:02 a.m., Yong Qiao Wang wrote: > > include/mesos/master/allocator.hpp, line 101 > > > > > > When allocator initialize, their should be no active frameworks in > > mesos, can we consider to

Re: Review Request 41075: Added support for implicit roles.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41075/#review109326 --- src/master/allocator/mesos/hierarchical.hpp (line 367)

Re: Review Request 41075: Added support for implicit roles.

> On Dec. 8, 2015, noon, Yong Qiao Wang wrote: > > src/master/allocator/mesos/hierarchical.cpp, line 320 > > > > > > In the current implementation, quota(quotaSorter) does not been removed > > after the

Re: Review Request 41075: Added support for implicit roles.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41075/#review109347 --- Would it be a good idea to split this review into smaller bits for

Re: Review Request 41075: Added support for implicit roles.

> On Dec. 8, 2015, 3:20 p.m., Qian Zhang wrote: > > src/master/allocator/mesos/hierarchical.hpp, line 407 > > > > > > s/iff/if/ "iff" is what was intended

Re: Review Request 41075: Added support for implicit roles.

> On Dec. 8, 2015, 5:26 p.m., Anand Mazumdar wrote: > > Would it be a good idea to split this review into smaller bits for ease of > > reviewing ? It would make it easier to commit the individual sub system > > component changes. > > > > One possible split can be: > > > > 1. Removal of

Re: Review Request 41075: Added support for implicit roles.

> On Dec. 8, 2015, 4:45 p.m., Alexander Rukletsov wrote: > > src/master/allocator/mesos/hierarchical.hpp, line 367 > > > > > > s/Frameworks/Roles? Thanks, fixed. > On Dec. 8, 2015, 4:45 p.m., Alexander Rukletsov

Re: Review Request 41075: Added support for implicit roles.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41075/ --- (Updated Dec. 8, 2015, 7:18 p.m.) Review request for mesos, Adam B, Alexander

Re: Review Request 41075: Added support for implicit roles.

> On Dec. 8, 2015, 4:45 p.m., Alexander Rukletsov wrote: > > src/master/http.cpp, lines 1543-1545 > > > > > > Why don't we consider roles without frameworks but with a non-default > > weight active? Or roles with

Re: Review Request 41075: Added support for implicit roles.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41075/ --- (Updated Dec. 9, 2015, 5:54 a.m.) Review request for mesos, Adam B, Alexander

Re: Review Request 41075: Added support for implicit roles.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41075/#review109287 --- include/mesos/master/allocator.hpp (line 99)

Re: Review Request 41075: Added support for implicit roles.

> On Dec. 8, 2015, 7:02 a.m., Yong Qiao Wang wrote: > > include/mesos/master/allocator.hpp, line 101 > > > > > > When allocator initialize, their should be no active frameworks in > > mesos, can we consider to

Re: Review Request 41075: Added support for implicit roles.

> On Dec. 8, 2015, 7:02 a.m., Yong Qiao Wang wrote: > > I have talked with our shepherd Adam B yesterday, and we all agree to improve /roles endpoint to update/remove/list active roles, and Implicit Roles will focus on removing the static role list(specified by --roles flag) and let

Re: Review Request 41075: Added support for implicit roles.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41075/#review109281 --- The test failure that I noticed on Ubuntu 15.10 seems to occur

Re: Review Request 41075: Added support for implicit roles.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41075/ --- (Updated Dec. 8, 2015, 5:41 a.m.) Review request for mesos, Adam B, Alexander