Re: Review Request 69064: Added unit tests for Master HTTP endpoints.

2018-12-18 Thread Greg Mann
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69064/#review211377 --- src/tests/master_load_tests.cpp Lines 96-102 (patched)

Re: Review Request 69064: Added unit tests for Master HTTP endpoints.

2018-12-18 Thread Greg Mann
> On Nov. 26, 2018, 12:25 p.m., Benno Evers wrote: > > src/tests/master_load_tests.cpp > > Lines 216 (patched) > > > > > > I'm not 100% happy about this part, but I also couldn't think of a > > better way to

Re: Review Request 69543: Implemented recovery for volume gid manager.

2018-12-18 Thread Qian Zhang
> On Dec. 12, 2018, 2:38 a.m., Andrei Budnik wrote: > > src/slave/containerizer/mesos/volume_gid_manager/volume_gid_manager.cpp > > Lines 168 (patched) > > > > > > Consider adding `gidRanges` to the `message

Re: Review Request 69544: Made non-root containers can access shared persistent volume.

2018-12-18 Thread Qian Zhang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69544/ --- (Updated Dec. 18, 2018, 4:08 p.m.) Review request for mesos, Andrei Budnik,

Re: Review Request 69543: Implemented recovery for volume gid manager.

2018-12-18 Thread Qian Zhang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69543/ --- (Updated Dec. 18, 2018, 4:09 p.m.) Review request for mesos, Andrei Budnik,

Re: Review Request 69541: Added volume gid manager.

2018-12-18 Thread Qian Zhang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69541/ --- (Updated Dec. 18, 2018, 4:11 p.m.) Review request for mesos, Andrei Budnik,

Review Request 69581: Fixed an interleaving bug on the master actor.

2018-12-18 Thread Greg Mann
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69581/ --- Review request for mesos, Benno Evers, Chun-Hung Hsiao, Gastón Kleiman, and

Review Request 69582: Added a test to verify a bug fix for the master.

2018-12-18 Thread Greg Mann
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69582/ --- Review request for mesos, Benno Evers, Chun-Hung Hsiao, Gastón Kleiman, and

Review Request 69579: Added a test `ROOT_UNPRIVILEGED_USER_TaskSandboxLocalPersistentVolume`.

2018-12-18 Thread Qian Zhang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69579/ --- Review request for mesos, Andrei Budnik, Gilbert Song, Greg Mann, Ilya Pronin,

Re: Review Request 69422: WIP: Add new metric for cache hits.

2018-12-18 Thread Greg Mann
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69422/#review211387 --- Ship it! I would advocate integrating this with the tests now.

Re: Review Request 69582: Added a test to verify a bug fix for the master.

2018-12-18 Thread Mesos Reviewbot Windows
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69582/#review211389 --- PASS: Mesos patch 69582 was successfully built and tested.

Re: Review Request 69579: Added a test `ROOT_UNPRIVILEGED_USER_TaskSandboxLocalPersistentVolume`.

2018-12-18 Thread Mesos Reviewbot Windows
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69579/#review211388 --- FAIL: Some of the unit tests failed. Please check the relevant

Re: Review Request 67844: Bundled libseccomp v2.3.3 into 3rdparty libraries.

2018-12-18 Thread Qian Zhang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67844/#review211394 --- Ship it! Ship It! - Qian Zhang On Aug. 6, 2018, 9:37 p.m.,

Re: Review Request 69493: Documented the `linux/seccomp` isolator.

2018-12-18 Thread Mesos Reviewbot Windows
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69493/#review211392 --- FAIL: Failed to apply the dependent review: 67844. Failed

Re: Review Request 69572: Validated that resource providers use correct ID in operation states.

2018-12-18 Thread Benjamin Bannier
> On Dec. 17, 2018, 11:33 p.m., Chun-Hung Hsiao wrote: > > src/tests/master_slave_reconciliation_tests.cpp > > Lines 501 (patched) > > > > > > Not sure if this is gramatically correct. > > Should it be (The

Review Request 69584: Relaxed matching criteria for benchmark filter.

2018-12-18 Thread Benno Evers
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69584/ --- Review request for mesos and Meng Zhu. Repository: mesos Description ---

Re: Review Request 69571: Made master process all authorization results for `LAUNCH_GROUP`.

2018-12-18 Thread Jan Schlicht
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69571/#review211395 --- Ship it! Ship It! - Jan Schlicht On Dec. 17, 2018, 11:58

Re: Review Request 69563: Made master to authorize `CREATE_DISK` and `DESTROY_DISK` properly.

2018-12-18 Thread Jan Schlicht
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69563/#review211396 --- Ship it! Ship It! - Jan Schlicht On Dec. 18, 2018,

Re: Review Request 69584: Relaxed matching criteria for test filters.

2018-12-18 Thread Benno Evers
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69584/ --- (Updated Dec. 18, 2018, 2:59 p.m.) Review request for mesos and Meng Zhu.

Re: Review Request 68016: Added libseccomp to the build.

2018-12-18 Thread Andrei Budnik
> On Dec. 12, 2018, 11:51 p.m., Gilbert Song wrote: > > configure.ac > > Lines 352 (patched) > > > > > > Do we have a plan to deprecate this configuraton flag in the future? > > E.g., always only compile the

Re: Review Request 69559: Simplified verify-reviews.py to be more similar to the Python 2 script.

2018-12-18 Thread Till Toenshoff via Review Board
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69559/#review211397 --- Ship it! Ship It! - Till Toenshoff On Dec. 17, 2018, 3:55

Re: Review Request 69577: Documented `Object.value` authorization field for certain operations.

2018-12-18 Thread Benjamin Bannier
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69577/#review211401 --- Fix it, then Ship it! Ship It!

Re: Review Request 69572: Validated that resource providers use correct ID in operation states.

2018-12-18 Thread Mesos Reviewbot Windows
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69572/#review211398 --- PASS: Mesos patch 69572 was successfully built and tested.

Re: Review Request 69578: Set up `Object.value` for `CREATE_DISK` and `DESTROY_DISK` authorization.

2018-12-18 Thread Benjamin Bannier
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69578/#review211402 --- Fix it, then Ship it! src/master/master.cpp Lines 3869-3872

Re: Review Request 67844: Bundled libseccomp v2.3.3 into 3rdparty libraries.

2018-12-18 Thread Andrei Budnik
> On Dec. 6, 2018, 7:01 p.m., Gilbert Song wrote: > > Could you post this patch as a github PR? so that I could apply and commit. https://github.com/apache/mesos/pull/320 - Andrei --- This is an automatically generated e-mail. To

Re: Review Request 69541: Added volume gid manager.

2018-12-18 Thread Andrei Budnik
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69541/#review211408 --- Fix it, then Ship it! Ship It!

Re: Review Request 68795: Added deduplication for read-only master requests.

2018-12-18 Thread Benno Evers
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68795/ --- (Updated Dec. 18, 2018, 6:33 p.m.) Review request for mesos, Alexander

Re: Review Request 69543: Implemented recovery for volume gid manager.

2018-12-18 Thread Andrei Budnik
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69543/#review211409 --- Fix it, then Ship it! Ship It!

Re: Review Request 69584: Relaxed matching criteria for test filters.

2018-12-18 Thread Joseph Wu
> On Dec. 18, 2018, 10:44 a.m., Meng Zhu wrote: > > Not quite sure about the false positive issue: > > > > "This change is also safe regarding false positives, since our > > naming conventions forbid the matched strings from appearing > > naturally in any test name." > > > > Ah, I wasn't aware

Re: Review Request 69064: Added unit tests for Master HTTP endpoints.

2018-12-18 Thread Benno Evers
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69064/ --- (Updated Dec. 18, 2018, 6:41 p.m.) Review request for mesos, Alexander

Re: Review Request 69584: Relaxed matching criteria for test filters.

2018-12-18 Thread Meng Zhu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69584/#review211410 --- Not quite sure about the false positive issue: "This change is

Re: Review Request 69422: Added new metric for cache hits.

2018-12-18 Thread Benno Evers
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69422/ --- (Updated Dec. 18, 2018, 6:41 p.m.) Review request for mesos and Joseph Wu.

Re: Review Request 69064: Added unit tests for Master HTTP endpoints.

2018-12-18 Thread Benno Evers
> On Dec. 18, 2018, 8:32 a.m., Greg Mann wrote: > > src/tests/master_load_tests.cpp > > Lines 293-297 (patched) > > > > > > Should we add something like > > > > ASSERT_TRUE(Clock::now() - whileLoopStartTime

Re: Review Request 69578: Set up `Object.value` for `CREATE_DISK` and `DESTROY_DISK` authorization.

2018-12-18 Thread Chun-Hung Hsiao
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69578/ --- (Updated Dec. 18, 2018, 9:24 p.m.) Review request for mesos, Benjamin Bannier,

Re: Review Request 69581: Fixed an interleaving bug on the master actor.

2018-12-18 Thread Chun-Hung Hsiao
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69581/#review211413 --- Ship it! Ship It! - Chun-Hung Hsiao On Dec. 18, 2018, 9:06

Re: Review Request 69577: Documented `Object.value` authorization field for certain operations.

2018-12-18 Thread Chun-Hung Hsiao
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69577/ --- (Updated Dec. 18, 2018, 8:33 p.m.) Review request for mesos, Benjamin Bannier,

Re: Review Request 69590: Moves CNI root directory to a persistent location.

2018-12-18 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69590/#review211423 --- Fix it, then Ship it!

Review Request 69592: Updated upgrades.md.

2018-12-18 Thread Deepak Goel
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69592/ --- Review request for mesos, Jie Yu and Qian Zhang. Bugs: MESOS-9492

Re: Review Request 69590: Moves CNI root directory to a persistent location.

2018-12-18 Thread Deepak Goel
> On Dec. 19, 2018, 12:48 a.m., Till Toenshoff wrote: > > src/slave/containerizer/mesos/isolators/network/cni/cni.cpp > > Lines 1307-1308 (original) > > > > > > Are we assuming that this location was never meant

Re: Review Request 69590: Moves CNI root directory to a persistent location.

2018-12-18 Thread Qian Zhang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69590/#review211418 --- src/slave/containerizer/mesos/isolators/network/cni/paths.hpp

Re: Review Request 69590: Moves CNI root directory to a persistent location.

2018-12-18 Thread Chun-Hung Hsiao
> On Dec. 19, 2018, 2:51 a.m., Qian Zhang wrote: > > src/slave/flags.cpp > > Lines 1191 (patched) > > > > > > Should the default value be `false` for backward compatibility? Hmm yeah maybe it's better if we make

Re: Review Request 69590: Moves CNI root directory to a persistent location.

2018-12-18 Thread Qian Zhang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69590/#review211424 --- Fix it, then Ship it! The commit message of this patch seems

Re: Review Request 69581: Fixed an interleaving bug on the master actor.

2018-12-18 Thread Gastón Kleiman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69581/#review211415 --- Ship it! Ship It! - Gastón Kleiman On Dec. 18, 2018, 1:06

Re: Review Request 69592: Updated upgrades.md.

2018-12-18 Thread Qian Zhang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69592/#review211427 --- Ship it! Ship It! - Qian Zhang On Dec. 19, 2018, 3:22 p.m.,

Re: Review Request 69590: Moves CNI root directory to a persistent location.

2018-12-18 Thread Deepak Goel
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69590/ --- (Updated Dec. 19, 2018, 3:25 a.m.) Review request for mesos, Gilbert Song, Jie

Re: Review Request 69590: Moves CNI root directory to a persistent location.

2018-12-18 Thread Chun-Hung Hsiao
> On Dec. 19, 2018, 12:48 a.m., Till Toenshoff wrote: > > src/slave/containerizer/mesos/isolators/network/cni/cni.cpp > > Lines 1307-1308 (original) > > > > > > Are we assuming that this location was never meant

Review Request 69591: Fixed allocator benchmark names to ensure proper filtering.

2018-12-18 Thread Meng Zhu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69591/ --- Review request for mesos, Benno Evers and Joseph Wu. Repository: mesos

Re: Review Request 69590: Moves CNI root directory to a persistent location.

2018-12-18 Thread Deepak Goel
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69590/ --- (Updated Dec. 19, 2018, 2:03 a.m.) Review request for mesos, Gilbert Song, Jie

Re: Review Request 69590: Moves CNI root directory to a persistent location.

2018-12-18 Thread Chun-Hung Hsiao
> On Dec. 19, 2018, 2:51 a.m., Qian Zhang wrote: > > src/slave/flags.cpp > > Lines 1191 (patched) > > > > > > Should the default value be `false` for backward compatibility? > > Chun-Hung Hsiao wrote: > Hmm

Re: Review Request 69590: Moves CNI root directory to a persistent location.

2018-12-18 Thread Deepak Goel
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69590/ --- (Updated Dec. 19, 2018, 4:52 a.m.) Review request for mesos, Gilbert Song, Jie

Re: Review Request 69590: Moves CNI root directory to a persistent location.

2018-12-18 Thread Deepak Goel
> On Dec. 19, 2018, 12:48 a.m., Till Toenshoff wrote: > > src/slave/containerizer/mesos/isolators/network/cni/cni.cpp > > Lines 1307-1308 (original) > > > > > > Are we assuming that this location was never meant

Review Request 69590: Moves CNI root directory to a persistent location.

2018-12-18 Thread Deepak Goel
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69590/ --- Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Sergey

Re: Review Request 69590: Moves CNI root directory to a persistent location.

2018-12-18 Thread Till Toenshoff via Review Board
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69590/#review211416 --- Thanks for this. Just some nitpicking below :)