Review Request 55749: Added CMake to standard documentation.

[Alex Clemmer]

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

Review request for mesos, Andrew Schwartzmeyer, Daniel Pravat, and Joseph Wu.


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


Repository: mesos


Description
---

Added CMake to standard documentation.


Diffs
-

  docs/configuration.md 2113d06f58dddc0a28ae1241a24096266fe39801 

Diff: https://reviews.apache.org/r/55749/diff/


Testing
---


Thanks,

Alex Clemmer

Review Request 55748: CMake: Deleted spurious configuration settings in agent and master.

[Alex Clemmer]

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

Review request for mesos, Andrew Schwartzmeyer, Daniel Pravat, and Joseph Wu.


Repository: mesos


Description
---

CMake: Deleted spurious configuration settings in agent and master.


Diffs
-

  src/master/cmake/MasterConfigure.cmake 
3d316d6ff2910fc360b0faecb5e6ac9687a77883 
  src/slave/cmake/AgentConfigure.cmake 1582127ccce773af6031a5b09252192b05a13cdc 

Diff: https://reviews.apache.org/r/55748/diff/


Testing
---


Thanks,

Alex Clemmer

Re: Review Request 55745: Added scheduler adapter to API client libraries doc.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [55745]

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

- Mesos Reviewbot


On Jan. 20, 2017, 4:14 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55745/
> ---
> 
> (Updated Jan. 20, 2017, 4:14 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   docs/api-client-libraries.md a12489e4c9efe9c74ab87d6806ea26ac0a6f45c9 
> 
> Diff: https://reviews.apache.org/r/55745/diff/
> 
> 
> Testing
> ---
> 
> https://gist.github.com/hatred/1a7ef232c6c789db4b467c139f545f9d
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 55732: Added `--3way` option to `git apply`.

[haosdent huang]

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




support/apply-reviews.py (lines 120 - 121)


Is this change necessary?


- haosdent huang


On Jan. 19, 2017, 10:49 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55732/
> ---
> 
> (Updated Jan. 19, 2017, 10:49 p.m.)
> 
> 
> Review request for mesos, Jason Lai, Joseph Wu, and Michael Park.
> 
> 
> Bugs: MESOS-4119
> https://issues.apache.org/jira/browse/MESOS-4119
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This can help on some patches which includes conflicts that can be
> resolved by 3 way merge.
> 
> 
> Diffs
> -
> 
>   support/apply-reviews.py c77b4c2b2d7a3d5e74e225403e71a84e23a9a1e7 
> 
> Diff: https://reviews.apache.org/r/55732/diff/
> 
> 
> Testing
> ---
> 
> With this fix, I was able to apply the chain in r/52534, which has a conflict 
> otherwise.
> 
> Bash log:
> ```
> $ python ./support/apply-reviews.py -r 52534 -c
> 2017-01-19 14:45:26 URL:https://reviews.apache.org/r/51027/diff/raw/ 
> [9830/9830] -> "51027.patch" [1]
> [3way fba53e2] Track allocation candidates to bound allocator.
>  Author: Jacob Janco 
>  2 files changed, 97 insertions(+), 54 deletions(-)
> 2017-01-19 14:45:28 URL:https://reviews.apache.org/r/52534/diff/raw/ 
> [2950/2950] -> "52534.patch" [1]
> error: patch failed: src/master/allocator/mesos/hierarchical.hpp:224
> Falling back to three-way merge...
> Applied patch to 'src/master/allocator/mesos/hierarchical.hpp' cleanly.
> [3way 261018a] Dispatch filter expiration twice.
>  Author: Jacob Janco 
>  2 files changed, 32 insertions(+), 8 deletions(-)
>  ```
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 55710: Added Agent capabilities in the response of /state endpoint.

[Jay Guo]

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

(Updated Jan. 20, 2017, 1:29 p.m.)


Review request for mesos, Benjamin Bannier, Benjamin Mahler, and Guangya Liu.


Changes
---

Added a test to check `capablities` is included in `/state` response.


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


Repository: mesos


Description
---

We introduced Capabilities to SlaveInfo protobuf message in ec1a326.
It is automatically wrapped into response of GetState v1 API. This
patch added this field to /state v0 API for consistency.


Diffs (updated)
-

  src/master/http.cpp a44621f39cb059e654a56f57f75b38947f3a4230 
  src/tests/master_tests.cpp da7094dbbafbb0ab1153a0a4a6fcabd63888d67a 

Diff: https://reviews.apache.org/r/55710/diff/


Testing (updated)
---

make check GTEST_FILTER="MasterTest.StateEndpointAgentCapabilities"
[   OK ] MasterTest.StateEndpointAgentCapabilities (85 ms)
[--] 1 test from MasterTest (94 ms total)

[--] Global test environment tear-down
[==] 1 test from 1 test case ran. (122 ms total)

make check on Ubuntu 14.04


Thanks,

Jay Guo

Re: Review Request 55740: Fixed unsafe usage of process pointer in loop.hpp.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [55701, 55740]

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

- Mesos Reviewbot


On Jan. 20, 2017, 1:34 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55740/
> ---
> 
> (Updated Jan. 20, 2017, 1:34 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Hindman, and Benjamin 
> Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The `loop(...)` helper spawns a libprocess actor to execute some
> lambda (in a loop, of course).  This actor is owned by the
> libprocess GC actor, but the body of the `loop` passes a copy of that
> pointer into a Future callback.  This will potentially segfault if
> the actor terminates outside of the `loop`.
> 
> Instead, the `loop` should use the `PID` of the actor.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/loop.hpp 
> 8bd9715246e72474a35a0f1af94c8a5a3e87dd7a 
> 
> Diff: https://reviews.apache.org/r/55740/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Review Request 55745: Added scheduler adapter to API client libraries doc.

[Anand Mazumdar]

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

Review request for mesos and Vinod Kone.


Repository: mesos


Description
---

See summary.


Diffs
-

  docs/api-client-libraries.md a12489e4c9efe9c74ab87d6806ea26ac0a6f45c9 

Diff: https://reviews.apache.org/r/55745/diff/


Testing
---

https://gist.github.com/hatred/1a7ef232c6c789db4b467c139f545f9d


Thanks,

Anand Mazumdar

Re: Review Request 55740: Fixed unsafe usage of process pointer in loop.hpp.

[Benjamin Mahler]

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


Ship it!




I would suggest clarifying in the commit that there isn't an issue here 
currently (right?), but the code is brittle and we want to make generally safer.

- Benjamin Mahler


On Jan. 20, 2017, 1:34 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55740/
> ---
> 
> (Updated Jan. 20, 2017, 1:34 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Hindman, and Benjamin 
> Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The `loop(...)` helper spawns a libprocess actor to execute some
> lambda (in a loop, of course).  This actor is owned by the
> libprocess GC actor, but the body of the `loop` passes a copy of that
> pointer into a Future callback.  This will potentially segfault if
> the actor terminates outside of the `loop`.
> 
> Instead, the `loop` should use the `PID` of the actor.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/loop.hpp 
> 8bd9715246e72474a35a0f1af94c8a5a3e87dd7a 
> 
> Diff: https://reviews.apache.org/r/55740/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 55732: Added `--3way` option to `git apply`.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [55732]

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

- Mesos Reviewbot


On Jan. 19, 2017, 10:49 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55732/
> ---
> 
> (Updated Jan. 19, 2017, 10:49 p.m.)
> 
> 
> Review request for mesos, Jason Lai, Joseph Wu, and Michael Park.
> 
> 
> Bugs: MESOS-4119
> https://issues.apache.org/jira/browse/MESOS-4119
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This can help on some patches which includes conflicts that can be
> resolved by 3 way merge.
> 
> 
> Diffs
> -
> 
>   support/apply-reviews.py c77b4c2b2d7a3d5e74e225403e71a84e23a9a1e7 
> 
> Diff: https://reviews.apache.org/r/55732/diff/
> 
> 
> Testing
> ---
> 
> With this fix, I was able to apply the chain in r/52534, which has a conflict 
> otherwise.
> 
> Bash log:
> ```
> $ python ./support/apply-reviews.py -r 52534 -c
> 2017-01-19 14:45:26 URL:https://reviews.apache.org/r/51027/diff/raw/ 
> [9830/9830] -> "51027.patch" [1]
> [3way fba53e2] Track allocation candidates to bound allocator.
>  Author: Jacob Janco 
>  2 files changed, 97 insertions(+), 54 deletions(-)
> 2017-01-19 14:45:28 URL:https://reviews.apache.org/r/52534/diff/raw/ 
> [2950/2950] -> "52534.patch" [1]
> error: patch failed: src/master/allocator/mesos/hierarchical.hpp:224
> Falling back to three-way merge...
> Applied patch to 'src/master/allocator/mesos/hierarchical.hpp' cleanly.
> [3way 261018a] Dispatch filter expiration twice.
>  Author: Jacob Janco 
>  2 files changed, 32 insertions(+), 8 deletions(-)
>  ```
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Review Request 55740: Fixed unsafe usage of process pointer in loop.hpp.

[Joseph Wu]

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

Review request for mesos, Benjamin Bannier, Benjamin Hindman, and Benjamin 
Mahler.


Repository: mesos


Description
---

The `loop(...)` helper spawns a libprocess actor to execute some
lambda (in a loop, of course).  This actor is owned by the
libprocess GC actor, but the body of the `loop` passes a copy of that
pointer into a Future callback.  This will potentially segfault if
the actor terminates outside of the `loop`.

Instead, the `loop` should use the `PID` of the actor.


Diffs
-

  3rdparty/libprocess/include/process/loop.hpp 
8bd9715246e72474a35a0f1af94c8a5a3e87dd7a 

Diff: https://reviews.apache.org/r/55740/diff/


Testing
---

make check


Thanks,

Joseph Wu

Re: Review Request 55701: Fixed unsafe usage of process pointer in async.hpp.

[Joseph Wu]

> On Jan. 19, 2017, 6:08 a.m., Benjamin Bannier wrote:
> > Looking through other instances of `spawn(.*,\ true)`, should this one also 
> > be adjusted, 
> > https://github.com/apache/mesos/blob/745b3c7589e5252cf93f62e081b78fa420771d0c/3rdparty/libprocess/include/process/loop.hpp#L134-L144?

I'll open a separate review for this one.  (I haven't hit that particular 
segfault in tests.)


- Joseph


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


On Jan. 18, 2017, 6:41 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55701/
> ---
> 
> (Updated Jan. 18, 2017, 6:41 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Hindman, Benjamin 
> Mahler, and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The `async(...)` helper spawns a libprocess actor to execute some
> lambda (asynchronously, of course).  This actor is owned by the
> libprocess GC actor, but the `AsyncExecutor` stores a copy of that
> pointer and dereferences it in several possible locations.
> 
> Instead, the `AsyncExecutor` should store the `PID` of the actor,
> which can be safely used, even if the actor has already terminated;
> such as turning libprocess finalization.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/async.hpp 
> 8565a52c6ba4008edb852e898b8f0b6d598a194f 
> 
> Diff: https://reviews.apache.org/r/55701/diff/
> 
> 
> Testing
> ---
> 
> Applied the following change:
> ```
> diff --git a/src/launcher/executor.cpp b/src/launcher/executor.cpp
> index e035a4e..4c60ef9 100644
> --- a/src/launcher/executor.cpp
> +++ b/src/launcher/executor.cpp
> @@ -936,6 +930,8 @@ int main(int argc, char** argv)
>  
>process::spawn(executor.get());
>process::wait(executor.get());
> +  executor.reset();
>  
> +  process::finalize(true);
>return EXIT_SUCCESS;
>  }
> ```
> 
> And then ran:
> ```
> make check GTEST_FILTER="HTTPCommandExecutorTest.TerminateWithACK"
> ```
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 55727: Checkpoint and track docker image layer sizes.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [53105, 53330, 55727]

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

- Mesos Reviewbot


On Jan. 19, 2017, 8:09 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55727/
> ---
> 
> (Updated Jan. 19, 2017, 8:09 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.
> 
> 
> Bugs: MESOS-6893
> https://issues.apache.org/jira/browse/MESOS-6893
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch added the following capabilities:
> 1. add the size and fetch timestamp of each docker image layer in metadata;
> 2. checkpoint and recover size (timestamp will be added later);
> 3. reuse the `DiskUsageCollector` class to track size of each downloaded 
> store;
> 4. for upgrade path, reuse same collector to back fill the size of layers.
> 
> Tests to be added later. Sending out review for earlier feedback.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/docker/message.proto 
> c93c7a92ec152bd9747a70392adfe6a0e863e839 
>   src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp 
> 954da1681778878c8aff6150002e52ecb648d1bb 
>   src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp 
> 395c36b0f6b8f8e037681ec5f2df99e83a7cf155 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> 9dccd0673dbc0c61abfd4b88097f86e7d7131c46 
> 
> Diff: https://reviews.apache.org/r/55727/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 55714: Added fs::magic() mapping function for fs id to fs type mapping.

[James Peach]

> On Jan. 19, 2017, 11:23 p.m., James Peach wrote:
> > src/linux/fs.hpp, line 244
> > 
> >
> > I get the analogy that leads to `magic`, but I'd prefer that it was 
> > named something more obvious. Maybe `typeName()`?

If you change this name, consider the names of the `FS_MAGIC` constants too. 
Perhaps `FS_TYPE_XXX`?


- James


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


On Jan. 19, 2017, 11:33 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55714/
> ---
> 
> (Updated Jan. 19, 2017, 11:33 p.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Jie Yu, James Peach, Timothy 
> Chen, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6958
> https://issues.apache.org/jira/browse/MESOS-6958
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> A simple uint32_t -> string mapping.
> 
> 
> Diffs
> -
> 
>   src/linux/fs.hpp da49c9ebfa938d169152ed3b6e4df7378711b013 
>   src/linux/fs.cpp 913e23317291db164fe6bdf77f3eca146dedec9b 
> 
> Diff: https://reviews.apache.org/r/55714/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 50871: Supported auto backend in Unified Containerizer.

[James Peach]

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




src/slave/containerizer/mesos/provisioner/provisioner.cpp (line 69)


For future maintenance, I think it would be very helpful to give some 
extended rationale for this support matrix.



src/slave/containerizer/mesos/provisioner/provisioner.cpp (line 80)


You really only need the strings to generate a sane error message. For 
actual validation, you can use the `f_type` constants directly.



src/slave/containerizer/mesos/provisioner/provisioner.cpp (line 83)


This might be a bit picky, but I'd be inclined to avoid the hashset 
allocation in this case.

Maybe:

```
const std::array o {
FS_TYPE_AUFS, FS_TYPE_BTRFS, FS_TYPE_ECRYPTFS,
...
};

std::find(o.begin(), o.end(), needle);

```

It's unfortunately verbose though :(


- James Peach


On Jan. 19, 2017, 6:45 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50871/
> ---
> 
> (Updated Jan. 19, 2017, 6:45 p.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Artem Harutyunyan, Jie Yu, and 
> Timothy Chen.
> 
> 
> Bugs: MESOS-5931
> https://issues.apache.org/jira/browse/MESOS-5931
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Select a backend smartly. Currently, the logic is really
> simple:
> 1. Use overlayfs backend if it exists.
> 2. Use aufs backend if the overlayfs does not exists.
> 3. Use copy backend of both overlayfs and aufs do not exist.
> Please note that the bind backend needs to be specified explicitly
> through the agent flag '--image_provisioner_backend' since it
> requires the sandbox already existed.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md 2113d06f58dddc0a28ae1241a24096266fe39801 
>   src/slave/containerizer/mesos/provisioner/appc/store.hpp 
> 26cdd2fba142874ab1d3eca61222bb22bfbb2e13 
>   src/slave/containerizer/mesos/provisioner/appc/store.cpp 
> e63ae419e24212b887edddeb5cae114cd39b39c8 
>   src/slave/containerizer/mesos/provisioner/docker/local_puller.hpp 
> abb8e7e48422896f207a475661ced0530fc75e68 
>   src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp 
> 817e30c5d6d6a4b011193e3209301fc3cdf88b06 
>   src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp 
> 954da1681778878c8aff6150002e52ecb648d1bb 
>   src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp 
> 395c36b0f6b8f8e037681ec5f2df99e83a7cf155 
>   src/slave/containerizer/mesos/provisioner/docker/paths.hpp 
> 232c027f8f96da0cb30b957bce4607d3695050d2 
>   src/slave/containerizer/mesos/provisioner/docker/paths.cpp 
> cd684b33eb308ce1eeb4539a5b2d51985d835db7 
>   src/slave/containerizer/mesos/provisioner/docker/puller.hpp 
> 191f3fc8cd1e0f706b5b1e6afddf856e8d53ae6e 
>   src/slave/containerizer/mesos/provisioner/docker/puller.cpp 
> ac9dae8ecbb897b8ff942d11ac70281a63e06831 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.hpp 
> bbd6005317bed3fff3d86e2527ca3ead839d49e3 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
> cecf34a23329a64fdbce7de4b83827a30975e9a4 
>   src/slave/containerizer/mesos/provisioner/docker/store.hpp 
> 1c2b149823e83dc5a3feb0af599d651d1dc05682 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> 9dccd0673dbc0c61abfd4b88097f86e7d7131c46 
>   src/slave/containerizer/mesos/provisioner/provisioner.hpp 
> 0a48617d6f9ade928993e1d5205de6486ef657c7 
>   src/slave/containerizer/mesos/provisioner/provisioner.cpp 
> 519028236305e9c8c1b6cded1919a5dd7ca3dbed 
>   src/slave/containerizer/mesos/provisioner/store.hpp 
> a312ad953b406aa75506051593dcc1c27cdc93af 
>   src/slave/containerizer/mesos/provisioner/store.cpp 
> 7141d63fcf2dbc3fbf00508c7f92945aab014fb2 
>   src/slave/flags.hpp 6ac0d45072157f6741b96266886a326e9337c153 
>   src/slave/flags.cpp d976fb80b9d1e634ce0ca8e8ad35aa64959a4853 
>   src/tests/containerizer/provisioner_appc_tests.cpp 
> 4d4ebba92ae66767903edc7a4f6edd9c6fee2489 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> d9472bb44cde999f95a8a65e6eee13f1f0fc09ed 
> 
> Diff: https://reviews.apache.org/r/50871/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> sudo ./bin/mesos-tests.sh
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 55713: Added linux helper function fs::type() for filesystem id detection.

[Gilbert Song]

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

(Updated Jan. 19, 2017, 3:33 p.m.)


Review request for mesos, Avinash sridharan, Jie Yu, James Peach, Timothy Chen, 
and Jiang Yan Xu.


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


Repository: mesos


Description
---

Get filesystem type id by using statfs() from .


Diffs
-

  src/linux/fs.hpp da49c9ebfa938d169152ed3b6e4df7378711b013 
  src/linux/fs.cpp 913e23317291db164fe6bdf77f3eca146dedec9b 
  src/tests/containerizer/fs_tests.cpp ccdd5967d01abfe4fa7746e3588ff93c6e0fe7e4 

Diff: https://reviews.apache.org/r/55713/diff/


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 55714: Added fs::magic() mapping function for fs id to fs type mapping.

[Gilbert Song]

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

(Updated Jan. 19, 2017, 3:33 p.m.)


Review request for mesos, Avinash sridharan, Jie Yu, James Peach, Timothy Chen, 
and Jiang Yan Xu.


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


Repository: mesos


Description
---

A simple uint32_t -> string mapping.


Diffs
-

  src/linux/fs.hpp da49c9ebfa938d169152ed3b6e4df7378711b013 
  src/linux/fs.cpp 913e23317291db164fe6bdf77f3eca146dedec9b 

Diff: https://reviews.apache.org/r/55714/diff/


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 55713: Added linux helper function fs::type() for filesystem id detection.

[James Peach]

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




src/linux/fs.hpp (line 161)


`fstype()` would be a bit more obvious to me, though this is consistent 
with Mesos style.



src/linux/fs.cpp (line 94)


In the past I've been told that the caller should add context to the error 
string, in which case this should just return `ErrnoError()` and the caller 
should be responsible for adding the path.



src/linux/fs.cpp (line 96)


Why are you truncating the return value? `__fsword_t` can be 8 bytes. Maybe 
it is worth defining a cleaner typedef for the fs type?

```
typedef fstype_t __fsword_t;
```


- James Peach


On Jan. 19, 2017, 6:56 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55713/
> ---
> 
> (Updated Jan. 19, 2017, 6:56 p.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Jie Yu, James Peach, Timothy 
> Chen, and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Get filesystem type id by using statfs() from .
> 
> 
> Diffs
> -
> 
>   src/linux/fs.hpp da49c9ebfa938d169152ed3b6e4df7378711b013 
>   src/linux/fs.cpp 913e23317291db164fe6bdf77f3eca146dedec9b 
>   src/tests/containerizer/fs_tests.cpp 
> ccdd5967d01abfe4fa7746e3588ff93c6e0fe7e4 
> 
> Diff: https://reviews.apache.org/r/55713/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 55464: Made the Agent API able to handle containers nested at arbitrary levels.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [55676, 55722, 55677, 55678, 55679, 55464]

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

- Mesos Reviewbot


On Jan. 19, 2017, 4:37 p.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55464/
> ---
> 
> (Updated Jan. 19, 2017, 4:37 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Anand Mazumdar, 
> Alexander Rojas, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: MESOS-6864
> https://issues.apache.org/jira/browse/MESOS-6864
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Made the Agent API able to handle containers nested at arbitrary levels.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp ee7119179a6ddd935c3f43a618ef645619c305ee 
>   src/slave/slave.cpp 205138add7a63289ff8ed81138f8b603828c748e 
>   src/tests/api_tests.cpp ea62226fbc5dd0bcc5ea60d1bbcd3748f9367467 
> 
> Diff: https://reviews.apache.org/r/55464/diff/
> 
> 
> Testing
> ---
> 
> `GTEST_FILTER="" make -j 8 check` in macOS.
> `make check` in Linux.
> 
> I also did some manual testing using a proof of concept  that makes the 
> `DefaultExecutor` leverage this change to perform CMD health checks.
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Re: Review Request 55714: Added fs::magic() mapping function for fs id to fs type mapping.

[James Peach]

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




src/linux/fs.hpp (line 152)


AFAICT `` doesn't contain these constants.



src/linux/fs.hpp (line 153)


Why do we need the `ifndef` here? On my systems there is no existing header 
file that might define these constants, so there should be no need to guard 
against that.



src/linux/fs.hpp (line 154)


I think these comments are superfluous since the constants are obvioud 
given the naming. Suggest that we remove these comments, which also removes the 
need for `NOLINT`.



src/linux/fs.hpp (line 244)


I get the analogy that leads to `magic`, but I'd prefer that it was named 
something more obvious. Maybe `typeName()`?



src/linux/fs.cpp (line 100)


Does Mesos typically pass `uint32_t` by const reference? Personally, I'd 
pass this by value.



src/linux/fs.cpp (line 103)


Can you make this `static const`?


- James Peach


On Jan. 19, 2017, 6:56 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55714/
> ---
> 
> (Updated Jan. 19, 2017, 6:56 p.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Jie Yu, James Peach, Timothy 
> Chen, and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> A simple uint32_t -> string mapping.
> 
> 
> Diffs
> -
> 
>   src/linux/fs.hpp da49c9ebfa938d169152ed3b6e4df7378711b013 
>   src/linux/fs.cpp 913e23317291db164fe6bdf77f3eca146dedec9b 
> 
> Diff: https://reviews.apache.org/r/55714/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Review Request 55732: Added `--3way` option to `git apply`.

[Zhitao Li]

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

Review request for mesos, Jason Lai, Joseph Wu, and Michael Park.


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


Repository: mesos


Description
---

This can help on some patches which includes conflicts that can be
resolved by 3 way merge.


Diffs
-

  support/apply-reviews.py c77b4c2b2d7a3d5e74e225403e71a84e23a9a1e7 

Diff: https://reviews.apache.org/r/55732/diff/


Testing
---

With this fix, I was able to apply the chain in r/52534, which has a conflict 
otherwise.

Bash log:
```
$ python ./support/apply-reviews.py -r 52534 -c
2017-01-19 14:45:26 URL:https://reviews.apache.org/r/51027/diff/raw/ 
[9830/9830] -> "51027.patch" [1]
[3way fba53e2] Track allocation candidates to bound allocator.
 Author: Jacob Janco 
 2 files changed, 97 insertions(+), 54 deletions(-)
2017-01-19 14:45:28 URL:https://reviews.apache.org/r/52534/diff/raw/ 
[2950/2950] -> "52534.patch" [1]
error: patch failed: src/master/allocator/mesos/hierarchical.hpp:224
Falling back to three-way merge...
Applied patch to 'src/master/allocator/mesos/hierarchical.hpp' cleanly.
[3way 261018a] Dispatch filter expiration twice.
 Author: Jacob Janco 
 2 files changed, 32 insertions(+), 8 deletions(-)
 ```


Thanks,

Zhitao Li

Re: Review Request 55381: Added test for framework upgrading to multi-role capability.

[Benjamin Mahler]

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


Fix it, then Ship it!




Thanks, looks good as far as validation is concerned. Seems we'll need a more 
comprehensive upgrade test to ensure a framework with running tasks can do the 
upgrade and continue to function normally, as well as launch new tasks in the 
new model.

Just some minor comments below, I'll make the updates and get this committed.


src/tests/master_validation_tests.cpp (lines 2589 - 2590)


Both empty would signify a change of roles since an empty `roles` indicates 
no roles rather than the default role of `"*"`.

This test appears to be covering the case where they are both set to the 
same non-default value (so I'll update the comment), do you want to add another 
test that covers the `"*`" case or is the implementation agnostic to this case?



src/tests/master_validation_tests.cpp (line 2602)


How about using Duration here? e.g.

```
  frameworkInfo.set_failover_timeout(Weeks(1).secs());
```



src/tests/master_validation_tests.cpp (lines 2618 - 2619)


How about a `driver.stop(failover=true); driver.join()` here to be more 
explicit?



src/tests/master_validation_tests.cpp (lines 2642 - 2643)


How about a stop and join here to be consistent with other tests that 
register a scheduler driver above?


- Benjamin Mahler


On Jan. 18, 2017, 3:26 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55381/
> ---
> 
> (Updated Jan. 18, 2017, 3:26 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Jay Guo, and Guangya Liu.
> 
> 
> Bugs: MESOS-6900
> https://issues.apache.org/jira/browse/MESOS-6900
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/tests/master_validation_tests.cpp 
> c092362152e1fe8a6b615c2eda171d852c1bbd86 
> 
> Diff: https://reviews.apache.org/r/55381/diff/
> 
> 
> Testing
> ---
> 
> make check (OS X)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 55691: Fix XSS vulnerability in pailer invocation.

[Jacob Janco]

> On Jan. 19, 2017, 4:27 p.m., haosdent huang wrote:
> > Hi, seems set `document.cookie` could work instead of use localstorage. The 
> > problem of localstorage is not supported some old browsers. Have you try 
> > set cookie before?

I think the attack vector would be similar if we were to pass the url through a 
cookie. localStorage is supported by: 

   
Feature Chrome  Firefox (Gecko) Internet Explorer   Opera   Safari 
(WebKit)
localStorage4   3.5 8   10.50   4
sessionStorage  5   2   8   10.50   4

I think this is fairly good coverage especially considering Microsoft's end of 
support for legacy browsers. If it becomes an issue we can definitely rethink 
this.


> On Jan. 19, 2017, 4:27 p.m., haosdent huang wrote:
> > src/webui/master/static/pailer.html, lines 46-68
> > 
> >
> > I think we remove this snippet?

This block of code keeps localStorage clean and sets the sessionStorage for the 
life of the open pailer window, so we need to persist the value through reloads.


> On Jan. 19, 2017, 4:27 p.m., haosdent huang wrote:
> > src/webui/master/static/pailer.html, line 80
> > 
> >
> > I think we could `localStorage.getItem/removeItem` above and use it 
> > here directly?

storageKey is scoped above this to keep it out of the global namespace


- Jacob


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


On Jan. 18, 2017, 11:40 p.m., Jacob Janco wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55691/
> ---
> 
> (Updated Jan. 18, 2017, 11:40 p.m.)
> 
> 
> Review request for mesos, haosdent huang and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6947
> https://issues.apache.org/jira/browse/MESOS-6947
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix XSS vulnerability in pailer invocation.
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/js/controllers.js 
> 388ca2447716cbc7141da6a20daf2340621a16e8 
>   src/webui/master/static/pailer.html 
> 19e0981143bd7e8372b49f4f036867e9dd05727a 
> 
> Diff: https://reviews.apache.org/r/55691/diff/
> 
> 
> Testing
> ---
> 
> make -j8 + test framework + checking pailer representation of files in sandbox
> 
> 
> Thanks,
> 
> Jacob Janco
> 
>

Review Request 55727: Checkpoint and track docker image layer sizes.

[Zhitao Li]

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

Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.


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


Repository: mesos


Description
---

This patch added the following capabilities:
1. add the size and fetch timestamp of each docker image layer in metadata;
2. checkpoint and recover size (timestamp will be added later);
3. reuse the `DiskUsageCollector` class to track size of each downloaded store;
4. for upgrade path, reuse same collector to back fill the size of layers.

Tests to be added later. Sending out review for earlier feedback.


Diffs
-

  src/slave/containerizer/mesos/provisioner/docker/message.proto 
c93c7a92ec152bd9747a70392adfe6a0e863e839 
  src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp 
954da1681778878c8aff6150002e52ecb648d1bb 
  src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp 
395c36b0f6b8f8e037681ec5f2df99e83a7cf155 
  src/slave/containerizer/mesos/provisioner/docker/store.cpp 
9dccd0673dbc0c61abfd4b88097f86e7d7131c46 

Diff: https://reviews.apache.org/r/55727/diff/


Testing
---


Thanks,

Zhitao Li

Re: Review Request 50871: Supported auto backend in Unified Containerizer.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [54216, 54212, 54213, 54214, 54816, 54215, 55713, 55714, 50871]

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

- Mesos Reviewbot


On Jan. 19, 2017, 6:45 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50871/
> ---
> 
> (Updated Jan. 19, 2017, 6:45 p.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Artem Harutyunyan, Jie Yu, and 
> Timothy Chen.
> 
> 
> Bugs: MESOS-5931
> https://issues.apache.org/jira/browse/MESOS-5931
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Select a backend smartly. Currently, the logic is really
> simple:
> 1. Use overlayfs backend if it exists.
> 2. Use aufs backend if the overlayfs does not exists.
> 3. Use copy backend of both overlayfs and aufs do not exist.
> Please note that the bind backend needs to be specified explicitly
> through the agent flag '--image_provisioner_backend' since it
> requires the sandbox already existed.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md 2113d06f58dddc0a28ae1241a24096266fe39801 
>   src/slave/containerizer/mesos/provisioner/appc/store.hpp 
> 26cdd2fba142874ab1d3eca61222bb22bfbb2e13 
>   src/slave/containerizer/mesos/provisioner/appc/store.cpp 
> e63ae419e24212b887edddeb5cae114cd39b39c8 
>   src/slave/containerizer/mesos/provisioner/docker/local_puller.hpp 
> abb8e7e48422896f207a475661ced0530fc75e68 
>   src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp 
> 817e30c5d6d6a4b011193e3209301fc3cdf88b06 
>   src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp 
> 954da1681778878c8aff6150002e52ecb648d1bb 
>   src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp 
> 395c36b0f6b8f8e037681ec5f2df99e83a7cf155 
>   src/slave/containerizer/mesos/provisioner/docker/paths.hpp 
> 232c027f8f96da0cb30b957bce4607d3695050d2 
>   src/slave/containerizer/mesos/provisioner/docker/paths.cpp 
> cd684b33eb308ce1eeb4539a5b2d51985d835db7 
>   src/slave/containerizer/mesos/provisioner/docker/puller.hpp 
> 191f3fc8cd1e0f706b5b1e6afddf856e8d53ae6e 
>   src/slave/containerizer/mesos/provisioner/docker/puller.cpp 
> ac9dae8ecbb897b8ff942d11ac70281a63e06831 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.hpp 
> bbd6005317bed3fff3d86e2527ca3ead839d49e3 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
> cecf34a23329a64fdbce7de4b83827a30975e9a4 
>   src/slave/containerizer/mesos/provisioner/docker/store.hpp 
> 1c2b149823e83dc5a3feb0af599d651d1dc05682 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> 9dccd0673dbc0c61abfd4b88097f86e7d7131c46 
>   src/slave/containerizer/mesos/provisioner/provisioner.hpp 
> 0a48617d6f9ade928993e1d5205de6486ef657c7 
>   src/slave/containerizer/mesos/provisioner/provisioner.cpp 
> 519028236305e9c8c1b6cded1919a5dd7ca3dbed 
>   src/slave/containerizer/mesos/provisioner/store.hpp 
> a312ad953b406aa75506051593dcc1c27cdc93af 
>   src/slave/containerizer/mesos/provisioner/store.cpp 
> 7141d63fcf2dbc3fbf00508c7f92945aab014fb2 
>   src/slave/flags.hpp 6ac0d45072157f6741b96266886a326e9337c153 
>   src/slave/flags.cpp d976fb80b9d1e634ce0ca8e8ad35aa64959a4853 
>   src/tests/containerizer/provisioner_appc_tests.cpp 
> 4d4ebba92ae66767903edc7a4f6edd9c6fee2489 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> d9472bb44cde999f95a8a65e6eee13f1f0fc09ed 
> 
> Diff: https://reviews.apache.org/r/50871/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> sudo ./bin/mesos-tests.sh
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 50871: Supported auto backend in Unified Containerizer.

[Gilbert Song]

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

(Updated Jan. 19, 2017, 10:45 a.m.)


Review request for mesos, Avinash sridharan, Artem Harutyunyan, Jie Yu, and 
Timothy Chen.


Changes
---

Fixed `#ifdef __linux__`


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


Repository: mesos


Description
---

Select a backend smartly. Currently, the logic is really
simple:
1. Use overlayfs backend if it exists.
2. Use aufs backend if the overlayfs does not exists.
3. Use copy backend of both overlayfs and aufs do not exist.
Please note that the bind backend needs to be specified explicitly
through the agent flag '--image_provisioner_backend' since it
requires the sandbox already existed.


Diffs (updated)
-

  docs/configuration.md 2113d06f58dddc0a28ae1241a24096266fe39801 
  src/slave/containerizer/mesos/provisioner/appc/store.hpp 
26cdd2fba142874ab1d3eca61222bb22bfbb2e13 
  src/slave/containerizer/mesos/provisioner/appc/store.cpp 
e63ae419e24212b887edddeb5cae114cd39b39c8 
  src/slave/containerizer/mesos/provisioner/docker/local_puller.hpp 
abb8e7e48422896f207a475661ced0530fc75e68 
  src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp 
817e30c5d6d6a4b011193e3209301fc3cdf88b06 
  src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp 
954da1681778878c8aff6150002e52ecb648d1bb 
  src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp 
395c36b0f6b8f8e037681ec5f2df99e83a7cf155 
  src/slave/containerizer/mesos/provisioner/docker/paths.hpp 
232c027f8f96da0cb30b957bce4607d3695050d2 
  src/slave/containerizer/mesos/provisioner/docker/paths.cpp 
cd684b33eb308ce1eeb4539a5b2d51985d835db7 
  src/slave/containerizer/mesos/provisioner/docker/puller.hpp 
191f3fc8cd1e0f706b5b1e6afddf856e8d53ae6e 
  src/slave/containerizer/mesos/provisioner/docker/puller.cpp 
ac9dae8ecbb897b8ff942d11ac70281a63e06831 
  src/slave/containerizer/mesos/provisioner/docker/registry_puller.hpp 
bbd6005317bed3fff3d86e2527ca3ead839d49e3 
  src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
cecf34a23329a64fdbce7de4b83827a30975e9a4 
  src/slave/containerizer/mesos/provisioner/docker/store.hpp 
1c2b149823e83dc5a3feb0af599d651d1dc05682 
  src/slave/containerizer/mesos/provisioner/docker/store.cpp 
9dccd0673dbc0c61abfd4b88097f86e7d7131c46 
  src/slave/containerizer/mesos/provisioner/provisioner.hpp 
0a48617d6f9ade928993e1d5205de6486ef657c7 
  src/slave/containerizer/mesos/provisioner/provisioner.cpp 
519028236305e9c8c1b6cded1919a5dd7ca3dbed 
  src/slave/containerizer/mesos/provisioner/store.hpp 
a312ad953b406aa75506051593dcc1c27cdc93af 
  src/slave/containerizer/mesos/provisioner/store.cpp 
7141d63fcf2dbc3fbf00508c7f92945aab014fb2 
  src/slave/flags.hpp 6ac0d45072157f6741b96266886a326e9337c153 
  src/slave/flags.cpp d976fb80b9d1e634ce0ca8e8ad35aa64959a4853 
  src/tests/containerizer/provisioner_appc_tests.cpp 
4d4ebba92ae66767903edc7a4f6edd9c6fee2489 
  src/tests/containerizer/provisioner_docker_tests.cpp 
d9472bb44cde999f95a8a65e6eee13f1f0fc09ed 

Diff: https://reviews.apache.org/r/50871/diff/


Testing
---

make check

sudo ./bin/mesos-tests.sh


Thanks,

Gilbert Song

Re: Review Request 55458: Added validation for a general check.

[Alexander Rukletsov]

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

(Updated Jan. 19, 2017, 5:46 p.m.)


Review request for mesos, Gastón Kleiman, haosdent huang, and Vinod Kone.


Changes
---

Rebased.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/CMakeLists.txt c06baf3a926687c7c6161025ae4a534dbae637e4 
  src/Makefile.am 3d0e82cb522d379923e00e5e98e9c9f646fdd3db 
  src/checks/checker.hpp PRE-CREATION 
  src/checks/checker.cpp PRE-CREATION 
  src/master/validation.hpp 78b70a78536cd850cf56b832b575091b2e115284 
  src/master/validation.cpp 5f134b781901f2de6a90fa6a10d42cc7629c27a0 

Diff: https://reviews.apache.org/r/55458/diff/


Testing
---

See https://reviews.apache.org/r/55459/


Thanks,

Alexander Rukletsov

Re: Review Request 55457: Added protobufs for a general check.

[Alexander Rukletsov]

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

(Updated Jan. 19, 2017, 5:42 p.m.)


Review request for mesos, Gastón Kleiman, haosdent huang, and Vinod Kone.


Changes
---

Vinod's comments.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  include/mesos/mesos.proto 8f1d6957a97eff1e0a94817d38e7a7de6d69 
  include/mesos/type_utils.hpp 7824407a6bbfc06edeec69479ed82cabae34a440 
  include/mesos/v1/mesos.proto 74e7851b147ab821dceeab6e838d34b092f101c3 
  src/common/type_utils.cpp 8270f808277d84a73823992c639d6536a99353b8 

Diff: https://reviews.apache.org/r/55457/diff/


Testing
---

See https://reviews.apache.org/r/55459/


Thanks,

Alexander Rukletsov

Re: Review Request 55456: Fixed include order in "launcher/executor.cpp".

[Alexander Rukletsov]

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

(Updated Jan. 19, 2017, 5:41 p.m.)


Review request for mesos, Gastón Kleiman, haosdent huang, and Vinod Kone.


Changes
---

Rebased.


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/launcher/executor.cpp e035a4ee4438a3342f68a5548a5fd8d57a315cfa 

Diff: https://reviews.apache.org/r/55456/diff/


Testing
---

None: not a functional change


Thanks,

Alexander Rukletsov

Re: Review Request 55455: Moved `HealthChecker` into "checks" folder and namespace.

[Alexander Rukletsov]

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

(Updated Jan. 19, 2017, 5:40 p.m.)


Review request for mesos, Gastón Kleiman, haosdent huang, and Vinod Kone.


Changes
---

Rebased.


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  docs/health-checks.md 890e2983c2269d4b5fdbf70d9d19bec2b63cb2f1 
  src/CMakeLists.txt c06baf3a926687c7c6161025ae4a534dbae637e4 
  src/Makefile.am 3d0e82cb522d379923e00e5e98e9c9f646fdd3db 
  src/docker/executor.cpp 9b5c469e2d0f33e228ec746711e6bc6ed352cbc7 
  src/health-check/CMakeLists.txt  
  src/health-check/health_checker.hpp 59831b12b99b87c15a51a7aa6c43b23be2c42a26 
  src/health-check/health_checker.cpp a8424b75927d15dc1b897faf0e47cf075c70ff26 
  src/health-check/tcp_connect.cpp  
  src/launcher/default_executor.cpp 57e4799e750f8f5352a9fec58af40efe432ea865 
  src/launcher/executor.cpp e035a4ee4438a3342f68a5548a5fd8d57a315cfa 
  src/master/validation.cpp 5f134b781901f2de6a90fa6a10d42cc7629c27a0 
  src/tests/health_check_tests.cpp 0a6d2dd295408dcc0434f3573e307e685f9abfe4 

Diff: https://reviews.apache.org/r/55455/diff/


Testing
---

make check


Thanks,

Alexander Rukletsov

Re: Review Request 55454: Ensured zero health check timeout means infinite timeout.

[Alexander Rukletsov]

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

(Updated Jan. 19, 2017, 5:39 p.m.)


Review request for mesos, Gastón Kleiman, haosdent huang, and Vinod Kone.


Changes
---

Rebased.


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


Repository: mesos


Description
---

Prior to this patch, zero health check timeout was interpreted
literally, which is not very helpful since a health check did not
even get a chance to finish. This patch fixes this behaviour by
interpreting zero as `Duration::max()` effectively rendering the
timeout infinite.


Diffs (updated)
-

  include/mesos/mesos.proto 8f1d6957a97eff1e0a94817d38e7a7de6d69 
  include/mesos/v1/mesos.proto 74e7851b147ab821dceeab6e838d34b092f101c3 
  src/health-check/health_checker.cpp a8424b75927d15dc1b897faf0e47cf075c70ff26 

Diff: https://reviews.apache.org/r/55454/diff/


Testing
---

make check


Thanks,

Alexander Rukletsov

Re: Review Request 55453: Updated comments in `HealthCheck` protobuf for clarity.

[Alexander Rukletsov]

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

(Updated Jan. 19, 2017, 5:39 p.m.)


Review request for mesos, Gastón Kleiman, haosdent huang, and Vinod Kone.


Changes
---

Vinod's comments.


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  include/mesos/mesos.proto 8f1d6957a97eff1e0a94817d38e7a7de6d69 
  include/mesos/v1/mesos.proto 74e7851b147ab821dceeab6e838d34b092f101c3 

Diff: https://reviews.apache.org/r/55453/diff/


Testing
---

None: not a functional change.


Thanks,

Alexander Rukletsov

Re: Review Request 55458: Added validation for a general check.

[Alexander Rukletsov]

> On Jan. 18, 2017, 11:33 a.m., Vinod Kone wrote:
> > src/checks/checker.cpp, line 45
> > 
> >
> > don't you want to validate that `check.command().has_command()` ?

Probably not, this is a required field.


- Alexander


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


On Jan. 18, 2017, 9:31 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55458/
> ---
> 
> (Updated Jan. 18, 2017, 9:31 a.m.)
> 
> 
> Review request for mesos, Gastón Kleiman, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: MESOS-6906
> https://issues.apache.org/jira/browse/MESOS-6906
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt c06baf3a926687c7c6161025ae4a534dbae637e4 
>   src/Makefile.am 3d0e82cb522d379923e00e5e98e9c9f646fdd3db 
>   src/checks/checker.hpp PRE-CREATION 
>   src/checks/checker.cpp PRE-CREATION 
>   src/master/validation.hpp 78b70a78536cd850cf56b832b575091b2e115284 
>   src/master/validation.cpp 5f134b781901f2de6a90fa6a10d42cc7629c27a0 
> 
> Diff: https://reviews.apache.org/r/55458/diff/
> 
> 
> Testing
> ---
> 
> See https://reviews.apache.org/r/55459/
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 55458: Added validation for a general check.

[Alexander Rukletsov]

> On Jan. 18, 2017, 11:33 a.m., Vinod Kone wrote:
> > src/checks/checker.hpp, line 31
> > 
> >
> > This should be
> > 
> > ```
> > Option validate(const CheckInfo& check);
> > ```

I don't think so, since it lives in `validation` namespace. This is also what 
we do for health checks.


- Alexander


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


On Jan. 18, 2017, 9:31 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55458/
> ---
> 
> (Updated Jan. 18, 2017, 9:31 a.m.)
> 
> 
> Review request for mesos, Gastón Kleiman, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: MESOS-6906
> https://issues.apache.org/jira/browse/MESOS-6906
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt c06baf3a926687c7c6161025ae4a534dbae637e4 
>   src/Makefile.am 3d0e82cb522d379923e00e5e98e9c9f646fdd3db 
>   src/checks/checker.hpp PRE-CREATION 
>   src/checks/checker.cpp PRE-CREATION 
>   src/master/validation.hpp 78b70a78536cd850cf56b832b575091b2e115284 
>   src/master/validation.cpp 5f134b781901f2de6a90fa6a10d42cc7629c27a0 
> 
> Diff: https://reviews.apache.org/r/55458/diff/
> 
> 
> Testing
> ---
> 
> See https://reviews.apache.org/r/55459/
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 55474: Renamed `taskTerminated` for Slave/Framework to `recoverResources`.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On Jan. 12, 2017, 10:52 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55474/
> ---
> 
> (Updated Jan. 12, 2017, 10:52 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-6619
> https://issues.apache.org/jira/browse/MESOS-6619
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The old name was misleading: these functions are invoked when a task
> becomes unreachable, which does not count as "task termination".
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp 368ee1d5e97784fa54e0f141906405ee8f104317 
>   src/master/master.cpp 1746a88953dbdc148d98881bcf7027b62ad6b040 
> 
> Diff: https://reviews.apache.org/r/55474/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 55472: Moved `Slave` definitions out-of-line to master.cpp.

[Neil Conway]

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

(Updated Jan. 19, 2017, 4:57 p.m.)


Review request for mesos and Vinod Kone.


Changes
---

Whitespace fixes.


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


Repository: mesos


Description
---

Previously, one of the `Slave` member functions was defined out-of-line,
but the rest were defined inline; make them all defined out-of-line for
consistency, and also to allow the function implementations to access
members of `Master` in the future.


Diffs (updated)
-

  src/master/master.hpp 8e8a9037af11cf95961b6498540a0fd486ed091b 
  src/master/master.cpp 73159328ce3fd838e02eba0e6a30cf69efc319ba 

Diff: https://reviews.apache.org/r/55472/diff/


Testing
---

`make check`


Thanks,

Neil Conway

Re: Review Request 55473: Marked a member function `const`.

[Neil Conway]

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

(Updated Jan. 19, 2017, 4:57 p.m.)


Review request for mesos and Vinod Kone.


Changes
---

Whitespace fix.


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


Repository: mesos


Description
---

Marked a member function `const`.


Diffs (updated)
-

  src/master/master.hpp 8e8a9037af11cf95961b6498540a0fd486ed091b 
  src/master/master.cpp 73159328ce3fd838e02eba0e6a30cf69efc319ba 

Diff: https://reviews.apache.org/r/55473/diff/


Testing
---

`make check`


Thanks,

Neil Conway

Re: Review Request 55472: Moved `Slave` definitions out-of-line to master.cpp.

[Neil Conway]

> On Jan. 19, 2017, 4:48 p.m., Vinod Kone wrote:
> > src/master/master.cpp, lines 8874-8876
> > 
> >
> > does this not fit in one line?
> > 
> > i know in the headers we sometimes put them on different lines for 
> > consistency with closeby methods, but we don't do that in cpp files.

This does not fit on one line (91 characters wide). I put it on two lines 
instead of three.


> On Jan. 19, 2017, 4:48 p.m., Vinod Kone wrote:
> > src/master/master.cpp, lines 8883-8885
> > 
> >
> > one line?

Does not fit on one line (89 chars), but I but it on two lines instead of three.


- Neil


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


On Jan. 18, 2017, 5 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55472/
> ---
> 
> (Updated Jan. 18, 2017, 5 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-6619
> https://issues.apache.org/jira/browse/MESOS-6619
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, one of the `Slave` member functions was defined out-of-line,
> but the rest were defined inline; make them all defined out-of-line for
> consistency, and also to allow the function implementations to access
> members of `Master` in the future.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp 8e8a9037af11cf95961b6498540a0fd486ed091b 
>   src/master/master.cpp 73159328ce3fd838e02eba0e6a30cf69efc319ba 
> 
> Diff: https://reviews.apache.org/r/55472/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 55473: Marked a member function `const`.

[Vinod Kone]

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


Fix it, then Ship it!





src/master/master.cpp (lines 8758 - 8760)


keep it on one line.


- Vinod Kone


On Jan. 12, 2017, 10:52 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55473/
> ---
> 
> (Updated Jan. 12, 2017, 10:52 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-6619
> https://issues.apache.org/jira/browse/MESOS-6619
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Marked a member function `const`.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp 368ee1d5e97784fa54e0f141906405ee8f104317 
>   src/master/master.cpp 1746a88953dbdc148d98881bcf7027b62ad6b040 
> 
> Diff: https://reviews.apache.org/r/55473/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 55472: Moved `Slave` definitions out-of-line to master.cpp.

[Vinod Kone]

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




src/master/master.cpp (lines 8871 - 8873)


does this not fit in one line?

i know in the headers we sometimes put them on different lines for 
consistency with closeby methods, but we don't do that in cpp files.



src/master/master.cpp (lines 8880 - 8882)


one line?



src/master/master.cpp (line 8891)


2 lines.


- Vinod Kone


On Jan. 18, 2017, 5 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55472/
> ---
> 
> (Updated Jan. 18, 2017, 5 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-6619
> https://issues.apache.org/jira/browse/MESOS-6619
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, one of the `Slave` member functions was defined out-of-line,
> but the rest were defined inline; make them all defined out-of-line for
> consistency, and also to allow the function implementations to access
> members of `Master` in the future.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp 8e8a9037af11cf95961b6498540a0fd486ed091b 
>   src/master/master.cpp 73159328ce3fd838e02eba0e6a30cf69efc319ba 
> 
> Diff: https://reviews.apache.org/r/55472/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 55722: Removed redundant `Times(1)` statements from `api_tests.cpp`.

[Anand Mazumdar]

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


Ship it!




Ship It!

- Anand Mazumdar


On Jan. 19, 2017, 4:18 p.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55722/
> ---
> 
> (Updated Jan. 19, 2017, 4:18 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Anand Mazumdar, 
> Alexander Rojas, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: MESOS-6864
> https://issues.apache.org/jira/browse/MESOS-6864
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed redundant `Times(1)` statements from `api_tests.cpp`.
> 
> 
> Diffs
> -
> 
>   src/tests/api_tests.cpp ea62226fbc5dd0bcc5ea60d1bbcd3748f9367467 
> 
> Diff: https://reviews.apache.org/r/55722/diff/
> 
> 
> Testing
> ---
> 
> `make check` in Linux
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Re: Review Request 55472: Moved `Slave` definitions out-of-line to master.cpp.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On Jan. 18, 2017, 5 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55472/
> ---
> 
> (Updated Jan. 18, 2017, 5 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-6619
> https://issues.apache.org/jira/browse/MESOS-6619
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, one of the `Slave` member functions was defined out-of-line,
> but the rest were defined inline; make them all defined out-of-line for
> consistency, and also to allow the function implementations to access
> members of `Master` in the future.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp 8e8a9037af11cf95961b6498540a0fd486ed091b 
>   src/master/master.cpp 73159328ce3fd838e02eba0e6a30cf69efc319ba 
> 
> Diff: https://reviews.apache.org/r/55472/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 55464: Made the Agent API able to handle containers nested at arbitrary levels.

[Gastón Kleiman]

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

(Updated Jan. 19, 2017, 4:37 p.m.)


Review request for mesos, Adam B, Alexander Rukletsov, Anand Mazumdar, 
Alexander Rojas, haosdent huang, and Vinod Kone.


Changes
---

Addressed feedback.


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


Repository: mesos


Description
---

Made the Agent API able to handle containers nested at arbitrary levels.


Diffs (updated)
-

  src/slave/http.cpp ee7119179a6ddd935c3f43a618ef645619c305ee 
  src/slave/slave.cpp 205138add7a63289ff8ed81138f8b603828c748e 
  src/tests/api_tests.cpp ea62226fbc5dd0bcc5ea60d1bbcd3748f9367467 

Diff: https://reviews.apache.org/r/55464/diff/


Testing
---

`GTEST_FILTER="" make -j 8 check` in macOS.
`make check` in Linux.

I also did some manual testing using a proof of concept  that makes the 
`DefaultExecutor` leverage this change to perform CMD health checks.


Thanks,

Gastón Kleiman

Re: Review Request 55691: Fix XSS vulnerability in pailer invocation.

[haosdent huang]

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



Hi, seems set `document.cookie` could work instead of use localstorage. The 
problem of localstorage is not supported some old browsers. Have you try set 
cookie before?


src/webui/master/static/pailer.html (lines 46 - 68)


I think we remove this snippet?



src/webui/master/static/pailer.html (line 80)


I think we could `localStorage.getItem/removeItem` above and use it here 
directly?


- haosdent huang


On Jan. 18, 2017, 11:40 p.m., Jacob Janco wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55691/
> ---
> 
> (Updated Jan. 18, 2017, 11:40 p.m.)
> 
> 
> Review request for mesos, haosdent huang and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6947
> https://issues.apache.org/jira/browse/MESOS-6947
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix XSS vulnerability in pailer invocation.
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/js/controllers.js 
> 388ca2447716cbc7141da6a20daf2340621a16e8 
>   src/webui/master/static/pailer.html 
> 19e0981143bd7e8372b49f4f036867e9dd05727a 
> 
> Diff: https://reviews.apache.org/r/55691/diff/
> 
> 
> Testing
> ---
> 
> make -j8 + test framework + checking pailer representation of files in sandbox
> 
> 
> Thanks,
> 
> Jacob Janco
> 
>

Re: Review Request 54183: Improved management of unreachable and completed tasks in master.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On Jan. 18, 2017, 7:18 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54183/
> ---
> 
> (Updated Jan. 18, 2017, 7:18 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-6619
> https://issues.apache.org/jira/browse/MESOS-6619
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Before partition-awareness, when an agent failed health checks, the
> master removed the agent from the registry, marked all of its tasks
> TASK_LOST, and moved them to the `completedTasks` list in the master's
> memory. Although "lost" tasks might still be running, partitioned agents
> would only be allowed to re-register if the master failed over, in which
> case the `completedTasks` map would be emptied.
> 
> When partition-awareness was introduced, we initially followed the same
> scheme, with the only difference that partition-aware tasks are marked
> TASK_UNREACHABLE, not TASK_LOST.
> 
> This scheme has a few shortcomings. First, partition-aware tasks might
> resume running when the partitioned agent re-registers. Second, we
> re-added non-partition aware tasks when the agent re-registered but then
> marked them completed when the framework is shutdown, resulting in two
> entries in `completedTasks`.
> 
> This commit introduces a separate bounded map, `unreachableTasks`. These
> tasks are reported separately via the HTTP endpoints, because they have
> different semantics (unlike completed tasks, unreachable tasks can
> resume running). The size of this map is limited by a new master flag,
> `--max_unreachable_tasks_per_framework`. This commit also changes the
> master to omit re-adding non-partition-aware tasks on re-registering
> agents (unless the master has failed over): those tasks will shortly be
> shutdown anyway.
> 
> Finally, this commit fixes a minor bug in the previous code: the
> previous coding neglected to shutdown non-partition-aware frameworks
> running on pre-1.0 Mesos agents that re-register with the master after
> a network partition.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md 2113d06f58dddc0a28ae1241a24096266fe39801 
>   include/mesos/master/master.proto 03203c8ddd05876339b1c56789b7bb140499d49d 
>   include/mesos/v1/master/master.proto 
> f8edf39a68752c8601cece345f52bce8b9a8a68b 
>   src/master/constants.hpp 900b69460de9280f30b16b86925e436054e080ca 
>   src/master/flags.hpp 6a17b763dc76daa10073394f416b049e97a44238 
>   src/master/flags.cpp 124bfbca6be9d5d60597b9a1793bd807202da8ec 
>   src/master/http.cpp a44621f39cb059e654a56f57f75b38947f3a4230 
>   src/master/master.hpp 8e8a9037af11cf95961b6498540a0fd486ed091b 
>   src/master/master.cpp 73159328ce3fd838e02eba0e6a30cf69efc319ba 
>   src/tests/partition_tests.cpp 72013d1bfee275c6f3cb90173f0c408d55e0bc5d 
> 
> Diff: https://reviews.apache.org/r/54183/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 55677: Made `AttachContainerOutput/Input` tests use an existing container ID.

[Gastón Kleiman]

> On Jan. 19, 2017, 2:23 p.m., Vinod Kone wrote:
> > src/tests/api_tests.cpp, line 4266
> > 
> >
> > we don't do "Times(1)" since that is the default.
> > 
> > just do
> > 
> > ```
> > EXPECT_CALL(exec, registered(_, _, _, _));
> > 
> > ```

https://reviews.apache.org/r/55722/


- Gastón


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


On Jan. 19, 2017, 4:19 p.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55677/
> ---
> 
> (Updated Jan. 19, 2017, 4:19 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Anand Mazumdar, 
> Alexander Rojas, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: MESOS-6864
> https://issues.apache.org/jira/browse/MESOS-6864
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> These tests verify that the Agent returns a 500 if the containerizer
> doesn't support the `attach` call.
> 
> This chain refactors those methods and make them return a 404 instead of
> a 500 if the container can't be found, so we need to pass the ID of an
> existing container.
> 
> 
> Diffs
> -
> 
>   src/tests/api_tests.cpp ea62226fbc5dd0bcc5ea60d1bbcd3748f9367467 
> 
> Diff: https://reviews.apache.org/r/55677/diff/
> 
> 
> Testing
> ---
> 
> `make check` in macOS and Linux.
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Re: Review Request 55722: Removed redundant `Times(1)` statements from `api_tests.cpp`.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On Jan. 19, 2017, 4:18 p.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55722/
> ---
> 
> (Updated Jan. 19, 2017, 4:18 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Anand Mazumdar, 
> Alexander Rojas, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: MESOS-6864
> https://issues.apache.org/jira/browse/MESOS-6864
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed redundant `Times(1)` statements from `api_tests.cpp`.
> 
> 
> Diffs
> -
> 
>   src/tests/api_tests.cpp ea62226fbc5dd0bcc5ea60d1bbcd3748f9367467 
> 
> Diff: https://reviews.apache.org/r/55722/diff/
> 
> 
> Testing
> ---
> 
> `make check` in Linux
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Re: Review Request 55677: Made `AttachContainerOutput/Input` tests use an existing container ID.

[Gastón Kleiman]

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

(Updated Jan. 19, 2017, 4:19 p.m.)


Review request for mesos, Adam B, Alexander Rukletsov, Anand Mazumdar, 
Alexander Rojas, haosdent huang, and Vinod Kone.


Changes
---

Addressed feedback.


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


Repository: mesos


Description
---

These tests verify that the Agent returns a 500 if the containerizer
doesn't support the `attach` call.

This chain refactors those methods and make them return a 404 instead of
a 500 if the container can't be found, so we need to pass the ID of an
existing container.


Diffs (updated)
-

  src/tests/api_tests.cpp ea62226fbc5dd0bcc5ea60d1bbcd3748f9367467 

Diff: https://reviews.apache.org/r/55677/diff/


Testing
---

`make check` in macOS and Linux.


Thanks,

Gastón Kleiman

Review Request 55722: Removed redundant `Times(1)` statements from `api_tests.cpp`.

[Gastón Kleiman]

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

Review request for mesos, Adam B, Alexander Rukletsov, Anand Mazumdar, 
Alexander Rojas, haosdent huang, and Vinod Kone.


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


Repository: mesos


Description
---

Removed redundant `Times(1)` statements from `api_tests.cpp`.


Diffs
-

  src/tests/api_tests.cpp ea62226fbc5dd0bcc5ea60d1bbcd3748f9367467 

Diff: https://reviews.apache.org/r/55722/diff/


Testing
---

`make check` in Linux


Thanks,

Gastón Kleiman

Re: Review Request 55464: Made the Agent API able to handle containers nested at arbitrary levels.

[Anand Mazumdar]

> On Jan. 19, 2017, 3:20 p.m., Vinod Kone wrote:
> > src/tests/api_tests.cpp, line 3896
> > 
> >
> > Maybe also verify parent container is still running.
> > 
> > ```
> > EXPECT_TRUE(waitParent.isPending());
> > 
> > ```

The subsequent call to kill the parent container would fail if the parent 
container is not running/already destroyed (!= 200 OK response) on L3910.


- Anand


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


On Jan. 18, 2017, 2:48 p.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55464/
> ---
> 
> (Updated Jan. 18, 2017, 2:48 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Anand Mazumdar, 
> Alexander Rojas, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: MESOS-6864
> https://issues.apache.org/jira/browse/MESOS-6864
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Made the Agent API able to handle containers nested at arbitrary levels.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp ee7119179a6ddd935c3f43a618ef645619c305ee 
>   src/slave/slave.cpp ae183e6b8b40094b4764525a6d63164f210ef9d8 
>   src/tests/api_tests.cpp ea62226fbc5dd0bcc5ea60d1bbcd3748f9367467 
> 
> Diff: https://reviews.apache.org/r/55464/diff/
> 
> 
> Testing
> ---
> 
> `GTEST_FILTER="" make -j 8 check` in macOS.
> `make check` in Linux.
> 
> I also did some manual testing using a proof of concept  that makes the 
> `DefaultExecutor` leverage this change to perform CMD health checks.
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Re: Review Request 55464: Made the Agent API able to handle containers nested at arbitrary levels.

[Anand Mazumdar]

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


Fix it, then Ship it!




Modulo other comments from Vinod


src/slave/slave.cpp (line 6476)


s/ContainerID&/ContainerID


- Anand Mazumdar


On Jan. 18, 2017, 2:48 p.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55464/
> ---
> 
> (Updated Jan. 18, 2017, 2:48 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Anand Mazumdar, 
> Alexander Rojas, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: MESOS-6864
> https://issues.apache.org/jira/browse/MESOS-6864
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Made the Agent API able to handle containers nested at arbitrary levels.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp ee7119179a6ddd935c3f43a618ef645619c305ee 
>   src/slave/slave.cpp ae183e6b8b40094b4764525a6d63164f210ef9d8 
>   src/tests/api_tests.cpp ea62226fbc5dd0bcc5ea60d1bbcd3748f9367467 
> 
> Diff: https://reviews.apache.org/r/55464/diff/
> 
> 
> Testing
> ---
> 
> `GTEST_FILTER="" make -j 8 check` in macOS.
> `make check` in Linux.
> 
> I also did some manual testing using a proof of concept  that makes the 
> `DefaultExecutor` leverage this change to perform CMD health checks.
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Re: Review Request 55679: Unified the way in which the Slave API handlers perform AuthZ.

[Anand Mazumdar]

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


Ship it!




Ship It!

- Anand Mazumdar


On Jan. 19, 2017, 10:17 a.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55679/
> ---
> 
> (Updated Jan. 19, 2017, 10:17 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Anand Mazumdar, 
> Alexander Rojas, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: MESOS-6864
> https://issues.apache.org/jira/browse/MESOS-6864
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Unified the way in which the Slave API handlers perform AuthZ.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp ee7119179a6ddd935c3f43a618ef645619c305ee 
> 
> Diff: https://reviews.apache.org/r/55679/diff/
> 
> 
> Testing
> ---
> 
> `make check` in macOS and Linux.
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Re: Review Request 55678: Renamed `locateExecutor` to `getExecutor`.

[Anand Mazumdar]

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


Ship it!




Ship It!

- Anand Mazumdar


On Jan. 19, 2017, 10:17 a.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55678/
> ---
> 
> (Updated Jan. 19, 2017, 10:17 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Anand Mazumdar, 
> Alexander Rojas, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: MESOS-6864
> https://issues.apache.org/jira/browse/MESOS-6864
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Rnamed `locateExecutor` to `getExecutor` in `slave.cpp` to be consistent
> with other similar functions in the same file.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp ee7119179a6ddd935c3f43a618ef645619c305ee 
>   src/slave/slave.hpp 8c21ce95683b3d8f0f732e87cf3b3dbe7860f1da 
>   src/slave/slave.cpp 205138add7a63289ff8ed81138f8b603828c748e 
> 
> Diff: https://reviews.apache.org/r/55678/diff/
> 
> 
> Testing
> ---
> 
> `make check` in macOS and Linux.
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Re: Review Request 55464: Made the Agent API able to handle containers nested at arbitrary levels.

[Vinod Kone]

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


Fix it, then Ship it!





src/tests/api_tests.cpp (line 3644)


kill this.



src/tests/api_tests.cpp (line 3656)


kill this.



src/tests/api_tests.cpp (lines 3797 - 3799)


move this to #3819



src/tests/api_tests.cpp (line 3894)


Maybe also verify parent container is still running.

```
EXPECT_TRUE(waitParent.isPending());

```


- Vinod Kone


On Jan. 18, 2017, 2:48 p.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55464/
> ---
> 
> (Updated Jan. 18, 2017, 2:48 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Anand Mazumdar, 
> Alexander Rojas, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: MESOS-6864
> https://issues.apache.org/jira/browse/MESOS-6864
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Made the Agent API able to handle containers nested at arbitrary levels.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp ee7119179a6ddd935c3f43a618ef645619c305ee 
>   src/slave/slave.cpp ae183e6b8b40094b4764525a6d63164f210ef9d8 
>   src/tests/api_tests.cpp ea62226fbc5dd0bcc5ea60d1bbcd3748f9367467 
> 
> Diff: https://reviews.apache.org/r/55464/diff/
> 
> 
> Testing
> ---
> 
> `GTEST_FILTER="" make -j 8 check` in macOS.
> `make check` in Linux.
> 
> I also did some manual testing using a proof of concept  that makes the 
> `DefaultExecutor` leverage this change to perform CMD health checks.
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Re: Review Request 55677: Made `AttachContainerOutput/Input` tests use an existing container ID.

[Anand Mazumdar]

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


Fix it, then Ship it!




Modulo comments from Vinod.


src/tests/api_tests.cpp (line 4248)


Nit: Remove this? You don't need to ignore subsequent offers as there won't 
be any due to your task never reaching a terminal state before the driver was 
stopped.



src/tests/api_tests.cpp (line 4334)


Ditto as above.


- Anand Mazumdar


On Jan. 19, 2017, 10:17 a.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55677/
> ---
> 
> (Updated Jan. 19, 2017, 10:17 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Anand Mazumdar, 
> Alexander Rojas, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: MESOS-6864
> https://issues.apache.org/jira/browse/MESOS-6864
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> These tests verify that the Agent returns a 500 if the containerizer
> doesn't support the `attach` call.
> 
> This chain refactors those methods and make them return a 404 instead of
> a 500 if the container can't be found, so we need to pass the ID of an
> existing container.
> 
> 
> Diffs
> -
> 
>   src/tests/api_tests.cpp ea62226fbc5dd0bcc5ea60d1bbcd3748f9367467 
> 
> Diff: https://reviews.apache.org/r/55677/diff/
> 
> 
> Testing
> ---
> 
> `make check` in macOS and Linux.
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Re: Review Request 55676: Moved `getRootContainerId` to `protobuf_utils`.

[Anand Mazumdar]

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


Ship it!





src/slave/containerizer/mesos/containerizer.cpp (line 749)


Not yours, we don't capture temporaries by const reference as per our style 
guide. You might want to clean this up in a separate patch.


- Anand Mazumdar


On Jan. 19, 2017, 10:15 a.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55676/
> ---
> 
> (Updated Jan. 19, 2017, 10:15 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Anand Mazumdar, 
> Alexander Rojas, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: MESOS-6864
> https://issues.apache.org/jira/browse/MESOS-6864
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This method is pretty generic and should not be a part of the Mesos
> containerizer code.
> 
> 
> Diffs
> -
> 
>   src/common/protobuf_utils.hpp bca9ace6cb37047bb08466c4fb14028bbb818446 
>   src/common/protobuf_utils.cpp af3c2a660f91c366ca5ffe8671d10c4a270bbbcc 
>   src/slave/containerizer/composing.cpp 
> 4fb7ba5cadd36be00be61aa00a69c38c44649aee 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 8bf8a7774a38131c53f6d91c7f09f5dedd9d4cb4 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
> b3a269535517222a1d85d8be1fdf5b865d6a3fc6 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
> e1cac954848e618a03ddb82fd6d040ae1d948e82 
>   src/slave/containerizer/mesos/utils.hpp 
> a54106dc4893bb222f42ede936ac9029e817faf9 
>   src/slave/containerizer/mesos/utils.cpp 
> 4e2a01495909c07f320af416ff4dc59f7328c710 
> 
> Diff: https://reviews.apache.org/r/55676/diff/
> 
> 
> Testing
> ---
> 
> `make check` in macOS and Linux.
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Re: Review Request 55678: Renamed `locateExecutor` to `getExecutor`.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On Jan. 19, 2017, 10:17 a.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55678/
> ---
> 
> (Updated Jan. 19, 2017, 10:17 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Anand Mazumdar, 
> Alexander Rojas, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: MESOS-6864
> https://issues.apache.org/jira/browse/MESOS-6864
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Rnamed `locateExecutor` to `getExecutor` in `slave.cpp` to be consistent
> with other similar functions in the same file.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp ee7119179a6ddd935c3f43a618ef645619c305ee 
>   src/slave/slave.hpp 8c21ce95683b3d8f0f732e87cf3b3dbe7860f1da 
>   src/slave/slave.cpp 205138add7a63289ff8ed81138f8b603828c748e 
> 
> Diff: https://reviews.apache.org/r/55678/diff/
> 
> 
> Testing
> ---
> 
> `make check` in macOS and Linux.
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Re: Review Request 55677: Made `AttachContainerOutput/Input` tests use an existing container ID.

[Vinod Kone]

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


Fix it, then Ship it!





src/tests/api_tests.cpp (line 4256)


we don't do "Times(1)" since that is the default.

just do

```
EXPECT_CALL(exec, registered(_, _, _, _));

```



src/tests/api_tests.cpp (line 4342)


ditto.


- Vinod Kone


On Jan. 19, 2017, 10:17 a.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55677/
> ---
> 
> (Updated Jan. 19, 2017, 10:17 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Anand Mazumdar, 
> Alexander Rojas, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: MESOS-6864
> https://issues.apache.org/jira/browse/MESOS-6864
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> These tests verify that the Agent returns a 500 if the containerizer
> doesn't support the `attach` call.
> 
> This chain refactors those methods and make them return a 404 instead of
> a 500 if the container can't be found, so we need to pass the ID of an
> existing container.
> 
> 
> Diffs
> -
> 
>   src/tests/api_tests.cpp ea62226fbc5dd0bcc5ea60d1bbcd3748f9367467 
> 
> Diff: https://reviews.apache.org/r/55677/diff/
> 
> 
> Testing
> ---
> 
> `make check` in macOS and Linux.
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Re: Review Request 55679: Unified the way in which the Slave API handlers perform AuthZ.

[Alexander Rojas]

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


Ship it!




Ship It!

- Alexander Rojas


On Jan. 19, 2017, 11:17 a.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55679/
> ---
> 
> (Updated Jan. 19, 2017, 11:17 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Anand Mazumdar, 
> Alexander Rojas, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: MESOS-6864
> https://issues.apache.org/jira/browse/MESOS-6864
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Unified the way in which the Slave API handlers perform AuthZ.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp ee7119179a6ddd935c3f43a618ef645619c305ee 
> 
> Diff: https://reviews.apache.org/r/55679/diff/
> 
> 
> Testing
> ---
> 
> `make check` in macOS and Linux.
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Re: Review Request 55676: Moved `getRootContainerId` to `protobuf_utils`.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On Jan. 19, 2017, 10:15 a.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55676/
> ---
> 
> (Updated Jan. 19, 2017, 10:15 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Anand Mazumdar, 
> Alexander Rojas, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: MESOS-6864
> https://issues.apache.org/jira/browse/MESOS-6864
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This method is pretty generic and should not be a part of the Mesos
> containerizer code.
> 
> 
> Diffs
> -
> 
>   src/common/protobuf_utils.hpp bca9ace6cb37047bb08466c4fb14028bbb818446 
>   src/common/protobuf_utils.cpp af3c2a660f91c366ca5ffe8671d10c4a270bbbcc 
>   src/slave/containerizer/composing.cpp 
> 4fb7ba5cadd36be00be61aa00a69c38c44649aee 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 8bf8a7774a38131c53f6d91c7f09f5dedd9d4cb4 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
> b3a269535517222a1d85d8be1fdf5b865d6a3fc6 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
> e1cac954848e618a03ddb82fd6d040ae1d948e82 
>   src/slave/containerizer/mesos/utils.hpp 
> a54106dc4893bb222f42ede936ac9029e817faf9 
>   src/slave/containerizer/mesos/utils.cpp 
> 4e2a01495909c07f320af416ff4dc59f7328c710 
> 
> Diff: https://reviews.apache.org/r/55676/diff/
> 
> 
> Testing
> ---
> 
> `make check` in macOS and Linux.
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Re: Review Request 55701: Fixed unsafe usage of process pointer in async.hpp.

[Benjamin Bannier]

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


Ship it!




Looking through other instances of `spawn(.*,\ true)`, should this one also be 
adjusted, 
https://github.com/apache/mesos/blob/745b3c7589e5252cf93f62e081b78fa420771d0c/3rdparty/libprocess/include/process/loop.hpp#L134-L144?

- Benjamin Bannier


On Jan. 19, 2017, 3:41 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55701/
> ---
> 
> (Updated Jan. 19, 2017, 3:41 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Hindman, Benjamin 
> Mahler, and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The `async(...)` helper spawns a libprocess actor to execute some
> lambda (asynchronously, of course).  This actor is owned by the
> libprocess GC actor, but the `AsyncExecutor` stores a copy of that
> pointer and dereferences it in several possible locations.
> 
> Instead, the `AsyncExecutor` should store the `PID` of the actor,
> which can be safely used, even if the actor has already terminated;
> such as turning libprocess finalization.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/async.hpp 
> 8565a52c6ba4008edb852e898b8f0b6d598a194f 
> 
> Diff: https://reviews.apache.org/r/55701/diff/
> 
> 
> Testing
> ---
> 
> Applied the following change:
> ```
> diff --git a/src/launcher/executor.cpp b/src/launcher/executor.cpp
> index e035a4e..4c60ef9 100644
> --- a/src/launcher/executor.cpp
> +++ b/src/launcher/executor.cpp
> @@ -936,6 +930,8 @@ int main(int argc, char** argv)
>  
>process::spawn(executor.get());
>process::wait(executor.get());
> +  executor.reset();
>  
> +  process::finalize(true);
>return EXIT_SUCCESS;
>  }
> ```
> 
> And then ran:
> ```
> make check GTEST_FILTER="HTTPCommandExecutorTest.TerminateWithACK"
> ```
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 55678: Renamed `locateExecutor` to `getExecutor`.

[Alexander Rojas]

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


Ship it!




Ship It!

- Alexander Rojas


On Jan. 19, 2017, 11:17 a.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55678/
> ---
> 
> (Updated Jan. 19, 2017, 11:17 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Anand Mazumdar, 
> Alexander Rojas, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: MESOS-6864
> https://issues.apache.org/jira/browse/MESOS-6864
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Rnamed `locateExecutor` to `getExecutor` in `slave.cpp` to be consistent
> with other similar functions in the same file.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp ee7119179a6ddd935c3f43a618ef645619c305ee 
>   src/slave/slave.hpp 8c21ce95683b3d8f0f732e87cf3b3dbe7860f1da 
>   src/slave/slave.cpp 205138add7a63289ff8ed81138f8b603828c748e 
> 
> Diff: https://reviews.apache.org/r/55678/diff/
> 
> 
> Testing
> ---
> 
> `make check` in macOS and Linux.
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Re: Review Request 55701: Fixed unsafe usage of process pointer in async.hpp.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [55701]

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

- Mesos Reviewbot


On Jan. 19, 2017, 2:41 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55701/
> ---
> 
> (Updated Jan. 19, 2017, 2:41 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Hindman, Benjamin 
> Mahler, and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The `async(...)` helper spawns a libprocess actor to execute some
> lambda (asynchronously, of course).  This actor is owned by the
> libprocess GC actor, but the `AsyncExecutor` stores a copy of that
> pointer and dereferences it in several possible locations.
> 
> Instead, the `AsyncExecutor` should store the `PID` of the actor,
> which can be safely used, even if the actor has already terminated;
> such as turning libprocess finalization.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/async.hpp 
> 8565a52c6ba4008edb852e898b8f0b6d598a194f 
> 
> Diff: https://reviews.apache.org/r/55701/diff/
> 
> 
> Testing
> ---
> 
> Applied the following change:
> ```
> diff --git a/src/launcher/executor.cpp b/src/launcher/executor.cpp
> index e035a4e..4c60ef9 100644
> --- a/src/launcher/executor.cpp
> +++ b/src/launcher/executor.cpp
> @@ -936,6 +930,8 @@ int main(int argc, char** argv)
>  
>process::spawn(executor.get());
>process::wait(executor.get());
> +  executor.reset();
>  
> +  process::finalize(true);
>return EXIT_SUCCESS;
>  }
> ```
> 
> And then ran:
> ```
> make check GTEST_FILTER="HTTPCommandExecutorTest.TerminateWithACK"
> ```
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 55271: Disallow multi-role frameworks to change their roles.

[Benjamin Bannier]

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

(Updated Jan. 19, 2017, 2:01 p.m.)


Review request for mesos, Benjamin Mahler, Jay Guo, and Guangya Liu.


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


Repository: mesos


Description
---

We currently do not allow `MULTI_ROLE` frameworks to change their
roles. This restriction will be lifted later.


Diffs (updated)
-

  src/master/master.hpp 8e8a9037af11cf95961b6498540a0fd486ed091b 
  src/tests/master_validation_tests.cpp 
c092362152e1fe8a6b615c2eda171d852c1bbd86 

Diff: https://reviews.apache.org/r/55271/diff/


Testing
---

Tested on various Linux configurations in internal CI.


Thanks,

Benjamin Bannier

Re: Review Request 55677: Made `AttachContainerOutput/Input` tests use an existing container ID.

[Alexander Rojas]

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


Ship it!




Ship It!

- Alexander Rojas


On Jan. 19, 2017, 11:17 a.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55677/
> ---
> 
> (Updated Jan. 19, 2017, 11:17 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Anand Mazumdar, 
> Alexander Rojas, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: MESOS-6864
> https://issues.apache.org/jira/browse/MESOS-6864
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> These tests verify that the Agent returns a 500 if the containerizer
> doesn't support the `attach` call.
> 
> This chain refactors those methods and make them return a 404 instead of
> a 500 if the container can't be found, so we need to pass the ID of an
> existing container.
> 
> 
> Diffs
> -
> 
>   src/tests/api_tests.cpp ea62226fbc5dd0bcc5ea60d1bbcd3748f9367467 
> 
> Diff: https://reviews.apache.org/r/55677/diff/
> 
> 
> Testing
> ---
> 
> `make check` in macOS and Linux.
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Re: Review Request 55683: Rationalize process wait error checking.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [55683]

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

- Mesos Reviewbot


On Jan. 19, 2017, 1:11 a.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55683/
> ---
> 
> (Updated Jan. 19, 2017, 1:11 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Joris Van Remoortere, Michael 
> Park, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6946
> https://issues.apache.org/jira/browse/MESOS-6946
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Introduce the WSUCCEEDED() API to test that a forked process exited
> successfully. Use it in all the places that were performing this test
> bespokely, and update error logs to user WSTRINGIFY() if appropriate.
> 
> This also fixes called that were checking the exit status but not the
> signaled status.
> 
> 
> Diffs
> -
> 
>   src/common/status_utils.hpp 26b94d49973dd61a6138e6a64d5ec0bdec6dea7e 
>   src/docker/executor.cpp 9b5c469e2d0f33e228ec746711e6bc6ed352cbc7 
>   src/hdfs/hdfs.cpp bebba8bb3c16f40293d21b5e00c353135a0f8f26 
>   src/launcher/default_executor.cpp 57e4799e750f8f5352a9fec58af40efe432ea865 
>   src/launcher/executor.cpp e035a4ee4438a3342f68a5548a5fd8d57a315cfa 
>   src/launcher/fetcher.cpp 5f9a38d3ae8fb9b4b21203f96d53c5bef0909eba 
>   src/linux/ns.hpp da43266ba67f76c9ca856584a540794062097691 
>   src/slave/containerizer/docker.cpp 7a8a7271b54af0b4dcdae7a2aa8a90a8a7d05fd6 
>   src/slave/containerizer/mesos/io/switchboard.cpp 
> 2c4eed8f2dc7f91b9fd08f91f1b08484fca9a9af 
>   src/slave/containerizer/mesos/launch.cpp 
> e482ab8bdfc358f695b87cda72ca59fb64cd8c4d 
>   src/tests/script.cpp ef6b22aaeb056ed489f2c519e1a02644f2653329 
> 
> Diff: https://reviews.apache.org/r/55683/diff/
> 
> 
> Testing
> ---
> 
> make check && sudo make check (Fedora 25)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 54212: Fixed overlay backend provisioning multi images symlink.

[Gilbert Song]

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

(Updated Jan. 19, 2017, 4:08 a.m.)


Review request for mesos, Avinash sridharan, Artem Harutyunyan, Jie Yu, Qian 
Zhang, and Zhitao Li.


Changes
---

Rebased.


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


Repository: mesos


Description
---

Since the fix of MESOS-6000, symlinks are used in overlayfs
backend to shorten the arguments when mounting the rootfs.
E.g., '.../backends/overlay/links' is the symlink created
for a provisioned image. It becomes problematic if a
container image is specified while some image volumes are
specified for the same container. An unique symlink is
needed for each image to be provisioned.


Diffs (updated)
-

  src/slave/containerizer/mesos/provisioner/backends/overlay.cpp 
a6e6c8b84adadc24b915b45533d1789f8de758dd 

Diff: https://reviews.apache.org/r/54212/diff/


Testing
---

make check

Verified that the unit test `ROOT_ImageInVolumeWithRootFilesystem` passed.


Thanks,

Gilbert Song

Re: Review Request 54216: Fixed unsufficient root privileges check by geteuid().

[Gilbert Song]

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

(Updated Jan. 19, 2017, 4:08 a.m.)


Review request for mesos, Avinash sridharan, Artem Harutyunyan, James DeFelice, 
and Jie Yu.


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


Repository: mesos


Description
---

We should check the root privileges by using the effective user
id, instead of comparing os::user() to 'root'.


Diffs
-

  src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 
74ab1fda4587d887342f8e63b3a38d450d9c6941 
  src/slave/containerizer/mesos/isolators/gpu/volume.cpp 
478752f372200b79cb3bd1e1c0f254f529930b78 
  src/slave/containerizer/mesos/provisioner/backends/aufs.cpp 
c92c6c7174421158b9190ed0bfb00c1c97ef0f7b 
  src/slave/containerizer/mesos/provisioner/backends/bind.cpp 
94dac40a12b6fd2e7d9733444d84763c77785402 
  src/slave/containerizer/mesos/provisioner/backends/overlay.cpp 
a6e6c8b84adadc24b915b45533d1789f8de758dd 

Diff: https://reviews.apache.org/r/54216/diff/


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 54215: Fixed duplicate image layer ids returned by docker store.

[Gilbert Song]

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

(Updated Jan. 19, 2017, 4:08 a.m.)


Review request for mesos, Avinash sridharan, Artem Harutyunyan, Jie Yu, Qian 
Zhang, and Zhitao Li.


Changes
---

Rebased.


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


Repository: mesos


Description
---

This issue is exposed by pulling the 'mesosphere/inky' docker
image using registry puller. Due to the duplicate layer id
from the manifest, there are duplicate layer pathes passed
to the backend. The aufs backend cannot handle this case and
returns 'invalid arguments' error. Ideally, we should make
sure that layer paths that are passed to the backend are
unique.


Diffs (updated)
-

  src/slave/containerizer/mesos/provisioner/docker/store.cpp 
9dccd0673dbc0c61abfd4b88097f86e7d7131c46 

Diff: https://reviews.apache.org/r/54215/diff/


Testing
---

make check

Tested by the unit test `ROOT_CURL_INTERNET_DockerDefaultEntryptRegistryPuller`.

Manually tested using the `mesosphere/inky` image, which contains duplicate 
layer ids.


Thanks,

Gilbert Song

Re: Review Request 54816: Replaced os::getcwd() to sandbox.get() in provisioner test.

[Gilbert Song]

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

(Updated Jan. 19, 2017, 4:08 a.m.)


Review request for mesos, Avinash sridharan, Artem Harutyunyan, Jie Yu, Shuai 
Lin, and Timothy Chen.


Changes
---

Rebased.


Repository: mesos


Description
---

Replaced os::getcwd() to sandbox.get() in provisioner test.


Diffs (updated)
-

  src/tests/containerizer/provisioner_backend_tests.cpp 
727bf645dd9337a94f098ab0a816b7e0e0651083 

Diff: https://reviews.apache.org/r/54816/diff/


Testing
---

make check


Thanks,

Gilbert Song

Review Request 55713: Added linux helper function fs::type() for filesystem id detection.

[Gilbert Song]

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

Review request for mesos, Avinash sridharan, Jie Yu, Timothy Chen, and Jiang 
Yan Xu.


Repository: mesos


Description
---

Get filesystem type id by using statfs() from .


Diffs
-

  src/linux/fs.hpp da49c9ebfa938d169152ed3b6e4df7378711b013 
  src/linux/fs.cpp 913e23317291db164fe6bdf77f3eca146dedec9b 
  src/tests/containerizer/fs_tests.cpp ccdd5967d01abfe4fa7746e3588ff93c6e0fe7e4 

Diff: https://reviews.apache.org/r/55713/diff/


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 54216: Fixed unsufficient root privileges check by geteuid().

[Gilbert Song]

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

(Updated Jan. 19, 2017, 4:07 a.m.)


Review request for mesos, Avinash sridharan, Artem Harutyunyan, James DeFelice, 
and Jie Yu.


Changes
---

Rebased.


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


Repository: mesos


Description
---

We should check the root privileges by using the effective user
id, instead of comparing os::user() to 'root'.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 
74ab1fda4587d887342f8e63b3a38d450d9c6941 
  src/slave/containerizer/mesos/isolators/gpu/volume.cpp 
478752f372200b79cb3bd1e1c0f254f529930b78 
  src/slave/containerizer/mesos/provisioner/backends/aufs.cpp 
c92c6c7174421158b9190ed0bfb00c1c97ef0f7b 
  src/slave/containerizer/mesos/provisioner/backends/bind.cpp 
94dac40a12b6fd2e7d9733444d84763c77785402 
  src/slave/containerizer/mesos/provisioner/backends/overlay.cpp 
a6e6c8b84adadc24b915b45533d1789f8de758dd 

Diff: https://reviews.apache.org/r/54216/diff/


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 54213: Supported more layers through symlink for aufs backend.

[Gilbert Song]

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

(Updated Jan. 19, 2017, 4:08 a.m.)


Review request for mesos, Avinash sridharan, Artem Harutyunyan, Jie Yu, Qian 
Zhang, and Zhitao Li.


Changes
---

Rebased.


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


Repository: mesos


Description
---

This issue is similar to MESOS-6000. We use the same solution
(using symlinks) to resolve many-layer image issue with aufs
backend.


Diffs (updated)
-

  src/slave/containerizer/mesos/provisioner/backends/aufs.cpp 
c92c6c7174421158b9190ed0bfb00c1c97ef0f7b 

Diff: https://reviews.apache.org/r/54213/diff/


Testing
---

make check

Tested by the new unit test `ROOT_AUFS_AufsBackendWithManyLayers` (see the 
following patch). 

Manually tested using a 37-layer image `gilbertsong/cirros:34`. It failed 
before the fix, and it passed after the fix.


Thanks,

Gilbert Song

Re: Review Request 50871: Supported auto backend in Unified Containerizer.

[Gilbert Song]

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

(Updated Jan. 19, 2017, 4:08 a.m.)


Review request for mesos, Avinash sridharan, Artem Harutyunyan, Jie Yu, and 
Timothy Chen.


Changes
---

Fixed the backing filesystem issue.


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


Repository: mesos


Description
---

Select a backend smartly. Currently, the logic is really
simple:
1. Use overlayfs backend if it exists.
2. Use aufs backend if the overlayfs does not exists.
3. Use copy backend of both overlayfs and aufs do not exist.
Please note that the bind backend needs to be specified explicitly
through the agent flag '--image_provisioner_backend' since it
requires the sandbox already existed.


Diffs (updated)
-

  docs/configuration.md 2113d06f58dddc0a28ae1241a24096266fe39801 
  src/slave/containerizer/mesos/provisioner/appc/store.hpp 
26cdd2fba142874ab1d3eca61222bb22bfbb2e13 
  src/slave/containerizer/mesos/provisioner/appc/store.cpp 
e63ae419e24212b887edddeb5cae114cd39b39c8 
  src/slave/containerizer/mesos/provisioner/docker/local_puller.hpp 
abb8e7e48422896f207a475661ced0530fc75e68 
  src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp 
817e30c5d6d6a4b011193e3209301fc3cdf88b06 
  src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp 
954da1681778878c8aff6150002e52ecb648d1bb 
  src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp 
395c36b0f6b8f8e037681ec5f2df99e83a7cf155 
  src/slave/containerizer/mesos/provisioner/docker/paths.hpp 
232c027f8f96da0cb30b957bce4607d3695050d2 
  src/slave/containerizer/mesos/provisioner/docker/paths.cpp 
cd684b33eb308ce1eeb4539a5b2d51985d835db7 
  src/slave/containerizer/mesos/provisioner/docker/puller.hpp 
191f3fc8cd1e0f706b5b1e6afddf856e8d53ae6e 
  src/slave/containerizer/mesos/provisioner/docker/puller.cpp 
ac9dae8ecbb897b8ff942d11ac70281a63e06831 
  src/slave/containerizer/mesos/provisioner/docker/registry_puller.hpp 
bbd6005317bed3fff3d86e2527ca3ead839d49e3 
  src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
cecf34a23329a64fdbce7de4b83827a30975e9a4 
  src/slave/containerizer/mesos/provisioner/docker/store.hpp 
1c2b149823e83dc5a3feb0af599d651d1dc05682 
  src/slave/containerizer/mesos/provisioner/docker/store.cpp 
9dccd0673dbc0c61abfd4b88097f86e7d7131c46 
  src/slave/containerizer/mesos/provisioner/provisioner.hpp 
0a48617d6f9ade928993e1d5205de6486ef657c7 
  src/slave/containerizer/mesos/provisioner/provisioner.cpp 
519028236305e9c8c1b6cded1919a5dd7ca3dbed 
  src/slave/containerizer/mesos/provisioner/store.hpp 
a312ad953b406aa75506051593dcc1c27cdc93af 
  src/slave/containerizer/mesos/provisioner/store.cpp 
7141d63fcf2dbc3fbf00508c7f92945aab014fb2 
  src/slave/flags.hpp 6ac0d45072157f6741b96266886a326e9337c153 
  src/slave/flags.cpp d976fb80b9d1e634ce0ca8e8ad35aa64959a4853 
  src/tests/containerizer/provisioner_appc_tests.cpp 
4d4ebba92ae66767903edc7a4f6edd9c6fee2489 
  src/tests/containerizer/provisioner_docker_tests.cpp 
d9472bb44cde999f95a8a65e6eee13f1f0fc09ed 

Diff: https://reviews.apache.org/r/50871/diff/


Testing
---

make check

sudo ./bin/mesos-tests.sh


Thanks,

Gilbert Song

Re: Review Request 54214: Added unit test for aufs backend supporting many layers.

[Gilbert Song]

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

(Updated Jan. 19, 2017, 4:08 a.m.)


Review request for mesos, Avinash sridharan, Artem Harutyunyan, Jie Yu, Qian 
Zhang, and Zhitao Li.


Changes
---

Rebased.


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


Repository: mesos


Description
---

Added unit test for aufs backend supporting many layers.


Diffs (updated)
-

  src/tests/containerizer/provisioner_backend_tests.cpp 
727bf645dd9337a94f098ab0a816b7e0e0651083 

Diff: https://reviews.apache.org/r/54214/diff/


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 54215: Fixed duplicate image layer ids returned by docker store.

[Gilbert Song]

> On Jan. 6, 2017, 11:18 p.m., Avinash sridharan wrote:
> > src/slave/containerizer/mesos/provisioner/docker/store.cpp, line 268
> > 
> >
> > Given that the `layerPaths` are meant to be unique why not make the 
> > `layerPaths` itself a `hashset`. Will be more explicit in terms of its 
> > usage.

We need to keep the layer ids in order. There is dependency on that.


- Gilbert


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


On Jan. 3, 2017, 2:47 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54215/
> ---
> 
> (Updated Jan. 3, 2017, 2:47 p.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Artem Harutyunyan, Jie Yu, Qian 
> Zhang, and Zhitao Li.
> 
> 
> Bugs: MESOS-6654
> https://issues.apache.org/jira/browse/MESOS-6654
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This issue is exposed by pulling the 'mesosphere/inky' docker
> image using registry puller. Due to the duplicate layer id
> from the manifest, there are duplicate layer pathes passed
> to the backend. The aufs backend cannot handle this case and
> returns 'invalid arguments' error. Ideally, we should make
> sure that layer paths that are passed to the backend are
> unique.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> 9dccd0673dbc0c61abfd4b88097f86e7d7131c46 
> 
> Diff: https://reviews.apache.org/r/54215/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> Tested by the unit test 
> `ROOT_CURL_INTERNET_DockerDefaultEntryptRegistryPuller`.
> 
> Manually tested using the `mesosphere/inky` image, which contains duplicate 
> layer ids.
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Review Request 55714: Added fs::magic() mapping function for fs id to fs type mapping.

[Gilbert Song]

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

Review request for mesos, Avinash sridharan, Jie Yu, Timothy Chen, and Jiang 
Yan Xu.


Repository: mesos


Description
---

A simple uint32_t -> string mapping.


Diffs
-

  src/linux/fs.hpp da49c9ebfa938d169152ed3b6e4df7378711b013 
  src/linux/fs.cpp 913e23317291db164fe6bdf77f3eca146dedec9b 

Diff: https://reviews.apache.org/r/55714/diff/


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 55490: Used the `mesos/mesos-tidy` image from DockerHub.

[Benjamin Bannier]

> On Jan. 15, 2017, 10:55 a.m., Benjamin Bannier wrote:
> > This is great. Could you make sure to follow up with setting 
> > `mesos/mesos-tidy` up for automated builds? Before building the image in 
> > the dockerhub time constraints was hard, but I am optimistic this would 
> > work now with the changes from https://reviews.apache.org/r/55488/.

FYI: I tried creating this docker image with an automated build, but also ran 
into the time limit.


- Benjamin


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


On Jan. 18, 2017, 9:58 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55490/
> ---
> 
> (Updated Jan. 18, 2017, 9:58 a.m.)
> 
> 
> Review request for mesos and Benjamin Bannier.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Used the `mesos/mesos-tidy` image from DockerHub.
> 
> 
> Diffs
> -
> 
>   support/mesos-tidy.sh 96d3ecb91f5476ff499ca5f043c527681c30efe9 
> 
> Diff: https://reviews.apache.org/r/55490/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 55679: Unified the way in which the Slave API handlers perform AuthZ.

[Gastón Kleiman]

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

(Updated Jan. 19, 2017, 10:17 a.m.)


Review request for mesos, Adam B, Alexander Rukletsov, Anand Mazumdar, 
Alexander Rojas, haosdent huang, and Vinod Kone.


Changes
---

Rebased.


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


Repository: mesos


Description
---

Unified the way in which the Slave API handlers perform AuthZ.


Diffs (updated)
-

  src/slave/http.cpp ee7119179a6ddd935c3f43a618ef645619c305ee 

Diff: https://reviews.apache.org/r/55679/diff/


Testing
---

`make check` in macOS and Linux.


Thanks,

Gastón Kleiman

Re: Review Request 55678: Renamed `locateExecutor` to `getExecutor`.

[Gastón Kleiman]

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

(Updated Jan. 19, 2017, 10:17 a.m.)


Review request for mesos, Adam B, Alexander Rukletsov, Anand Mazumdar, 
Alexander Rojas, haosdent huang, and Vinod Kone.


Changes
---

Rebased.


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


Repository: mesos


Description
---

Rnamed `locateExecutor` to `getExecutor` in `slave.cpp` to be consistent
with other similar functions in the same file.


Diffs (updated)
-

  src/slave/http.cpp ee7119179a6ddd935c3f43a618ef645619c305ee 
  src/slave/slave.hpp 8c21ce95683b3d8f0f732e87cf3b3dbe7860f1da 
  src/slave/slave.cpp 205138add7a63289ff8ed81138f8b603828c748e 

Diff: https://reviews.apache.org/r/55678/diff/


Testing
---

`make check` in macOS and Linux.


Thanks,

Gastón Kleiman

Re: Review Request 55676: Moved `getRootContainerId` to `protobuf_utils`.

[Alexander Rojas]

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


Ship it!




Ship It!

- Alexander Rojas


On Jan. 19, 2017, 11:15 a.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55676/
> ---
> 
> (Updated Jan. 19, 2017, 11:15 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Anand Mazumdar, 
> Alexander Rojas, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: MESOS-6864
> https://issues.apache.org/jira/browse/MESOS-6864
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This method is pretty generic and should not be a part of the Mesos
> containerizer code.
> 
> 
> Diffs
> -
> 
>   src/common/protobuf_utils.hpp bca9ace6cb37047bb08466c4fb14028bbb818446 
>   src/common/protobuf_utils.cpp af3c2a660f91c366ca5ffe8671d10c4a270bbbcc 
>   src/slave/containerizer/composing.cpp 
> 4fb7ba5cadd36be00be61aa00a69c38c44649aee 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 8bf8a7774a38131c53f6d91c7f09f5dedd9d4cb4 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
> b3a269535517222a1d85d8be1fdf5b865d6a3fc6 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
> e1cac954848e618a03ddb82fd6d040ae1d948e82 
>   src/slave/containerizer/mesos/utils.hpp 
> a54106dc4893bb222f42ede936ac9029e817faf9 
>   src/slave/containerizer/mesos/utils.cpp 
> 4e2a01495909c07f320af416ff4dc59f7328c710 
> 
> Diff: https://reviews.apache.org/r/55676/diff/
> 
> 
> Testing
> ---
> 
> `make check` in macOS and Linux.
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Re: Review Request 55677: Made `AttachContainerOutput/Input` tests use an existing container ID.

[Gastón Kleiman]

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

(Updated Jan. 19, 2017, 10:17 a.m.)


Review request for mesos, Adam B, Alexander Rukletsov, Anand Mazumdar, 
Alexander Rojas, haosdent huang, and Vinod Kone.


Changes
---

Rebased.


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


Repository: mesos


Description
---

These tests verify that the Agent returns a 500 if the containerizer
doesn't support the `attach` call.

This chain refactors those methods and make them return a 404 instead of
a 500 if the container can't be found, so we need to pass the ID of an
existing container.


Diffs (updated)
-

  src/tests/api_tests.cpp ea62226fbc5dd0bcc5ea60d1bbcd3748f9367467 

Diff: https://reviews.apache.org/r/55677/diff/


Testing
---

`make check` in macOS and Linux.


Thanks,

Gastón Kleiman

Re: Review Request 55676: Moved `getRootContainerId` to `protobuf_utils`.

[Gastón Kleiman]

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

(Updated Jan. 19, 2017, 10:15 a.m.)


Review request for mesos, Adam B, Alexander Rukletsov, Anand Mazumdar, 
Alexander Rojas, haosdent huang, and Vinod Kone.


Changes
---

Rebased.


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


Repository: mesos


Description
---

This method is pretty generic and should not be a part of the Mesos
containerizer code.


Diffs (updated)
-

  src/common/protobuf_utils.hpp bca9ace6cb37047bb08466c4fb14028bbb818446 
  src/common/protobuf_utils.cpp af3c2a660f91c366ca5ffe8671d10c4a270bbbcc 
  src/slave/containerizer/composing.cpp 
4fb7ba5cadd36be00be61aa00a69c38c44649aee 
  src/slave/containerizer/mesos/containerizer.cpp 
8bf8a7774a38131c53f6d91c7f09f5dedd9d4cb4 
  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
b3a269535517222a1d85d8be1fdf5b865d6a3fc6 
  src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
e1cac954848e618a03ddb82fd6d040ae1d948e82 
  src/slave/containerizer/mesos/utils.hpp 
a54106dc4893bb222f42ede936ac9029e817faf9 
  src/slave/containerizer/mesos/utils.cpp 
4e2a01495909c07f320af416ff4dc59f7328c710 

Diff: https://reviews.apache.org/r/55676/diff/


Testing
---

`make check` in macOS and Linux.


Thanks,

Gastón Kleiman

Re: Review Request 55676: Moved `getRootContainerId` to `protobuf_utils`.

[Alexander Rojas]

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




src/slave/slave.cpp 


Rebase problem?



src/tests/slave_recovery_tests.cpp 


Rebase problem?


- Alexander Rojas


On Jan. 18, 2017, 8:01 p.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55676/
> ---
> 
> (Updated Jan. 18, 2017, 8:01 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Anand Mazumdar, 
> Alexander Rojas, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: MESOS-6864
> https://issues.apache.org/jira/browse/MESOS-6864
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This method is pretty generic and should not be a part of the Mesos
> containerizer code.
> 
> 
> Diffs
> -
> 
>   CHANGELOG a8d080555a3948c796116af90c17dd71e1dea5d2 
>   src/common/protobuf_utils.hpp bca9ace6cb37047bb08466c4fb14028bbb818446 
>   src/common/protobuf_utils.cpp af3c2a660f91c366ca5ffe8671d10c4a270bbbcc 
>   src/slave/containerizer/composing.cpp 
> 4fb7ba5cadd36be00be61aa00a69c38c44649aee 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 8bf8a7774a38131c53f6d91c7f09f5dedd9d4cb4 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
> b3a269535517222a1d85d8be1fdf5b865d6a3fc6 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
> e1cac954848e618a03ddb82fd6d040ae1d948e82 
>   src/slave/containerizer/mesos/utils.hpp 
> a54106dc4893bb222f42ede936ac9029e817faf9 
>   src/slave/containerizer/mesos/utils.cpp 
> 4e2a01495909c07f320af416ff4dc59f7328c710 
>   src/slave/slave.cpp 205138add7a63289ff8ed81138f8b603828c748e 
>   src/tests/slave_recovery_tests.cpp 9cf1d461cf5c6ac1f51ac963360a17e37f558505 
> 
> Diff: https://reviews.apache.org/r/55676/diff/
> 
> 
> Testing
> ---
> 
> `make check` in macOS and Linux.
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Re: Review Request 55271: Disallow multi-role frameworks to change their roles.

[Jay Guo]

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




src/tests/master_validation_tests.cpp (lines 2774 - 2776)


s/One failover/On failover/
s/and then from the agent/and then from the framework/


- Jay Guo


On Jan. 19, 2017, 3:07 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55271/
> ---
> 
> (Updated Jan. 19, 2017, 3:07 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Jay Guo, and Guangya Liu.
> 
> 
> Bugs: MESOS-6631
> https://issues.apache.org/jira/browse/MESOS-6631
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We currently do not allow `MULTI_ROLE` frameworks to change their
> roles. This restriction will be lifted later.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp 8e8a9037af11cf95961b6498540a0fd486ed091b 
>   src/tests/master_validation_tests.cpp 
> c092362152e1fe8a6b615c2eda171d852c1bbd86 
> 
> Diff: https://reviews.apache.org/r/55271/diff/
> 
> 
> Testing
> ---
> 
> Tested on various Linux configurations in internal CI.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Review Request 55710: Added Agent capabilities in the response of /state endpoint.

[Jay Guo]

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

Review request for mesos, Benjamin Bannier, Benjamin Mahler, and Guangya Liu.


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


Repository: mesos


Description
---

We introduced Capabilities to SlaveInfo protobuf message in ec1a326.
It is automatically wrapped into response of GetState v1 API. This
patch added this field to /state v0 API for consistency.


Diffs
-

  src/master/http.cpp a44621f39cb059e654a56f57f75b38947f3a4230 

Diff: https://reviews.apache.org/r/55710/diff/


Testing
---

WIP. We need to figure out how to inject `SlaveInfo` during Agent registration 
for testing purpose, before we actually implement Agent Capabilities.


Thanks,

Jay Guo