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

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

[Qian Zhang]

> On June 20, 2019, 8:44 a.m., Gilbert Song wrote:
> > include/mesos/mesos.proto
> > Lines 3269-3270 (patched)
> > 
> >
> > delete: with a possibility to share them with its child containers
> > 
> > Is a nested container allowed to have its own ipc namespace and 
> > /dev/shm?

I would suggest to keep `"with a possibility to share them with its child 
containers"`, actually Docker has something similar, see `--ipc=shareable` in 
https://docs.docker.com/engine/reference/run/#ipc-settings---ipc

> Is a nested container allowed to have its own ipc namespace and /dev/shm?

Yes.


> On June 20, 2019, 8:44 a.m., Gilbert Song wrote:
> > include/mesos/mesos.proto
> > Lines 3279 (patched)
> > 
> >
> > s/do it/ share from the agent's/g

The word `share from the agent's` makes the whole sentence a bit redundant, so 
I would suggest to keep the word `do it`.


- Qian


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


On June 7, 2019, 7:23 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70775/
> ---
> 
> (Updated June 7, 2019, 7:23 a.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Gilbert Song.
> 
> 
> Bugs: MESOS-9827
> https://issues.apache.org/jira/browse/MESOS-9827
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added `ipc_mode` and `shm_size` fields into `LinuxInfo`.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto dc6a87f14864cb4d46b9856f251c2946a475162d 
>   include/mesos/v1/mesos.proto e8086e0f70cac73876a8ae31db3365b5059b5c44 
> 
> 
> Diff: https://reviews.apache.org/r/70775/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 70904: Added tests for task killing when draining the agent.

[Mesos Reviewbot]

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



Patch looks great!

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

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, 12:33 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70904/
> ---
> 
> (Updated June 19, 2019, 12:33 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benno Evers, Benjamin Mahler, and 
> Joseph Wu.
> 
> 
> Bugs: MESOS-9821
> https://issues.apache.org/jira/browse/MESOS-9821
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added tests for task killing when draining the agent.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_tests.cpp c2035976713abb31b3646c0d23771fa40df93271 
> 
> 
> Diff: https://reviews.apache.org/r/70904/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> `bin/mesos-tests.sh --gtest_filter="*DrainAgent*" --gtest_repeat=-1 
> --gtest_break_on_failure`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

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

[Gilbert Song]

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


Fix it, then Ship it!





include/mesos/mesos.proto
Lines 3269-3270 (patched)


delete: with a possibility to share them with its child containers

Is a nested container allowed to have its own ipc namespace and /dev/shm?



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


s/with/from/g



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


s/with/from/g



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


ditto



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


ditto



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


s/do it/ share from the agent's/g



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


period `.` at the end.



include/mesos/mesos.proto
Lines 3290-3294 (patched)


Can we split into different sentences to make it more clear?



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


s/handling/support/g



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


s/,/./g



include/mesos/mesos.proto
Lines 3299-3301 (patched)


add `()` to i.e.,...



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


ditto


- Gilbert Song


On June 6, 2019, 4:23 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70775/
> ---
> 
> (Updated June 6, 2019, 4:23 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Gilbert Song.
> 
> 
> Bugs: MESOS-9827
> https://issues.apache.org/jira/browse/MESOS-9827
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added `ipc_mode` and `shm_size` fields into `LinuxInfo`.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto dc6a87f14864cb4d46b9856f251c2946a475162d 
>   include/mesos/v1/mesos.proto e8086e0f70cac73876a8ae31db3365b5059b5c44 
> 
> 
> Diff: https://reviews.apache.org/r/70775/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 70905: Avoided a double map lookup in the allocator.

[Benjamin Mahler]

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


Ship it!




Ship It!

- Benjamin Mahler


On June 19, 2019, 10:38 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70905/
> ---
> 
> (Updated June 19, 2019, 10:38 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Avoided a double map lookup in the allocator.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> 0d8b0ca66e2c0f6b9bb6f92f43ad993b1838d103 
> 
> 
> Diff: https://reviews.apache.org/r/70905/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

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

[Benjamin Mahler]

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


Ship it!




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


src/java/jni/construct.hpp
Lines 30 (patched)


brace on next line



src/java/jni/construct.hpp
Lines 31 (patched)


consider a subsequent patch that reserves the vector capacity based on the 
.size() result of the iterable?


- Benjamin Mahler


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

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

[Benjamin Mahler]

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



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.


src/sched/sched.cpp
Lines 1697 (patched)


why bother storing it as a repeated field instead of a vector? we still 
have to copy it


- Benjamin Mahler


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

Review Request 70905: Avoided a double map lookup in the allocator.

[Meng Zhu]

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

Review request for mesos and Benjamin Mahler.


Repository: mesos


Description
---

Avoided a double map lookup in the allocator.


Diffs
-

  src/master/allocator/mesos/hierarchical.cpp 
0d8b0ca66e2c0f6b9bb6f92f43ad993b1838d103 


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


Testing
---

make check


Thanks,

Meng Zhu

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

```
../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)`.


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

Good point, I'll give that a try. IME `CHECK_NE` on pointers doesn't always 
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 70898: Fixed flaky UpdateFrameworkV0Test.DriverErrorOnFrameworkIDMismatch.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On June 19, 2019, 6:37 p.m., Andrei Sekretenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70898/
> ---
> 
> (Updated June 19, 2019, 6:37 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 removes a spurious call of driver.start() before setting
> expectations on the scheduler's mock methods.
> 
> 
> Diffs
> -
> 
>   src/tests/master/update_framework_tests.cpp 
> 3ac62c61db90eee626e59fbb09554f983d96f391 
> 
> 
> Diff: https://reviews.apache.org/r/70898/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrei Sekretenko
> 
>

Re: Review Request 70904: Added tests for task killing when draining the agent.

[Greg Mann]

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

(Updated June 19, 2019, 7:33 p.m.)


Review request for mesos, Benjamin Bannier, Benno Evers, Benjamin Mahler, and 
Joseph Wu.


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


Repository: mesos


Description
---

Added tests for task killing when draining the agent.


Diffs
-

  src/tests/slave_tests.cpp c2035976713abb31b3646c0d23771fa40df93271 


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


Testing (updated)
---

`make check`
`bin/mesos-tests.sh --gtest_filter="*DrainAgent*" --gtest_repeat=-1 
--gtest_break_on_failure`


Thanks,

Greg Mann

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/#review215976
---



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 70904: Added tests for task killing when draining the agent.

[Greg Mann]

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

Review request for mesos, Benjamin Bannier, Benno Evers, Benjamin Mahler, and 
Joseph Wu.


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


Repository: mesos


Description
---

Added tests for task killing when draining the agent.


Diffs
-

  src/tests/slave_tests.cpp c2035976713abb31b3646c0d23771fa40df93271 


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


Testing
---

`make check`


Thanks,

Greg Mann

Review Request 70903: Killed all tasks on the agent when draining.

[Greg Mann]

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

Review request for mesos, Benjamin Bannier, Benno Evers, Benjamin Mahler, and 
Joseph Wu.


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


Repository: mesos


Description
---

This patch updates the agent's `DrainSlaveMessage` handler
to kill all tasks on the agent when the message is received.


Diffs
-

  include/mesos/type_utils.hpp 57b1893160dbe874aa9fec00a3d1b640b9c54906 
  src/slave/slave.cpp 30039b0857a4d85b4b96fa95d7f8724d57cdec6e 


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


Testing
---

Testing details at the end of this chain.


Thanks,

Greg Mann

Re: Review Request 70899: Refactored the agent's task-killing code.

[Greg Mann]

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

(Updated June 19, 2019, 7:05 p.m.)


Review request for mesos, Benjamin Bannier, Benno Evers, Benjamin Mahler, and 
Joseph Wu.


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


Repository: mesos


Description
---

This patch factors the code responsible for killing tasks
out into two helper functions. This will facilitate the
calling of this common code by the agent-draining handler.


Diffs (updated)
-

  src/slave/slave.hpp 6954f53ff1531b9fcb688ef76acddf6a3d849a41 
  src/slave/slave.cpp 30039b0857a4d85b4b96fa95d7f8724d57cdec6e 


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

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


Testing
---

Testing details at the end of this chain.


Thanks,

Greg Mann

Review Request 70901: Added kill policy to the 'Task' message.

[Greg Mann]

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

Review request for mesos, Benjamin Bannier, Benno Evers, Benjamin Mahler, and 
Joseph Wu.


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


Repository: mesos


Description
---

Added kill policy to the 'Task' message.


Diffs
-

  include/mesos/mesos.proto 2b4f350815935220c2d2b0dd0e52346bc74c91d9 
  include/mesos/v1/mesos.proto bafc27499f810791700c4a30dcb1da33b6f31d2e 
  src/common/protobuf_utils.cpp 9ff0cf59c658c7c6a3a439a77aff13aff3c20fe5 


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


Testing
---


Thanks,

Greg Mann

Review Request 70900: Updated an equality operator.

[Greg Mann]

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

Review request for mesos, Benjamin Bannier, Benno Evers, Benjamin Mahler, and 
Joseph Wu.


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


Repository: mesos


Description
---

This patch updates the equality operator for the `Task`
message to make use of the `MessageDifferencer`, which
will eliminate the need to update the operator
implementation when new fields are added in the future.


Diffs
-

  src/common/type_utils.cpp ef1b3ea15cde1c7a8e0735fb9d7566dd1fd2cfdb 


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


Testing
---

Testing details at the end of this chain.


Thanks,

Greg Mann

Review Request 70899: Refactored the agent's task-killing code.

[Greg Mann]

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

Review request for mesos, Benjamin Bannier, Benno Evers, Benjamin Mahler, and 
Joseph Wu.


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


Repository: mesos


Description
---

This patch factors the code responsible for killing tasks
out into two helper functions. This will facilitate the
calling of this common code by the agent-draining handler.


Diffs
-

  src/slave/slave.hpp 6954f53ff1531b9fcb688ef76acddf6a3d849a41 
  src/slave/slave.cpp 30039b0857a4d85b4b96fa95d7f8724d57cdec6e 


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


Testing
---

Testing details at the end of this chain.


Thanks,

Greg Mann

Review Request 70898: Fixed flaky UpdateFrameworkV0Test.DriverErrorOnFrameworkIDMismatch.

[Andrei Sekretenko]

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

Review request for mesos and Benjamin Mahler.


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


Repository: mesos


Description
---

This patch removes a spurious call of driver.start() before setting
expectations on the scheduler's mock methods.


Diffs
-

  src/tests/master/update_framework_tests.cpp 
3ac62c61db90eee626e59fbb09554f983d96f391 


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


Testing
---


Thanks,

Andrei Sekretenko

Review Request 70897: Supported suppressedRoles in updateFramework() in V0 Java bindings.

[Andrei Sekretenko]

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

Review request for mesos and Benjamin Mahler.


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


Repository: mesos


Description
---

Supported suppressedRoles in updateFramework() in V0 Java bindings.


Diffs
-

  src/java/jni/org_apache_mesos_MesosSchedulerDriver.cpp 
b47976376ce49b518d97a702a794b704cb79dcdd 
  src/java/src/org/apache/mesos/MesosSchedulerDriver.java 
55ebc8772d9183286c908b4dba342109f28394f4 
  src/java/src/org/apache/mesos/SchedulerDriver.java 
ee5a9e24956299d84ff03a0fc5a22e641470a03f 


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


Testing
---

`./bin/mesos-tests.sh --gtest_filter="ExamplesTest.JavaFramework" 
--gtest_break_on_failure --gtest_repeat=10` with a patch from 
https://reviews.apache.org/r/70815/


Thanks,

Andrei Sekretenko

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

[Andrei Sekretenko]

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

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 70894: Provided ability to change suppressed roles via V0 updateFramework().

[Andrei Sekretenko]

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

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

Review Request 70895: Added test for supperssing roles via V0 updateFramework().

[Andrei Sekretenko]

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

Review request for mesos and Benjamin Mahler.


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


Repository: mesos


Description
---

Added test for supperssing roles via V0 updateFramework().


Diffs
-

  src/tests/master/update_framework_tests.cpp 
3ac62c61db90eee626e59fbb09554f983d96f391 


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


Testing
---

`./bin/mesos-tests.sh --gtest_filter="UpdateFrameworkV0*SuppressedRoles" 
--gtest_break_on_failure --gtest_repeat=1000`


Thanks,

Andrei Sekretenko

Re: Review Request 70773: Added `--disallow_sharing_agent_ipc_namespace` agent flag.

[Qian Zhang]

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

(Updated June 19, 2019, 11:02 a.m.)


Review request for mesos, Andrei Budnik and Gilbert Song.


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


Repository: mesos


Description
---

Added `--disallow_sharing_agent_ipc_namespace` agent flag.


Diffs
-

  docs/configuration/agent.md 91a5e931c35fb353c87bee35004cb73e8546c8d3 
  src/slave/flags.hpp 9684a09071be930f25a7d10821ee65b5a965fa83 
  src/slave/flags.cpp e23061ad929c8e3c24806f52c95f82766e3d1911 


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


Testing
---


Thanks,

Qian Zhang

Re: Review Request 70881: Improve master CHECK messages.

[Greg Mann]

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


Fix it, then Ship it!





src/master/master.cpp
Line 4346 (original), 4346 (patched)


Nit: you could use `CHECK_NE` here and elsewhere instead of just `CHECK`?


- Greg Mann


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 70774: Added `--default_shm_size` agent flag.

[Gilbert Song]

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


Ship it!




Ship It!

- Gilbert Song


On June 18, 2019, 11:26 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70774/
> ---
> 
> (Updated June 18, 2019, 11:26 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Gilbert Song.
> 
> 
> Bugs: MESOS-9833
> https://issues.apache.org/jira/browse/MESOS-9833
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added `--default_shm_size` agent flag.
> 
> 
> Diffs
> -
> 
>   docs/configuration/agent.md 91a5e931c35fb353c87bee35004cb73e8546c8d3 
>   src/slave/flags.hpp 9684a09071be930f25a7d10821ee65b5a965fa83 
>   src/slave/flags.cpp e23061ad929c8e3c24806f52c95f82766e3d1911 
> 
> 
> Diff: https://reviews.apache.org/r/70774/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 70822: Added common protobufs for agent draining.

[Vinod Kone]

> On June 14, 2019, 9:34 a.m., Benjamin Bannier wrote:
> > src/messages/messages.proto
> > Lines 986 (patched)
> > 
> >
> > What does it mean if the master sends a `DRAINED` state to the agent? 
> > Is that something we need to reject in validation?
> > 
> > Does it maybe make sense to instead break out `DrainInfo.state` into 
> > its own message to use in state reporting?
> 
> Greg Mann wrote:
> Yes we can have a CHECK on the agent to make sure the master doesn't send 
> DRAINED in a DrainSlaveMessage. That will happen in another patch.
> 
> What's the benefit of moving `DrainInfo.State` outside of `DrainInfo`?
> 
> Benjamin Bannier wrote:
> The goal would be to use dedicated messages for triggering draining 
> (master->agent) and to report draining (agent->master). That might not only 
> be conceptually simplier, but would likely also lead to simpler, less 
> redundant code. Is there any benefit in using the same message?
> 
> Greg Mann wrote:
> What do you mean by "report draining"? The design doesn't involve any 
> explicit communication of draining progress between agent and master, the 
> master just receives task status updates.
> 
> Benjamin Bannier wrote:
> Sorry, there's no process of communicating `DrainInfo` back to master, 
> but we do report it to users with https://reviews.apache.org/r/70835/.
> 
> Greg Mann wrote:
> Are you proposing having a separate message for inclusion in the agent 
> API outputs?
> 
> Vinod Kone wrote:
> IIUC, Benjamin is questioning why we are storing both the spec (max grace 
> period, mark_gone) and the status (state) in the same proto (do we do this 
> elsewhere in the code base?). If they were separate protos, say DrainRequest 
> and DrainStatus, master would send DrainRequest to the agent, and show 
> DrainStatus (and possibly DrainRequest) in the operator API response. It 
> looks a bit weird that we would send DrainInfo (as it is currently written  
> to the agent and do a CHECK on valid state. The fact that an agent got a 
> Drain message is enough for the agent to know that it needs to drain, it 
> doesn't need to look into the `DrainInfo::State`?

Let me repharse my first sentence. I think storing both spec and status in the 
same proto is probably fine if it makes thing easy to expose in API endpoints 
and use it internally in the master. But I still think it's weird to send the 
status in a message to the agent which is not expected to use it. So, if 
there's a way we can avoid it that would be great.


- Vinod


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


On June 18, 2019, 11:17 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70822/
> ---
> 
> (Updated June 18, 2019, 11:17 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benno Evers, Benjamin Mahler, 
> Joseph Wu, and Vinod Kone.
> 
> 
> Bugs: MESOS-9753
> https://issues.apache.org/jira/browse/MESOS-9753
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch makes protobuf message updates which will be used
> by both the master and the agent to facilitate automatic
> draining of agents.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 2b4f350815935220c2d2b0dd0e52346bc74c91d9 
>   include/mesos/type_utils.hpp 57b1893160dbe874aa9fec00a3d1b640b9c54906 
>   include/mesos/v1/mesos.proto bafc27499f810791700c4a30dcb1da33b6f31d2e 
>   src/common/type_utils.cpp ef1b3ea15cde1c7a8e0735fb9d7566dd1fd2cfdb 
>   src/internal/devolve.hpp fefe86e450fa5083b9ff50e92f4594ffb30a54c8 
>   src/internal/devolve.cpp 1d300b49d5cc3de4b8ed409902eb881c7afc07ea 
>   src/internal/evolve.hpp 1044d9df75b6fc1f60d3704be9cb5751e6d4321d 
>   src/internal/evolve.cpp 19c155967bf090fb2ec39211805ff1385787ab59 
>   src/messages/messages.proto e30ad34cc9212b05f85ba5e1d4fcfc9e49ae92c0 
> 
> 
> Diff: https://reviews.apache.org/r/70822/diff/5/
> 
> 
> Testing
> ---
> 
> `make`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 70773: Added `--disallow_sharing_agent_ipc_namespace` agent flag.

[Gilbert Song]

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


Ship it!




Ship It!

- Gilbert Song


On June 18, 2019, 11:21 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70773/
> ---
> 
> (Updated June 18, 2019, 11:21 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Gilbert Song.
> 
> 
> Bugs: MESOS-9788
> https://issues.apache.org/jira/browse/MESOS-9788
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added `--disallow_sharing_agent_ipc_namespace` agent flag.
> 
> 
> Diffs
> -
> 
>   docs/configuration/agent.md 91a5e931c35fb353c87bee35004cb73e8546c8d3 
>   src/slave/flags.hpp 9684a09071be930f25a7d10821ee65b5a965fa83 
>   src/slave/flags.cpp e23061ad929c8e3c24806f52c95f82766e3d1911 
> 
> 
> Diff: https://reviews.apache.org/r/70773/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 70797: WIP: Unit tests for hostname validation.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [70747, 70748, 70749, 70795, 70796, 70797]

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:47 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70797/
> ---
> 
> (Updated June 19, 2019, 2:47 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Joseph Wu, and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> WIP: Unit tests for hostname validation.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/ssl_tests.cpp 
> 6b8496aeeed79ae1bd39d7013f4f403b248fdd4c 
> 
> 
> Diff: https://reviews.apache.org/r/70797/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 70747: Updated some flag description for libprocess SSL flags.

[Till Toenshoff via Review Board]

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


Ship it!




Ship It!

- Till Toenshoff


On May 29, 2019, 1:50 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70747/
> ---
> 
> (Updated May 29, 2019, 1:50 p.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Changed `SSL` to `TLS` in some flag descriptions where the
> flag was referring to versions of the TLS protocol.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/openssl.cpp 
> e7dbd67913fa8e7fbbf60dee428e7e38895f86ce 
> 
> 
> Diff: https://reviews.apache.org/r/70747/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Review Request 70891: Wrapped launcher in `LauncherTracker`.

[Andrei Budnik]

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

Review request for mesos, Gilbert Song and Qian Zhang.


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


Repository: mesos


Description
---

This patch wraps a container launcher in instance of `LauncherTracker`
class. If the launcher gets stuck in some operation, `pendingFutures`
will return the method name along with its arguments such as
`containerId`, `pid`, etc.


Diffs
-

  src/slave/containerizer/mesos/containerizer.cpp 
043244841a73fa3f5f7119bc38f6d3a04be8990b 


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


Testing
---


Thanks,

Andrei Budnik

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

[Andrei Budnik]

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

Review request for mesos, Gilbert Song, James Peach, and Qian Zhang.


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


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

Review Request 70889: Wrapped isolators in `IsolatorTracker`.

[Andrei Budnik]

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

Review request for mesos, Gilbert Song and Qian Zhang.


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


Repository: mesos


Description
---

This patch wraps every isolator in instance of `IsolatorTracker` class.
If an isolator gets stuck in some operation, `pendingFutures` will
return the isolator name, method name along with its arguments such as
`containerId`, `pid`, etc.


Diffs
-

  src/slave/containerizer/mesos/containerizer.cpp 
043244841a73fa3f5f7119bc38f6d3a04be8990b 


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


Testing
---


Thanks,

Andrei Budnik

Review Request 70890: Added `LauncherTracker` for tracking calls of launcher methods.

[Andrei Budnik]

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

Review request for mesos, Gilbert Song and Qian Zhang.


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


Repository: mesos


Description
---

This patch adds a new `LauncherTracker` class that intercepts
`Launcher` calls and uses `track` function for detecting very
slow or stuck operations on the real launcher.


Diffs
-

  src/CMakeLists.txt 1d4f541b73c07a307a8b61f217e0cfad5dc095e4 
  src/Makefile.am 761dde1d63e0f4f1ac4ab86f129f84f3746d3153 
  src/slave/containerizer/mesos/launcher_tracker.hpp PRE-CREATION 
  src/slave/containerizer/mesos/launcher_tracker.cpp PRE-CREATION 


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


Testing
---


Thanks,

Andrei Budnik

Review Request 70888: Added `IsolatorTracker` for tracking calls of isolator methods.

[Andrei Budnik]

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

Review request for mesos, Gilbert Song and Qian Zhang.


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


Repository: mesos


Description
---

This patch adds a new `IsolatorTracker` class that intercepts
`Isolator` calls and uses `track` function for detecting very
slow or stuck operations on the real isolator.


Diffs
-

  src/CMakeLists.txt 1d4f541b73c07a307a8b61f217e0cfad5dc095e4 
  src/Makefile.am 761dde1d63e0f4f1ac4ab86f129f84f3746d3153 
  src/slave/constants.hpp cc81e5111e8c3082c940b918230f660a1121a1ac 
  src/slave/containerizer/mesos/isolator_tracker.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolator_tracker.cpp PRE-CREATION 


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


Testing
---


Thanks,

Andrei Budnik

Review Request 70887: Added `track`, `pendingFutures` functions for tracking pending futures.

[Andrei Budnik]

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

Review request for mesos, Benjamin Mahler, Gilbert Song, James Peach, Meng Zhu, 
and Qian Zhang.


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


Repository: mesos


Description
---

This patch introduces a mechanism for tracking of pending futures.
This feature can be used to detect hanging operations, which might
get stuck on a blocking operation or asynchronously. Both `track`
and `pendingFutures` functions depend on `PendingFutureTracker`
class, which is instantiated the first time they are accessed.


Diffs
-

  src/slave/containerizer/mesos/future_track.hpp PRE-CREATION 


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


Testing
---


Thanks,

Andrei Budnik

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

[Benno Evers]

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

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

Re: Review Request 70797: WIP: Unit tests for hostname validation.

[Benno Evers]

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

(Updated June 19, 2019, 2:47 p.m.)


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


Repository: mesos


Description
---

WIP: Unit tests for hostname validation.


Diffs (updated)
-

  3rdparty/libprocess/src/tests/ssl_tests.cpp 
6b8496aeeed79ae1bd39d7013f4f403b248fdd4c 


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

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


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 19, 2019, 2:46 p.m.)


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


Summary (updated)
-

Updated SSL docs to include new libprocess flag.


Repository: mesos


Description (updated)
---

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

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


Testing
---


Thanks,

Benno Evers

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

[Benno Evers]

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

(Updated June 19, 2019, 2:45 p.m.)


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


Summary (updated)
-

Introduced optional new algorithm for hostname validation.


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


Repository: mesos


Description
---

This commit introduces a new libprocess SSL flag
`hostname_validation_algorithm`, 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 algorithm 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 algorithms.


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

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


Testing
---

Todo!


Thanks,

Benno Evers

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

[Benno Evers]

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


Summary (updated)
-

Fixed Mesos unit tests after Address API change.


Repository: mesos


Description (updated)
---

Adjusted some Mesos unit tests after the renaming
of `Address::hostname()` -> `Address::lookup_hostname()`.


Diffs (updated)
-

  src/tests/master_tests.cpp 097f1b77a59e29c6690210773d1556ebf2bb701e 


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

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


Testing
---


Thanks,

Benno Evers

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

[Benno Evers]

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

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

Review Request 70884: WIP: Added optional hostname field to UPID.

[Benno Evers]

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

Review request for mesos, Joseph Wu and Till Toenshoff.


Repository: mesos


Description
---

This allows client code to access the original hostname
that was used to specify a libprocess address.

NOTE: Right now, this does not yet contain the code to
set the hostname when creating a UPID from a string
description.


Diffs
-

  3rdparty/libprocess/include/process/pid.hpp 
9f09ab46fa3ceaeac09b0fbf9f532728c4ed2d7a 
  3rdparty/libprocess/src/process.cpp 799666f03d6a78708aa9336c2dd04bc9b5023aa0 


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


Testing
---


Thanks,

Benno Evers

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

[Benno Evers]

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

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.

[Benno Evers]

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

Updated SSL docs to reflect the changes introduced in the previous commit and 
added a section with suggested runtime configuration intended for operators.


Diffs (updated)
-

  docs/ssl.md ce5058896144aa7824986d40d996899d92cb7c1c 


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

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


Testing
---


Thanks,

Benno Evers

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

[Benno Evers]

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


Changes
---

Updated commit description.


Summary (updated)
-

Changed semantics of some libprocess TLS flags.


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


Repository: mesos


Description (updated)
---

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 (updated)
-

  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/

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


Testing (updated)
---

So far, mostly manual testing.


Thanks,

Benno Evers

Re: Review Request 70879: Updated webui Roles tab to consistently display '-' for 0 entries.

[Armand Grillet]

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


Ship it!




LGTM

- Armand Grillet


On June 18, 2019, 10:22 p.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70879/
> ---
> 
> (Updated June 18, 2019, 10:22 p.m.)
> 
> 
> Review request for mesos, Armand Grillet and Meng Zhu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The Roles tab currently uses an inconsistent mix of '0', '0 B' and '-'
> for displaying the allocation, guarantee, and limit. Since it's easier
> to scan the table for non-empty entries when '-' is used, this updates
> the every empty case to use '-'.
> 
> We may also want to consider using the empty string '' in the future,
> as that may help make the table even easier to scan visually than '-'.
> 
> 
> Diffs
> -
> 
>   src/webui/app/roles/roles.html c8ac1ef6cdfbc6d6a3d5c2e619896e8656a9badf 
> 
> 
> Diff: https://reviews.apache.org/r/70879/diff/1/
> 
> 
> Testing
> ---
> 
> Ran manually, see screenshot.
> 
> 
> File Attachments
> 
> 
> post-changes
>   
> https://reviews.apache.org/media/uploaded/files/2019/06/18/ab42cf05-9252-4eed-a765-4126c5ed23a4__Screen_Shot_2019-06-18_at_4.16.49_PM.png
> pre-changes
>   
> https://reviews.apache.org/media/uploaded/files/2019/06/18/29122c1d-dae3-4d4c-8a52-062971d603c0__Screen_Shot_2019-06-18_at_4.21.27_PM.png
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 70870: Updated `namespaces-ipc.md` for configurable IPC namespace and /dev/shm.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [70773, 70774, 70775, 70798, 70844, 70820, 70845, 70849, 
70852, 70857, 70859, 70860, 70870]

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 18, 2019, 2:42 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70870/
> ---
> 
> (Updated June 18, 2019, 2:42 a.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Gilbert Song.
> 
> 
> Bugs: MESOS-9828
> https://issues.apache.org/jira/browse/MESOS-9828
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated `namespaces-ipc.md` for configurable IPC namespace and /dev/shm.
> 
> 
> Diffs
> -
> 
>   docs/isolators/namespaces-ipc.md e550e752ef0ca4bc363fd3c5c190c44f5dd4187a 
> 
> 
> Diff: https://reviews.apache.org/r/70870/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

[GitHub] [mesos] carlonelong opened a new pull request #336: Cgroups isolators do not require Linux launcher.

[GitBox]

carlonelong opened a new pull request #336: Cgroups isolators do not require 
Linux launcher.
URL: https://github.com/apache/mesos/pull/336
 
 
   Change-Id: I9b8bdf7c6e8cb314cdf5e45b3d4a122653ce79c2


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

Re: Review Request 70881: Improve master CHECK messages.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [70881]

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