Re: Review Request 70811: Fixed compilation error on Mac OS.

2019-06-07 Thread Mesos Reviewbot

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



Patch looks great!

Reviews applied: [70811]

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 7, 2019, 5:21 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70811/
> ---
> 
> (Updated June 7, 2019, 5:21 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Gilbert Song, James Peach, and 
> Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds missing switch case to fix compilation error introduced
> in `bc5a57122635`.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/launch.cpp 
> a69a68823842171bb15ddf34a504e5ce4af232b0 
> 
> 
> Diff: https://reviews.apache.org/r/70811/diff/1/
> 
> 
> Testing
> ---
> 
> internal CI
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>



Re: Review Request 70815: Added using updateFramework() to java V0 example framework.

2019-06-07 Thread Mesos Reviewbot

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



Bad patch!

Reviews applied: [70815, 70814, 70813, 70755, 70754, 70753, 70752]

Failed command: cd .. && /usr/bin/python3 support/apply-reviews.py -n -r 70753

Error:
2019-06-07 23:01:03 URL:https://reviews.apache.org/r/70753/diff/raw/ 
[3109/3109] -> "70753.patch" [1]
error: src/tests/master/update_framework_tests.cpp: does not exist in index

Full log: https://builds.apache.org/job/Mesos-Reviewbot-Linux/1964/console

- Mesos Reviewbot


On June 7, 2019, 11:49 a.m., Andrei Sekretenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70815/
> ---
> 
> (Updated June 7, 2019, 11:49 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added using updateFramework() to java V0 example framework.
> 
> 
> Diffs
> -
> 
>   src/examples/java/TestFramework.java 
> c8b0ceacd1305f7ff67f2ef490e14513d2757f5a 
>   src/examples/python/test_framework.py 
> 27bc052dc4e57e0290ef58a39b2f1ac82fdb42fd 
>   src/tests/java_framework_test.sh c53e4abd75e5ff5d1fa1d1aad7f7d54806f9f899 
> 
> 
> Diff: https://reviews.apache.org/r/70815/diff/1/
> 
> 
> Testing
> ---
> 
> `./bin/mesos-tests.sh --gtest_filter="ExamplesTest.JavaFramework" 
> --gtest_break_on_failure --gtest_repeat=100`
> 
> 
> Thanks,
> 
> Andrei Sekretenko
> 
>



Re: Review Request 70782: Added a non-deterministic test for MESOS-9808.

2019-06-07 Thread Mesos Reviewbot

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



Patch looks great!

Reviews applied: [70778, 70782]

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 7, 2019, 6:05 p.m., Andrei Sekretenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70782/
> ---
> 
> (Updated June 7, 2019, 6:05 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Chun-Hung Hsiao, and Greg Mann.
> 
> 
> Bugs: MESOS-9808
> https://issues.apache.org/jira/browse/MESOS-9808
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a non-deterministic test for MESOS-9808.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/.process.cpp.swp PRE-CREATION 
>   3rdparty/libprocess/src/tests/process_tests.cpp 
> 05dc5ec2fdc74a989689e4378bef775bcf2b7a87 
> 
> 
> Diff: https://reviews.apache.org/r/70782/diff/3/
> 
> 
> Testing
> ---
> 
> Without any of two fixes from https://reviews.apache.org/r/70778/ - deadlocks 
> 100 out of 100 times on the hardware I used. 
> Without the first fix the deadlock is due to the same reason as initially 
> observed in in MESOS-9808.
> 
> 
> With both fixes applied, one run takes around 400 ms on a release build. No 
> deadlock observed in 1000 runs.
> 
> 
> Thanks,
> 
> Andrei Sekretenko
> 
>



Re: Review Request 70756: Refactored some of `MasterAPITest`s using MockMasterAPISubscriber.

2019-06-07 Thread Mesos Reviewbot

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



Patch looks great!

Reviews applied: [70532, 70663, 70664, 70665, 70668, 70669, 70670, 70533, 
70778, 70671, 70756]

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 May 29, 2019, 6:42 p.m., Andrei Sekretenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70756/
> ---
> 
> (Updated May 29, 2019, 6:42 p.m.)
> 
> 
> Review request for mesos, Benno Evers, Benjamin Mahler, Greg Mann, and Joseph 
> Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch replaces reading out all Master API events (both interesting
> and uninteresting ones) in the code of the tests with setting
> expectations on `MOCK_METHOD`s of the dedicated class.
> 
> 
> Diffs
> -
> 
>   src/tests/api_tests.cpp 2220cecc22778a86f0c29317adf495927e1a900d 
> 
> 
> Diff: https://reviews.apache.org/r/70756/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrei Sekretenko
> 
>



Re: Review Request 70811: Fixed compilation error on Mac OS.

2019-06-07 Thread Benjamin Bannier

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




src/slave/containerizer/mesos/launch.cpp
Line 508 (original), 509 (patched)


For non-`__linux-_` cases this silently drops any configuration. Why 
wouldn't we return an error in that case instead?


- Benjamin Bannier


On June 7, 2019, 7:21 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70811/
> ---
> 
> (Updated June 7, 2019, 7:21 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Gilbert Song, James Peach, and 
> Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds missing switch case to fix compilation error introduced
> in `bc5a57122635`.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/launch.cpp 
> a69a68823842171bb15ddf34a504e5ce4af232b0 
> 
> 
> Diff: https://reviews.apache.org/r/70811/diff/1/
> 
> 
> Testing
> ---
> 
> internal CI
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>



Re: Review Request 70814: Added explicit roles to ACL of script tests for UpdateFramework testing - WIP

2019-06-07 Thread Andrei Sekretenko

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

(Updated June 7, 2019, 6:52 p.m.)


Review request for mesos and Benjamin Mahler.


Summary (updated)
-

Added explicit roles to ACL of script tests for UpdateFramework testing - WIP


Repository: mesos


Description (updated)
---

Added explicit roles to ACL of script tests for UpdateFramework testing - WIP


Diffs
-

  src/tests/script.cpp 2e50861bf40ceeaf6a40476a742e6e82c23ba86f 


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


Testing (updated)
---

make check

NOTE: without this change it is not possible to subscribe the java example 
framework with a role/roles other than `*`


Thanks,

Andrei Sekretenko



Review Request 70815: Added using updateFramework() to java V0 example framework.

2019-06-07 Thread Andrei Sekretenko

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

Review request for mesos and Benjamin Mahler.


Repository: mesos


Description
---

Added using updateFramework() to java V0 example framework.


Diffs
-

  src/examples/java/TestFramework.java c8b0ceacd1305f7ff67f2ef490e14513d2757f5a 
  src/examples/python/test_framework.py 
27bc052dc4e57e0290ef58a39b2f1ac82fdb42fd 
  src/tests/java_framework_test.sh c53e4abd75e5ff5d1fa1d1aad7f7d54806f9f899 


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


Testing
---

`./bin/mesos-tests.sh --gtest_filter="ExamplesTest.JavaFramework" 
--gtest_break_on_failure --gtest_repeat=100`


Thanks,

Andrei Sekretenko



Review Request 70813: Added updateFramework() method to java V0 scheduler driver bindings.

2019-06-07 Thread Andrei Sekretenko

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

Review request for mesos and Benjamin Mahler.


Repository: mesos


Description
---

Added updateFramework() method to java V0 scheduler driver bindings.


Diffs
-

  src/java/jni/org_apache_mesos_MesosSchedulerDriver.cpp 
e0421406958bdad9235f394ea3854c236da9430c 
  src/java/src/org/apache/mesos/MesosSchedulerDriver.java 
07bdd4b273137bad11742e9024947c7f1ad905e4 
  src/java/src/org/apache/mesos/SchedulerDriver.java 
85faa8e6f8b3420ce140eff7ac860a5f3d417928 


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


Testing
---


Thanks,

Andrei Sekretenko



Re: Review Request 70738: Allow roles to burst up to quota limits in the allocator.

2019-06-07 Thread Benjamin Mahler

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


Fix it, then Ship it!




We've come a long way, the allocation cycle code is a lot more pleasant to read 
than when we started working on it!


src/master/allocator/mesos/hierarchical.cpp
Line 1772 (original), 1774 (patched)


Hm.. I would expect this to say something more like:

"In the 1st stage, we allocate to satisfy quota guarantees"

The fact that the default guarantees == 0 guarantees -> and therefore no 
guarantees to satsify seems like an extra mental step for the reader?



src/master/allocator/mesos/hierarchical.cpp
Lines 1776-1784 (original), 1776-1784 (patched)


ditto here, the non-default guarantee seems like an extra mental step for 
the reader?



src/master/allocator/mesos/hierarchical.cpp
Lines 1796-1797 (patched)


Hm.. why?



src/master/allocator/mesos/hierarchical.cpp
Line 1860 (original), 1874 (patched)


"Rationale":

https://writingexplained.org/rational-vs-rationale-difference



src/master/allocator/mesos/hierarchical.cpp
Line 1874 (original), 1888 (patched)


"Rationale"



src/master/allocator/mesos/hierarchical.cpp
Line 1890 (original), 1904 (patched)


... to increase a role's quota consumption

("allocate to" seems to fit reservations also, whereas reservations don't 
increase consumption when allocated)



src/master/allocator/mesos/hierarchical.cpp
Lines 1930 (patched)


not sure I understand the comment, "explicitly here" as opposed to?



src/master/allocator/mesos/hierarchical.cpp
Line 1917 (original), 1942 (patched)


Can we check against "no limits" rather than default?

(Also, just so I understand, this `if` condition is an optimization right?)



src/master/allocator/mesos/hierarchical.cpp
Line 1925 (original), 1950 (patched)


This if condition is an optimization right?



src/master/allocator/mesos/hierarchical.cpp
Lines 1978-1979 (original), 2010-2011 (patched)


"since allocating these does not make progress towards satsifying quota 
guarantees"?



src/master/allocator/mesos/hierarchical.cpp
Lines 2049-2055 (original), 2081-2087 (patched)


Isn't this just:

```
Resources toAllocate = available.allocatableTo(role).reserved();
```

No need for the TODO?



src/master/allocator/mesos/hierarchical.cpp
Line 2064 (original), 2096 (patched)


Ditto here, can we check against no limits?

Ah, I also just realized that this check is needed at the current time 
because since `rolesConsumedQuota` does not store roles with empty limits.



src/master/allocator/mesos/hierarchical.cpp
Lines 2145 (patched)


Fragile to not fill the map with every role, hoping to see this if 
condition go away soon



src/tests/hierarchical_allocator_tests.cpp
Line 4425 (original), 4424-4426 (patched)


s/.get()/->/


- Benjamin Mahler


On June 4, 2019, 9:15 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70738/
> ---
> 
> (Updated June 4, 2019, 9:15 p.m.)
> 
> 
> Review request for mesos, Andrei Sekretenko and Benjamin Mahler.
> 
> 
> Bugs: MESOS-8456
> https://issues.apache.org/jira/browse/MESOS-8456
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch allows roles to burst above their quota guarantees
> up to the quota limits.
> 
> In the first allocation stage which is dedicated to roles
> with non-default guarantees, we allow these roles to burst
> up to their limits for resources that they have guarantees
> set as well as for other resources allocated together
> with the guaranteed resources. These bursts are subject to
> global headroom enforcement.
> 
> In the second allocation stage, we allow all roles to burst
> up to their limits while maintaining the global headroom.
> In this stage, if some unreserved non-revocable scalar resources
> on the agent need to be held back to maintain the headroom,
> we will "chop" them to 

Review Request 70814: Added explicit roles to ACL of script tests for UpdateFramework testing.

2019-06-07 Thread Andrei Sekretenko

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

Review request for mesos.


Repository: mesos


Description
---

Added explicit roles to ACL of script tests for UpdateFramework testing.


Diffs
-

  src/tests/script.cpp 2e50861bf40ceeaf6a40476a742e6e82c23ba86f 


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


Testing
---

Without this change it is not possible to subscribe the java example framework 
with a role/roles other than `*`


Thanks,

Andrei Sekretenko



Re: Review Request 70811: Fixed compilation error on Mac OS.

2019-06-07 Thread Gilbert Song

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


Ship it!




Ship It!

- Gilbert Song


On June 7, 2019, 10:21 a.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70811/
> ---
> 
> (Updated June 7, 2019, 10:21 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Gilbert Song, James Peach, and 
> Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds missing switch case to fix compilation error introduced
> in `bc5a57122635`.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/launch.cpp 
> a69a68823842171bb15ddf34a504e5ce4af232b0 
> 
> 
> Diff: https://reviews.apache.org/r/70811/diff/1/
> 
> 
> Testing
> ---
> 
> internal CI
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>



Re: Review Request 70782: Added a non-deterministic test for MESOS-9808.

2019-06-07 Thread Andrei Sekretenko

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

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


Review request for mesos, Benjamin Mahler, Chun-Hung Hsiao, and Greg Mann.


Changes
---

int -> size_t


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


Repository: mesos


Description
---

Added a non-deterministic test for MESOS-9808.


Diffs (updated)
-

  3rdparty/libprocess/src/.process.cpp.swp PRE-CREATION 
  3rdparty/libprocess/src/tests/process_tests.cpp 
05dc5ec2fdc74a989689e4378bef775bcf2b7a87 


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

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


Testing
---

Without any of two fixes from https://reviews.apache.org/r/70778/ - deadlocks 
100 out of 100 times on the hardware I used. 
Without the first fix the deadlock is due to the same reason as initially 
observed in in MESOS-9808.


With both fixes applied, one run takes around 400 ms on a release build. No 
deadlock observed in 1000 runs.


Thanks,

Andrei Sekretenko



Re: Review Request 70782: Added a non-deterministic test for MESOS-9808.

2019-06-07 Thread Andrei Sekretenko


> On June 6, 2019, 9:11 p.m., Benjamin Mahler wrote:
> > 3rdparty/libprocess/src/tests/process_tests.cpp
> > Lines 2196-2200 (patched)
> > 
> >
> > Oh, this gate doesn't really accomplish much?
> > 
> > I had suggested using it in order to ensure we could fill ProcessA's 
> > queue with dispatches, and it accomplished that by preventing 
> > ProcessA::initialize() from completing.
> > 
> > Did that not work?
> > 
> > ```
> > Promise gate;
> > 
> > PID pid = spawn(new ProcessA(gate.future()), true);
> > 
> > for (size_t i = 0; i < 1000; ++i) {
> >   dispatch(pid, ::f, std::unique_ptr(new B()));
> > }
> > 
> > gate.set(Nothing());
> > ```

It it the other way round: with the gate added, filling ProcessA's queue does 
not increase the deadlock probability.

Why it is necessary to delay return of `ProcessA::initialize()` in the current 
implementation (and why it was necessary to fill ProcessA's queue in the 
previous implementation), is actually a very good question.


An attempt to add a counter and count the number of `~B()` calls happened 
before waiting for `reference->expired()` in the `ProcessManager::cleanup()` 
shows that `cleanup()` gets to that point after 3 deleted events on average in 
the current implementation.
When I remove the gate, cleanup occurs earlier: after < 1 event on average.

Basically, the reliability of this test depends on two factors:
1. Contention between the dispatches on one side and the 
`state.store(TERMINATING)` + `events->consumer.decomission()` in `cleanup()` on 
the other side.  The slower the body of the inner loop of the test, the higher 
the probability of a false negative.
2. Timing between dispatches and that part of cleanup. If this delay changes 
(due to some modifications in libprocess/kernel/hardware/ etc.), this test 
might became unable to introduce the race at all, i.e. it will become invalid.

These possibilities can be mitigated to some degree:
1. We can move everything possible out of the inner loop and add more threads 
calling dispatch until the cleanup completes (or libprocess deadlocks).
2. We can do a longer series of dispatches and call terminate() in another 
thread.

However, I'm not sure if these ideas are sane, as they will significantly 
complicate the test setup...


- Andrei


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


On June 5, 2019, 3:47 p.m., Andrei Sekretenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70782/
> ---
> 
> (Updated June 5, 2019, 3:47 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Chun-Hung Hsiao, and Greg Mann.
> 
> 
> Bugs: MESOS-9808
> https://issues.apache.org/jira/browse/MESOS-9808
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a non-deterministic test for MESOS-9808.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/process_tests.cpp 
> 05dc5ec2fdc74a989689e4378bef775bcf2b7a87 
> 
> 
> Diff: https://reviews.apache.org/r/70782/diff/2/
> 
> 
> Testing
> ---
> 
> Without any of two fixes from https://reviews.apache.org/r/70778/ - deadlocks 
> 100 out of 100 times on the hardware I used. 
> Without the first fix the deadlock is due to the same reason as initially 
> observed in in MESOS-9808.
> 
> 
> With both fixes applied, one run takes around 400 ms on a release build. No 
> deadlock observed in 1000 runs.
> 
> 
> Thanks,
> 
> Andrei Sekretenko
> 
>



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

2019-06-07 Thread Mesos Reviewbot

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



Patch looks great!

Reviews applied: [70810]

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 7, 2019, 2:56 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70810/
> ---
> 
> (Updated June 7, 2019, 2:56 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/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>



Review Request 70811: Fixed compilation error on Mac OS.

2019-06-07 Thread Andrei Budnik

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

Review request for mesos, Benjamin Bannier, Gilbert Song, James Peach, and 
Jiang Yan Xu.


Repository: mesos


Description
---

This patch adds missing switch case to fix compilation error introduced
in `bc5a57122635`.


Diffs
-

  src/slave/containerizer/mesos/launch.cpp 
a69a68823842171bb15ddf34a504e5ce4af232b0 


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


Testing
---

internal CI


Thanks,

Andrei Budnik



Re: Review Request 70752: Supported updating framework in MesosSchedulerDriver.

2019-06-07 Thread Andrei Sekretenko

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

(Updated June 7, 2019, 4:26 p.m.)


Review request for mesos, Benjamin Mahler and Meng Zhu.


Changes
---

Made handling of empty `user` and `hostname` by updateFramework() consistent 
with that of driver's constructor.


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


Repository: mesos


Description
---

This patch adds a method to the MesosSchedulerDriver which updates the
FrameworkInfo (by making an UPDATE_FRAMEWORK call to the master and also
updating the FrameworkInfo stored in the driver for the purposes of
(re-)subscribing).


Diffs (updated)
-

  include/mesos/scheduler.hpp 2ea7cdf1f9473c7b94d4282dd0df129947c97e69 
  src/sched/sched.cpp e77a02951831a7e0c5d9a9068f8d014cb1478382 


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

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


Testing
---


Thanks,

Andrei Sekretenko



Re: Review Request 70752: Supported updating framework in MesosSchedulerDriver.

2019-06-07 Thread Andrei Sekretenko

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

(Updated June 7, 2019, 4:26 p.m.)


Review request for mesos, Benjamin Mahler and Meng Zhu.


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


Repository: mesos


Description
---

This patch adds a method to the MesosSchedulerDriver which updates the
FrameworkInfo (by making an UPDATE_FRAMEWORK call to the master and also
updating the FrameworkInfo stored in the driver for the purposes of
(re-)subscribing).


Diffs
-

  include/mesos/scheduler.hpp 2ea7cdf1f9473c7b94d4282dd0df129947c97e69 
  src/sched/sched.cpp e77a02951831a7e0c5d9a9068f8d014cb1478382 


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


Testing
---


Thanks,

Andrei Sekretenko



Re: Review Request 70671: Added a class for setting expectations on master V1 API events in tests.

2019-06-07 Thread Andrei Sekretenko

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

(Updated June 7, 2019, 4:21 p.m.)


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


Changes
---

Removed workaround for libprocess deadlock (which was fixed in MESOS-9808).


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


Repository: mesos


Description
---

This patch introduces a class with mock methods for subscribing to the
events of master's V1 streaming API and setting expectations for them.


Diffs (updated)
-

  src/Makefile.am 93b2606841745c74dc33713e6b1b2fd62a1c7e74 
  src/tests/CMakeLists.txt e6b1d8a097246f0921467d7f778034d6754fcff6 
  src/tests/master/mock_master_api_subscriber.hpp PRE-CREATION 
  src/tests/master/mock_master_api_subscriber.cpp PRE-CREATION 


Diff: https://reviews.apache.org/r/70671/diff/6/

Changes: https://reviews.apache.org/r/70671/diff/5-6/


Testing
---

`./bin/mesos-tests.sh --verbose --gtest_filter="*UpdateFramework*" 
--gtest_break_on_failure --gtest_repeat=1000` with new tests from 
https://reviews.apache.org/r/70534/

`./bin/mesos-tests.sh --verbose --gtest_filter="*MasterAPITest*" 
--gtest_break_on_failure --gtest_repeat=1000` with refactored tests from 
https://reviews.apache.org/r/70756/


Thanks,

Andrei Sekretenko



Re: Review Request 70748: Disallow verification of empty TLS server certificates.

2019-06-07 Thread Benno Evers


> On May 31, 2019, 3:05 p.m., Alexander Rukletsov wrote:
> > The code looks good to me. Thanks!
> > 
> > As part of this change, either here or in separate reviews, let us:
> > - call out the modification in the changelog;
> > - mention taken approach in MESOS-9791 and explain why (based on the 
> > discussion we had with Jan-Philip Gehrcke);
> > - update SSL flags description, in particular calling out that 
> > `REQUIRE_CERT` is applicable for client mode only;
> > - update SSL documentation, in particular introduce suggested configuration 
> > and clarify the behaviour in client and server modes based on set arguments 
> > (based on the table you showed me offline);
> > - send a message to the user list regarding the use of anonymous ciphers

I posted the updated SSL documentation as a separate review at 
https://reviews.apache.org/r/70810/


- Benno


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


On June 6, 2019, 11:12 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70748/
> ---
> 
> (Updated June 6, 2019, 11:12 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
> ---
> 
> When in SSL client mode and `LIBPROCESS_SSL_VERIFY_CERT=true` has
> been set, enforce that the server actually presents a certificate
> that can be verified.
> 
> Note that in most cases, the TLS stack would rejected the connection
> before the code ever reaches `openssl::verify()`, since the TLS
> specification that a server MUST always send a certificate unless
> an anonymous cipher is used.
> 
> 
> 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/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>



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

2019-06-07 Thread Benno Evers

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

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


Testing
---


Thanks,

Benno Evers



Re: Review Request 70809: Fixed non-deferred callback calling Master::send() in UpdateFramework.

2019-06-07 Thread Mesos Reviewbot

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



Patch looks great!

Reviews applied: [70809]

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 7, 2019, 12:01 p.m., Andrei Sekretenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70809/
> ---
> 
> (Updated June 7, 2019, 12:01 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Benjamin Mahler.
> 
> 
> Bugs: MESOS-9793
> https://issues.apache.org/jira/browse/MESOS-9793
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed non-deferred callback calling Master::send() in UpdateFramework.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp c41d912817a8e84dd3c97008dda76249fd197745 
> 
> 
> Diff: https://reviews.apache.org/r/70809/diff/1/
> 
> 
> Testing
> ---
> 
> make check + tests from https://reviews.apache.org/r/70755/ which is not 
> committed yet:
> 
> `./bin/mesos-tests.sh --verbose --gtest_filter="UpdateFrameworkV0Test*" 
> --gtest_break_on_failure --gtest_repeat=1000`
> 
> 
> Thanks,
> 
> Andrei Sekretenko
> 
>



Re: Review Request 70777: WIP: Stopped using rDNS for hostname validation.

2019-06-07 Thread Benno Evers

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

(Updated June 7, 2019, 1:25 p.m.)


Review request for mesos, Alexander Rukletsov, Benjamin Mahler, and Joseph Wu.


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


Repository: mesos


Description
---

Before this change, libprocess would validate that a given server
certificate matches the first hostname configured in a PTR record
for that servers IP address.

Instead, we now check that the server certificate matches the
hostname that was originally passed to `connect()`.


Diffs
-

  3rdparty/libprocess/include/process/address.hpp 
e740e840c38381bafd7a1a7fcde5f963832ac1fb 
  3rdparty/libprocess/src/http.cpp 3e73ee936f5c6329f41704a179f3d88ab65dfb6d 
  3rdparty/libprocess/src/posix/libevent/libevent_ssl_socket.cpp 
29a1bf71c1df9d80370455a6269ecea0ec4193b0 
  3rdparty/libprocess/src/tests/http_tests.cpp 
97aaf3ed3d4fab6d717d5c9b6d12402562ac6b46 
  3rdparty/libprocess/src/tests/ssl_tests.cpp 
6b8496aeeed79ae1bd39d7013f4f403b248fdd4c 


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


Testing
---

Some tests are failing, hence WIP status :/


Thanks,

Benno Evers



Review Request 70809: Fixed non-deferred callback calling Master::send() in UpdateFramework.

2019-06-07 Thread Andrei Sekretenko

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

Review request for mesos, Benjamin Bannier and Benjamin Mahler.


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


Repository: mesos


Description
---

Fixed non-deferred callback calling Master::send() in UpdateFramework.


Diffs
-

  src/master/master.cpp c41d912817a8e84dd3c97008dda76249fd197745 


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


Testing
---

make check + tests from https://reviews.apache.org/r/70755/ which is not 
committed yet:

`./bin/mesos-tests.sh --verbose --gtest_filter="UpdateFrameworkV0Test*" 
--gtest_break_on_failure --gtest_repeat=1000`


Thanks,

Andrei Sekretenko



Re: Review Request 70804: Used the new quota struct for the allocator recover call.

2019-06-07 Thread Mesos Reviewbot

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



Patch looks great!

Reviews applied: [70799, 70800, 70801, 70802, 70803, 70804]

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 7, 2019, 5:25 a.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70804/
> ---
> 
> (Updated June 7, 2019, 5:25 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-9807
> https://issues.apache.org/jira/browse/MESOS-9807
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Used the new quota struct for the allocator recover call.
> 
> 
> Diffs
> -
> 
>   include/mesos/allocator/allocator.hpp 
> eafcdfdab7f0fc9570e65db15274b52495aa6de3 
>   src/master/allocator/mesos/allocator.hpp 
> 2d83f382eed9d9dca218adf84bc03883f7486349 
>   src/master/allocator/mesos/hierarchical.hpp 
> 1a3329d0e50ad2adb6aeadcd5d315eb572a85704 
>   src/master/allocator/mesos/hierarchical.cpp 
> dff1eabfc34ba132b905de9ad8ab5bd0656d8977 
>   src/master/master.cpp c41d912817a8e84dd3c97008dda76249fd197745 
>   src/tests/allocator.hpp 0718bef4de39a82dbcadebb9967d2ff1cceb1cae 
> 
> 
> Diff: https://reviews.apache.org/r/70804/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>