Re: Review Request 65073: Fixed post-reviews for posting reviews from `master` branch.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65073/ --- (Updated Jan. 11, 2018, 8:56 a.m.) Review request for mesos and Armand

Re: Review Request 64978: Made task's volume directory visible in the /files endpoints.

> On Jan. 9, 2018, 8:12 a.m., Jie Yu wrote: > > src/slave/slave.cpp > > Lines 2795-2829 (patched) > > > > > > I am flying by. I am wondering if we should add this logic to > > `Executor::addLaunchedTask` and

Re: Review Request 65072: Fixed handling of terminal operations in updateSlave handler.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65072/#review195202 --- src/master/master.cpp Line 10419 (original), 10447 (patched)

Re: Review Request 64978: Made task's volume directory visible in the /files endpoints.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64978/#review195178 --- src/slave/slave.cpp Line 2795 (original), 2795 (patched)

Re: Review Request 65085: WIP: Added a unit test for fd leakage with blocking HTTP calls.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65085/#review195203 --- FAIL: Failed to apply the dependent review: 64994. Failed

Re: Review Request 64992: Added SLRP unit tests for profile updates and corner cases.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64992/#review195198 --- Fix it, then Ship it!

Re: Review Request 65056: Added missing fields to the GET_MASTER operator API call.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65056/#review195197 --- PASS: Mesos patch 65056 was successfully built and tested.

Re: Review Request 65056: Added missing fields to the GET_MASTER operator API call.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65056/#review195195 --- Ship it! Ship It! - Gaston Kleiman On Jan. 10, 2018, 3:11

Re: Review Request 65072: Fixed handling of terminal operations in updateSlave handler.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65072/#review195192 --- src/master/master.cpp Lines 7718 (patched)

Review Request 65085: WIP: Added a unit test for fd leakage with blocking HTTP calls.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65085/ --- Review request for mesos, Greg Mann and Jie Yu. Bugs: MESOS-8428

Re: Review Request 64516: Improved documentation on resource reservations.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64516/#review195190 --- PASS: Mesos patch 64516 was successfully built and tested.

Re: Review Request 65056: Added missing fields to the GET_MASTER operator API call.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65056/ --- (Updated Jan. 10, 2018, 11:11 p.m.) Review request for mesos, Anand Mazumdar,

Re: Review Request 65022: Updated uses of `protobuf::read(path)` which now returns `Try`.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65022/#review195189 --- Ship it!

Re: Review Request 64930: Unified the marking agent unreachable logic in the master.

> On Jan. 8, 2018, 5:31 p.m., Benno Evers wrote: > > src/master/master.cpp > > Line 8304 (original), 8235 (patched) > > > > > > After a similar a change I got the following comment from @dzhuk, which > > I'll

Re: Review Request 64387: Windows: Ported docker health check tests.

> On Jan. 10, 2018, 1:41 p.m., Alexander Rukletsov wrote: > > src/tests/health_check_tests.cpp > > Lines 120-122 (patched) > > > > > > Why only on windows? The original code didn't pre-pull the images, because the

Re: Review Request 63862: Windows: Ported docker_tests.cpp.

> On Jan. 10, 2018, 1:27 p.m., Alexander Rukletsov wrote: > > src/tests/containerizer/docker_tests.cpp > > Lines 130-137 (patched) > > > > > > Looks like instead of this adhoc routine we should rather introduce a >

Re: Review Request 64386: Windows: Enabled docker health checks.

> On Jan. 10, 2018, 1:16 p.m., Alexander Rukletsov wrote: > > src/checks/checker.hpp > > Lines 73 (patched) > > > > > > I think all such commands should not be passed from outside, but be > > part of the library

Re: Review Request 63861: Windows: Updated networking doc.

> On Jan. 10, 2018, 12:32 p.m., Alexander Rukletsov wrote: > > docs/networking.md > > Lines 118-122 (patched) > > > > > > Can you please mention that other network modes than `NAT` and `USER` > > are not supported

Re: Review Request 63860: Windows: Mapped the Docker network info types.

> On Jan. 10, 2018, 12:30 p.m., Alexander Rukletsov wrote: > > src/docker/docker.cpp > > Lines 744-749 (patched) > > > > > > Does it make sense to also remove the default here? > >

Re: Review Request 64516: Improved documentation on resource reservations.

> On Jan. 9, 2018, 8:27 a.m., James Peach wrote: > > docs/reservation.md > > Lines 88 (patched) > > > > > > "Similarly, agents can register ..." Meaning s/by default// because of repetition? It doesn't feel too

Re: Review Request 64516: Improved documentation on resource reservations.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64516/ --- (Updated Jan. 10, 2018, 2:23 p.m.) Review request for mesos, Alexander Rojas,

Re: Review Request 64515: Used `reserve_resources` ACL for static reservations.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64515/ --- (Updated Jan. 10, 2018, 2:22 p.m.) Review request for mesos, Alexander Rojas

Re: Review Request 64515: Used `reserve_resources` ACL for static reservations.

> On Jan. 9, 2018, 8:45 a.m., James Peach wrote: > > What do you think about testing the re-registration case? There's no > > coverage here, but equally, it would be annoying to duplicate all these > > tests for re-registration ... Two other tests `AuthorizedToRegisterAndReregisterAgent` and

Re: Review Request 65022: Updated uses of `protobuf::read(path)` which now returns `Try`.

> On Jan. 9, 2018, 5:49 p.m., Benjamin Mahler wrote: > > Looks like we need to take a closer look at the change being made w.r.t. to > > the none case now becoming an error. > > > > Can you include vinod on this review? Consulted Chun and Jie to audit this. Since we introduced

Re: Review Request 65042: Adjusted CSI example framework for recent changes.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65042/#review195183 --- Ship it! Ship It! - Gaston Kleiman On Jan. 10, 2018, 1:16

Re: Review Request 63422: Added os::eraseenv to clear the old environment value.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63422/ --- (Updated Jan. 10, 2018, 10:08 p.m.) Review request for mesos and Greg Mann.

Re: Review Request 63422: Added os::eraseenv to clear the old environment value.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63422/ --- (Updated Jan. 10, 2018, 10 p.m.) Review request for mesos and Greg Mann.

Re: Review Request 65056: Added missing fields to the GET_MASTER operator API call.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65056/#review195180 --- include/mesos/v1/mesos.proto Lines 905-909 (patched)

Re: Review Request 65022: Updated uses of `protobuf::read(path)` which now returns `Try`.

> On Jan. 10, 2018, 1:29 p.m., Chun-Hung Hsiao wrote: > > src/slave/containerizer/mesos/paths.cpp > > Line 263 (original), 263 (patched) > > > > > > Return a `Try`. Oops. Sorry, turns out this is actually correct

Re: Review Request 65022: Updated uses of `protobuf::read(path)` which now returns `Try`.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65022/#review195174 --- src/slave/containerizer/mesos/paths.cpp Line 263 (original), 263

Re: Review Request 63859: Windows: Fixed mock signal values in stout.

> On Jan. 10, 2018, 12:09 p.m., Alexander Rukletsov wrote: > > 3rdparty/stout/include/stout/windows.hpp > > Lines 343-348 (original), 343-348 (patched) > > > > > > Could you please clarify, what the fix is here? At

Re: Review Request 65072: Fixed handling of terminal operations in updateSlave handler.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65072/#review195169 --- src/master/master.cpp Lines 7722 (patched)

Re: Review Request 65077: Added a test to check `wait` of composing c'zer after recovery.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65077/#review195166 --- FAIL: Mesos failed to build. Reviews applied: `['65071',

Review Request 65077: Added a test to check `wait` of composing c'zer after recovery.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65077/ --- Review request for mesos, Alexander Rukletsov, Gilbert Song, and Jie Yu. Bugs:

Re: Review Request 65071: Fixed composing containerizer hanging in `wait()` after recovery.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65071/#review195163 --- FAIL: Mesos failed to build. Reviews applied: `['65071']`

Re: Review Request 65071: Fixed composing containerizer hanging in `wait()` after recovery.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65071/ --- (Updated Jan. 10, 2018, 6:48 p.m.) Review request for mesos, Alexander

Re: Review Request 64936: Improved the documentation of protos related to operation feedback.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64936/#review195162 --- Ship it! Ship It! - Greg Mann On Jan. 10, 2018, 1:22 a.m.,

Re: Review Request 64931: Added a TODO for informing frameworks that an agent is unreachable.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64931/#review195160 --- FAIL: mesos-java failed to build. Reviews applied: `['64930',

Re: Review Request 65072: Fixed handling of terminal operations in updateSlave handler.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65072/#review195157 --- PASS: Mesos patch 65072 was successfully built and tested.

Re: Review Request 63423: Cleared the executor auth token after using it.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63423/#review195156 --- FAIL: mesos-java failed to build. Reviews applied: `['63422',

Re: Review Request 65074: Fixed comparison logic for additive reconfiguration policy.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65074/#review195153 --- FAIL: mesos-java failed to build. Reviews applied: `['65074']`

Re: Review Request 64931: Added a TODO for informing frameworks that an agent is unreachable.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64931/#review195152 --- Ship it! Ship It! - Benno Evers On Jan. 3, 2018, 10:48

Re: Review Request 64930: Unified the marking agent unreachable logic in the master.

> On Jan. 8, 2018, 5:31 p.m., Benno Evers wrote: > > The structure of the new `markUnreachable()` seems to be `[recover slave > > info depending on bool parameter] -> common code path to apply registry > > operation -> [notification/cleanup depending on bool parameter]`. > > Intuitively I'd

Re: Review Request 64930: Unified the marking agent unreachable logic in the master.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64930/#review195151 --- Ship it! Ship It! - Benno Evers On Jan. 9, 2018, 8:06 a.m.,

Re: Review Request 65073: Fixed post-reviews for posting reviews from `master` branch.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65073/#review195149 --- FAIL: mesos-java failed to build. Reviews applied: `['65073']`

Re: Review Request 63862: Windows: Ported docker_tests.cpp.

> On Jan. 10, 2018, 5:27 a.m., Alexander Rukletsov wrote: > > src/tests/containerizer/docker_tests.cpp > > Lines 130-137 (patched) > > > > > > Looks like instead of this adhoc routine we should rather introduce a >

Re: Review Request 63859: Windows: Fixed mock signal values in stout.

> On Jan. 10, 2018, 4:09 a.m., Alexander Rukletsov wrote: > > 3rdparty/stout/include/stout/windows.hpp > > Lines 343-348 (original), 343-348 (patched) > > > > > > Could you please clarify, what the fix is here? At

Re: Review Request 63859: Windows: Fixed mock signal values in stout.

> On Jan. 10, 2018, 4:09 a.m., Alexander Rukletsov wrote: > > 3rdparty/stout/include/stout/windows.hpp > > Lines 343-348 (original), 343-348 (patched) > > > > > > Could you please clarify, what the fix is here? At

Re: Review Request 65071: Fixed composing containerizer hanging in `wait()` after recovery.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65071/#review195145 --- FAIL: mesos-java failed to build. Reviews applied: `['65071']`

Re: Review Request 65070: WIP: Updated `ROOT_TaskSandboxPersistentVolume` to check `/files` endpoint.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65070/#review195142 --- FAIL: Some Mesos tests failed. Reviews applied: `['64978',

Re: Review Request 63423: Cleared the executor auth token after using it.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63423/ --- (Updated Jan. 10, 2018, 4:37 p.m.) Review request for mesos and Greg Mann.

Re: Review Request 63422: Added os::eraseenv to clear the old environment value.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63422/ --- (Updated Jan. 10, 2018, 4:37 p.m.) Review request for mesos and Greg Mann.

Review Request 65074: Fixed comparison logic for additive reconfiguration policy.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65074/ --- Review request for mesos and Vinod Kone. Bugs: MESOS-8410

Re: Review Request 64849: Added authentication to some example frameworks.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64849/#review195141 --- Ship it! Ship It! - Armand Grillet On Jan. 9, 2018, 3:19

Re: Review Request 65073: Fixed post-reviews for posting reviews from `master` branch.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65073/#review195139 --- support/post-reviews.py Lines 189 (patched)

Review Request 65073: Fixed post-reviews for posting reviews from `master` branch.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65073/ --- Review request for mesos and Armand Grillet. Repository: mesos Description

Review Request 65072: Fixed handling of terminal operations in updateSlave handler.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65072/ --- Review request for mesos, Gaston Kleiman and Greg Mann. Repository: mesos

Review Request 65071: Fixed composing containerizer hanging in `wait()` after recovery.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65071/ --- Review request for mesos, Alexander Rukletsov, Gilbert Song, and Jie Yu. Bugs:

Re: Review Request 63862: Windows: Ported docker_tests.cpp.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63862/#review195138 --- PASS: Mesos patch 63862 was successfully built and tested.

Re: Review Request 65070: WIP: Updated `ROOT_TaskSandboxPersistentVolume` to check `/files` endpoint.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65070/ --- (Updated Jan. 10, 2018, 10:14 p.m.) Review request for mesos, Benjamin Mahler,

Re: Review Request 64978: Made task's volume directory visible in the /files endpoints.

> On Jan. 9, 2018, 8:04 a.m., Vinod Kone wrote: > > Can you add a test or update an existing test to verify the /files endpoint > > for task volume? Ideally, you could also verify that once the executor's > > work directory is gc'ed the files endpoint no longer serves the task volume > >

Review Request 65070: Updated `ROOT_TaskSandboxPersistentVolume` to check `/files` endpoint.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65070/ --- Review request for mesos, Benjamin Mahler, Gilbert Song, Jie Yu, and Vinod Kone.

Re: Review Request 64387: Windows: Ported docker health check tests.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64387/#review195134 --- src/tests/health_check_tests.cpp Lines 120-122 (patched)

Re: Review Request 63862: Windows: Ported docker_tests.cpp.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63862/#review195133 --- src/tests/containerizer/docker_tests.cpp Lines 130-137 (patched)

Re: Review Request 64386: Windows: Enabled docker health checks.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64386/#review195132 --- src/checks/checker.hpp Lines 73 (patched)

Re: Review Request 63861: Windows: Updated networking doc.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63861/#review195131 --- Fix it, then Ship it! docs/networking.md Lines 118-122

Re: Review Request 63860: Windows: Mapped the Docker network info types.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63860/#review195130 --- src/docker/docker.cpp Lines 744-749 (patched)

Re: Review Request 63859: Windows: Fixed mock signal values in stout.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63859/#review195129 --- 3rdparty/stout/include/stout/windows.hpp Lines 343-348

Re: Review Request 65042: Adjusted CSI example framework for recent changes.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65042/#review195124 --- FAIL: mesos-java failed to build. Reviews applied: `['64932',

Re: Review Request 65063: Replace `stringify()` with `jsonify()` in http serialization.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65063/#review195122 --- FAIL: Some Mesos tests failed. Reviews applied: `['65063']`

Re: Review Request 65042: Adjusted CSI example framework for recent changes.

> On Jan. 9, 2018, 10:49 p.m., Gaston Kleiman wrote: > > src/examples/test_csi_user_framework.cpp > > Lines 422-432 (patched) > > > > > > It is not very clear to me why we need this block. > > > > Do we

Re: Review Request 65042: Adjusted CSI example framework for recent changes.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65042/ --- (Updated Jan. 10, 2018, 10:16 a.m.) Review request for mesos, Gaston Kleiman

Re: Review Request 64848: Updated example frameworks to make use of added flags.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64848/#review195120 --- Ship it! Ship It! - Armand Grillet On Jan. 9, 2018, 3:19