Re: Review Request 60013: Resources: Updated the validation logic.

2017-06-16 Thread Benjamin Mahler

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


Fix it, then Ship it!





src/common/resources.cpp
Lines 511-512 (original), 511-512 (patched)


Can you commit this unrelated formatting change separately? Would be good 
to minimize the diff for posterity



src/common/resources.cpp
Lines 522-523 (original), 522-523 (patched)


Ditto here



src/master/validation.cpp
Lines 1854-1856 (original), 1902-1904 (patched)


Commit this separately to minimize the diff?



src/master/validation.cpp
Lines 2027-2030 (original), 2091-2094 (patched)


Commit this separately to minimize the diff?



src/tests/master_validation_tests.cpp
Lines 1987-1989 (original), 1987 (patched)


This seems strictly less readable to me, the intention of capturing the 
'error' was to have a clear expectation:

```
EXPECT_NONE(error);
```

Ditto on the others below. It's also unrelated to this change :)



src/v1/resources.cpp
Lines 513-525 (original), 513-525 (patched)


Ditto v0 comments. Could you commit this separately in the interest of 
getting this diff down to its essential?


- Benjamin Mahler


On June 15, 2017, 7:39 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60013/
> ---
> 
> (Updated June 15, 2017, 7:39 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-7655
> https://issues.apache.org/jira/browse/MESOS-7655
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch updates `Resources::validate`, and `resource::validate`.
> 
> The `Resources::validate` function ensures that the resources are
> either in the 'pre-' or 'post-' reservation-refinement formats,
> but not both.
> 
> The `resource::validate` function takes an optional framework
> capabilities and ensures that the resources format matches
> the framework's RESERVATION_REFINEMENT capbility.
> 
> 
> Diffs
> -
> 
>   src/common/resources.cpp e37519ea83c564b72c842c662fe784731c6e44b0 
>   src/master/validation.hpp 1be9f082024a5295e6a20bb047c15217c1866ca3 
>   src/master/validation.cpp 1d10e6bb0dfada7d6aa7085aa99c5bfa8099643e 
>   src/tests/master_validation_tests.cpp 
> 83370fa5653fe5da666ec577b05001c4a5499848 
>   src/v1/resources.cpp 40ef32f11be3f2710a0e634f3225314fb9ae5aec 
> 
> 
> Diff: https://reviews.apache.org/r/60013/diff/13/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Michael Park
> 
>



Re: Review Request 60145: Update Linux capabilities isolator tests.

2017-06-16 Thread Jie Yu

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


Ship it!




Ship It!

- Jie Yu


On June 16, 2017, 4:50 a.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60145/
> ---
> 
> (Updated June 16, 2017, 4:50 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-7477
> https://issues.apache.org/jira/browse/MESOS-7477
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Update the Linux capabilities tests to separate the operator
> bounding and effective sets clearly and add an additional test
> case that wasn't already covered. This better demonstrates
> how the various capability set configurations interact.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/linux_capabilities_isolator_tests.cpp 
> 40376a03fdb8f931f8d3f83b1c3fa6207e02c1d1 
> 
> 
> Diff: https://reviews.apache.org/r/60145/diff/1/
> 
> 
> Testing
> ---
> 
> make check (Fedora 25)
> 
> 
> Thanks,
> 
> James Peach
> 
>



Re: Review Request 60101: Prevent the fetcher from setting overly-permissive fs permissions.

2017-06-16 Thread Jiang Yan Xu

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



Chatted with Silas offline.

So the problem we are trying to solve here is that all Mesos created 
directories have a [755 
permission](https://github.com/apache/mesos/blob/9cfae1bdea14ab07b879d5aba8ec2c8ac2f76693/3rdparty/stout/include/stout/os/mkdir.hpp#L40)
 and in some environments for some directories it's too open.

We need to restrict certain directories but I think with this RR:
- Simply use another hard-coded value to restrict "others" and not "group" is 
probably too inflexible.
- Right now even if you do 'umask 0027', the fetcher is going to [chmod 
executables back to 
755](https://github.com/apache/mesos/blob/9cfae1bdea14ab07b879d5aba8ec2c8ac2f76693/src/launcher/fetcher.cpp#L267).
- We should probably give the same treatment to sandboxes since the security 
concern there is the same.


So perhaps an agent flag `--executor_dir_mode` defaulting to `755` and chmoding 
both the user fetcher cache dir and the executor dir using this mode would 
solve this problem?

- Jiang Yan Xu


On June 16, 2017, 4:45 p.m., Silas Snider wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60101/
> ---
> 
> (Updated June 16, 2017, 4:45 p.m.)
> 
> 
> Review request for mesos, Joseph Wu and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-7298
> https://issues.apache.org/jira/browse/MESOS-7298
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Prevent the fetcher from setting overly-permissive fs permissions.
> 
> 
> Diffs
> -
> 
>   src/launcher/fetcher.cpp 42980f5a4a40b72f754156469e9fe60a952d1d87 
> 
> 
> Diff: https://reviews.apache.org/r/60101/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Silas Snider
> 
>



Re: Review Request 59739: Replaced use of `net::IPNetwork` by `net::IP::Network`.

2017-06-16 Thread Avinash sridharan

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

(Updated June 16, 2017, 11:30 p.m.)


Review request for mesos, Benjamin Hindman and Jie Yu.


Changes
---

Rebased.


Bugs: MESOS-7488
https://issues.apache.org/jira/browse/MESOS-7488


Repository: mesos


Description
---

Replaced use of `net::IPNetwork` by `net::IP::Network`.


Diffs (updated)
-

  3rdparty/stout/include/stout/posix/ip.hpp 
0d7e85a4b4b8263691dfb7654835312bbd117db6 
  src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
b386fbed17698dd6c59bc6c53edfe8a7ece6a3eb 
  
src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/port_mapper.cpp
 ce795628f19f0e8138ad9a0b50b7d644b9d734a8 
  src/tests/containerizer/cni_isolator_tests.cpp 
17afdf45a839bb80aa19663f97eaebec332ceee8 
  src/tests/utils.hpp a2ae43c6a20ab3753a3501d8ce23ffb0f6ac0005 
  src/tests/utils.cpp 053976d3c4cf120ff5e7dff6e96c6862a5b617b1 


Diff: https://reviews.apache.org/r/59739/diff/4/

Changes: https://reviews.apache.org/r/59739/diff/3-4/


Testing
---

make.


Thanks,

Avinash sridharan



Re: Review Request 59861: Added protobuf changes for reservation refinement.

2017-06-16 Thread Neil Conway

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


Fix it, then Ship it!





include/mesos/mesos.proto
Lines 349 (patched)


"The framework is offered resources in a new format."



include/mesos/mesos.proto
Lines 359 (patched)


"The framework can create refined reservations."



include/mesos/mesos.proto
Lines 368 (patched)


Typo: "capability"

Tweak: "A resource is said to have"



include/mesos/mesos.proto
Lines 980 (patched)


Is this consistent with the "set both old and new fields for endpoints" 
approach?



include/mesos/mesos.proto
Lines 1005 (patched)


"a framework or agent"? Seems technically correct but maybe confusing to 
mention here.



include/mesos/mesos.proto
Lines 1101 (patched)


Can we add a note here that the contents of the `reservations` field are 
ordered from "least refined" to "most refined"?



include/mesos/v1/mesos.proto
Lines 367 (patched)


Typo



include/mesos/v1/mesos.proto
Lines 1151 (patched)


Remove comma: "reservation's role,"


- Neil Conway


On June 16, 2017, 10:51 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59861/
> ---
> 
> (Updated June 16, 2017, 10:51 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Neil Conway.
> 
> 
> Bugs: MESOS-7575
> https://issues.apache.org/jira/browse/MESOS-7575
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> With reservation refinement, we now represent the reservation state in
> \`Resource.reservations\` rather than with \`Resource.role\` and
> \`Resource.reservation\`. We also keep track of the type of reservation,
> and the role the reservation is for in the \`Resource.ReservationInfo\`.
> This patch introduces these changes and explains the different formats
> in detail.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto e9a8b1513150b394f4986a1f34483cfca32b35e3 
>   include/mesos/v1/mesos.proto 5e8c15c07a02999e3376d2104c1c642f00511969 
>   src/common/protobuf_utils.hpp 7b1a1289c16edde44143432b645ecce68576 
>   src/common/protobuf_utils.cpp 46331b379c732edc597b46bb10665c95c9640e96 
> 
> 
> Diff: https://reviews.apache.org/r/59861/diff/12/
> 
> 
> Testing
> ---
> 
> Updated tests + `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>



Re: Review Request 59861: Added protobuf changes for reservation refinement.

2017-06-16 Thread Michael Park

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

(Updated June 16, 2017, 3:51 p.m.)


Review request for mesos, Benjamin Mahler and Neil Conway.


Changes
---

Added a few references.


Bugs: MESOS-7575
https://issues.apache.org/jira/browse/MESOS-7575


Repository: mesos


Description
---

With reservation refinement, we now represent the reservation state in
\`Resource.reservations\` rather than with \`Resource.role\` and
\`Resource.reservation\`. We also keep track of the type of reservation,
and the role the reservation is for in the \`Resource.ReservationInfo\`.
This patch introduces these changes and explains the different formats
in detail.


Diffs (updated)
-

  include/mesos/mesos.proto e9a8b1513150b394f4986a1f34483cfca32b35e3 
  include/mesos/v1/mesos.proto 5e8c15c07a02999e3376d2104c1c642f00511969 
  src/common/protobuf_utils.hpp 7b1a1289c16edde44143432b645ecce68576 
  src/common/protobuf_utils.cpp 46331b379c732edc597b46bb10665c95c9640e96 


Diff: https://reviews.apache.org/r/59861/diff/12/

Changes: https://reviews.apache.org/r/59861/diff/11-12/


Testing
---

Updated tests + `make check`


Thanks,

Michael Park



Re: Review Request 59861: Added protobuf changes for reservation refinement.

2017-06-16 Thread Michael Park

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

(Updated June 16, 2017, 3:29 p.m.)


Review request for mesos, Benjamin Mahler and Neil Conway.


Changes
---

Added a more descriptive comment for agent `RESERVATION_REFINEMNET` capability.


Bugs: MESOS-7575
https://issues.apache.org/jira/browse/MESOS-7575


Repository: mesos


Description (updated)
---

With reservation refinement, we now represent the reservation state in
\`Resource.reservations\` rather than with \`Resource.role\` and
\`Resource.reservation\`. We also keep track of the type of reservation,
and the role the reservation is for in the \`Resource.ReservationInfo\`.
This patch introduces these changes and explains the different formats
in detail.


Diffs (updated)
-

  include/mesos/mesos.proto e9a8b1513150b394f4986a1f34483cfca32b35e3 
  include/mesos/v1/mesos.proto 5e8c15c07a02999e3376d2104c1c642f00511969 
  src/common/protobuf_utils.hpp 7b1a1289c16edde44143432b645ecce68576 
  src/common/protobuf_utils.cpp 46331b379c732edc597b46bb10665c95c9640e96 


Diff: https://reviews.apache.org/r/59861/diff/11/

Changes: https://reviews.apache.org/r/59861/diff/10-11/


Testing
---

Updated tests + `make check`


Thanks,

Michael Park



Re: Review Request 59861: Added protobuf changes for reservation refinement.

2017-06-16 Thread Michael Park

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

(Updated June 16, 2017, 2:43 p.m.)


Review request for mesos, Benjamin Mahler and Neil Conway.


Changes
---

Addressed comments.


Bugs: MESOS-7575
https://issues.apache.org/jira/browse/MESOS-7575


Repository: mesos


Description
---

With reservation refinement, we now represent the reservation state in
`Resource.reservations` rather than with `Resource.role` and
`Resource.reservation`. We also keep track of the type of reservation,
and the role the reservation is for in the `Resource.ReservationInfo`.
This patch introduces these changes and explains the different formats
in detail.


Diffs (updated)
-

  include/mesos/mesos.proto e9a8b1513150b394f4986a1f34483cfca32b35e3 
  include/mesos/v1/mesos.proto 5e8c15c07a02999e3376d2104c1c642f00511969 
  src/common/protobuf_utils.hpp 7b1a1289c16edde44143432b645ecce68576 
  src/common/protobuf_utils.cpp 46331b379c732edc597b46bb10665c95c9640e96 


Diff: https://reviews.apache.org/r/59861/diff/10/

Changes: https://reviews.apache.org/r/59861/diff/9-10/


Testing
---

Updated tests + `make check`


Thanks,

Michael Park



Re: Review Request 59739: Replaced use of `net::IPNetwork` by `net::IP::Network`.

2017-06-16 Thread Avinash sridharan

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

(Updated June 16, 2017, 8:51 p.m.)


Review request for mesos, Benjamin Hindman and Jie Yu.


Changes
---

Rebased and fixed compilation errors i `cni_isolator_tests`.


Bugs: MESOS-7488
https://issues.apache.org/jira/browse/MESOS-7488


Repository: mesos


Description
---

Replaced use of `net::IPNetwork` by `net::IP::Network`.


Diffs (updated)
-

  3rdparty/stout/include/stout/posix/ip.hpp 
0d7e85a4b4b8263691dfb7654835312bbd117db6 
  src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
b386fbed17698dd6c59bc6c53edfe8a7ece6a3eb 
  
src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/port_mapper.cpp
 ce795628f19f0e8138ad9a0b50b7d644b9d734a8 
  src/tests/containerizer/cni_isolator_tests.cpp 
17afdf45a839bb80aa19663f97eaebec332ceee8 
  src/tests/utils.hpp a2ae43c6a20ab3753a3501d8ce23ffb0f6ac0005 
  src/tests/utils.cpp 053976d3c4cf120ff5e7dff6e96c6862a5b617b1 


Diff: https://reviews.apache.org/r/59739/diff/3/

Changes: https://reviews.apache.org/r/59739/diff/2-3/


Testing
---

make.


Thanks,

Avinash sridharan



Re: Review Request 60062: Prevented reserve/create with hierarchical roles on a non-capable agent.

2017-06-16 Thread Neil Conway

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


Ship it!




Ship It!

- Neil Conway


On June 16, 2017, 7:48 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60062/
> ---
> 
> (Updated June 16, 2017, 7:48 p.m.)
> 
> 
> Review request for mesos and Neil Conway.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch makes it such that reserve / create operations involving
> resources with a hierarchical role are invalid if they are attempted on
> an agent without a HIERARCHICAL_ROLE capability. Such attempts from an
> operator endpoint will result in a `BadRequest` response. On the
> frameworks side, we've opted to not offer resources from
> a non-HIERARCHICAL_ROLE agent to hierarchical roles (ca8d33ab).
> 
> If we were to undo ca8d33ab, an attempt from a framework to perform such
> an operation would be considered invalid due to this patch and the
> operation would be dropped by the master.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 1dcfe6ef00b0e3984deb79a511e665f638661323 
>   src/master/master.cpp 47a76ba9f549f2a170b5f424ac351e120ba75fb2 
>   src/master/validation.hpp 6b53e34f67f6a8b338b92db6cb7398f1dccce5a9 
>   src/master/validation.cpp f45f9e816bdaf6304794604570edbd68d5faf715 
>   src/tests/master_validation_tests.cpp 
> 4e7ce74edf0069b9317f869b299694a45e2a62f2 
> 
> 
> Diff: https://reviews.apache.org/r/60062/diff/10/
> 
> 
> Testing
> ---
> 
> Added new tests to `master_validation_tests.cpp` + `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>



Re: Review Request 60062: Prevented reserve/create with hierarchical roles on a non-capable agent.

2017-06-16 Thread Michael Park

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

(Updated June 16, 2017, 12:48 p.m.)


Review request for mesos and Neil Conway.


Changes
---

Addressed the rest of the comments.


Repository: mesos


Description
---

This patch makes it such that reserve / create operations involving
resources with a hierarchical role are invalid if they are attempted on
an agent without a HIERARCHICAL_ROLE capability. Such attempts from an
operator endpoint will result in a `BadRequest` response. On the
frameworks side, we've opted to not offer resources from
a non-HIERARCHICAL_ROLE agent to hierarchical roles (ca8d33ab).

If we were to undo ca8d33ab, an attempt from a framework to perform such
an operation would be considered invalid due to this patch and the
operation would be dropped by the master.


Diffs (updated)
-

  src/master/http.cpp 1dcfe6ef00b0e3984deb79a511e665f638661323 
  src/master/master.cpp 47a76ba9f549f2a170b5f424ac351e120ba75fb2 
  src/master/validation.hpp 6b53e34f67f6a8b338b92db6cb7398f1dccce5a9 
  src/master/validation.cpp f45f9e816bdaf6304794604570edbd68d5faf715 
  src/tests/master_validation_tests.cpp 
4e7ce74edf0069b9317f869b299694a45e2a62f2 


Diff: https://reviews.apache.org/r/60062/diff/9/

Changes: https://reviews.apache.org/r/60062/diff/8-9/


Testing
---

Added new tests to `master_validation_tests.cpp` + `make check`


Thanks,

Michael Park



Re: Review Request 60139: Avoided needless copy of `Owned` in libprocess.

2017-06-16 Thread Benjamin Bannier


> On June 16, 2017, 1:25 p.m., Benjamin Bannier wrote:
> > I do not agree with this change.
> > 
> > An `Owned` cannot be copied semantically. If we pass an `Owned` at all, it 
> > should be by value, not by reference, so potential copies happen on 
> > interface boundaries and we do not need to perform potentially unneeded 
> > copies inside the consuming functions. See e.g., 
> > https://herbsutter.com/2013/06/05/gotw-91-solution-smart-pointer-parameters/
> >  for a more detailed write-up.
> > 
> > I believe the existing code is still broken since it performs copies of an 
> > `Owned` (which is currently allowed syntactically, 
> > https://issues.apache.org/jira/browse/MESOS-5122). IMHO the correct fix 
> > would be to keep the existing signatures, but to `std::move` when passing 
> > the `authenticator` value on, both when used as function argument and when 
> > ultimatly stored in `AuthenticatorManagerProcess::authenticators_`. This 
> > would both prevent copies and express ownership semantics correctly.
> 
> Neil Conway wrote:
> In principle, I agree with you 100%. `Owned` should have `unique_ptr` 
> semantics (or we should just remove `Owned` and use `unique_ptr`), and a 
> function that takes over the ownership of an `Owned` pointer should use 
> `std::move`.
> 
> But until `Owned` actually enforces `unique_ptr` semantics and disallows 
> copies, do we actually want to encourage people using `std::move`? We use 
> `std::move` relatively rarely, and in the places where we use it, we often 
> get it wrong :) In contrast, we use `const Owned&` parameters reasonably 
> often right now...
> 
> At least in this case, we'd need to add quite a few `std::move`s to 
> account for all the call-sites. I guess for now I'll just give up on this, 
> and we can try to clean it up properly as part of fixing `Owned`.

> But until Owned actually enforces unique_ptr semantics and disallows copies, 
> do we actually want to encourage people using std::move?

I believe we should and I point out incorrect usage if I come across it. Using 
`Owned` incorrectly (e.g., copying them) can introduce blatant bugs which are 
not very apparent from looking at variable declarations alone. Semantically 
correct use of `Owned` from the start will also ease the transition to a true 
owning smart pointer (which we will need to tackle eventually).


- Benjamin


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


On June 16, 2017, 1:16 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60139/
> ---
> 
> (Updated June 16, 2017, 1:16 a.m.)
> 
> 
> Review request for mesos and Alexander Rojas.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Spotted via clang-tidy.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/authenticator_manager.hpp 
> 0dc8fd24b411d649bcc62208bde5784cac4ea997 
>   3rdparty/libprocess/src/authenticator_manager.cpp 
> 5cbed53e7085f227d90679e1b56ad803d9b74a47 
> 
> 
> Diff: https://reviews.apache.org/r/60139/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Neil Conway
> 
>



Re: Review Request 60139: Avoided needless copy of `Owned` in libprocess.

2017-06-16 Thread Neil Conway


> On June 16, 2017, 11:25 a.m., Benjamin Bannier wrote:
> > I do not agree with this change.
> > 
> > An `Owned` cannot be copied semantically. If we pass an `Owned` at all, it 
> > should be by value, not by reference, so potential copies happen on 
> > interface boundaries and we do not need to perform potentially unneeded 
> > copies inside the consuming functions. See e.g., 
> > https://herbsutter.com/2013/06/05/gotw-91-solution-smart-pointer-parameters/
> >  for a more detailed write-up.
> > 
> > I believe the existing code is still broken since it performs copies of an 
> > `Owned` (which is currently allowed syntactically, 
> > https://issues.apache.org/jira/browse/MESOS-5122). IMHO the correct fix 
> > would be to keep the existing signatures, but to `std::move` when passing 
> > the `authenticator` value on, both when used as function argument and when 
> > ultimatly stored in `AuthenticatorManagerProcess::authenticators_`. This 
> > would both prevent copies and express ownership semantics correctly.

In principle, I agree with you 100%. `Owned` should have `unique_ptr` semantics 
(or we should just remove `Owned` and use `unique_ptr`), and a function that 
takes over the ownership of an `Owned` pointer should use `std::move`.

But until `Owned` actually enforces `unique_ptr` semantics and disallows 
copies, do we actually want to encourage people using `std::move`? We use 
`std::move` relatively rarely, and in the places where we use it, we often get 
it wrong :) In contrast, we use `const Owned&` parameters reasonably often 
right now...

At least in this case, we'd need to add quite a few `std::move`s to account for 
all the call-sites. I guess for now I'll just give up on this, and we can try 
to clean it up properly as part of fixing `Owned`.


- Neil


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


On June 15, 2017, 11:16 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60139/
> ---
> 
> (Updated June 15, 2017, 11:16 p.m.)
> 
> 
> Review request for mesos and Alexander Rojas.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Spotted via clang-tidy.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/authenticator_manager.hpp 
> 0dc8fd24b411d649bcc62208bde5784cac4ea997 
>   3rdparty/libprocess/src/authenticator_manager.cpp 
> 5cbed53e7085f227d90679e1b56ad803d9b74a47 
> 
> 
> Diff: https://reviews.apache.org/r/60139/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Neil Conway
> 
>



Review Request 60162: Fixed bug in GroupTest.ConnectTimer, GroupTest.TimerCleanup.

2017-06-16 Thread Neil Conway

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

Review request for mesos and Andrei Budnik.


Repository: mesos


Description
---

These tests looked for dispatches to `GroupProces::expired` as a way to
determine when the current Zk session has expired. However, the previous
implementation of the connect timer (`GroupProcess::timedout`) invoked
`GroupProcess::expired` directly, which meant that an `EXPECT_DISPATCH`
on the `expired` method should not fire.

However, a separate bug in `EXPECT_DISPATCH` (MESOS-5886) meant that the
test expectations were actually being satisfied by a dispatch to
`GroupProcess::timedout`, which meant the tests happened to work (!).

Fix this by changing `GroupProcess::timedout` to dispatch to `expired`
rather than invoking it directly.


Diffs
-

  src/zookeeper/group.cpp 20f1928e05123d362147ce4ab0d6a752d95e466d 


Diff: https://reviews.apache.org/r/60162/diff/1/


Testing
---

`make check`

Validated that if the fix for MESOS-5886 is applied without this change, 
`GroupTest.ConnectTimer` fails. With this change, the test passes (both with 
and without the fix for MESOS-5886 applied).


Thanks,

Neil Conway



Review Request 60161: Cleaned up zookeeper binding code slightly.

2017-06-16 Thread Neil Conway

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

Review request for mesos and Andrei Budnik.


Repository: mesos


Description
---

Cleaned up zookeeper binding code slightly.


Diffs
-

  src/zookeeper/detector.cpp 973f5f297208dac2050d38fc5140a78c244666b7 
  src/zookeeper/group.cpp 20f1928e05123d362147ce4ab0d6a752d95e466d 


Diff: https://reviews.apache.org/r/60161/diff/1/


Testing
---

`make check`


Thanks,

Neil Conway



Re: Review Request 60008: Fixed bug causing FUTURE_DISPATCH to react on irrelevant dispatch (WIP).

2017-06-16 Thread Andrei Budnik

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

(Updated June 16, 2017, 5:34 p.m.)


Review request for mesos, Alexander Rukletsov, Michael Park, and Neil Conway.


Summary (updated)
-

Fixed bug causing FUTURE_DISPATCH to react on irrelevant dispatch (WIP).


Bugs: MESOS-5886
https://issues.apache.org/jira/browse/MESOS-5886


Repository: mesos


Description
---

FUTURE_DISPATCH uses DispatchMatcher to figure out whether a processed
DispatchEvent is the same the user is waiting for. Currently, we
compare std::type_info of function pointers, which is not enough:
different class methods with same signatures will be matched (see
MESOS-5886 for an example).
This patch adds value of pointer-to-member function in addition to
std::type_info in DispatchEvent to uniquely identify class methods.


Diffs
-

  3rdparty/libprocess/include/process/dispatch.hpp 
3a0793888dc0df5e3ec31b06f47cd920c71e0db9 
  3rdparty/libprocess/include/process/event.hpp 
8afe6266eb0dc5a17af35d79efb6bfdf9e6a0ee9 
  3rdparty/libprocess/include/process/gmock.hpp 
e9af943b39436f365fe687301febb5c7fbefffc4 
  3rdparty/libprocess/src/process.cpp 4ff7448d171f39dbb8cbb81dd9bed136ad43d62d 
  3rdparty/libprocess/src/tests/process_tests.cpp 
38d787a083a5eb31e922d283f4b4bed2bd62eb0a 


Diff: https://reviews.apache.org/r/60008/diff/1/


Testing
---

1. make check (mac os x 10.12, fedora 25)
2. internal CI

NOTE: Test GroupTest.ConnectTimer is broken, because it uses FUTURE_DISPATCH on 
a wrong method (GroupProcess::expired),
which has the same signature as a private method (GroupProcess::timedout). The 
later method is actually called,
thus causing the error after applying this patch.


Thanks,

Andrei Budnik



Re: Review Request 60067: Updated of documents including authorization in operator endpoints.

2017-06-16 Thread Till Toenshoff

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


Ship it!




Ship It!

- Till Toenshoff


On June 14, 2017, 9:48 a.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60067/
> ---
> 
> (Updated June 14, 2017, 9:48 a.m.)
> 
> 
> Review request for mesos, Greg Mann and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Automatically generated by running
> `./support/generate-endpoint-help.py`
> 
> 
> Diffs
> -
> 
>   docs/endpoints/master/machine/down.md 
> a605a99f6b3a5b05ddc6cdd9e71bc326c4f96c27 
>   docs/endpoints/master/machine/up.md 
> 67cfc159b579740f9c82edc35fe1a70165efb36d 
>   docs/endpoints/master/maintenance/schedule.md 
> d3d7154ed863d5c9f39099f945d30a5ef2d3475f 
>   docs/endpoints/master/maintenance/status.md 
> 1a35eba2c4b9ae236df1266fffa8ed007ccb4d6e 
> 
> 
> Diff: https://reviews.apache.org/r/60067/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>



Re: Review Request 59688: Moved `net::IPNetwork` to `net::IP:Network`.

2017-06-16 Thread Avinash sridharan

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

(Updated June 16, 2017, 4:50 p.m.)


Review request for mesos, Benjamin Hindman and Jie Yu.


Changes
---

Rebased.


Bugs: MESOS-7488
https://issues.apache.org/jira/browse/MESOS-7488


Repository: mesos


Description
---

Moved `net::IPNetwork` to `net::IP:Network`.


Diffs (updated)
-

  3rdparty/stout/include/stout/ip.hpp ee48befac8b9bb4956aec0c5bdebd69347bd5890 
  3rdparty/stout/tests/ip_tests.cpp 39ee0598e3580510a30894fd3f47d2014f596ef7 


Diff: https://reviews.apache.org/r/59688/diff/3/

Changes: https://reviews.apache.org/r/59688/diff/2-3/


Testing
---

make check


Thanks,

Avinash sridharan



Re: Review Request 59127: Added IPv6 flags for Mesos agent.

2017-06-16 Thread Avinash sridharan

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

(Updated June 16, 2017, 4:48 p.m.)


Review request for mesos, Benjamin Hindman and Jie Yu.


Changes
---

Rebased.


Bugs: MESOS-7488
https://issues.apache.org/jira/browse/MESOS-7488


Repository: mesos


Description
---

Added IPv6 flags for Mesos agent.


Diffs (updated)
-

  src/slave/flags.hpp 2f9d52e94c2c31e95208cd8b0640a5de2d2a61fd 
  src/slave/flags.cpp 93c8ffb5c822cf6c99071be7aca52a6b3d187619 


Diff: https://reviews.apache.org/r/59127/diff/3/

Changes: https://reviews.apache.org/r/59127/diff/2-3/


Testing
---

make check


Thanks,

Avinash sridharan



Re: Review Request 59128: Added initialization logic in Mesos agent for IPv6 flags.

2017-06-16 Thread Avinash sridharan

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

(Updated June 16, 2017, 4:48 p.m.)


Review request for mesos, Benjamin Hindman and Jie Yu.


Changes
---

Rebased.


Bugs: MESOS-7488
https://issues.apache.org/jira/browse/MESOS-7488


Repository: mesos


Description
---

Operators can now set the IPv6 address for the agent using the `--ip6`
and `--ip6_discovery_command` flags. Currently, the only place the agent
will use the IPv6 address would be to advertise v6 addresses for
containers running on the host network.


Diffs (updated)
-

  src/slave/main.cpp 66ae7db4039eedba7ff4eff51495384317c09354 


Diff: https://reviews.apache.org/r/59128/diff/2/

Changes: https://reviews.apache.org/r/59128/diff/1-2/


Testing
---

make check.


Thanks,

Avinash sridharan



Re: Review Request 59739: Replaced use of `net::IPNetwork` by `net::IP::Network`.

2017-06-16 Thread Avinash sridharan

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

(Updated June 16, 2017, 4:47 p.m.)


Review request for mesos, Benjamin Hindman and Jie Yu.


Changes
---

Rebased.


Bugs: MESOS-7488
https://issues.apache.org/jira/browse/MESOS-7488


Repository: mesos


Description
---

Replaced use of `net::IPNetwork` by `net::IP::Network`.


Diffs (updated)
-

  3rdparty/stout/include/stout/posix/ip.hpp 
0d7e85a4b4b8263691dfb7654835312bbd117db6 
  src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
b386fbed17698dd6c59bc6c53edfe8a7ece6a3eb 
  
src/slave/containerizer/mesos/isolators/network/cni/plugins/port_mapper/port_mapper.cpp
 ce795628f19f0e8138ad9a0b50b7d644b9d734a8 
  src/tests/containerizer/cni_isolator_tests.cpp 
17afdf45a839bb80aa19663f97eaebec332ceee8 
  src/tests/utils.hpp a2ae43c6a20ab3753a3501d8ce23ffb0f6ac0005 
  src/tests/utils.cpp 053976d3c4cf120ff5e7dff6e96c6862a5b617b1 


Diff: https://reviews.apache.org/r/59739/diff/2/

Changes: https://reviews.apache.org/r/59739/diff/1-2/


Testing
---

make.


Thanks,

Avinash sridharan



Re: Review Request 59129: Introduced `inet6::Address` to handle IPv6 addresses in `libprocess`.

2017-06-16 Thread Avinash sridharan

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

(Updated June 16, 2017, 4:47 p.m.)


Review request for mesos, Benjamin Hindman and Jie Yu.


Bugs: MESOS-7488
https://issues.apache.org/jira/browse/MESOS-7488


Repository: mesos


Description
---

Introduced `inet6::Address` to handle IPv6 addresses in `libprocess`.


Diffs
-

  3rdparty/libprocess/include/process/address.hpp 
6b143c3d00c1d7ebd1697c26b6d312a64f30839a 
  3rdparty/libprocess/src/socket.cpp 85920a1abb3a2b2da167647dcf1351b825c72c80 


Diff: https://reviews.apache.org/r/59129/diff/4/


Testing
---

make check.


Thanks,

Avinash sridharan



Re: Review Request 59721: Refactored net::IP and added net::IPv4 and net::IPv6.

2017-06-16 Thread Avinash sridharan

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

(Updated June 16, 2017, 4:46 p.m.)


Review request for mesos, Benjamin Hindman and Jie Yu.


Changes
---

Rebased.


Bugs: MESOS-7488
https://issues.apache.org/jira/browse/MESOS-7488


Repository: mesos


Description
---

Classes can now initialize the specialized classes whenever they need to
create explicit v4 and v6 addresses, rather than relying on the
`create/Try` semantics of net::IP.


Diffs (updated)
-

  3rdparty/stout/include/stout/ip.hpp ee48befac8b9bb4956aec0c5bdebd69347bd5890 
  3rdparty/stout/tests/ip_tests.cpp 39ee0598e3580510a30894fd3f47d2014f596ef7 


Diff: https://reviews.apache.org/r/59721/diff/4/

Changes: https://reviews.apache.org/r/59721/diff/3-4/


Testing
---

make check.


Thanks,

Avinash sridharan



Review Request 60136: Refactored inet::Address to base of inet4::Address and inet6::Address.

2017-06-16 Thread Avinash sridharan

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

Review request for mesos, Benjamin Hindman and Jie Yu.


Bugs: MESOS-7488
https://issues.apache.org/jira/browse/MESOS-7488


Repository: mesos


Description
---

Refactored inet::Address to base of inet4::Address and inet6::Address.


Diffs
-

  3rdparty/libprocess/include/process/address.hpp 
6b143c3d00c1d7ebd1697c26b6d312a64f30839a 
  3rdparty/libprocess/include/process/pid.hpp 
c634916a37f570194945b9c7d7b786ffeae0408a 
  3rdparty/libprocess/include/process/socket.hpp 
42287c128fb352af6a196b7abc77f913a2ddbba0 
  3rdparty/libprocess/src/http.cpp f317f2f0826e006342cfea20b15b80e7c8fc68a9 
  3rdparty/libprocess/src/libevent_ssl_socket.cpp 
7d493301bd5c0f24bf89e0b213f07ffe7801508b 
  3rdparty/libprocess/src/pid.cpp 023f881841f466d91243d2773961b67f82da4e91 
  3rdparty/libprocess/src/poll_socket.cpp 
9d1b2769ac31928d5ce4f64467c12ca0f58c11cf 
  3rdparty/libprocess/src/process.cpp 2a19078205e4f758edac868d4d5894a6b04d3cd9 
  3rdparty/libprocess/src/socket.cpp 85920a1abb3a2b2da167647dcf1351b825c72c80 
  3rdparty/libprocess/src/tests/http_tests.cpp 
09c7297aa0e80accd09ce6d81390a0f662bec6f2 
  3rdparty/libprocess/src/tests/process_tests.cpp 
38d787a083a5eb31e922d283f4b4bed2bd62eb0a 
  3rdparty/libprocess/src/tests/socket_tests.cpp 
fdcd23dcb05d683cfb3ae14956fe3fd8fe166603 
  3rdparty/libprocess/src/tests/test_linkee.cpp 
77df385d3388788658fa40d033816e1fbb8d8f2c 


Diff: https://reviews.apache.org/r/60136/diff/1/


Testing
---

make check


Thanks,

Avinash sridharan



Review Request 60148: Mesos updates for new inet4::Address.

2017-06-16 Thread Avinash sridharan

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

Review request for mesos and Benjamin Hindman.


Bugs: MESOS-7488
https://issues.apache.org/jira/browse/MESOS-7488


Repository: mesos


Description
---

Mesos updates for new inet4::Address.


Diffs
-

  src/tests/utils.cpp 053976d3c4cf120ff5e7dff6e96c6862a5b617b1 


Diff: https://reviews.apache.org/r/60148/diff/1/


Testing
---

make check.


Thanks,

Avinash sridharan



Re: Review Request 60149: Added support for `net::IPv4` and `net::IPv6` in `FlagsBase`.

2017-06-16 Thread Avinash sridharan

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

(Updated June 16, 2017, 4:41 p.m.)


Review request for mesos, Benjamin Hindman and Jie Yu.


Changes
---

Added MESOS-JIRA.


Bugs: MESOS-7488
https://issues.apache.org/jira/browse/MESOS-7488


Repository: mesos


Description
---

Added support for `net::IPv4` and `net::IPv6` in `FlagsBase`.


Diffs
-

  3rdparty/stout/include/stout/flags/parse.hpp 
3448c571ece36b8d45bbe1317efc824468fe3466 


Diff: https://reviews.apache.org/r/60149/diff/1/


Testing
---

make check.


Thanks,

Avinash sridharan



Review Request 60149: Added support for `net::IPv4` and `net::IPv6` in `FlagsBase`.

2017-06-16 Thread Avinash sridharan

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

Review request for mesos, Benjamin Hindman and Jie Yu.


Repository: mesos


Description
---

Added support for `net::IPv4` and `net::IPv6` in `FlagsBase`.


Diffs
-

  3rdparty/stout/include/stout/flags/parse.hpp 
3448c571ece36b8d45bbe1317efc824468fe3466 


Diff: https://reviews.apache.org/r/60149/diff/1/


Testing
---

make check.


Thanks,

Avinash sridharan



Re: Review Request 59131: Added an IPv6 address storage to UPID.

2017-06-16 Thread Avinash sridharan

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

(Updated June 16, 2017, 4:38 p.m.)


Review request for mesos and Jie Yu.


Changes
---

Rebased.


Bugs: MESOS-7488
https://issues.apache.org/jira/browse/MESOS-7488


Repository: mesos


Description (updated)
---

Added an IPv6 address storage to UPID.


Diffs (updated)
-

  3rdparty/libprocess/include/process/pid.hpp 
c634916a37f570194945b9c7d7b786ffeae0408a 


Diff: https://reviews.apache.org/r/59131/diff/3/

Changes: https://reviews.apache.org/r/59131/diff/2-3/


Testing
---

make check.


Thanks,

Avinash sridharan



Re: Review Request 59130: Added storage for IPv6 in a `libprocess` process.

2017-06-16 Thread Avinash sridharan

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

(Updated June 16, 2017, 4:36 p.m.)


Review request for mesos and Jie Yu.


Changes
---

Rebased and addressed BenH's comemnts.


Bugs: MESOS-7488
https://issues.apache.org/jira/browse/MESOS-7488


Repository: mesos


Description (updated)
---

Added storage for IPv6 in a `libprocess` process.


Diffs (updated)
-

  3rdparty/libprocess/src/process.cpp 2a19078205e4f758edac868d4d5894a6b04d3cd9 


Diff: https://reviews.apache.org/r/59130/diff/4/

Changes: https://reviews.apache.org/r/59130/diff/3-4/


Testing
---

make check.


Thanks,

Avinash sridharan



Re: Review Request 59233: Updated v6 address for containers running on host network.

2017-06-16 Thread Avinash sridharan

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

(Updated June 16, 2017, 4:36 p.m.)


Review request for mesos and Jie Yu.


Changes
---

Rebased.


Bugs: MESOS-7488
https://issues.apache.org/jira/browse/MESOS-7488


Repository: mesos


Description
---

Currently the agent is populating only the v4 address for containers
running on the host network. However, clusters running on a dual stack
network need to report v4 and v6 address for containers running on the
host network. This change allows v6 address specified by operators to be
advertised for containers running on the host network.


Diffs (updated)
-

  include/mesos/mesos.proto e9a8b1513150b394f4986a1f34483cfca32b35e3 
  include/mesos/v1/mesos.proto 5e8c15c07a02999e3376d2104c1c642f00511969 
  src/slave/slave.cpp 91a1bf330c3d5e91ef1b5ed016f9b3d0ad78400f 


Diff: https://reviews.apache.org/r/59233/diff/4/

Changes: https://reviews.apache.org/r/59233/diff/3-4/


Testing
---

make check


Thanks,

Avinash sridharan



Re: Review Request 59233: Updated v6 address for containers running on host network.

2017-06-16 Thread Avinash sridharan

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

(Updated June 16, 2017, 3:54 p.m.)


Review request for mesos and Jie Yu.


Changes
---

Rebased and addressed BenH's comments


Bugs: MESOS-7488
https://issues.apache.org/jira/browse/MESOS-7488


Repository: mesos


Description
---

Currently the agent is populating only the v4 address for containers
running on the host network. However, clusters running on a dual stack
network need to report v4 and v6 address for containers running on the
host network. This change allows v6 address specified by operators to be
advertised for containers running on the host network.


Diffs (updated)
-

  include/mesos/mesos.proto e9a8b1513150b394f4986a1f34483cfca32b35e3 
  include/mesos/v1/mesos.proto 5e8c15c07a02999e3376d2104c1c642f00511969 
  src/slave/slave.cpp 91a1bf330c3d5e91ef1b5ed016f9b3d0ad78400f 


Diff: https://reviews.apache.org/r/59233/diff/3/

Changes: https://reviews.apache.org/r/59233/diff/2-3/


Testing
---

make check


Thanks,

Avinash sridharan



Re: Review Request 60139: Avoided needless copy of `Owned` in libprocess.

2017-06-16 Thread Benjamin Bannier

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



I do not agree with this change.

An `Owned` cannot be copied semantically. If we pass an `Owned` at all, it 
should be by value, not by reference, so potential copies happen on interface 
boundaries and we do not need to perform potentially unneeded copies inside the 
consuming functions. See e.g., 
https://herbsutter.com/2013/06/05/gotw-91-solution-smart-pointer-parameters/ 
for a more detailed write-up.

I believe the existing code is still broken since it performs copies of an 
`Owned` (which is currently allowed syntactically, 
https://issues.apache.org/jira/browse/MESOS-5122). IMHO the correct fix would 
be to keep the existing signatures, but to `std::move` when passing the 
`authenticator` value on, both when used as function argument and when 
ultimatly stored in `AuthenticatorManagerProcess::authenticators_`. This would 
both prevent copies and express ownership semantics correctly.

- Benjamin Bannier


On June 16, 2017, 1:16 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60139/
> ---
> 
> (Updated June 16, 2017, 1:16 a.m.)
> 
> 
> Review request for mesos and Alexander Rojas.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Spotted via clang-tidy.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/authenticator_manager.hpp 
> 0dc8fd24b411d649bcc62208bde5784cac4ea997 
>   3rdparty/libprocess/src/authenticator_manager.cpp 
> 5cbed53e7085f227d90679e1b56ad803d9b74a47 
> 
> 
> Diff: https://reviews.apache.org/r/60139/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Neil Conway
> 
>



Re: Review Request 60088: CLI: Added 'agent' key as an acceptable key in config.toml.

2017-06-16 Thread Andrei Budnik

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



LGTM

- Andrei Budnik


On June 14, 2017, 3:17 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60088/
> ---
> 
> (Updated June 14, 2017, 3:17 p.m.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Bugs: MESOS-7284
> https://issues.apache.org/jira/browse/MESOS-7284
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This key is a dictionnary accepting two keys: ip and port.
> It will be used by future plugins.
> 
> 
> Diffs
> -
> 
>   src/cli_new/README.md c5475c7f4632fde5303ed1c901918d9088eac660 
>   src/cli_new/lib/cli/config.py 36a32f94bb12a033705b20f3c91d8bba972ba6c5 
> 
> 
> Diff: https://reviews.apache.org/r/60088/diff/1/
> 
> 
> Testing
> ---
> 
> Tested manually, PEP8 and Pylint used to make sure that the code style is 
> correct.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>