Re: Review Request 70907: Added recovery of agent drain information.

[Mesos Reviewbot]

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



Bad review!

Reviews applied: [70907, 70906, 70904, 70903, 70901, 70900, 70899, 70836, 
70835, 70834, 70839, 70822]

Error:
2019-06-21 03:36:28 URL:https://reviews.apache.org/r/70907/diff/raw/ 
[6759/6759] -> "70907.patch" [1]
error: patch failed: src/slave/state.cpp:203
error: src/slave/state.cpp: patch does not apply

- Mesos Reviewbot


On June 20, 2019, 9:45 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70907/
> ---
> 
> (Updated June 20, 2019, 9:45 p.m.)
> 
> 
> Review request for mesos, Greg Mann and Joseph Wu.
> 
> 
> Bugs: MESOS-9822
> https://issues.apache.org/jira/browse/MESOS-9822
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> With this patch the agent will, after executor reregistration finished,
> replay any active drain information so remaining tasks are drained as
> well. We need to wait until executors had a chance to register so they
> are not terminated should we try to send kill task request before the
> executor has registered.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp 30039b0857a4d85b4b96fa95d7f8724d57cdec6e 
>   src/slave/state.hpp 45836e5805a5038c0b3b5563ea995c3fa90ab808 
>   src/slave/state.cpp b4bf6292eb482f4e99a27442af0ea03e31c1ddc2 
>   src/tests/slave_tests.cpp c2035976713abb31b3646c0d23771fa40df93271 
> 
> 
> Diff: https://reviews.apache.org/r/70907/diff/2/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 70912: Removed const ref lifetime extension use in `slave::state::recover`.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [70822, 70839, 70834, 70835, 70836, 70899, 70900, 70901, 
70903, 70904, 70912]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose --disable-libtool-wrappers 
--disable-parallel-test-execution' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker-build.sh

- Mesos Reviewbot


On June 20, 2019, 2:44 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70912/
> ---
> 
> (Updated June 20, 2019, 2:44 p.m.)
> 
> 
> Review request for mesos and Greg Mann.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed const ref lifetime extension use in `slave::state::recover`.
> 
> 
> Diffs
> -
> 
>   src/slave/state.cpp b4bf6292eb482f4e99a27442af0ea03e31c1ddc2 
> 
> 
> Diff: https://reviews.apache.org/r/70912/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Review Request 70919: Replaced the old `struct Quota`.

[Meng Zhu]

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

Review request for mesos and Benjamin Mahler.


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


Repository: mesos


Description
---

The new `struct Quota` is consistent with the proto `QuotaConfig`
where guarantees and limits are decoupled and uses more proper
abstractions: `ResourceQuantities` and `ResourceLimits`.


Diffs
-

  include/mesos/allocator/allocator.hpp 
0aef0030e78514bef731b80de5b4bb3a567811e4 
  include/mesos/quota/quota.hpp edfd1c0b1e63b1aab66beb23c6dd8abc7432bab0 
  src/common/http.hpp b8c71ec3cf94b607ded689cafb91ab11308a1369 
  src/common/http.cpp 6962959326e8844a463eb8d63be04e5dfcc6bc6e 
  src/master/allocator/mesos/allocator.hpp 
0ae7e3697327832a0f95e2cd7339d8876e815be8 
  src/master/allocator/mesos/hierarchical.hpp 
25472eb7f5cb8342d2c88449c9c4145d816fb718 
  src/master/allocator/mesos/hierarchical.cpp 
c9ed094fcd67c1d8896fe380274ef8e08d5af892 
  src/master/allocator/mesos/metrics.hpp 
a0953b0cf716489ab94ae54026e8328830ea719e 
  src/master/allocator/mesos/metrics.cpp 
b61a39b1cd8e15043a68337b20b057c70903115f 
  src/master/constants.hpp 850224064cfba4859275d2d5bc44248e053b85e5 
  src/master/master.hpp 7acaa8264eb7a37e17394bc4940971f872ab2de3 
  src/master/master.cpp 8a238aa1d5592f91d8c3b3c443aa30acd3a0cfd0 
  src/master/quota.cpp 4f2a59c234308be8a5ba16cb1f32e6729b79343d 
  src/master/quota_handler.cpp 854189cd052cc88b4dc0ade72c63ac9a1b0bcbb5 
  src/tests/allocator.hpp 765968d5bf9846324d8d7f1d3f6687155c9ea2b5 
  src/tests/allocator.cpp 553418778350d921f4f1bcec50cd7eeb3a74cc28 
  src/tests/hierarchical_allocator_tests.cpp 
7dc3dc1392b120e2798dcd68f7290cae4afa890d 
  src/tests/master_quota_tests.cpp 9b4cde1c154613f6af50ddc3484f689a9fe836c0 


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


Testing
---


Thanks,

Meng Zhu

Review Request 70918: Removed `setQuota` and `removeQuota` methods in the allocator.

[Meng Zhu]

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

Review request for mesos and Benjamin Mahler.


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


Repository: mesos


Description
---

These are replaced by the `updateQuota` method.


Diffs
-

  include/mesos/allocator/allocator.hpp 
0aef0030e78514bef731b80de5b4bb3a567811e4 
  src/master/allocator/mesos/allocator.hpp 
0ae7e3697327832a0f95e2cd7339d8876e815be8 
  src/master/allocator/mesos/hierarchical.hpp 
25472eb7f5cb8342d2c88449c9c4145d816fb718 
  src/master/allocator/mesos/hierarchical.cpp 
c9ed094fcd67c1d8896fe380274ef8e08d5af892 
  src/master/allocator/mesos/metrics.hpp 
a0953b0cf716489ab94ae54026e8328830ea719e 
  src/master/allocator/mesos/metrics.cpp 
b61a39b1cd8e15043a68337b20b057c70903115f 
  src/master/quota_handler.cpp 854189cd052cc88b4dc0ade72c63ac9a1b0bcbb5 
  src/tests/allocator.hpp 765968d5bf9846324d8d7f1d3f6687155c9ea2b5 


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


Testing
---

make check


Thanks,

Meng Zhu

Review Request 70917: Refactored the allocator test to use the `updateQuota` method.

[Meng Zhu]

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

Review request for mesos and Benjamin Mahler.


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


Repository: mesos


Description
---

This paves the way to remove `setQuota` and `removeQuota` methods.


Diffs
-

  src/tests/allocator.hpp 765968d5bf9846324d8d7f1d3f6687155c9ea2b5 
  src/tests/allocator.cpp 553418778350d921f4f1bcec50cd7eeb3a74cc28 
  src/tests/hierarchical_allocator_benchmarks.cpp 
04c124985ac807d13ff09e4142d407d788e66abe 
  src/tests/hierarchical_allocator_tests.cpp 
7dc3dc1392b120e2798dcd68f7290cae4afa890d 


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


Testing
---

make check


Thanks,

Meng Zhu

Review Request 70916: Replaced `Quota` with `Quota2` in the master state.

[Meng Zhu]

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

Review request for mesos and Benjamin Mahler.


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


Repository: mesos


Description
---

This paves way to remove `struct Quota`.


Diffs
-

  src/master/master.hpp 7acaa8264eb7a37e17394bc4940971f872ab2de3 
  src/master/master.cpp 8a238aa1d5592f91d8c3b3c443aa30acd3a0cfd0 
  src/master/quota_handler.cpp 854189cd052cc88b4dc0ade72c63ac9a1b0bcbb5 
  src/master/readonly_handler.cpp d62f139ed25dc6d03e91f76bf6574ad8c3c6baa0 


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


Testing
---

make check


Thanks,

Meng Zhu

Review Request 70915: Made `/roles` endpoint also return quota limits.

[Meng Zhu]

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

Review request for mesos and Benjamin Mahler.


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


Repository: mesos


Description
---

Now that guarantees is decoupled from limits, we should return
limits and guarantees separately in the `/roles` endpoint.

Also fixed an affected test.


Diffs
-

  src/common/http.hpp b8c71ec3cf94b607ded689cafb91ab11308a1369 
  src/common/http.cpp 6962959326e8844a463eb8d63be04e5dfcc6bc6e 
  src/master/readonly_handler.cpp d62f139ed25dc6d03e91f76bf6574ad8c3c6baa0 
  src/tests/role_tests.cpp e428fb48344c721386aa3d3ffcf18a84e4b6c9f5 


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


Testing
---

make check


Thanks,

Meng Zhu

Review Request 70914: Defined `mapped_type` for `ResourceQuantities` and `ResourceLimits`.

[Meng Zhu]

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

Review request for mesos and Benjamin Mahler.


Repository: mesos


Description
---

This let the two pass the `HasMappedType` and benefit from
templated functions such as `json`.


Diffs
-

  include/mesos/resource_quantities.hpp 
c861df8a8a167b19ea8374c22cdd2a8fe567a6a6 


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


Testing
---

make check


Thanks,

Meng Zhu

Re: Review Request 70795: Updated SSL docs to include new libprocess flag.

[Till Toenshoff via Review Board]

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


Fix it, then Ship it!




Great! 

Please don't forget to update the review description to match the changed 
naming.


docs/ssl.md
Lines 176 (patched)


s/can not/cannot/



docs/ssl.md
Lines 194 (patched)


I wonder if we should already start a deprecation of the `libprocess` 
scheme - that would be:
- announcing that `openssl` will be standard soon on compatible boxes
- announcing it to be gone at some point

Or am I too eager for unification here?


- Till Toenshoff


On June 20, 2019, 5:27 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70795/
> ---
> 
> (Updated June 20, 2019, 5:27 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Jan-Philip Gehrcke, Joseph Wu, 
> and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a description of the new `--hostname_validation_algorithm` flag
> and corresponding `LIBPROCESS_SSL_HOSTNAME_VALIDATION_ALGORITHM`
> environment variable.
> 
> 
> Diffs
> -
> 
>   docs/ssl.md ce5058896144aa7824986d40d996899d92cb7c1c 
> 
> 
> Diff: https://reviews.apache.org/r/70795/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 70749: Introduced optional new scheme for hostname validation.

[Till Toenshoff via Review Board]

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




3rdparty/libprocess/src/openssl.cpp
Lines 147 (patched)


s/algorithm/scheme/?



3rdparty/libprocess/src/openssl.cpp
Lines 153 (patched)


s/algorithm/scheme/?

Also, we should tell the user which version he needs at least here, nice 
touch ;)



3rdparty/libprocess/src/openssl.cpp
Lines 565-566 (patched)


Should we 
```
EXIT(EXIT_FAILURE)
 << "Unknown value for hostname_validation_scheme: "
   << ssl_flags->hostname_validation_scheme;  
```
instead here and below to be perfectly explicit and consistent?



3rdparty/libprocess/src/openssl.cpp
Lines 576 (patched)


s/algorithm/scheme/?



3rdparty/libprocess/src/openssl.cpp
Line 755 (original), 788 (patched)


Maybe
```
// When using the OpenSSL built-in scheme,...
```



3rdparty/libprocess/src/openssl.cpp
Lines 795 (patched)


s/algorithm/scheme/



3rdparty/libprocess/src/openssl.cpp
Lines 808 (patched)


I know you just moved it, but where do these 100ms come from and how could 
we be more explicit about that choice?

I would suggest to use a const with some explaining comment how that value 
was chosen - can we please? :D



3rdparty/libprocess/src/openssl.cpp
Lines 984-985 (patched)


This sounds like a great idea worth spending 1 more cycle on -- can we 
create and reference a ticket that explains this jazz as nicely as we do here 
in the code?

My thought is that being open about this idea in JIRA,  we would provide 
more chances of getting community support for it.



3rdparty/libprocess/src/openssl.cpp
Lines 989 (patched)


Shall we explain why this is the `right` way - aka best practices?



3rdparty/libprocess/src/openssl.cpp
Lines 1016 (patched)


s/ip/IP/



3rdparty/libprocess/src/posix/libevent/libevent_ssl_socket.cpp
Lines 444-448 (patched)


Great comment - thanks!



3rdparty/libprocess/src/posix/libevent/libevent_ssl_socket.cpp
Lines 569 (patched)


Now that we log the `peer_hostname`, I wonder if it made sense to also log 
the `peer_ip` like this - what do you think? If you agree, then we should even 
try to render a single log-line out of those two - it will be noisy enough on 
level 2.



3rdparty/libprocess/src/posix/libevent/libevent_ssl_socket.cpp
Lines 1129 (patched)


I feel `peername` comes in a bit surprising here if one does not know about 
`getpeername()`.

Maybe s/peername/IP address/?



3rdparty/libprocess/src/posix/libevent/libevent_ssl_socket.cpp
Lines 1136 (patched)


As mentioned in some other place of this chain, let's add a neat JIRA on 
this plan.


- Till Toenshoff


On June 20, 2019, 5:48 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70749/
> ---
> 
> (Updated June 20, 2019, 5:48 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Joseph Wu, and Till Toenshoff.
> 
> 
> Bugs: MESOS-9809
> https://issues.apache.org/jira/browse/MESOS-9809
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This commit introduces a new libprocess SSL flag
> `hostname_validation_scheme`, which can be used to select
> between the previous hostname validation behaviour and a new
> option to use standardized OpenSSL algorithms to handle
> hostname validation as part of the handshake.
> 
> As a nice side-effect, the new scheme gets rid of reverse DNS
> lookups during TLS connection establishment, which used to be
> a common source of hard-to-debug unresponsiveness in Mesos
> components.
> 
> See `docs/ssl.md` in the follow-up commit for details of and
> differences between the schemes.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/ssl/flags.hpp 
> f3483f97f93bb29117b2c78f0f2ed9735d9c4b3a 
>   3rdparty/libprocess/src/openssl.hpp 
> 17bec246e516261f8d772f1647c17f092fae82d1 
>   3rdparty/libprocess/src/openssl.cpp 
> 

Re: Review Request 70796: Fixed Mesos unit tests after Address API change.

[Till Toenshoff via Review Board]

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


Ship it!




- Till Toenshoff


On June 19, 2019, 2:44 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70796/
> ---
> 
> (Updated June 19, 2019, 2:44 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Joseph Wu, and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adjusts some Mesos unit tests after renaming
> `Address::hostname()` to `Address::lookup_hostname()`.
> 
> 
> Diffs
> -
> 
>   src/tests/master_tests.cpp 097f1b77a59e29c6690210773d1556ebf2bb701e 
> 
> 
> Diff: https://reviews.apache.org/r/70796/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 70885: Renamed 'libprocess::network::Address::hostname()'.

[Till Toenshoff via Review Board]

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


Ship it!




Thanks a bunch - much better signal for a pricy function now :)

- Till Toenshoff


On June 19, 2019, 2:44 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70885/
> ---
> 
> (Updated June 19, 2019, 2:44 p.m.)
> 
> 
> Review request for mesos, Joseph Wu and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Renamed member function `hostname()` to `lookup_hostname()`,
> since the former name hides the fact that a call to this
> function involves a synchronous network access in order
> to make a reverse DNS lookup.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/address.hpp 
> e740e840c38381bafd7a1a7fcde5f963832ac1fb 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> 97aaf3ed3d4fab6d717d5c9b6d12402562ac6b46 
>   3rdparty/libprocess/src/tests/ssl_tests.cpp 
> 6b8496aeeed79ae1bd39d7013f4f403b248fdd4c 
> 
> 
> Diff: https://reviews.apache.org/r/70885/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 70883: Added optional 'peer_hostname' argument to Socket::connect().

[Till Toenshoff via Review Board]

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


Ship it!




We have discussed this patch a bit out of band and agreed that passing the 
`peer_hostname` as an additional argument into the `process::connect` function 
and socket stack below was the most sensible we can do. Alternatives like 
passing as a new `network::Address` member has surprising 
conversion/typecasting results while exploiting the idea of a network address. 
Adding a setter to our `Socket` might be another option but here we would have 
to supply a noop member of the non SSL variant. With both variants we would 
have widened the scope of this argument to areas where it does not play a role.


3rdparty/libprocess/include/process/socket.hpp
Lines 371 (patched)


Too bad we can't default.


- Till Toenshoff


On June 19, 2019, 2:34 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70883/
> ---
> 
> (Updated June 19, 2019, 2:34 p.m.)
> 
> 
> Review request for mesos, Joseph Wu and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The Socket::connect() function now takes an optional string
> as an additional argument. This is to prepare support for proper
> TLS hostname validation.
> 
> With TCP, a connection is always made to a specific IP address,
> with the hostname just serving as an artifact to help humans
> remember that address.
> 
> With TLS, the roles are switched: A connection is made to a
> specific hostname (which is recorded in a TLS certificate),
> with the IP address just being a network-layer artifact to
> help packets route to that hostname.
> 
> Therefore, a connecting TLS socket must be aware of the
> hostname it is supposed to connect to.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> 029605eaff72e80206cb7dfd64c2f898084155e0 
>   3rdparty/libprocess/include/process/socket.hpp 
> 4f0f6e9aa6e95e826e3de96e518a7200ad7a8f83 
>   3rdparty/libprocess/src/http.cpp 3e73ee936f5c6329f41704a179f3d88ab65dfb6d 
>   3rdparty/libprocess/src/poll_socket.hpp 
> 15b7902ba2b10fad63e2ba7b8d5081d4b9e2d1c7 
>   3rdparty/libprocess/src/posix/libevent/libevent_ssl_socket.hpp 
> 6ef5a86566af3439cfe0b06ab3576076623f7be0 
>   3rdparty/libprocess/src/posix/libevent/libevent_ssl_socket.cpp 
> 29a1bf71c1df9d80370455a6269ecea0ec4193b0 
>   3rdparty/libprocess/src/posix/poll_socket.cpp 
> 74acb6942682a9d9626df81b303eba0a1c24ecf7 
>   3rdparty/libprocess/src/windows/poll_socket.cpp 
> 565b0088dc2b270193e615655f57f48419eb2c12 
> 
> 
> Diff: https://reviews.apache.org/r/70883/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 70810: Updated SSL docs with suggested runtime configuration.

[Till Toenshoff via Review Board]

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


Ship it!




Awesome - thanks!

- Till Toenshoff


On June 20, 2019, 5:32 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70810/
> ---
> 
> (Updated June 20, 2019, 5:32 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Joseph Wu, and Till Toenshoff.
> 
> 
> Bugs: MESOS-9791
> https://issues.apache.org/jira/browse/MESOS-9791
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated SSL docs with suggested runtime configuration.
> 
> 
> Diffs
> -
> 
>   docs/ssl.md ce5058896144aa7824986d40d996899d92cb7c1c 
> 
> 
> Diff: https://reviews.apache.org/r/70810/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 70748: Changed semantics of some libprocess TLS flags.

[Till Toenshoff via Review Board]

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


Fix it, then Ship it!




Great - thanks so much!


3rdparty/libprocess/src/openssl.cpp
Line 90 (original), 90 (patched)


`verify_cert` effectively becomes a TLS client mode specific setting - we 
should explain that here. In my mind, in TLS server mode, `verify_cert` stricly 
follows `require_cert` in its state.



3rdparty/libprocess/src/openssl.cpp
Line 95 (original), 95 (patched)


This is a TLS server mode only setting by now - we need to call that out I 
feel. In TLS client mode, we have `require_cert` enabled implicitly.



3rdparty/libprocess/src/openssl.cpp
Line 96 (original), 96 (patched)


Additional to the updates above, we also need to call this out in the 
CHANGELOG. Can you please include that in this chain?



3rdparty/libprocess/src/openssl.cpp
Lines 743-745 (patched)


This is so important, I would like to add more meat to it;

Let's clarify that the OpenSSL handshake itself will do this, before we 
even reach our additional verification. Also, the use of `remote` confuses me - 
do local servers have an exception here? 

Maybe something like this?
```
  // NOTE: Even without this check, the OpenSSL handshake will
  // not complete when connecting to servers that do not present
  // a certificate, unless an anonymous cipher is used.
```



3rdparty/libprocess/src/tests/ssl_tests.cpp
Lines 296-297 (patched)


Nice, thank you!


- Till Toenshoff


On June 19, 2019, 2:28 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70748/
> ---
> 
> (Updated June 19, 2019, 2:28 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Jan-Philip Gehrcke, Joseph Wu, 
> and Till Toenshoff.
> 
> 
> Bugs: MESOS-9810
> https://issues.apache.org/jira/browse/MESOS-9810
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This commit slightly updates the semants of the
> `LIBPROCESS_SSL_VERIFY_CERT` and `LIBPROCESS_SSL_REQUIRE_CERT`
> environment variables. The former now only applies to connections
> in client mode and the latter now only applies to connections in
> server mode.
> 
> In particular, in TLS server mode we now *only* verify client
> certificates when `LIBPROCESS_SSL_REQUIRE_CERT` is set to `true`,
> regardless of the value of `LIBPROCESS_SSL_VERIFY_CERT`.
> 
> In addition, when in SSL client mode and  `LIBPROCESS_SSL_VERIFY_CERT`
> has been set to `true`, enforce that the server actually presents a
> certificate that can be verified. Note that this is expected to be
> not a behavioural change in practice, since the TLS specification
> already states that a server MUST always send a certificate unless an
> anonymous cipher is used, and most TLS ciphersuites are configured to
> exclude anonymous ciphers.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/openssl.hpp 
> 17bec246e516261f8d772f1647c17f092fae82d1 
>   3rdparty/libprocess/src/openssl.cpp 
> e7dbd67913fa8e7fbbf60dee428e7e38895f86ce 
>   3rdparty/libprocess/src/posix/libevent/libevent_ssl_socket.cpp 
> 29a1bf71c1df9d80370455a6269ecea0ec4193b0 
>   3rdparty/libprocess/src/tests/ssl_tests.cpp 
> 6b8496aeeed79ae1bd39d7013f4f403b248fdd4c 
> 
> 
> Diff: https://reviews.apache.org/r/70748/diff/4/
> 
> 
> Testing
> ---
> 
> So far, mostly manual testing.
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 70820: Updated `filesystem/linux` isolator for configurable IPC support.

[Gilbert Song]

> On June 20, 2019, 5:27 p.m., Gilbert Song wrote:
> > could we consider to do the decouple now?

probably still need a deprecation cycle :(


- Gilbert


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


On June 12, 2019, 7:32 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70820/
> ---
> 
> (Updated June 12, 2019, 7:32 a.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Gilbert Song.
> 
> 
> Bugs: MESOS-9826
> https://issues.apache.org/jira/browse/MESOS-9826
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> If `namespaces/ipc` isolator is not enabled, for backward
> compatibility /dev/shm will still be handled in `filesystem/linux`
> isolator as before. Otherwise, both /dev/shm and IPC namespace
> will be handled by `namespaces/ipc` isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 
> 3cfb6e97a565420c8be2a0e31b481b39cd09d9da 
> 
> 
> Diff: https://reviews.apache.org/r/70820/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 70820: Updated `filesystem/linux` isolator for configurable IPC support.

[Gilbert Song]

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



could we consider to do the decouple now?

- Gilbert Song


On June 12, 2019, 7:32 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70820/
> ---
> 
> (Updated June 12, 2019, 7:32 a.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Gilbert Song.
> 
> 
> Bugs: MESOS-9826
> https://issues.apache.org/jira/browse/MESOS-9826
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> If `namespaces/ipc` isolator is not enabled, for backward
> compatibility /dev/shm will still be handled in `filesystem/linux`
> isolator as before. Otherwise, both /dev/shm and IPC namespace
> will be handled by `namespaces/ipc` isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 
> 3cfb6e97a565420c8be2a0e31b481b39cd09d9da 
> 
> 
> Diff: https://reviews.apache.org/r/70820/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 70844: Implemented `cleanup` method of the `namespaces/ipc` isolator.

[Gilbert Song]

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




src/slave/containerizer/mesos/isolators/namespaces/ipc.cpp
Lines 400 (patched)


if the shmPath not exists, should we log a warning?


- Gilbert Song


On June 12, 2019, 7:36 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70844/
> ---
> 
> (Updated June 12, 2019, 7:36 a.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Gilbert Song.
> 
> 
> Bugs: MESOS-9788
> https://issues.apache.org/jira/browse/MESOS-9788
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented `cleanup` method of the `namespaces/ipc` isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/namespaces/ipc.hpp 
> 32c888309ca536d944e4d73641aed214805ccce2 
>   src/slave/containerizer/mesos/isolators/namespaces/ipc.cpp 
> 6c8e8eed69ed355094f7d9b54bde25a8a9bc63f7 
> 
> 
> Diff: https://reviews.apache.org/r/70844/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 70798: Improved `namespaces/ipc` isolator for configurable IPC support.

[Gilbert Song]

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




src/slave/containerizer/mesos/isolators/namespaces/ipc.cpp
Lines 52 (patched)


consider to move it to containerizer/mesos/path.hpp?



src/slave/containerizer/mesos/isolators/namespaces/ipc.cpp
Lines 52-54 (patched)


consider to return Result for no parent case?



src/slave/containerizer/mesos/isolators/namespaces/ipc.cpp
Lines 56 (patched)


a bit confusing. the parentId could be assigned as a containerId without a 
parent?



src/slave/containerizer/mesos/isolators/namespaces/ipc.cpp
Lines 90-93 (patched)


what if for SHARE_PARENT case but the container does not have a parent?



src/slave/containerizer/mesos/isolators/namespaces/ipc.cpp
Lines 137-143 (patched)


is this  a new dependency?



src/slave/containerizer/mesos/isolators/namespaces/ipc.cpp
Lines 212 (patched)


are we going to remove the `createContainerMount()` in linux filesystem 
isolator?



src/slave/containerizer/mesos/isolators/namespaces/ipc.cpp
Line 94 (original), 382 (patched)


if we introduce cleanup(), does it mean we could avoid the dependency on 
linux filesystem isolator?



src/slave/containerizer/mesos/paths.hpp
Lines 85 (patched)


how about `CONTAINER_SHM_DIRECTORY` or just `CONTAINER_SHM`?


- Gilbert Song


On June 11, 2019, 7:33 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70798/
> ---
> 
> (Updated June 11, 2019, 7:33 a.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Gilbert Song.
> 
> 
> Bugs: MESOS-9788
> https://issues.apache.org/jira/browse/MESOS-9788
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Improved `namespaces/ipc` isolator for configurable IPC support.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/namespaces/ipc.hpp 
> 32c888309ca536d944e4d73641aed214805ccce2 
>   src/slave/containerizer/mesos/isolators/namespaces/ipc.cpp 
> 6c8e8eed69ed355094f7d9b54bde25a8a9bc63f7 
>   src/slave/containerizer/mesos/paths.hpp 
> a5e0920bd4d753f6ba6f3b092c92cb405e9edf35 
>   src/slave/containerizer/mesos/paths.cpp 
> 4281abc522c942c87fcfd811af26f95cbd6f734f 
>   src/tests/containerizer/isolator_tests.cpp 
> 9c14f3acbc19631b2f5cac4dc7cd9caba8527712 
> 
> 
> Diff: https://reviews.apache.org/r/70798/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 70907: Added recovery of agent drain information.

[Benjamin Bannier]

> On June 20, 2019, 5:31 p.m., Greg Mann wrote:
> > src/slave/slave.cpp
> > Lines 5639 (patched)
> > 
> >
> > Are we sure this operation will be idempotent for all executor types?

The way this test is written we are more checking the agent's behavior and less 
that of executors, i.e., agent resumes draining if it fails over after 
receiving a drain request.

That being said, the test here uses a `MockExecutor` which currently only 
instantiated for `MesosContainerizer`. Testing multiple containerizers doesn't 
seem that interesting here.

Moved this to `SlaveTest`.


- Benjamin


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


On June 20, 2019, 11:45 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70907/
> ---
> 
> (Updated June 20, 2019, 11:45 p.m.)
> 
> 
> Review request for mesos, Greg Mann and Joseph Wu.
> 
> 
> Bugs: MESOS-9822
> https://issues.apache.org/jira/browse/MESOS-9822
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> With this patch the agent will, after executor reregistration finished,
> replay any active drain information so remaining tasks are drained as
> well. We need to wait until executors had a chance to register so they
> are not terminated should we try to send kill task request before the
> executor has registered.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp 30039b0857a4d85b4b96fa95d7f8724d57cdec6e 
>   src/slave/state.hpp 45836e5805a5038c0b3b5563ea995c3fa90ab808 
>   src/slave/state.cpp b4bf6292eb482f4e99a27442af0ea03e31c1ddc2 
>   src/tests/slave_tests.cpp c2035976713abb31b3646c0d23771fa40df93271 
> 
> 
> Diff: https://reviews.apache.org/r/70907/diff/2/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 70907: Added recovery of agent drain information.

[Benjamin Bannier]

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

(Updated June 20, 2019, 11:45 p.m.)


Review request for mesos, Greg Mann and Joseph Wu.


Changes
---

Address Greg's comments


Summary (updated)
-

Added recovery of agent drain information.


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


Repository: mesos


Description
---

With this patch the agent will, after executor reregistration finished,
replay any active drain information so remaining tasks are drained as
well. We need to wait until executors had a chance to register so they
are not terminated should we try to send kill task request before the
executor has registered.


Diffs (updated)
-

  src/slave/slave.cpp 30039b0857a4d85b4b96fa95d7f8724d57cdec6e 
  src/slave/state.hpp 45836e5805a5038c0b3b5563ea995c3fa90ab808 
  src/slave/state.cpp b4bf6292eb482f4e99a27442af0ea03e31c1ddc2 
  src/tests/slave_tests.cpp c2035976713abb31b3646c0d23771fa40df93271 


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

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


Testing
---

`make check`


Thanks,

Benjamin Bannier

Review Request 70912: Removed const ref lifetime extension use in `slave::state::recover`.

[Benjamin Bannier]

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

Review request for mesos and Greg Mann.


Repository: mesos


Description
---

Removed const ref lifetime extension use in `slave::state::recover`.


Diffs
-

  src/slave/state.cpp b4bf6292eb482f4e99a27442af0ea03e31c1ddc2 


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


Testing
---

`make check`


Thanks,

Benjamin Bannier

Re: Review Request 70886: WIP: Override source address for executors.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [70747, 70748, 70810, 70883, 70884, 70885, 70796, 70749, 
70795, 70797, 70886]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose --disable-libtool-wrappers 
--disable-parallel-test-execution' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker-build.sh

- Mesos Reviewbot


On June 19, 2019, 7:48 a.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70886/
> ---
> 
> (Updated June 19, 2019, 7:48 a.m.)
> 
> 
> Review request for mesos, Jan-Philip Gehrcke, Joseph Wu, and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Ignore an executor's reported IP address and enforce the hostname
> "localhost" instead for TLS hostname validation.
> 
> NOTE: This change is for illustration purposes only, and is not
> intended to be upstreamed.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp 30039b0857a4d85b4b96fa95d7f8724d57cdec6e 
> 
> 
> Diff: https://reviews.apache.org/r/70886/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Review Request 70910: Added ACLs for agent draining APIs.

[Joseph Wu]

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

Review request for mesos, Benjamin Bannier, Benjamin Mahler, Greg Mann, and 
Vinod Kone.


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


Repository: mesos


Description
---

This adds three ACLs for the three endpoints added as part of the
agent draining feature:
  * DRAIN_AGENT
  * DEACTIVATE_AGENT
  * REACTIVATE_AGENT

All three ACLs are coarse-grained; a principal is either allowed to
perform the action, or unauthorized. However, there is scope to
restrict the action to particular subsets of agents in future.


Diffs
-

  include/mesos/authorizer/acls.proto 78acfe622603f3697150c81bcf8c7a31deed9699 
  include/mesos/authorizer/authorizer.proto 
ad40d3add9db7540c328ebcf3aae771ff966528e 
  src/authorizer/local/authorizer.cpp b89010d09ef3ee564e7659c0cfe335281656e1cd 
  src/tests/authorization_tests.cpp 7bb0bd831c7c71558cc981ce00ab399e0034bbdf 


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


Testing
---

make check


Thanks,

Joseph Wu

Review Request 70911: WIP: Added master endpoints for agent draining.

[Joseph Wu]

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

Review request for mesos, Benjamin Bannier, Benjamin Mahler, Greg Mann, and 
Vinod Kone.


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


Repository: mesos


Description
---

This adds three master calls:
  * DRAIN_AGENT
  * DEACTIVATE_AGENT
  * REACTIVATE_AGENT

DRAIN_AGENT starts automated draining of tasks on the specified agent.
When marked for draining, the agent's resources will not be offered
and all the agent's tasks will be gracefully killed.

DEACTIVATE_AGENT stops offers for a specific agent.  This can be used
for manual draining of tasks, but is not limited to draining.
For example, tn operator could deactivate an agent prior to
reserving resources.

REACTIVATE_AGENT restarts offers for a specific agent, that had been
drained or deactivated previously.


Diffs
-

  include/google/duration.proto PRE-CREATION 
  include/mesos/master/master.proto 4a653ce59268a47d69362d0e2a3d5eb867ca6fa9 
  include/mesos/v1/master/master.proto d4dd3fcb0abc947097a5a281c4082ccf04db8614 
  src/CMakeLists.txt 1d4f541b73c07a307a8b61f217e0cfad5dc095e4 


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


Testing
---

make check

TODO: Has the hacky google Duration object thing included, which is not 
pretty...
TODO: Missing autotools build modification.
TODO: Should also add the relevant endpoints, since the patch would generate 
warnings otherwise (because new switch cases are not being handled).


Thanks,

Joseph Wu

Re: Review Request 70881: Improve master CHECK messages.

[James Peach]

> On June 19, 2019, 5:41 p.m., Greg Mann wrote:
> > src/master/master.cpp
> > Line 4346 (original), 4346 (patched)
> > 
> >
> > Nit: you could use `CHECK_NE` here and elsewhere instead of just 
> > `CHECK`?
> 
> James Peach wrote:
> Good point, I'll give that a try. IME `CHECK_NE` on pointers doesn't 
> always compile :)
> 
> James Peach wrote:
> ```
> ../3rdparty/glog-0.4.0/src/glog/logging.h:720:1:   required from 
> ‘std::string* google::Check_NEImpl(const T1&, const T2&, const char*) [with 
> T1 = mesos::internal::master::Slave*; T2 = std::nullptr_t; std::string = 
> std::__cxx11::basic_string]’
> ../../src/master/master.cpp:4346:3:   required from here
> ../3rdparty/glog-0.4.0/src/glog/logging.h:640:9: error: ambiguous 
> overload for ‘operator<<’ (operand types are ‘std::ostream’ {aka 
> ‘std::basic_ostream’} and ‘std::nullptr_t’)
>   640 |   (*os) << v;
>   |   ~~^~~~
> ```
>   
> 
> In `master.cpp`, there are a lot of other places that do `CHECK(foo != 
> nullptr)`.

Dropping this issue, since `CHECK_NE` doesn't compile :)


- James


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


On June 19, 2019, 5:06 a.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70881/
> ---
> 
> (Updated June 19, 2019, 5:06 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Greg Mann.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> For master CHECKs that fail by looking up some unique key, log
> the key to aid debugging.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 8a238aa1d5592f91d8c3b3c443aa30acd3a0cfd0 
> 
> 
> Diff: https://reviews.apache.org/r/70881/diff/1/
> 
> 
> Testing
> ---
> 
> make check (Fedora 30)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 70877: Updated master's /roles endpoint to use jsonify.

[Benjamin Mahler]

> On June 20, 2019, 6:36 p.m., Meng Zhu wrote:
> > src/common/http.hpp
> > Lines 219-221 (patched)
> > 
> >
> > Why the namespace here and below?

It's needed to compile, clang can't seem to match the aapropriate overload 
otherwise


> On June 20, 2019, 6:36 p.m., Meng Zhu wrote:
> > src/master/readonly_handler.cpp
> > Line 744 (original), 701 (patched)
> > 
> >
> > what is this for?

notice that `master` is used in the lambdas (this is the same style as the 
other jsonify handlers)


> On June 20, 2019, 6:36 p.m., Meng Zhu wrote:
> > src/master/readonly_handler.cpp
> > Lines 766-770 (original), 721-736 (patched)
> > 
> >
> > Not very important, but I find putting shorter/simpler case on the top 
> > would improve readibilty.

Sounds good


- Benjamin


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


On June 18, 2019, 7:13 p.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70877/
> ---
> 
> (Updated June 18, 2019, 7:13 p.m.)
> 
> 
> Review request for mesos and Meng Zhu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated master's /roles endpoint to use jsonify.
> 
> 
> Diffs
> -
> 
>   src/common/http.hpp a0f9b67dca86f7c035035abed5d497aaf759bb10 
>   src/common/http.cpp dcf3cb0234fc81e155dae2267764c1dd7bed5201 
>   src/master/readonly_handler.cpp 66d6160c5e951e6c3a51eb02c31ead066c322202 
> 
> 
> Diff: https://reviews.apache.org/r/70877/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 70877: Updated master's /roles endpoint to use jsonify.

[Meng Zhu]

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


Fix it, then Ship it!




Looks like we do not have quota information in `GET_ROLES`: 
https://github.com/apache/mesos/blob/master/include/mesos/v1/mesos.proto#L3628-L3634
Also, maybe we want to consider adding role info to the state?


src/common/http.hpp
Lines 219-221 (patched)


Why the namespace here and below?



src/master/readonly_handler.cpp
Line 744 (original), 701 (patched)


what is this for?



src/master/readonly_handler.cpp
Lines 766-770 (original), 721-736 (patched)


Not very important, but I find putting shorter/simpler case on the top 
would improve readibilty.


- Meng Zhu


On June 18, 2019, 12:13 p.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70877/
> ---
> 
> (Updated June 18, 2019, 12:13 p.m.)
> 
> 
> Review request for mesos and Meng Zhu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated master's /roles endpoint to use jsonify.
> 
> 
> Diffs
> -
> 
>   src/common/http.hpp a0f9b67dca86f7c035035abed5d497aaf759bb10 
>   src/common/http.cpp dcf3cb0234fc81e155dae2267764c1dd7bed5201 
>   src/master/readonly_handler.cpp 66d6160c5e951e6c3a51eb02c31ead066c322202 
> 
> 
> Diff: https://reviews.apache.org/r/70877/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 70749: Introduced optional new scheme for hostname validation.

[Benno Evers]

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

(Updated June 20, 2019, 5:48 p.m.)


Review request for mesos, Alexander Rukletsov, Joseph Wu, and Till Toenshoff.


Changes
---

Renamed algorithm -> scheme


Summary (updated)
-

Introduced optional new scheme for hostname validation.


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


Repository: mesos


Description (updated)
---

This commit introduces a new libprocess SSL flag
`hostname_validation_scheme`, which can be used to select
between the previous hostname validation behaviour and a new
option to use standardized OpenSSL algorithms to handle
hostname validation as part of the

As a nice side-effect, the new scheme gets rid of reverse DNS
lookups during TLS connection establishment, which used to be
a common source of hard-to-debug unresponsiveness in Mesos
components.

See `docs/ssl.md` in the follow-up commit for details of and
differences between the schemes.


Diffs (updated)
-

  3rdparty/libprocess/include/process/ssl/flags.hpp 
f3483f97f93bb29117b2c78f0f2ed9735d9c4b3a 
  3rdparty/libprocess/src/openssl.hpp 17bec246e516261f8d772f1647c17f092fae82d1 
  3rdparty/libprocess/src/openssl.cpp e7dbd67913fa8e7fbbf60dee428e7e38895f86ce 
  3rdparty/libprocess/src/posix/libevent/libevent_ssl_socket.hpp 
6ef5a86566af3439cfe0b06ab3576076623f7be0 
  3rdparty/libprocess/src/posix/libevent/libevent_ssl_socket.cpp 
29a1bf71c1df9d80370455a6269ecea0ec4193b0 


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

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


Testing
---

Todo!


Thanks,

Benno Evers

Re: Review Request 70810: Updated SSL docs with suggested runtime configuration.

[Benno Evers]

> On June 20, 2019, 12:49 p.m., Jan-Philip Gehrcke wrote:
> > docs/ssl.md
> > Lines 90 (patched)
> > 
> >
> > Let's define "and contain the correct hostname", let's refer to the 
> > available algorithms.

I did that in the other review, since strictly speaking this commit does not 
yet contain the hostname validation option.


- Benno


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


On June 20, 2019, 5:32 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70810/
> ---
> 
> (Updated June 20, 2019, 5:32 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Joseph Wu, and Till Toenshoff.
> 
> 
> Bugs: MESOS-9791
> https://issues.apache.org/jira/browse/MESOS-9791
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated SSL docs with suggested runtime configuration.
> 
> 
> Diffs
> -
> 
>   docs/ssl.md ce5058896144aa7824986d40d996899d92cb7c1c 
> 
> 
> Diff: https://reviews.apache.org/r/70810/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 70810: Updated SSL docs with suggested runtime configuration.

[Benno Evers]

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

(Updated June 20, 2019, 5:32 p.m.)


Review request for mesos, Alexander Rukletsov, Joseph Wu, and Till Toenshoff.


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


Repository: mesos


Description (updated)
---

Updated SSL docs with suggested runtime configuration.


Diffs (updated)
-

  docs/ssl.md ce5058896144aa7824986d40d996899d92cb7c1c 


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

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


Testing
---


Thanks,

Benno Evers

Re: Review Request 70795: Updated SSL docs to include new libprocess flag.

[Benno Evers]

> On June 20, 2019, 1:05 p.m., Jan-Philip Gehrcke wrote:
> > docs/ssl.md
> > Lines 145 (patched)
> > 
> >
> > "via DNS name" instead of "via hostname"?

I know that "DNS name" is more common in the context of x509 certification, but 
here I think it may be confusing for readers: It's the first only time a "DNS 
name" is mentioned in this document, and also there's no requirement that the 
DNS name must be resolved via DNS or stored in any DNS server.


- Benno


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


On June 20, 2019, 5:27 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70795/
> ---
> 
> (Updated June 20, 2019, 5:27 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Jan-Philip Gehrcke, Joseph Wu, 
> and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a description of the new `--hostname_validation_algorithm` flag
> and corresponding `LIBPROCESS_SSL_HOSTNAME_VALIDATION_ALGORITHM`
> environment variable.
> 
> 
> Diffs
> -
> 
>   docs/ssl.md ce5058896144aa7824986d40d996899d92cb7c1c 
> 
> 
> Diff: https://reviews.apache.org/r/70795/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 70795: Updated SSL docs to include new libprocess flag.

[Benno Evers]

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

(Updated June 20, 2019, 5:27 p.m.)


Review request for mesos, Alexander Rukletsov, Jan-Philip Gehrcke, Joseph Wu, 
and Till Toenshoff.


Repository: mesos


Description
---

Added a description of the new `--hostname_validation_algorithm` flag
and corresponding `LIBPROCESS_SSL_HOSTNAME_VALIDATION_ALGORITHM`
environment variable.


Diffs (updated)
-

  docs/ssl.md ce5058896144aa7824986d40d996899d92cb7c1c 


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

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


Testing
---


Thanks,

Benno Evers

Re: Review Request 70892: Added `/containerizer/debug` endpoint.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [70887, 70888, 70889, 70890, 70891, 70892]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose --disable-libtool-wrappers 
--disable-parallel-test-execution' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker-build.sh

- Mesos Reviewbot


On June 19, 2019, 2:49 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70892/
> ---
> 
> (Updated June 19, 2019, 2:49 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, James Peach, and Qian Zhang.
> 
> 
> Bugs: MESOS-9829
> https://issues.apache.org/jira/browse/MESOS-9829
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch introduces an agent's `/containerizer/debug` endpoint,
> which exposes the debug info related to Mesos containerizer.
> Currently, this endpoint returns a list of pending futures related to
> isolators or containerizer launcher. This endpoint is experimental,
> and the format of its output may change over time.
> 
> 
> Diffs
> -
> 
>   src/slave/http.hpp b8c83f1db95c2175bb94f6cd76cc2c58aa118c4e 
>   src/slave/http.cpp 69e6d74e8b113cc6c937f47df8984ff9a63e5bb4 
>   src/slave/slave.cpp 30039b0857a4d85b4b96fa95d7f8724d57cdec6e 
> 
> 
> Diff: https://reviews.apache.org/r/70892/diff/1/
> 
> 
> Testing
> ---
> 
> Manual testing.
> 1. Add `::sleep(5)` to the beginning of the 
> `LinuxSeccompIsolatorProcess::prepare` method.
> 2. Rebuild Mesos and then spin up a local Mesos cluster.
> 3. Run simple task, e.g.:
> ```
> ./src/mesos-execute --master="`hostname`:5050" --name="test" 
> --containerizer=mesos --command="sleep 1"
> ```
> 4. In the parallel terminal session:
> ```
> curl `hostname`:5051/containerizer/debug
> # will return: 
> {"pending":[["linux/seccomp::prepare",{"containerId":""}]]}
> ```
> 5. After 5 seconds this endpoint returns an empty list of pending futures: 
> `{"pending":[]}`
> 
> Unit tests: TBD in the following patches.
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 70907: WIP: Added recovery of agent drain information.

[Greg Mann]

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




src/slave/slave.cpp
Lines 5639 (patched)


Are we sure this operation will be idempotent for all executor types?



src/tests/slave_recovery_tests.cpp
Lines 5335 (patched)


Nit: not indented far enough.



src/tests/slave_recovery_tests.cpp
Lines 5338 (patched)


s/Ince/Once/


- Greg Mann


On June 20, 2019, 1:30 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70907/
> ---
> 
> (Updated June 20, 2019, 1:30 p.m.)
> 
> 
> Review request for mesos, Greg Mann and Joseph Wu.
> 
> 
> Bugs: MESOS-9822
> https://issues.apache.org/jira/browse/MESOS-9822
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> With this patch the agent will, after executor reregistration finished,
> replay any active drain information so remaining tasks are drained as
> well. We need to wait until executors had a chance to register so they
> are not terminated should we try to send kill task request before the
> executor has registered.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp 30039b0857a4d85b4b96fa95d7f8724d57cdec6e 
>   src/tests/slave_recovery_tests.cpp d99752ab082f1aca9fb77659378d0bca5a0598eb 
> 
> 
> Diff: https://reviews.apache.org/r/70907/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 70906: Fixed pid checkpointing for `TestContainerizer`.

[Greg Mann]

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


Fix it, then Ship it!





src/tests/containerizer.cpp
Lines 166-176 (patched)


Not indented far enough.



src/tests/containerizer.cpp
Line 166 (original), 176-179 (patched)


Extra newline


- Greg Mann


On June 20, 2019, 1:30 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70906/
> ---
> 
> (Updated June 20, 2019, 1:30 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Greg Mann, and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In order for a `MockExecutor` to be able to reregister after agent
> restart a persisted pid is required. This patch adds checkpointing of
> the pid.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer.cpp fab7e81339f3b7b3fec2a4f6cb7e3a52df2607ed 
> 
> 
> Diff: https://reviews.apache.org/r/70906/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 70894: Provided ability to change suppressed roles via V0 updateFramework().

[Andrei Sekretenko]

> On June 19, 2019, 10:54 p.m., Benjamin Mahler wrote:
> > Hm.. there's a relationship between the 'suppressedRoles' list and the 
> > reviveOffers() / suppressOffers() calls.
> > 
> > For example, if I call reviveOffers(), I would then expect that 
> > 'suppressedRoles' will be cleared and won't be sent if a re-registration 
> > happens under the covers due to a disconnection, e.g.
> > 
> > ```
> > updateFramework(f, ["role1", "role2"]); // UPDATE_FRAMEWORK is sent
> > reviveOffers(); // REVIVE is sent, logically the master moves the 
> > suppressed roles from ["role1", "role2"] to []!
> > // disconnection and re-registration happens
> > // at this point, the driver should send [] as suppressed, since the 
> > scheduler unsuppressed all roles
> > // (but it sends ["role1", "role2"] in this patch)
> > ```
> > 
> > Similarly for suppressOffers() where all of the roles become suppressed.
> > 
> > If we add role(s) arguments to reviveOffers() / suppressOffers() then they 
> > would be removing items from the suppressed set rather than clearing and 
> > assigning it entirely.

Good point.

Now I'm wondering if the already existing behaviour of suppressOffers() + 
reconnection is valid. 
Consder a similar chain of events:
- framework starts the driver
- framework calls suppressOffers()
- due to some reason, driver reconnects (to the same master or to the new one), 
all roles are active again

Is this a desired behaviour, or something that existing frameworks all have to 
prevent on their own? (By calling suppressOffers() on reregistered(), I would 
guess ?..)


- Andrei


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


On June 19, 2019, 6:26 p.m., Andrei Sekretenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70894/
> ---
> 
> (Updated June 19, 2019, 6:26 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-9793
> https://issues.apache.org/jira/browse/MESOS-9793
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds a list of suppressed roles to the arguments of
> scheduler driver's updateFramework() method to make it possible
> for V0 frameworks to selectively suppress/revive offers.
> 
> 
> Diffs
> -
> 
>   include/mesos/scheduler.hpp 8c6774885ceb3b0644c32842a17fba39e2c3e472 
>   src/sched/sched.cpp e6cc534264e3ff4edaa703a660afb2f896388802 
> 
> 
> Diff: https://reviews.apache.org/r/70894/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> `./bin/mesos-tests.sh --gtest_filter="UpdateFrameworkV0*" 
> --gtest_break_on_failure --gtest_repeat=1000`
> 
> +a new test from the depending patch
> 
> 
> Thanks,
> 
> Andrei Sekretenko
> 
>

Re: Review Request 70896: Consolidated creating std::vector from Collection in V0 Java bindings.

[Andrei Sekretenko]

> On June 19, 2019, 10:56 p.m., Benjamin Mahler wrote:
> > Very nice, thanks for doing this!
> > 
> > Is this committable without the previous changes in this chain? If not, can 
> > you make it committable so we don't have to wait for 
> > https://reviews.apache.org/r/70894/?

Yes, it is committable. Doesn't depend on any previous patches.


- Andrei


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


On June 19, 2019, 6:27 p.m., Andrei Sekretenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70896/
> ---
> 
> (Updated June 19, 2019, 6:27 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-9793
> https://issues.apache.org/jira/browse/MESOS-9793
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch replaces several pieces of code which build std::vector
> from a Java collection with a single function template.
> This is a prerequisite for adding one more collection argument to the
> bindings.
> 
> 
> Diffs
> -
> 
>   src/java/jni/construct.hpp 3d3f6c7eeda9136b9231b079ac3743c5cc634609 
>   src/java/jni/org_apache_mesos_MesosSchedulerDriver.cpp 
> b47976376ce49b518d97a702a794b704cb79dcdd 
> 
> 
> Diff: https://reviews.apache.org/r/70896/diff/1/
> 
> 
> Testing
> ---
> 
> `./bin/mesos-tests.sh --gtest_filter="*Java*" --gtest_break_on_failure 
> --gtest_repeat=10`
> 
> 
> Thanks,
> 
> Andrei Sekretenko
> 
>

Review Request 70907: WIP: Added recovery of agent drain information.

[Benjamin Bannier]

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

Review request for mesos, Greg Mann and Joseph Wu.


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


Repository: mesos


Description
---

With this patch the agent will, after executor reregistration finished,
replay any active drain information so remaining tasks are drained as
well. We need to wait until executors had a chance to register so they
are not terminated should we try to send kill task request before the
executor has registered.


Diffs
-

  src/slave/slave.cpp 30039b0857a4d85b4b96fa95d7f8724d57cdec6e 
  src/tests/slave_recovery_tests.cpp d99752ab082f1aca9fb77659378d0bca5a0598eb 


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


Testing
---

`make check`


Thanks,

Benjamin Bannier

Review Request 70906: Fixed pid checkpointing for `TestContainerizer`.

[Benjamin Bannier]

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

Review request for mesos, Gilbert Song, Greg Mann, and Joseph Wu.


Repository: mesos


Description
---

In order for a `MockExecutor` to be able to reregister after agent
restart a persisted pid is required. This patch adds checkpointing of
the pid.


Diffs
-

  src/tests/containerizer.cpp fab7e81339f3b7b3fec2a4f6cb7e3a52df2607ed 


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


Testing
---

`make check`


Thanks,

Benjamin Bannier

Re: Review Request 70795: Updated SSL docs to include new libprocess flag.

[Jan-Philip Gehrcke via Review Board]

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


Fix it, then Ship it!




Great work, left a few comments. Thanks!


docs/ssl.md
Lines 132 (patched)


Let's maybe call this VALIDATION_SCHEME instead of VALIDATION_ALGORITHM



docs/ssl.md
Lines 142 (patched)


backticks around `openssl`?



docs/ssl.md
Lines 145 (patched)


"via DNS name" instead of "via hostname"?



docs/ssl.md
Lines 147 (patched)


Even if this consumes a little more space let's make clear what happens 
when `LIBPROCESS_SSL_VERIFY_IPADD` is false 

(IIUC then the TLS handshake will fail when we use IP addresses for opening 
the connection)



docs/ssl.md
Lines 150 (patched)


"Don't"
or
"Do not"



docs/ssl.md
Lines 154 (patched)


backticks?



docs/ssl.md
Lines 177 (patched)


Great!


- Jan-Philip Gehrcke


On June 19, 2019, 2:46 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70795/
> ---
> 
> (Updated June 19, 2019, 2:46 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Jan-Philip Gehrcke, Joseph Wu, 
> and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a description of the new `--hostname_validation_algorithm` flag
> and corresponding `LIBPROCESS_SSL_HOSTNAME_VALIDATION_ALGORITHM`
> environment variable.
> 
> 
> Diffs
> -
> 
>   docs/ssl.md ce5058896144aa7824986d40d996899d92cb7c1c 
> 
> 
> Diff: https://reviews.apache.org/r/70795/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 70810: Updated SSL docs with suggested runtime configuration.

[Jan-Philip Gehrcke via Review Board]

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


Fix it, then Ship it!




Great work. I have added a few minor comments and would appreciate 
consideration. Thanks!


docs/ssl.md
Lines 24 (patched)


Should we write TLS instead of SSL in new text sections?



docs/ssl.md
Lines 30 (patched)


... provides external clients ... with the ability ...



docs/ssl.md
Lines 33 (patched)


ensures



docs/ssl.md
Lines 37 (patched)


maybe WARNING instead of NOTE



docs/ssl.md
Lines 47 (patched)


if `LIBPROCESS_SSL_ENABLE_DOWNGRADE` is set to `false`



docs/ssl.md
Lines 88 (patched)


Let's make clear that the server is _required_ to present a certificate.



docs/ssl.md
Lines 90 (patched)


Let's define "and contain the correct hostname", let's refer to the 
available algorithms.



docs/ssl.md
Lines 99 (patched)


Let's maybe clarify "including tools hitting endpoints" (HTTP clients 
hitting HTTP endpoints?)


- Jan-Philip Gehrcke


On June 19, 2019, 2:31 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70810/
> ---
> 
> (Updated June 19, 2019, 2:31 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Joseph Wu, and Till Toenshoff.
> 
> 
> Bugs: MESOS-9791
> https://issues.apache.org/jira/browse/MESOS-9791
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated SSL docs to reflect the changes introduced in the previous commit and 
> added a section with suggested runtime configuration intended for operators.
> 
> 
> Diffs
> -
> 
>   docs/ssl.md ce5058896144aa7824986d40d996899d92cb7c1c 
> 
> 
> Diff: https://reviews.apache.org/r/70810/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 70775: Added `ipc_mode` and `shm_size` fields into `LinuxInfo`.

[Qian Zhang]

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

(Updated June 20, 2019, 2:04 p.m.)


Review request for mesos, Andrei Budnik and Gilbert Song.


Changes
---

Addressed review comments.


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


Repository: mesos


Description
---

Added `ipc_mode` and `shm_size` fields into `LinuxInfo`.


Diffs (updated)
-

  include/mesos/mesos.proto 2b4f350815935220c2d2b0dd0e52346bc74c91d9 
  include/mesos/v1/mesos.proto bafc27499f810791700c4a30dcb1da33b6f31d2e 


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

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


Testing
---


Thanks,

Qian Zhang