Review Request 49569: Added an option to the launch helper binary to unshare mount namespace.

[Jie Yu]

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

Review request for mesos, Benjamin Mahler, Gilbert Song, Ian Downes, and Joshua 
Cohen.


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


Repository: mesos


Description
---

This allows a custom executor to use this command to launch a command in
a new root filesystem without worrying about creating a new mount
namespace first. For example, the following command can be used to
launch a command (`ls -al /`) using a root filesystem (`/tmp/alpine`).

`mesos-containerizer launch \
--unshare_namespace_mnt \
--rootfs=/tmp/alpine\
--command='{"shell":true,"value":"ls -al /"}'`


Diffs
-

  src/slave/containerizer/mesos/launch.hpp 
c716e0396736d1f2f60ec31540f12f4f7597d081 
  src/slave/containerizer/mesos/launch.cpp 
83f4d7f28c066a605aa84862eca9fde900ec96c6 

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


Testing
---

Manually tested the command on CentOS7:
```
[root@core-dev ~]# /home/jie/workspace/dist/mesos/build/src/mesos-containerizer 
launch --rootfs=/home/jie/alpine --unshare_namespace_mnt 
--command='{"shell":true,"value":"ls -al /"}' --user=jie
Changing root to /home/jie/alpine
total 24
drwxrwxr-x   17 1001 1002  4096 Jul  3 05:11 .
drwxrwxr-x   17 1001 1002  4096 Jul  3 05:11 ..
-rwxr-xr-x1 root root 0 Jul  3 05:09 .dockerenv
drwxr-xr-x2 root root  4096 Apr  1 18:56 bin
drwxr-xr-x4 root root   300 Jul  3 05:42 dev
drwxr-xr-x   13 root root  4096 Jul  3 05:09 etc
drwxr-xr-x2 root root 6 Apr  1 18:56 home
drwxr-xr-x5 root root  4096 Apr  1 18:56 lib
lrwxrwxrwx1 root root12 Apr  1 18:56 linuxrc -> /bin/busybox
drwxr-xr-x5 root root41 Apr  1 18:56 media
drwxr-xr-x2 root root 6 Apr  1 18:56 mnt
dr-xr-xr-x  685 root root 0 Jun 18 02:22 proc
drwx--2 root root26 Jul  3 05:14 root
drwxr-xr-x2 root root 6 Apr  1 18:56 run
drwxr-xr-x2 root root  4096 Apr  1 18:56 sbin
dr-xr-xr-x   13 root root 0 Jun 18 02:23 sys
drwxrwxrwt2 root root 6 Jul  3 05:13 tmp
drwxr-xr-x7 root root61 Apr  1 18:56 usr
drwxr-xr-x   10 root root93 Apr  1 18:56 var
```


Thanks,

Jie Yu

Review Request 49568: Simplified a flag in the launch helper binary.

[Jie Yu]

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

Review request for mesos, Gilbert Song and Ian Downes.


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


Repository: mesos


Description
---

Using JSON::Object is not necessary. Also, the current name is quite
confusing as we have another flag called '--command'. This patch
simplified this flag.


Diffs
-

  src/slave/containerizer/mesos/containerizer.cpp 
a96b382f22886362a1159e1166dfe041072985ba 
  src/slave/containerizer/mesos/launch.hpp 
c716e0396736d1f2f60ec31540f12f4f7597d081 
  src/slave/containerizer/mesos/launch.cpp 
83f4d7f28c066a605aa84862eca9fde900ec96c6 

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


Testing
---

make check


Thanks,

Jie Yu

Re: Review Request 49414: Added Executor PID in /containers endpoint. Also Added Test Cases.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On July 2, 2016, 9:50 p.m., Haris Choudhary wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49414/
> ---
> 
> (Updated July 2, 2016, 9:50 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-5737
> https://issues.apache.org/jira/browse/MESOS-5737
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added Executor PID in /containers endpoint. Also Added Test Cases.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 5b15a89bbac0b33c77e305c5b188823f2704a653 
>   include/mesos/v1/mesos.proto a1435278e81c8f3179d94cd9d2c3dd8c0ba82d35 
>   src/slave/containerizer/mesos/containerizer.cpp 
> a96b382f22886362a1159e1166dfe041072985ba 
>   src/slave/containerizer/mesos/launcher.hpp 
> 05320f462653c31fc2f093d6c67e2182e9c794fa 
>   src/slave/containerizer/mesos/launcher.cpp 
> ff675262af8947b89f8099828665e5e5d86491d8 
>   src/slave/containerizer/mesos/linux_launcher.hpp 
> 89bb2958a41dffe4ade9c2492b9a7412f90a432d 
>   src/slave/containerizer/mesos/linux_launcher.cpp 
> 5028854fa003615f158120e030866b7ec4402b66 
>   src/tests/containerizer/launcher.hpp 
> c352634c4766d289706c7cc738677619d7d02ccd 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> 6c14f6e20b4d14d5ed095670673571739101b0e4 
> 
> Diff: https://reviews.apache.org/r/49414/diff/
> 
> 
> Testing
> ---
> 
> make and make check.
> 
> 
> Thanks,
> 
> Haris Choudhary
> 
>

Re: Review Request 49518: Initial snapshot for v1 master event stream.

[haosdent huang]

> On July 3, 2016, 2:23 a.m., haosdent huang wrote:
> > src/tests/api_tests.cpp, line 1458
> > 
> >
> > Add a blank line above.

Refer to 
https://github.com/apache/mesos/blob/master/docs/c%2B%2B-style-guide.md#empty-lines

> Inside a code block, every multi-line statement should be followed by one 
> empty line.


- haosdent


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


On July 2, 2016, 12:20 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49518/
> ---
> 
> (Updated July 2, 2016, 12:20 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: MESOS-5498
> https://issues.apache.org/jira/browse/MESOS-5498
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Initial snapshot for v1 master event stream.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/master.proto d06258e9fd39c7eefd8ecd394e3bdfb888479b1e 
>   include/mesos/v1/master/master.proto 
> b7cb6fdf2e4f34a11d326ac3ad3ec26525f8f343 
>   src/master/http.cpp 7b2f77b9264242f31ab62eb9db7c621a1b8aa2fe 
>   src/tests/api_tests.cpp e2d8bf591667ec9d8c609e55a424b55561892b5f 
> 
> Diff: https://reviews.apache.org/r/49518/diff/
> 
> 
> Testing
> ---
> 
> Updated test and make check.
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 49518: Initial snapshot for v1 master event stream.

[haosdent huang]

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



I think we should split the test cases for `SUBSCRIBE` instead of put all of 
them in `MasterAPITest.Subscribe`

- haosdent huang


On July 2, 2016, 12:20 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49518/
> ---
> 
> (Updated July 2, 2016, 12:20 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: MESOS-5498
> https://issues.apache.org/jira/browse/MESOS-5498
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Initial snapshot for v1 master event stream.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/master.proto d06258e9fd39c7eefd8ecd394e3bdfb888479b1e 
>   include/mesos/v1/master/master.proto 
> b7cb6fdf2e4f34a11d326ac3ad3ec26525f8f343 
>   src/master/http.cpp 7b2f77b9264242f31ab62eb9db7c621a1b8aa2fe 
>   src/tests/api_tests.cpp e2d8bf591667ec9d8c609e55a424b55561892b5f 
> 
> Diff: https://reviews.apache.org/r/49518/diff/
> 
> 
> Testing
> ---
> 
> Updated test and make check.
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 49518: Initial snapshot for v1 master event stream.

[haosdent huang]

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




src/tests/api_tests.cpp (line 1425)


Add a blank line above.


- haosdent huang


On July 2, 2016, 12:20 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49518/
> ---
> 
> (Updated July 2, 2016, 12:20 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: MESOS-5498
> https://issues.apache.org/jira/browse/MESOS-5498
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Initial snapshot for v1 master event stream.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/master.proto d06258e9fd39c7eefd8ecd394e3bdfb888479b1e 
>   include/mesos/v1/master/master.proto 
> b7cb6fdf2e4f34a11d326ac3ad3ec26525f8f343 
>   src/master/http.cpp 7b2f77b9264242f31ab62eb9db7c621a1b8aa2fe 
>   src/tests/api_tests.cpp e2d8bf591667ec9d8c609e55a424b55561892b5f 
> 
> Diff: https://reviews.apache.org/r/49518/diff/
> 
> 
> Testing
> ---
> 
> Updated test and make check.
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 49445: Updated FilesProcess to support List_Files Call in Operator API v1.

[haosdent huang]

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




src/files/files.cpp (line 185)


I suggest use a different function name so we don't need this.


- haosdent huang


On July 2, 2016, 7:32 a.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49445/
> ---
> 
> (Updated July 2, 2016, 7:32 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, zhou xing, haosdent huang, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-5487
> https://issues.apache.org/jira/browse/MESOS-5487
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated FilesProcess to support List_Files Call in Operator API v1.
> 
> 
> Diffs
> -
> 
>   src/files/files.hpp b767d5bc5bee16e3bd98199773a6bc7d30c1c32d 
>   src/files/files.cpp a5a1b86e14f63e5e3834a2900270252fbe16f586 
>   src/tests/files_tests.cpp 31337e280c6224a8c949c8868a53e5a785b4573f 
> 
> Diff: https://reviews.apache.org/r/49445/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 49243: Create readFile method in FilesProcess.

[haosdent huang]

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




src/files/files.cpp (line 517)


Need `defer(self()` here to avoid multiple threads conflict.


- haosdent huang


On June 30, 2016, 6:59 a.m., zhou xing wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49243/
> ---
> 
> (Updated June 30, 2016, 6:59 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: mesos-5515
> https://issues.apache.org/jira/browse/mesos-5515
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This method helps to readFiles based on offset, length
> and path, which can be used for implementing master/agent's
> READ_FILE operator v1 API.
> 
> 
> Diffs
> -
> 
>   src/files/files.hpp b767d5bc5bee16e3bd98199773a6bc7d30c1c32d 
>   src/files/files.cpp a5a1b86e14f63e5e3834a2900270252fbe16f586 
> 
> Diff: https://reviews.apache.org/r/49243/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> zhou xing
> 
>

Re: Review Request 49509: Revised protobuf definition of GetState response.

[haosdent huang]

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


Ship it!




Ship It!

- haosdent huang


On July 1, 2016, 11:43 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49509/
> ---
> 
> (Updated July 1, 2016, 11:43 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: MESOS-5489
> https://issues.apache.org/jira/browse/MESOS-5489
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Revised protobuf definition of GetState response.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/master.proto d06258e9fd39c7eefd8ecd394e3bdfb888479b1e 
>   include/mesos/v1/master/master.proto 
> b7cb6fdf2e4f34a11d326ac3ad3ec26525f8f343 
> 
> Diff: https://reviews.apache.org/r/49509/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 49445: Updated FilesProcess to support List_Files Call in Operator API v1.

[haosdent huang]

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




src/files/files.cpp (lines 85 - 87)


Why update these?


- haosdent huang


On July 2, 2016, 7:32 a.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49445/
> ---
> 
> (Updated July 2, 2016, 7:32 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, zhou xing, haosdent huang, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-5487
> https://issues.apache.org/jira/browse/MESOS-5487
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated FilesProcess to support List_Files Call in Operator API v1.
> 
> 
> Diffs
> -
> 
>   src/files/files.hpp b767d5bc5bee16e3bd98199773a6bc7d30c1c32d 
>   src/files/files.cpp a5a1b86e14f63e5e3834a2900270252fbe16f586 
>   src/tests/files_tests.cpp 31337e280c6224a8c949c8868a53e5a785b4573f 
> 
> Diff: https://reviews.apache.org/r/49445/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 49446: Implemented LIST_FILES Call in v1 master API.

[haosdent huang]

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




src/master/http.cpp (line 2815)


not need here?


- haosdent huang


On July 2, 2016, 7:35 a.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49446/
> ---
> 
> (Updated July 2, 2016, 7:35 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, zhou xing, haosdent huang, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-5487
> https://issues.apache.org/jira/browse/MESOS-5487
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented LIST_FILES Call in v1 master API.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/master.proto 2e5d6eeb3a960e4f41c382d65321f18bb05ed6be 
>   include/mesos/v1/master/master.proto 
> 93157d57dcc53b54fed2ebbc4772c689ddba2119 
>   src/master/http.cpp e5acdb8e0bbcd7a2b7e8a8bc7f4bbeaae2c4fea1 
>   src/master/master.hpp e2ab2110fe5a287ab16ac9ef4222fed633e02ebe 
> 
> Diff: https://reviews.apache.org/r/49446/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 49550: Added evolve function for `FileInfo`.

[haosdent huang]

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


Ship it!




Ship It!

- haosdent huang


On July 2, 2016, 7:37 a.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49550/
> ---
> 
> (Updated July 2, 2016, 7:37 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, zhou xing, haosdent huang, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-5514
> https://issues.apache.org/jira/browse/MESOS-5514
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added evolve function for `FileInfo`.
> 
> 
> Diffs
> -
> 
>   src/internal/evolve.hpp 582f3476d0f61c9356cf5aff5f242f3e8103822d 
>   src/internal/evolve.cpp bb99cb29db84e5476a820b261ff24348c657bae0 
> 
> Diff: https://reviews.apache.org/r/49550/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 49551: Overloaded equality(==) operator for `FileInfo`.

[haosdent huang]

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




include/mesos/v1/mesos.hpp (line 55)


The order should be alphabetically?



src/v1/mesos.cpp (line 332)


ditto.


- haosdent huang


On July 2, 2016, 7:39 a.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49551/
> ---
> 
> (Updated July 2, 2016, 7:39 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, zhou xing, haosdent huang, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-5514
> https://issues.apache.org/jira/browse/MESOS-5514
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Overloaded equality(==) operator for `FileInfo`.
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/mesos.hpp d220966bb9464595c927474532ccdcaf41690ef6 
>   src/v1/mesos.cpp 30788dcc1dd744553ddd93d23b11fcce44502b46 
> 
> Diff: https://reviews.apache.org/r/49551/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 49529: Removed jsonFileInfo implementation from files.hpp.

[haosdent huang]

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


Ship it!




Ship It!

- haosdent huang


On July 2, 2016, 7:51 a.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49529/
> ---
> 
> (Updated July 2, 2016, 7:51 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, zhou xing, haosdent huang, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-5487
> https://issues.apache.org/jira/browse/MESOS-5487
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Remove jsonFileInfo implementation from files.hpp.
> 
> 
> Diffs
> -
> 
>   src/files/files.hpp b767d5bc5bee16e3bd98199773a6bc7d30c1c32d 
> 
> Diff: https://reviews.apache.org/r/49529/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 49447: Implemented LIST_FILES Call in v1 agent API.

[haosdent huang]

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




src/slave/http.cpp (line 748)


Not need here?


- haosdent huang


On July 2, 2016, 7:55 a.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49447/
> ---
> 
> (Updated July 2, 2016, 7:55 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, zhou xing, haosdent huang, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-5514
> https://issues.apache.org/jira/browse/MESOS-5514
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented LIST_FILES Call in v1 agent API.
> 
> 
> Diffs
> -
> 
>   include/mesos/agent/agent.proto 538d12f71df1943f91bafb99650625aa910affaa 
>   include/mesos/v1/agent/agent.proto 48f15173fe62b9ce7f648f6b54d74ec62f797c55 
>   src/slave/http.cpp 44d8cc98c0c1ada9d5313a3fe5c66029c9c373c6 
>   src/slave/slave.hpp 2afd7d152dcd2f5390014cd7bd4e926b62c292d1 
> 
> Diff: https://reviews.apache.org/r/49447/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 49448: Added testcases for LIST_FILES call.

[haosdent huang]

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


Ship it!




Ship It!

- haosdent huang


On July 2, 2016, 7:50 a.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49448/
> ---
> 
> (Updated July 2, 2016, 7:50 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, zhou xing, haosdent huang, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-5514
> https://issues.apache.org/jira/browse/MESOS-5514
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added testcases for LIST_FILES call.
> 
> 
> Diffs
> -
> 
>   src/tests/api_tests.cpp b3127b716480223a6f23b7908bf6bc1808120f80 
> 
> Diff: https://reviews.apache.org/r/49448/diff/
> 
> 
> Testing
> ---
> 
> On Ubuntu 16.04:
> sudo 
> GTEST_FILTER="*AgentAPITest.ListFiles*:FilesTest.BrowseTest:*MasterAPITest.ListFiles*"
>  make -j4 check
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 49517: Implement GetState V1 master API.

[haosdent huang]

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




src/master/http.cpp (line 1545)


Indent incorrect here?


- haosdent huang


On July 2, 2016, 12:19 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49517/
> ---
> 
> (Updated July 2, 2016, 12:19 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: MESOS-5489
> https://issues.apache.org/jira/browse/MESOS-5489
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The help function will also be used for snapshot of
> event stream.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 7b2f77b9264242f31ab62eb9db7c621a1b8aa2fe 
>   src/master/master.hpp be7cd239c49a0710a29a8187c01484f9f6d615e1 
>   src/tests/api_tests.cpp e2d8bf591667ec9d8c609e55a424b55561892b5f 
> 
> Diff: https://reviews.apache.org/r/49517/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 49546: Added device support to Docker::run.

[Benjamin Mahler]

> On July 2, 2016, 8:18 p.m., Kevin Klues wrote:
> > src/docker/docker.cpp, line 677
> > 
> >
> > If all permissions are `false`, do we still want to include the ":" 
> > here? Does this work with docker?

Thanks! Turns out docker doesn't handle this case, so I've guarded against it.


> On July 2, 2016, 8:18 p.m., Kevin Klues wrote:
> > src/tests/containerizer/docker_tests.cpp, line 661
> > 
> >
> > Now I see why this is a little weird to have `NVIDIA_GPU` here. This 
> > isn't technically a GPU specific test -- it is valid for injecting any 
> > device into a docker container.
> > 
> > If we wanted to make it non GPU specific. You could probe `/dev` for 
> > *some* device on the host that's not in docker's default whitelist for 
> > devices and inject that one instead.

Yeah, I've added a TODO to have this test not require GPUs.


- Benjamin


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


On July 2, 2016, 12:55 a.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49546/
> ---
> 
> (Updated July 2, 2016, 12:55 a.m.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added device support to Docker::run.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 4343213391bb9a86e87410ce3a2c5d8447a1ab8d 
>   src/docker/docker.cpp b356848ea8f65b58102b26b38e2e753c4b665a4b 
>   src/docker/executor.cpp 2089b547408a46c9b4fa91e3ab17b88f7d2d8397 
>   src/slave/containerizer/docker.cpp 915030bdbe5a5b55acc38042ee0475074a602ee0 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> f7b42836ceb80cdae901c4926bd702b6da36a55c 
>   src/tests/containerizer/docker_tests.cpp 
> c0ff9574633f8f72ccae9b9298f3b60bc2f3cf81 
>   src/tests/mesos.hpp 3173bf9cfc832be0aa18d745434afc45d352a269 
>   src/tests/mesos.cpp a433e5b2775f1d62b18927cf71af23ef28d6a503 
> 
> Diff: https://reviews.apache.org/r/49546/diff/
> 
> 
> Testing
> ---
> 
> Added a test.
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 49544: Made stout::Path default constructible / assignable / copyable.

[Benjamin Mahler]

> On July 2, 2016, 7:56 p.m., Kevin Klues wrote:
> > 3rdparty/stout/include/stout/path.hpp, lines 255-261
> > 
> >
> > Do we really need this function? It looks like just casting it to a 
> > string would be enough to turn it into a string?

Kevin and I chatted offline, mostly we avoid the convention of using an 
implicit cast (e.g. `(string) path`), so there are two options if leaving the 
cast operator:

```
string(path) // And
stringify(path)
```

The latter tends to be used for printing purposes and the former is 
unconventional in our code base.

Also, since we likely want to move towards  upcoming in the standard 
library, I followed the `.string()` convention from there.


- Benjamin


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


On July 2, 2016, 12:46 a.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49544/
> ---
> 
> (Updated July 2, 2016, 12:46 a.m.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This makes Path closer to boost's path, and more convenient to work with 
> (e.g. as map values, struct members, etc).
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/path.hpp 
> 0ca13c6a1aebcb980c29dd33414bfa50c4b8bf81 
> 
> Diff: https://reviews.apache.org/r/49544/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 49567: Cleaned up some logic in stout/elf.hpp.

[Benjamin Mahler]

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


Ship it!




Let's split the bug fix into a separate patch. Also let's include a description 
that mentions that this logic was working around the ELFIO bug that we now fix 
via a .patch file.

- Benjamin Mahler


On July 2, 2016, 11:10 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49567/
> ---
> 
> (Updated July 2, 2016, 11:10 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Cleaned up some logic in stout/elf.hpp.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/elf.hpp 
> 94b1de97b14d10c4796933e2aa395c195c94ee10 
> 
> Diff: https://reviews.apache.org/r/49567/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 49414: Added Executor PID in /containers endpoint. Also Added Test Cases.

[Jie Yu]

> On July 2, 2016, 5:46 a.m., Jie Yu wrote:
> > src/tests/containerizer/mesos_containerizer_tests.cpp, line 580
> > 
> >
> > Please use UUID::random here.
> 
> Haris Choudhary wrote:
> I've made the changes. But consider if both UUID's were the same as a 
> result of random, the test would fail.
> However I think its super rare for both to be generated as the same.

UUID stands for universal unique id. Many components in Mesos will break if the 
uniqueness assumption is broken. I won't be too worried.


- Jie


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


On July 2, 2016, 9:50 p.m., Haris Choudhary wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49414/
> ---
> 
> (Updated July 2, 2016, 9:50 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-5737
> https://issues.apache.org/jira/browse/MESOS-5737
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added Executor PID in /containers endpoint. Also Added Test Cases.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 5b15a89bbac0b33c77e305c5b188823f2704a653 
>   include/mesos/v1/mesos.proto a1435278e81c8f3179d94cd9d2c3dd8c0ba82d35 
>   src/slave/containerizer/mesos/containerizer.cpp 
> a96b382f22886362a1159e1166dfe041072985ba 
>   src/slave/containerizer/mesos/launcher.hpp 
> 05320f462653c31fc2f093d6c67e2182e9c794fa 
>   src/slave/containerizer/mesos/launcher.cpp 
> ff675262af8947b89f8099828665e5e5d86491d8 
>   src/slave/containerizer/mesos/linux_launcher.hpp 
> 89bb2958a41dffe4ade9c2492b9a7412f90a432d 
>   src/slave/containerizer/mesos/linux_launcher.cpp 
> 5028854fa003615f158120e030866b7ec4402b66 
>   src/tests/containerizer/launcher.hpp 
> c352634c4766d289706c7cc738677619d7d02ccd 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> 6c14f6e20b4d14d5ed095670673571739101b0e4 
> 
> Diff: https://reviews.apache.org/r/49414/diff/
> 
> 
> Testing
> ---
> 
> make and make check.
> 
> 
> Thanks,
> 
> Haris Choudhary
> 
>

Re: Review Request 49565: Added a new 'NvidiaVolume' component.

[Benjamin Mahler]

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


Fix it, then Ship it!





src/slave/containerizer/mesos/isolators/gpu/volume.hpp (line 34)


It would be nice to have an nvidia namespace for all of this stuff and 
avoid the `mesos::internal::slave` namespaceing. This could be 
`nvidia::Volume`. Will leave for now.



src/slave/containerizer/mesos/isolators/gpu/volume.hpp (line 37)


Include try.hpp?



src/slave/containerizer/mesos/isolators/gpu/volume.hpp (line 45)


const?



src/slave/containerizer/mesos/isolators/gpu/volume.cpp (lines 159 - 160)


"Failed to " to be consistent with the tense of the other error messages. 
Nice error messages in this patch!



src/slave/containerizer/mesos/isolators/gpu/volume.cpp (lines 184 - 192)


Seems like this belongs up in the .hpp interface documentation, the caller 
should be aware of this?



src/slave/containerizer/mesos/isolators/gpu/volume.cpp (line 209)


how about s/result/mkdir/ sytle naming here and below? note that this is 
what we generally do instead of 'result'



src/slave/containerizer/mesos/isolators/gpu/volume.cpp (line 260)


s/:/: /



src/slave/containerizer/mesos/isolators/gpu/volume.cpp (line 285)


We should probably use unique_ptr here to avoid the libprocess depedency 
(no need for an asynchronous library), but I'll leave for now since we 
generally need to polish the unique_ptr / Owned usage.



src/slave/containerizer/mesos/isolators/gpu/volume.cpp (line 290)


Error context?



src/slave/containerizer/mesos/isolators/gpu/volume.cpp (line 296)


We could do Option here and CHECK_SOME after the logic to ensure 
we're never passing through with the empty string case.



src/slave/containerizer/mesos/isolators/gpu/volume.cpp (lines 300 - 301)


quotes on the same line?



src/slave/containerizer/mesos/isolators/gpu/volume.cpp (line 306)


indentation is off here?



src/slave/containerizer/mesos/isolators/gpu/volume.cpp (line 308)


This compiles? I'd think you need a stringify here since you're doing 
+(char[], enum)



src/slave/containerizer/mesos/isolators/gpu/volume.cpp (lines 319 - 320)


quotes on the same line?



src/slave/containerizer/mesos/isolators/gpu/volume.cpp (line 325)


"libraries" typo



src/slave/containerizer/mesos/isolators/gpu/volume.cpp (line 350)


double backtick?



src/tests/containerizer/nvidia_gpu_isolator_tests.cpp (line 31)


Alphabetical? Also, we can do a finer-grained os/exists.hpp include here.



src/tests/containerizer/nvidia_gpu_isolator_tests.cpp (lines 477 - 481)


Looks like these can be EXPECTs?


- Benjamin Mahler


On July 2, 2016, 7:41 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49565/
> ---
> 
> (Updated July 2, 2016, 7:41 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Yubo Li, and Vikrama Ditya.
> 
> 
> Bugs: MESOS-5401
> https://issues.apache.org/jira/browse/MESOS-5401
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This component is responsible for building up a consolidated volume of
> binaries / libraries from the user-space portion of the Nvidia GPU
> drivers so that it can be injected into a container as a single
> volume. Its core functionality mimics that of the
> `nvidia-docker-plugin`: https://github.com/NVIDIA/nvidia-docker/
> 
> We currently only implement the 'create()' function which is
> responsible for building up the volume itself. In a subsequent commit
> we will add support for reading a Docker ImageManifest and deciding if
> we should inject the volume into the docker container or not.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 09dd0e0dd2f25fb71db5051d5f5a797a981c2e59 
>   src/Makefile.am 52d63f26

Re: Review Request 49566: Integrated the 'NvidiaVolume' component into 'NvidiaComponents'.

[Benjamin Mahler]

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


Ship it!




Ship It!

- Benjamin Mahler


On July 2, 2016, 7:43 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49566/
> ---
> 
> (Updated July 2, 2016, 7:43 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Yubo Li, and Vikrama Ditya.
> 
> 
> Bugs: MESOS-5401
> https://issues.apache.org/jira/browse/MESOS-5401
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This makes it so that the 'NvidiaVolume' component can be shared
> across containerizers in the same way that the 'NvidiaGpuAllocator' is
> currently being shared.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/containerizer.cpp 
> a436a5d6e367bfd310a003193c9d3770c3fcbac9 
>   src/slave/containerizer/mesos/isolators/gpu/components.hpp 
> 39c6126f5a5023e59b1bd2d90f1aaa47b09d6b28 
> 
> Diff: https://reviews.apache.org/r/49566/diff/
> 
> 
> Testing
> ---
> 
> `GTEST_FILTER="*NVIDIA_GPU_Volume*" make -j check`
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 49414: Added Executor PID in /containers endpoint. Also Added Test Cases.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [49414]

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 July 2, 2016, 9:50 p.m., Haris Choudhary wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49414/
> ---
> 
> (Updated July 2, 2016, 9:50 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-5737
> https://issues.apache.org/jira/browse/MESOS-5737
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added Executor PID in /containers endpoint. Also Added Test Cases.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 5b15a89bbac0b33c77e305c5b188823f2704a653 
>   include/mesos/v1/mesos.proto a1435278e81c8f3179d94cd9d2c3dd8c0ba82d35 
>   src/slave/containerizer/mesos/containerizer.cpp 
> a96b382f22886362a1159e1166dfe041072985ba 
>   src/slave/containerizer/mesos/launcher.hpp 
> 05320f462653c31fc2f093d6c67e2182e9c794fa 
>   src/slave/containerizer/mesos/launcher.cpp 
> ff675262af8947b89f8099828665e5e5d86491d8 
>   src/slave/containerizer/mesos/linux_launcher.hpp 
> 89bb2958a41dffe4ade9c2492b9a7412f90a432d 
>   src/slave/containerizer/mesos/linux_launcher.cpp 
> 5028854fa003615f158120e030866b7ec4402b66 
>   src/tests/containerizer/launcher.hpp 
> c352634c4766d289706c7cc738677619d7d02ccd 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> 6c14f6e20b4d14d5ed095670673571739101b0e4 
> 
> Diff: https://reviews.apache.org/r/49414/diff/
> 
> 
> Testing
> ---
> 
> make and make check.
> 
> 
> Thanks,
> 
> Haris Choudhary
> 
>

Review Request 49567: Cleaned up some logic in stout/elf.hpp.

[Kevin Klues]

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

Review request for mesos.


Repository: mesos


Description
---

Cleaned up some logic in stout/elf.hpp.


Diffs
-

  3rdparty/stout/include/stout/elf.hpp 94b1de97b14d10c4796933e2aa395c195c94ee10 

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


Testing
---


Thanks,

Kevin Klues

Re: Review Request 49564: Added `get_abi_version()` to the ELF parsing abstraction.

[Kevin Klues]

> On July 2, 2016, 10:22 p.m., Benjamin Mahler wrote:
> > 3rdparty/stout/include/stout/elf.hpp, line 84
> > 
> >
> > Cleanups in a separate patch next time?

Whoops! I must have squashed this into the wrong commit.  I meant to make this 
part of the original libelf --> ELFIO transition.


> On July 2, 2016, 10:22 p.m., Benjamin Mahler wrote:
> > 3rdparty/stout/include/stout/elf.hpp, line 159
> > 
> >
> > "only" suggests that there are more than 1 entries, but there can be 0 
> > here according to the condition being checked

There should be 1 and only one (if the section existst at all).


> On July 2, 2016, 10:22 p.m., Benjamin Mahler wrote:
> > 3rdparty/stout/include/stout/elf.hpp, lines 177-184
> > 
> >
> > Do we need to mention the patch here?

This comment can be removed. I put it in when I was messing with the 
workaround.  The check should actually jsut be "name == GNU" now.


> On July 2, 2016, 10:22 p.m., Benjamin Mahler wrote:
> > src/tests/ldcache_tests.cpp, lines 66-69
> > 
> >
> > This needs to be in a separate commit because of stout/libprocess/mesos 
> > split.

Yeah. Weird that it let me commit it then (usually the linter catches things 
like this).


- Kevin


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


On July 2, 2016, 7:38 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49564/
> ---
> 
> (Updated July 2, 2016, 7:38 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-5769
> https://issues.apache.org/jira/browse/MESOS-5769
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This function returns the ABI version of the ELF file by parsing the
> contents of its `.note.ABI-tag` section. This section is Linux
> specific and adheres to the format described in the links below.
> 
> https://refspecs.linuxfoundation.org/LSB_1.2.0/gLSB/noteabitag.html
> http://flint.cs.yale.edu/cs422/doc/ELF_Format.pdf
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/elf.hpp 
> 5382acc1c12901ce8d8256f6010f48579020fbca 
>   src/tests/ldcache_tests.cpp b387a8b763d241075392f65256d9b39429909d8b 
> 
> Diff: https://reviews.apache.org/r/49564/diff/
> 
> 
> Testing
> ---
> 
> `GTEST_FILTER="*Ldcache*" make -j check`
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 49559: Added ELFIO as bundled dependency in Mesos.

[Kevin Klues]

> On July 2, 2016, 9:51 p.m., Benjamin Mahler wrote:
> > 3rdparty/Makefile.am, line 175
> > 
> >
> > Looks like we need the following:
> > 
> > ```
> > $(ELFIO)/elfio/elf_types.hpp: $(ELFIO)-stamp
> > $(ELFIO)/elfio/elfio.hpp: $(ELFIO)-stamp
> > $(ELFIO)/elfio/elfio_dump.hpp: $(ELFIO)-stamp
> > $(ELFIO)/elfio/elfio_dynamic.hpp: $(ELFIO)-stamp
> > $(ELFIO)/elfio/elfio_header.hpp: $(ELFIO)-stamp
> > $(ELFIO)/elfio/elfio_note.hpp: $(ELFIO)-stamp
> > $(ELFIO)/elfio/elfio_relocation.hpp: $(ELFIO)-stamp
> > $(ELFIO)/elfio/elfio_section.hpp: $(ELFIO)-stamp
> > $(ELFIO)/elfio/elfio_segment.hpp: $(ELFIO)-stamp
> > $(ELFIO)/elfio/elfio_strings.hpp: $(ELFIO)-stamp
> > $(ELFIO)/elfio/elfio_symbols.hpp: $(ELFIO)-stamp
> > $(ELFIO)/elfio/elfio_utils.hpp: $(ELFIO)-stamp
> > ```

Strange. It built fine for me. Then again, I run with `make -j`, so maybe the 
stamp got created from another target for me concurrently, so I didn't notice. 
At one point I was using the target:

`$(ELFIO)/elfio/%.hpp`

But it didn't like that for some reason.


- Kevin


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


On July 2, 2016, 8:08 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49559/
> ---
> 
> (Updated July 2, 2016, 8:08 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-5767
> https://issues.apache.org/jira/browse/MESOS-5767
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This includes a patch file to ELFIO to fix 2 off-by-one errors in the
> parsing of NOTE sections. Patches for these bugs have been submitted
> upstream. Details of the patches are below:
> 
> 1) Fixed off-by-one error in 'name' of add_note() function.
> 
> Previously, when assigning 'name' as a string, it's length was
> specified using the full length of 'namesz'. However, this length
> includes the trailing '\0' of the underlying char[]. This ultimately
> causes the C++ string that is created to (incorrectly) contain the
> '\0' character as well. This leads to problems where e.g. the
> following will return false, even when 'name' itself contains the
> string "GNU\0":
> 
>   if (name == "GNU") {
> return true;
>   }
>   return false;
> 
> To fix this, we should only include the length of the string minus the
> trailing '\0'.
> 
> 2) Fixed alignment of 'desc' in add_note() function.
> 
> The ELF spec specifically lists the alignment of the namez char[] to
> be 4 bytes. To quote it:
> 
> "Padding is present, if necessary, to ensure 4-byte alignment for the
> descriptor. Such padding is not included in namesz."
> 
> However, the current implementation sets the alignment to either 4 or
> 8 bytes depending on the class of the ELF file (CLASS32 or CLASS64).
> This commit fixes the alignment to only 4 bytes in all cases.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt 15f3171 
>   3rdparty/Makefile.am bd990cc 
>   3rdparty/cmake/Versions.cmake 7b73f8f 
>   3rdparty/elfio-3.1.patch PRE-CREATION 
>   3rdparty/elfio-3.1.tar.gz PRE-CREATION 
>   3rdparty/versions.am 203656c 
>   LICENSE eb39f6d 
>   configure.ac 321436b 
>   src/Makefile.am 52d63f2 
>   support/coverage.sh ab9564b 
> 
> Diff: https://reviews.apache.org/r/49559/diff/
> 
> 
> Testing
> ---
> 
> Made sure `elfio-3.1` appears in `/build` after running `../configure`, 
> `make`.
> Made sure `elfio` appears in `/include` of installation folder after `make 
> install`.
> 
> This second one is necessary because `stout` relies on elfio in order to 
> function (similar to how we've done with `picojson.h` in the past).
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 49564: Added `get_abi_version()` to the ELF parsing abstraction.

[Benjamin Mahler]

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


Fix it, then Ship it!





3rdparty/stout/include/stout/elf.hpp (line 84)


Cleanups in a separate patch next time?



3rdparty/stout/include/stout/elf.hpp (line 122)


We probably don't need to say ELF here at all? The other messages seem to 
already omit ELF. :)

Ditto above in that we should try to avoid cleanups in the same patch.



3rdparty/stout/include/stout/elf.hpp (line 138)


stray // here?



3rdparty/stout/include/stout/elf.hpp (line 144)


Can we make this const?

How about using stout's Version class?

```
  Result get_abi_version()
  {
...
return Version(version[1], version[2], version[3]);
  }
```



3rdparty/stout/include/stout/elf.hpp (line 159)


"only" suggests that there are more than 1 entries, but there can be 0 here 
according to the condition being checked



3rdparty/stout/include/stout/elf.hpp (lines 177 - 184)


Do we need to mention the patch here?



src/tests/ldcache_tests.cpp (lines 66 - 69)


This needs to be in a separate commit because of stout/libprocess/mesos 
split.


- Benjamin Mahler


On July 2, 2016, 7:38 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49564/
> ---
> 
> (Updated July 2, 2016, 7:38 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-5769
> https://issues.apache.org/jira/browse/MESOS-5769
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This function returns the ABI version of the ELF file by parsing the
> contents of its `.note.ABI-tag` section. This section is Linux
> specific and adheres to the format described in the links below.
> 
> https://refspecs.linuxfoundation.org/LSB_1.2.0/gLSB/noteabitag.html
> http://flint.cs.yale.edu/cs422/doc/ELF_Format.pdf
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/elf.hpp 
> 5382acc1c12901ce8d8256f6010f48579020fbca 
>   src/tests/ldcache_tests.cpp b387a8b763d241075392f65256d9b39429909d8b 
> 
> Diff: https://reviews.apache.org/r/49564/diff/
> 
> 
> Testing
> ---
> 
> `GTEST_FILTER="*Ldcache*" make -j check`
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 49562: Reimplemented the stout ELF abstraction in terms of ELFIO.

[Benjamin Mahler]

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


Fix it, then Ship it!





3rdparty/stout/include/stout/elf.hpp (lines 20 - 24)


Looks like we can remove a bunch of these?



3rdparty/stout/include/stout/elf.hpp (line 66)


whoops, brace on the next line here and above



3rdparty/stout/include/stout/elf.hpp (line 76)


Unfortunately, it doesn't seem that there is any contract around using 
errno for errors, so let's avoid assuming this (in case we end up with 
incorrect error messages). :(



3rdparty/stout/include/stout/elf.hpp (line 104)


Seems this should maybe be a None? I'll leave as is for now.


- Benjamin Mahler


On July 2, 2016, 7:31 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49562/
> ---
> 
> (Updated July 2, 2016, 7:31 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-5768
> https://issues.apache.org/jira/browse/MESOS-5768
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> As part of this, we updated the API to include functions written in
> snake_case (as is standard for stout). We also removed the 'close()'
> call, as the new ELFIO library doesn't require it.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/elf.hpp 
> 5382acc1c12901ce8d8256f6010f48579020fbca 
> 
> Diff: https://reviews.apache.org/r/49562/diff/
> 
> 
> Testing
> ---
> 
> Tests coming in subsequent patch with updates to `ldcache_tests.cpp`
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 49561: Added ELFIO as bundled dependency in stout.

[Benjamin Mahler]

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


Ship it!




Ship It!

- Benjamin Mahler


On July 2, 2016, 8:29 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49561/
> ---
> 
> (Updated July 2, 2016, 8:29 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-5767
> https://issues.apache.org/jira/browse/MESOS-5767
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added ELFIO as bundled dependency in stout.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/Makefile.am eeaca480872b97d60891376b8c21fda7b65a6f00 
>   3rdparty/stout/cmake/StoutConfigure.cmake 
> e4e3217a6f6053d3555d4602f7173c6fe94daeda 
>   3rdparty/stout/configure.ac ca6f041b5ad5c92e683500f7b34a521fefa318f4 
> 
> Diff: https://reviews.apache.org/r/49561/diff/
> 
> 
> Testing
> ---
> 
> Made sure `elfio-3.1` appears in `/build` after running `../configure`, 
> `make`.
> Made sure `elfio` appears in `/include` of installation folder after `make 
> install`.
> 
> This second one is necessary because `stout` relies on `elfio` in order to 
> function (similar to how we've done with `picojson.h` in the past).
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 49563: Reimplemented ldcache_test.cpp using the new ELF abstraction in stout.

[Benjamin Mahler]

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


Ship it!




Ship It!

- Benjamin Mahler


On July 2, 2016, 7:35 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49563/
> ---
> 
> (Updated July 2, 2016, 7:35 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-5768
> https://issues.apache.org/jira/browse/MESOS-5768
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We also reenabled this test to run as part of the standard Linux test
> suite. It had been disabled due to its dependence on the external
> libelf library. Now that we bundle ELFIO instead of relying on this
> external dependence, we can always have it enabled now.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 52d63f26e0455ef31411e964e497a509d96aaf4a 
>   src/tests/ldcache_tests.cpp b387a8b763d241075392f65256d9b39429909d8b 
> 
> Diff: https://reviews.apache.org/r/49563/diff/
> 
> 
> Testing
> ---
> 
> `GTEST_FILTER="LdcacheTest*" make -j check`
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 49559: Added ELFIO as bundled dependency in Mesos.

[Benjamin Mahler]

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


Fix it, then Ship it!





3rdparty/Makefile.am (line 175)


Looks like we need the following:

```
$(ELFIO)/elfio/elf_types.hpp: $(ELFIO)-stamp
$(ELFIO)/elfio/elfio.hpp: $(ELFIO)-stamp
$(ELFIO)/elfio/elfio_dump.hpp: $(ELFIO)-stamp
$(ELFIO)/elfio/elfio_dynamic.hpp: $(ELFIO)-stamp
$(ELFIO)/elfio/elfio_header.hpp: $(ELFIO)-stamp
$(ELFIO)/elfio/elfio_note.hpp: $(ELFIO)-stamp
$(ELFIO)/elfio/elfio_relocation.hpp: $(ELFIO)-stamp
$(ELFIO)/elfio/elfio_section.hpp: $(ELFIO)-stamp
$(ELFIO)/elfio/elfio_segment.hpp: $(ELFIO)-stamp
$(ELFIO)/elfio/elfio_strings.hpp: $(ELFIO)-stamp
$(ELFIO)/elfio/elfio_symbols.hpp: $(ELFIO)-stamp
$(ELFIO)/elfio/elfio_utils.hpp: $(ELFIO)-stamp
```


- Benjamin Mahler


On July 2, 2016, 8:08 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49559/
> ---
> 
> (Updated July 2, 2016, 8:08 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-5767
> https://issues.apache.org/jira/browse/MESOS-5767
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This includes a patch file to ELFIO to fix 2 off-by-one errors in the
> parsing of NOTE sections. Patches for these bugs have been submitted
> upstream. Details of the patches are below:
> 
> 1) Fixed off-by-one error in 'name' of add_note() function.
> 
> Previously, when assigning 'name' as a string, it's length was
> specified using the full length of 'namesz'. However, this length
> includes the trailing '\0' of the underlying char[]. This ultimately
> causes the C++ string that is created to (incorrectly) contain the
> '\0' character as well. This leads to problems where e.g. the
> following will return false, even when 'name' itself contains the
> string "GNU\0":
> 
>   if (name == "GNU") {
> return true;
>   }
>   return false;
> 
> To fix this, we should only include the length of the string minus the
> trailing '\0'.
> 
> 2) Fixed alignment of 'desc' in add_note() function.
> 
> The ELF spec specifically lists the alignment of the namez char[] to
> be 4 bytes. To quote it:
> 
> "Padding is present, if necessary, to ensure 4-byte alignment for the
> descriptor. Such padding is not included in namesz."
> 
> However, the current implementation sets the alignment to either 4 or
> 8 bytes depending on the class of the ELF file (CLASS32 or CLASS64).
> This commit fixes the alignment to only 4 bytes in all cases.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt 15f3171 
>   3rdparty/Makefile.am bd990cc 
>   3rdparty/cmake/Versions.cmake 7b73f8f 
>   3rdparty/elfio-3.1.patch PRE-CREATION 
>   3rdparty/elfio-3.1.tar.gz PRE-CREATION 
>   3rdparty/versions.am 203656c 
>   LICENSE eb39f6d 
>   configure.ac 321436b 
>   src/Makefile.am 52d63f2 
>   support/coverage.sh ab9564b 
> 
> Diff: https://reviews.apache.org/r/49559/diff/
> 
> 
> Testing
> ---
> 
> Made sure `elfio-3.1` appears in `/build` after running `../configure`, 
> `make`.
> Made sure `elfio` appears in `/include` of installation folder after `make 
> install`.
> 
> This second one is necessary because `stout` relies on elfio in order to 
> function (similar to how we've done with `picojson.h` in the past).
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 49414: Added Executor PID in /containers endpoint. Also Added Test Cases.

[Haris Choudhary]

> On July 2, 2016, 5:46 a.m., Jie Yu wrote:
> > src/tests/containerizer/mesos_containerizer_tests.cpp, line 580
> > 
> >
> > Please use UUID::random here.

I've made the changes. But consider if both UUID's were the same as a result of 
random, the test would fail.
However I think its super rare for both to be generated as the same.


> On July 2, 2016, 5:46 a.m., Jie Yu wrote:
> > src/tests/containerizer/mesos_containerizer_tests.cpp, line 582
> > 
> >
> > Ditto on using UUID::random

See previous comment for UUID


> On July 2, 2016, 5:46 a.m., Jie Yu wrote:
> > src/tests/containerizer/mesos_containerizer_tests.cpp, lines 591-593
> > 
> >
> > NO need for those default variables.

I tried removing None() here but the code wouldnt compile because the arguments 
are required. Could you please elaborate what is required instead? Thanks


- Haris


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


On July 2, 2016, 9:50 p.m., Haris Choudhary wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49414/
> ---
> 
> (Updated July 2, 2016, 9:50 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-5737
> https://issues.apache.org/jira/browse/MESOS-5737
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added Executor PID in /containers endpoint. Also Added Test Cases.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 5b15a89bbac0b33c77e305c5b188823f2704a653 
>   include/mesos/v1/mesos.proto a1435278e81c8f3179d94cd9d2c3dd8c0ba82d35 
>   src/slave/containerizer/mesos/containerizer.cpp 
> a96b382f22886362a1159e1166dfe041072985ba 
>   src/slave/containerizer/mesos/launcher.hpp 
> 05320f462653c31fc2f093d6c67e2182e9c794fa 
>   src/slave/containerizer/mesos/launcher.cpp 
> ff675262af8947b89f8099828665e5e5d86491d8 
>   src/slave/containerizer/mesos/linux_launcher.hpp 
> 89bb2958a41dffe4ade9c2492b9a7412f90a432d 
>   src/slave/containerizer/mesos/linux_launcher.cpp 
> 5028854fa003615f158120e030866b7ec4402b66 
>   src/tests/containerizer/launcher.hpp 
> c352634c4766d289706c7cc738677619d7d02ccd 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> 6c14f6e20b4d14d5ed095670673571739101b0e4 
> 
> Diff: https://reviews.apache.org/r/49414/diff/
> 
> 
> Testing
> ---
> 
> make and make check.
> 
> 
> Thanks,
> 
> Haris Choudhary
> 
>

Re: Review Request 49560: Added ELFIO as bundled dependency in libprocess.

[Benjamin Mahler]

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


Ship it!




Ship It!

- Benjamin Mahler


On July 2, 2016, 7:26 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49560/
> ---
> 
> (Updated July 2, 2016, 7:26 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-5767
> https://issues.apache.org/jira/browse/MESOS-5767
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added ELFIO as bundled dependency in libprocess.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am 41bd8f427706072786f3ab555012b455d299d1fa 
>   3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake 
> fa84e7cd48d61cf30768e6a537db76e47ad8823f 
>   3rdparty/libprocess/configure.ac 41f4537bd70097057b3017dd94a372672476f5f3 
> 
> Diff: https://reviews.apache.org/r/49560/diff/
> 
> 
> Testing
> ---
> 
> Made sure `elfio-3.1` appears in `/build` after running `../configure`, 
> `make`.
> Made sure `elfio` appears in `/include` of installation folder after `make 
> install`.
> 
> This second one is necessary because `stout` relies on `elfio` in order to 
> function (similar to how we've done with `picojson.h` in the past).
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 49414: Added Executor PID in /containers endpoint. Also Added Test Cases.

[Haris Choudhary]

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

(Updated July 2, 2016, 9:50 p.m.)


Review request for mesos and Jie Yu.


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


Repository: mesos


Description (updated)
---

Added Executor PID in /containers endpoint. Also Added Test Cases.


Diffs (updated)
-

  include/mesos/mesos.proto 5b15a89bbac0b33c77e305c5b188823f2704a653 
  include/mesos/v1/mesos.proto a1435278e81c8f3179d94cd9d2c3dd8c0ba82d35 
  src/slave/containerizer/mesos/containerizer.cpp 
a96b382f22886362a1159e1166dfe041072985ba 
  src/slave/containerizer/mesos/launcher.hpp 
05320f462653c31fc2f093d6c67e2182e9c794fa 
  src/slave/containerizer/mesos/launcher.cpp 
ff675262af8947b89f8099828665e5e5d86491d8 
  src/slave/containerizer/mesos/linux_launcher.hpp 
89bb2958a41dffe4ade9c2492b9a7412f90a432d 
  src/slave/containerizer/mesos/linux_launcher.cpp 
5028854fa003615f158120e030866b7ec4402b66 
  src/tests/containerizer/launcher.hpp c352634c4766d289706c7cc738677619d7d02ccd 
  src/tests/containerizer/mesos_containerizer_tests.cpp 
6c14f6e20b4d14d5ed095670673571739101b0e4 

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


Testing
---

make and make check.


Thanks,

Haris Choudhary

Re: Review Request 49562: Reimplemented the stout ELF abstraction in terms of ELFIO.

[Kevin Klues]

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




3rdparty/stout/include/stout/elf.hpp (line 76)


This should be regular `Error()`, not `ErrnoError()`.


- Kevin Klues


On July 2, 2016, 7:31 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49562/
> ---
> 
> (Updated July 2, 2016, 7:31 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-5768
> https://issues.apache.org/jira/browse/MESOS-5768
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> As part of this, we updated the API to include functions written in
> snake_case (as is standard for stout). We also removed the 'close()'
> call, as the new ELFIO library doesn't require it.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/elf.hpp 
> 5382acc1c12901ce8d8256f6010f48579020fbca 
> 
> Diff: https://reviews.apache.org/r/49562/diff/
> 
> 
> Testing
> ---
> 
> Tests coming in subsequent patch with updates to `ldcache_tests.cpp`
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 49561: Added ELFIO as bundled dependency in stout.

[Kevin Klues]

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

(Updated July 2, 2016, 8:29 p.m.)


Review request for mesos and Benjamin Mahler.


Changes
---

Fixed bug in naming of ELFIO tarball


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


Repository: mesos


Description
---

Added ELFIO as bundled dependency in stout.


Diffs (updated)
-

  3rdparty/stout/Makefile.am eeaca480872b97d60891376b8c21fda7b65a6f00 
  3rdparty/stout/cmake/StoutConfigure.cmake 
e4e3217a6f6053d3555d4602f7173c6fe94daeda 
  3rdparty/stout/configure.ac ca6f041b5ad5c92e683500f7b34a521fefa318f4 

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


Testing
---

Made sure `elfio-3.1` appears in `/build` after running `../configure`, `make`.
Made sure `elfio` appears in `/include` of installation folder after `make 
install`.

This second one is necessary because `stout` relies on `elfio` in order to 
function (similar to how we've done with `picojson.h` in the past).


Thanks,

Kevin Klues

Re: Review Request 49558: Added Nvidia License information for our bundled NVML header.

[Benjamin Mahler]

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


Ship it!




Ship It!

- Benjamin Mahler


On July 2, 2016, 8:22 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49558/
> ---
> 
> (Updated July 2, 2016, 8:22 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Vikrama Ditya.
> 
> 
> Bugs: MESOS-5766
> https://issues.apache.org/jira/browse/MESOS-5766
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added Nvidia License information for our bundled NVML header.
> 
> 
> Diffs
> -
> 
>   LICENSE eb39f6d69a165f59c00e8bb0ba9e15be8c958a5b 
> 
> Diff: https://reviews.apache.org/r/49558/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 49558: Added Nvidia License information for our bundled NVML header.

[Kevin Klues]

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

(Updated July 2, 2016, 8:22 p.m.)


Review request for mesos, Benjamin Mahler and Vikrama Ditya.


Changes
---

Remove dependence on discarded patch: `46220`


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


Repository: mesos


Description
---

Added Nvidia License information for our bundled NVML header.


Diffs
-

  LICENSE eb39f6d69a165f59c00e8bb0ba9e15be8c958a5b 

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


Testing
---


Thanks,

Kevin Klues

Re: Review Request 49546: Added device support to Docker::run.

[Kevin Klues]

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


Fix it, then Ship it!





src/docker/docker.cpp (line 677)


If all permissions are `false`, do we still want to include the ":" here? 
Does this work with docker?



src/tests/containerizer/docker_tests.cpp (line 661)


Now I see why this is a little weird to have `NVIDIA_GPU` here. This isn't 
technically a GPU specific test -- it is valid for injecting any device into a 
docker container.

If we wanted to make it non GPU specific. You could probe `/dev` for *some* 
device on the host that's not in docker's default whitelist for devices and 
inject that one instead.


- Kevin Klues


On July 2, 2016, 12:55 a.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49546/
> ---
> 
> (Updated July 2, 2016, 12:55 a.m.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added device support to Docker::run.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 4343213391bb9a86e87410ce3a2c5d8447a1ab8d 
>   src/docker/docker.cpp b356848ea8f65b58102b26b38e2e753c4b665a4b 
>   src/docker/executor.cpp 2089b547408a46c9b4fa91e3ab17b88f7d2d8397 
>   src/slave/containerizer/docker.cpp 915030bdbe5a5b55acc38042ee0475074a602ee0 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> f7b42836ceb80cdae901c4926bd702b6da36a55c 
>   src/tests/containerizer/docker_tests.cpp 
> c0ff9574633f8f72ccae9b9298f3b60bc2f3cf81 
>   src/tests/mesos.hpp 3173bf9cfc832be0aa18d745434afc45d352a269 
>   src/tests/mesos.cpp a433e5b2775f1d62b18927cf71af23ef28d6a503 
> 
> Diff: https://reviews.apache.org/r/49546/diff/
> 
> 
> Testing
> ---
> 
> Added a test.
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 49473: Made control pipe to mesos-containerizer launch optional.

[Gilbert Song]

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


Fix it, then Ship it!




LGTM! Just find some nits not from your patch.


src/slave/containerizer/mesos/launch.cpp (lines 166 - 170)


Not yours, since you are on it, could we fix the indentation?

```
  while ((length = os::read(pipe[0], &dummy, sizeof(dummy))) == -1 &&
 errno == EINTR);
```



src/slave/containerizer/mesos/launch.cpp (lines 372 - 373)


Also not yours. Could we fix the style here? Seems like from 
https://github.com/apache/mesos/commit/81e893d5cdb8a5873dd0c61ad7eceaa323aff466



src/slave/containerizer/mesos/launch.cpp (line 375)


Should we add a TODO here? Consider to use `execvpe`.


- Gilbert Song


On June 30, 2016, 2:22 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49473/
> ---
> 
> (Updated June 30, 2016, 2:22 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Gilbert Song, and Ian Downes.
> 
> 
> Bugs: MESOS-5753
> https://issues.apache.org/jira/browse/MESOS-5753
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is the first step allowing the command executor to directly use
> `mesos-containerizer launch` to launch user task. The control pipe is
> used to synchronize with the parent process. In the command executor,
> this synchronization is not needed. Therefore, we make it optional.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/launch.cpp 
> 83f4d7f28c066a605aa84862eca9fde900ec96c6 
> 
> Diff: https://reviews.apache.org/r/49473/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 49559: Added ELFIO as bundled dependency in Mesos.

[Kevin Klues]

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

(Updated July 2, 2016, 8:08 p.m.)


Review request for mesos and Benjamin Mahler.


Changes
---

Uploaded binary diff for `elfio-3.1.tar.gz`


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


Repository: mesos


Description
---

This includes a patch file to ELFIO to fix 2 off-by-one errors in the
parsing of NOTE sections. Patches for these bugs have been submitted
upstream. Details of the patches are below:

1) Fixed off-by-one error in 'name' of add_note() function.

Previously, when assigning 'name' as a string, it's length was
specified using the full length of 'namesz'. However, this length
includes the trailing '\0' of the underlying char[]. This ultimately
causes the C++ string that is created to (incorrectly) contain the
'\0' character as well. This leads to problems where e.g. the
following will return false, even when 'name' itself contains the
string "GNU\0":

  if (name == "GNU") {
return true;
  }
  return false;

To fix this, we should only include the length of the string minus the
trailing '\0'.

2) Fixed alignment of 'desc' in add_note() function.

The ELF spec specifically lists the alignment of the namez char[] to
be 4 bytes. To quote it:

"Padding is present, if necessary, to ensure 4-byte alignment for the
descriptor. Such padding is not included in namesz."

However, the current implementation sets the alignment to either 4 or
8 bytes depending on the class of the ELF file (CLASS32 or CLASS64).
This commit fixes the alignment to only 4 bytes in all cases.


Diffs (updated)
-

  3rdparty/CMakeLists.txt 15f3171 
  3rdparty/Makefile.am bd990cc 
  3rdparty/cmake/Versions.cmake 7b73f8f 
  3rdparty/elfio-3.1.patch PRE-CREATION 
  3rdparty/elfio-3.1.tar.gz PRE-CREATION 
  3rdparty/versions.am 203656c 
  LICENSE eb39f6d 
  configure.ac 321436b 
  src/Makefile.am 52d63f2 
  support/coverage.sh ab9564b 

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


Testing
---

Made sure `elfio-3.1` appears in `/build` after running `../configure`, `make`.
Made sure `elfio` appears in `/include` of installation folder after `make 
install`.

This second one is necessary because `stout` relies on elfio in order to 
function (similar to how we've done with `picojson.h` in the past).


Thanks,

Kevin Klues

Re: Review Request 49566: Integrated the 'NvidiaVolume' component into 'NvidiaComponents'.

[Mesos ReviewBot]

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



Bad patch!

Reviews applied: [49566, 49565, 49564, 49563, 49562, 49561, 49560, 49559, 
49558, 46220, 45970, 46053, 45969]

Failed command: ./support/apply-review.sh -n -r 46220

Error:
2016-07-02 20:08:11 URL:https://reviews.apache.org/r/46220/diff/raw/ 
[21952/21952] -> "46220.patch" [1]
error: patch failed: docs/home.md:42
error: docs/home.md: patch does not apply

Full log: https://builds.apache.org/job/mesos-reviewbot/14087/console

- Mesos ReviewBot


On July 2, 2016, 7:43 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49566/
> ---
> 
> (Updated July 2, 2016, 7:43 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Yubo Li, and Vikrama Ditya.
> 
> 
> Bugs: MESOS-5401
> https://issues.apache.org/jira/browse/MESOS-5401
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This makes it so that the 'NvidiaVolume' component can be shared
> across containerizers in the same way that the 'NvidiaGpuAllocator' is
> currently being shared.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/containerizer.cpp 
> a436a5d6e367bfd310a003193c9d3770c3fcbac9 
>   src/slave/containerizer/mesos/isolators/gpu/components.hpp 
> 39c6126f5a5023e59b1bd2d90f1aaa47b09d6b28 
> 
> Diff: https://reviews.apache.org/r/49566/diff/
> 
> 
> Testing
> ---
> 
> `GTEST_FILTER="*NVIDIA_GPU_Volume*" make -j check`
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 49545: Updated code to use Path::string() instead of Path::value.

[Kevin Klues]

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



Assuming we still want to have the `.string()` function on the `Path` class, 
this patch looks fine.  I still think casting to a string seems cleaner than 
having a member function called `.string()` though.

- Kevin Klues


On July 2, 2016, 12:55 a.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49545/
> ---
> 
> (Updated July 2, 2016, 12:55 a.m.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated code to use Path::string() instead of Path::value.
> 
> 
> Diffs
> -
> 
>   src/credentials/credentials.hpp 253ea8353ac67fde40268e35ea16f773f11eb509 
>   src/linux/systemd.cpp 3e1ba935763383ba5d5bf9fad21a948a771101a0 
>   src/slave/containerizer/fetcher.cpp 
> 7b52cb1703e5b8a6ce0e995896fab3cd520a5090 
>   src/slave/containerizer/mesos/provisioner/appc/fetcher.cpp 
> b29fc79de7f1915778e43a7f4e72c2754bb64ab3 
>   src/slave/containerizer/mesos/provisioner/provisioner.cpp 
> 249acad49122d988e44744384bcf840b941c0997 
>   src/tests/common/command_utils_tests.cpp 
> c91a7f38263e4c8352ec56f79070baec2b1cb06f 
>   src/tests/fetcher_cache_tests.cpp ba7d3f84ee54d982d7dfe0eeb9c67c1a68f9cf40 
>   src/watcher/whitelist_watcher.cpp b252f2f1b35113996f72c2ff92bcd6a3728d02f9 
> 
> Diff: https://reviews.apache.org/r/49545/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 49544: Made stout::Path default constructible / assignable / copyable.

[Kevin Klues]

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




3rdparty/stout/include/stout/path.hpp (lines 255 - 261)


Do we really need this function? It looks like just casting it to a string 
would be enough to turn it into a string?



3rdparty/stout/include/stout/path.hpp (line 267)


If e just cast it to a string, this should be sufficeint.



3rdparty/stout/include/stout/path.hpp (line 279)


ditto



3rdparty/stout/include/stout/path.hpp (line 305)


ditto


- Kevin Klues


On July 2, 2016, 12:46 a.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49544/
> ---
> 
> (Updated July 2, 2016, 12:46 a.m.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This makes Path closer to boost's path, and more convenient to work with 
> (e.g. as map values, struct members, etc).
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/path.hpp 
> 0ca13c6a1aebcb980c29dd33414bfa50c4b8bf81 
> 
> Diff: https://reviews.apache.org/r/49544/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Review Request 49566: Integrated the 'NvidiaVolume' component into 'NvidiaComponents'.

[Kevin Klues]

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

Review request for mesos, Benjamin Mahler, Yubo Li, and Vikrama Ditya.


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


Repository: mesos


Description
---

This makes it so that the 'NvidiaVolume' component can be shared
across containerizers in the same way that the 'NvidiaGpuAllocator' is
currently being shared.


Diffs
-

  src/slave/containerizer/containerizer.cpp 
a436a5d6e367bfd310a003193c9d3770c3fcbac9 
  src/slave/containerizer/mesos/isolators/gpu/components.hpp 
39c6126f5a5023e59b1bd2d90f1aaa47b09d6b28 

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


Testing
---

`GTEST_FILTER="*NVIDIA_GPU_Volume*" make -j check`


Thanks,

Kevin Klues

Review Request 49565: Added a new 'NvidiaVolume' component.

[Kevin Klues]

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

Review request for mesos, Benjamin Mahler, Yubo Li, and Vikrama Ditya.


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


Repository: mesos


Description
---

This component is responsible for building up a consolidated volume of
binaries / libraries from the user-space portion of the Nvidia GPU
drivers so that it can be injected into a container as a single
volume. Its core functionality mimics that of the
`nvidia-docker-plugin`: https://github.com/NVIDIA/nvidia-docker/

We currently only implement the 'create()' function which is
responsible for building up the volume itself. In a subsequent commit
we will add support for reading a Docker ImageManifest and deciding if
we should inject the volume into the docker container or not.


Diffs
-

  src/CMakeLists.txt 09dd0e0dd2f25fb71db5051d5f5a797a981c2e59 
  src/Makefile.am 52d63f26e0455ef31411e964e497a509d96aaf4a 
  src/slave/containerizer/mesos/isolators/gpu/nvidia.hpp 
01cdab2f87d3ac823b8647c084c481593b6fa07f 
  src/slave/containerizer/mesos/isolators/gpu/volume.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/gpu/volume.cpp PRE-CREATION 
  src/tests/containerizer/nvidia_gpu_isolator_tests.cpp 
6770f05455f19d12b7984162f93a8b458951926f 

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


Testing
---

`GTEST_FILTER="*NVIDIA_GPU_Volume*" make -j check`


Thanks,

Kevin Klues

Review Request 49564: Added `get_abi_version()` to the ELF parsing abstraction.

[Kevin Klues]

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

Review request for mesos and Benjamin Mahler.


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


Repository: mesos


Description
---

This function returns the ABI version of the ELF file by parsing the
contents of its `.note.ABI-tag` section. This section is Linux
specific and adheres to the format described in the links below.

https://refspecs.linuxfoundation.org/LSB_1.2.0/gLSB/noteabitag.html
http://flint.cs.yale.edu/cs422/doc/ELF_Format.pdf


Diffs
-

  3rdparty/stout/include/stout/elf.hpp 5382acc1c12901ce8d8256f6010f48579020fbca 
  src/tests/ldcache_tests.cpp b387a8b763d241075392f65256d9b39429909d8b 

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


Testing
---

`GTEST_FILTER="*Ldcache*" make -j check`


Thanks,

Kevin Klues

Review Request 49563: Reimplemented ldcache_test.cpp using the new ELF abstraction in stout.

[Kevin Klues]

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

Review request for mesos and Benjamin Mahler.


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


Repository: mesos


Description
---

We also reenabled this test to run as part of the standard Linux test
suite. It had been disabled due to its dependence on the external
libelf library. Now that we bundle ELFIO instead of relying on this
external dependence, we can always have it enabled now.


Diffs
-

  src/Makefile.am 52d63f26e0455ef31411e964e497a509d96aaf4a 
  src/tests/ldcache_tests.cpp b387a8b763d241075392f65256d9b39429909d8b 

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


Testing
---

`GTEST_FILTER="LdcacheTest*" make -j check`


Thanks,

Kevin Klues

Review Request 49562: Reimplemented the stout ELF abstraction in terms of ELFIO.

[Kevin Klues]

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

Review request for mesos and Benjamin Mahler.


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


Repository: mesos


Description
---

As part of this, we updated the API to include functions written in
snake_case (as is standard for stout). We also removed the 'close()'
call, as the new ELFIO library doesn't require it.


Diffs
-

  3rdparty/stout/include/stout/elf.hpp 5382acc1c12901ce8d8256f6010f48579020fbca 

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


Testing
---

Tests coming in subsequent patch with updates to `ldcache_tests.cpp`


Thanks,

Kevin Klues

Review Request 49561: Added ELFIO as bundled dependency in stout.

[Kevin Klues]

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

Review request for mesos and Benjamin Mahler.


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


Repository: mesos


Description
---

Added ELFIO as bundled dependency in stout.


Diffs
-

  3rdparty/stout/Makefile.am eeaca480872b97d60891376b8c21fda7b65a6f00 
  3rdparty/stout/cmake/StoutConfigure.cmake 
e4e3217a6f6053d3555d4602f7173c6fe94daeda 
  3rdparty/stout/configure.ac ca6f041b5ad5c92e683500f7b34a521fefa318f4 

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


Testing
---

Made sure `elfio-3.1` appears in `/build` after running `../configure`, `make`.
Made sure `elfio` appears in `/include` of installation folder after `make 
install`.

This second one is necessary because `stout` relies on `elfio` in order to 
function (similar to how we've done with `picojson.h` in the past).


Thanks,

Kevin Klues

Review Request 49560: Added ELFIO as bundled dependency in libprocess.

[Kevin Klues]

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

Review request for mesos and Benjamin Mahler.


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


Repository: mesos


Description
---

Added ELFIO as bundled dependency in libprocess.


Diffs
-

  3rdparty/libprocess/Makefile.am 41bd8f427706072786f3ab555012b455d299d1fa 
  3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake 
fa84e7cd48d61cf30768e6a537db76e47ad8823f 
  3rdparty/libprocess/configure.ac 41f4537bd70097057b3017dd94a372672476f5f3 

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


Testing
---

Made sure `elfio-3.1` appears in `/build` after running `../configure`, `make`.
Made sure `elfio` appears in `/include` of installation folder after `make 
install`.

This second one is necessary because `stout` relies on `elfio` in order to 
function (similar to how we've done with `picojson.h` in the past).


Thanks,

Kevin Klues

Review Request 49559: Added ELFIO as bundled dependency in Mesos.

[Kevin Klues]

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

Review request for mesos and Benjamin Mahler.


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


Repository: mesos


Description
---

This includes a patch file to ELFIO to fix 2 off-by-one errors in the
parsing of NOTE sections. Patches for these bugs have been submitted
upstream. Details of the patches are below:

1) Fixed off-by-one error in 'name' of add_note() function.

Previously, when assigning 'name' as a string, it's length was
specified using the full length of 'namesz'. However, this length
includes the trailing '\0' of the underlying char[]. This ultimately
causes the C++ string that is created to (incorrectly) contain the
'\0' character as well. This leads to problems where e.g. the
following will return false, even when 'name' itself contains the
string "GNU\0":

  if (name == "GNU") {
return true;
  }
  return false;

To fix this, we should only include the length of the string minus the
trailing '\0'.

2) Fixed alignment of 'desc' in add_note() function.

The ELF spec specifically lists the alignment of the namez char[] to
be 4 bytes. To quote it:

"Padding is present, if necessary, to ensure 4-byte alignment for the
descriptor. Such padding is not included in namesz."

However, the current implementation sets the alignment to either 4 or
8 bytes depending on the class of the ELF file (CLASS32 or CLASS64).
This commit fixes the alignment to only 4 bytes in all cases.


Diffs
-

  3rdparty/CMakeLists.txt 15f3171bb8c7ab1ca28b047a552e80492dd1b157 
  3rdparty/Makefile.am bd990cc6298c6b564df8f152673d42a2a979fb55 
  3rdparty/cmake/Versions.cmake 7b73f8f4a0c4180f7ed037efb1076f967981f2f0 
  3rdparty/elfio-3.1.patch PRE-CREATION 
  3rdparty/elfio-3.1.tar.gz PRE-CREATION 
  3rdparty/versions.am 203656cd1c55576dd9d6302dfa7e076434ef089b 
  LICENSE eb39f6d69a165f59c00e8bb0ba9e15be8c958a5b 
  configure.ac 321436beb8ad87bb5727932eb2943986fe558237 
  src/Makefile.am 52d63f26e0455ef31411e964e497a509d96aaf4a 
  support/coverage.sh ab9564bc1b921da64c70c14915067c5d3e683808 

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


Testing
---

Made sure `elfio-3.1` appears in `/build` after running `../configure`, `make`.
Made sure `elfio` appears in `/include` of installation folder after `make 
install`.

This second one is necessary because `stout` relies on elfio in order to 
function (similar to how we've done with `picojson.h` in the past).


Thanks,

Kevin Klues

Review Request 49558: Added Nvidia License information for our bundled NVML header.

[Kevin Klues]

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

Review request for mesos, Benjamin Mahler and Vikrama Ditya.


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


Repository: mesos


Description
---

Added Nvidia License information for our bundled NVML header.


Diffs
-

  LICENSE eb39f6d69a165f59c00e8bb0ba9e15be8c958a5b 

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


Testing
---


Thanks,

Kevin Klues

Re: Review Request 49370: Updateted documentation for roles endpoint filtering.

[Joerg Schad]

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

(Updated July 2, 2016, 5:41 p.m.)


Review request for mesos, Adam B and Vinod Kone.


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


Repository: mesos


Description
---

Updateted documentation for roles endpoint filtering.


Diffs (updated)
-

  CHANGELOG 47a9dcbf5a45441e15d3e6b4a574696975efcaa9 
  docs/authorization.md 9bd6031d2a3aef921fa4e3f6683cc5c234832d47 
  src/master/http.cpp 528f01f2e00ddbd15da1cc4fca27b5347c9fc798 

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


Testing
---

Viewed docs via docker website container.


Thanks,

Joerg Schad

Re: Review Request 49369: Introduced authorization based filtering for /roles.

[Joerg Schad]

> On July 1, 2016, 6:58 p.m., Vinod Kone wrote:
> > src/master/http.cpp, lines 2908-2950
> > 
> >
> > not completely yours, but would you mind refactoring this part into 
> > `_roles()` so that both `roles()` and `getRoles()` can leverage it instead 
> > of duplicating the code?
> > 
> > ```
> > Future> _roles()
> > {
> >// Returns a filtered list of roles.
> > }
> > 
> > ```

Sure (that was already on my "when I have time TODO-list"), I would just prefer 
to that in a seperate review as it is has a different focus than this PR. So I 
would drop this issue here for now.


- Joerg


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


On July 1, 2016, 2:39 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49369/
> ---
> 
> (Updated July 1, 2016, 2:39 p.m.)
> 
> 
> Review request for mesos, Adam B and Vinod Kone.
> 
> 
> Bugs: MESOS-5709
> https://issues.apache.org/jira/browse/MESOS-5709
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously only /weights was filtered but /roles actually
> contains the same information and hence both endpoints
> should be filtered in the same way.
> As part of this work we renamed the GetWeight action to
> ViewRole (and same for the acls).
> 
> 
> Diffs
> -
> 
>   include/mesos/authorizer/acls.proto 
> 31d5c144f92749d685dbf75b65362bd633c54ea5 
>   include/mesos/authorizer/authorizer.proto 
> e22d3a491e0c5e7c384af8635a7f555e2194b939 
>   src/authorizer/local/authorizer.cpp 
> aadb7f6f0889514060744a7eca96cda00639eb4e 
>   src/common/http.hpp eb2d0156550b3ed90c7f72a2e1405efc45eab78a 
>   src/common/http.cpp 2ef264fc886be50d2091dbe24ab422c818b5ee0b 
>   src/master/http.cpp 528f01f2e00ddbd15da1cc4fca27b5347c9fc798 
>   src/master/weights_handler.cpp 5fc69c6a1c2663bad1774d9376b0aa9f90fa7275 
>   src/tests/authorization_tests.cpp c1e8ea64bb9008163ae8f0488306687d0f63a527 
>   src/tests/dynamic_weights_tests.cpp 
> c67ed75a050b9db5575ac2bb6100bcf01cfc04ff 
>   src/tests/master_authorization_tests.cpp 
> 0807469763b14f39b594769341d5eb9678d26946 
> 
> Diff: https://reviews.apache.org/r/49369/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 49554: Fixed HealthCheck typo in `launcher/executor.cpp`.

[Timothy Chen]

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


Ship it!




Ship It!

- Timothy Chen


On July 2, 2016, 3:12 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49554/
> ---
> 
> (Updated July 2, 2016, 3:12 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Gilbert Song, 
> Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-5727
> https://issues.apache.org/jira/browse/MESOS-5727
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed HealthCheck typo in `launcher/executor.cpp`.
> 
> 
> Diffs
> -
> 
>   src/launcher/executor.cpp 63010252ffd6b1dfbe3358a1f1f4447967b824d2 
> 
> Diff: https://reviews.apache.org/r/49554/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 49553: Fixed indentions of HealthCheck files in src/Makefile.am.

[Timothy Chen]

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


Ship it!




Ship It!

- Timothy Chen


On July 2, 2016, 3:12 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49553/
> ---
> 
> (Updated July 2, 2016, 3:12 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Gilbert Song, 
> Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-5727
> https://issues.apache.org/jira/browse/MESOS-5727
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed indentions of HealthCheck files in src/Makefile.am.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 52d63f26e0455ef31411e964e497a509d96aaf4a 
> 
> Diff: https://reviews.apache.org/r/49553/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 49424: Added an abstraction os::raw::Argv in stout.

[Joris Van Remoortere]

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




3rdparty/stout/include/stout/os/raw/argv.hpp (line 27)


I know you want to say `NULL` to be explicit here. Consider using `nullptr` 
to avoid tripping people grepping for `NULL`.



3rdparty/stout/include/stout/os/raw/argv.hpp (line 40)


It seems like this extra vector and the subsequent copy into the argv array 
is only necessary because you're supporting structures with an unknown size.

Would this be simpler if you restricted to inputs that support `.size()`?



3rdparty/stout/include/stout/os/raw/argv.hpp (line 43)


Wouldn't hurt to have a small comment here mentioning that `strcpy` does 
the null termination.


- Joris Van Remoortere


On June 30, 2016, 4:18 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49424/
> ---
> 
> (Updated June 30, 2016, 4:18 p.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added an abstraction os::raw::Argv in stout.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/Makefile.am f10c836c1ac008cc4055741648b5e7dd697e4c1e 
>   3rdparty/stout/include/stout/os.hpp 
> 53b00932693fba7cf6514da6a519269a904de345 
>   3rdparty/stout/include/stout/os/raw/argv.hpp PRE-CREATION 
>   3rdparty/stout/tests/os_tests.cpp 786da14addf62be5f5270f156fb1e011d3f403e3 
> 
> Diff: https://reviews.apache.org/r/49424/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 49436: Added test cases for tcp health check.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [49553, 49554, 49555, 49556, 49351, 36816, 49360, 49434, 
49435, 49436]

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 July 2, 2016, 3:14 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49436/
> ---
> 
> (Updated July 2, 2016, 3:14 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Gilbert Song, 
> Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-3567
> https://issues.apache.org/jira/browse/MESOS-3567
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test cases for tcp health check.
> 
> 
> Diffs
> -
> 
>   src/tests/health_check_tests.cpp 3e8e41badc33d7fca1e0b19f823a0aa353a64967 
> 
> Diff: https://reviews.apache.org/r/49436/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 49516: Refactor Master::Http::getExecutors into helper function.

[haosdent huang]

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


Ship it!




Ship It!

- haosdent huang


On July 1, 2016, 11:45 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49516/
> ---
> 
> (Updated July 1, 2016, 11:45 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: MESOS-5489
> https://issues.apache.org/jira/browse/MESOS-5489
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Helper function will be reused by `GetExecutors` and `GetState`.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 7b2f77b9264242f31ab62eb9db7c621a1b8aa2fe 
>   src/master/master.hpp be7cd239c49a0710a29a8187c01484f9f6d615e1 
> 
> Diff: https://reviews.apache.org/r/49516/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 49489: Refactor master::Http::getFrameworks to helper function.

[haosdent huang]

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


Ship it!




Ship It!

- haosdent huang


On July 1, 2016, 11:42 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49489/
> ---
> 
> (Updated July 1, 2016, 11:42 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The new helper function will be reused by `GET_FRAMEWORKS` and
> `GET_STATE` calls.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 7b2f77b9264242f31ab62eb9db7c621a1b8aa2fe 
>   src/master/master.hpp be7cd239c49a0710a29a8187c01484f9f6d615e1 
> 
> Diff: https://reviews.apache.org/r/49489/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 49489: Refactor master::Http::getFrameworks to helper function.

[haosdent huang]

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


Ship it!




Ship It!

- haosdent huang


On July 1, 2016, 11:42 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49489/
> ---
> 
> (Updated July 1, 2016, 11:42 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The new helper function will be reused by `GET_FRAMEWORKS` and
> `GET_STATE` calls.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 7b2f77b9264242f31ab62eb9db7c621a1b8aa2fe 
>   src/master/master.hpp be7cd239c49a0710a29a8187c01484f9f6d615e1 
> 
> Diff: https://reviews.apache.org/r/49489/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 49488: Refactor Master::Http::getAgents into helper function.

[haosdent huang]

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


Ship it!




Ship It!

- haosdent huang


On July 1, 2016, 11:34 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49488/
> ---
> 
> (Updated July 1, 2016, 11:34 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The new helper function will be reused by both `GET_AGENTS`
> and `GET_STATE` calls.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 7b2f77b9264242f31ab62eb9db7c621a1b8aa2fe 
>   src/master/master.hpp be7cd239c49a0710a29a8187c01484f9f6d615e1 
> 
> Diff: https://reviews.apache.org/r/49488/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 49487: Refactor Master::Http::getTasks into helper function.

[haosdent huang]

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


Ship it!




Ship It!

- haosdent huang


On July 1, 2016, 11:32 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49487/
> ---
> 
> (Updated July 1, 2016, 11:32 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, haosdent huang, and Vinod Kone.
> 
> 
> Bugs: MESOS-5489
> https://issues.apache.org/jira/browse/MESOS-5489
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Helper function will be also be reused for `GetState`.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 7b2f77b9264242f31ab62eb9db7c621a1b8aa2fe 
>   src/master/master.hpp be7cd239c49a0710a29a8187c01484f9f6d615e1 
> 
> Diff: https://reviews.apache.org/r/49487/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 49443: Include a function to construct FileInfo protobuf message.

[haosdent huang]

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


Ship it!




Ship It!

- haosdent huang


On July 1, 2016, 7:49 p.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49443/
> ---
> 
> (Updated July 1, 2016, 7:49 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, zhou xing, haosdent huang, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-5487
> https://issues.apache.org/jira/browse/MESOS-5487
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Include a function to construct FileInfo protobuf message.
> 
> 
> Diffs
> -
> 
>   src/common/protobuf_utils.hpp 3dd03da3fea10a01aa4508f53c5d1ebcc3d6a2a4 
>   src/common/protobuf_utils.cpp 040bdf82134289f0caf63e11c2ce8e7853a392b3 
> 
> Diff: https://reviews.apache.org/r/49443/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 49556: Removed the binary way of HealthCheck.

[haosdent huang]

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

(Updated July 2, 2016, 3:13 p.m.)


Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Gilbert Song, 
Jie Yu, and Timothy Chen.


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


Repository: mesos


Description
---

Removed the binary way of HealthCheck.


Diffs
-

  src/Makefile.am 52d63f26e0455ef31411e964e497a509d96aaf4a 
  src/health-check/main.cpp 0b139262ac4f12504202fd5f967724e52ecd1a08 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 49555: Updated docker executor to use HealthCheck via library way.

[haosdent huang]

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

(Updated July 2, 2016, 3:13 p.m.)


Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Gilbert Song, 
Jie Yu, and Timothy Chen.


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


Repository: mesos


Description
---

Updated docker executor to use HealthCheck via library way.


Diffs
-

  src/docker/executor.cpp 2089b547408a46c9b4fa91e3ab17b88f7d2d8397 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 49553: Fixed indentions of HealthCheck files in src/Makefile.am.

[haosdent huang]

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

(Updated July 2, 2016, 3:12 p.m.)


Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Gilbert Song, 
Jie Yu, and Timothy Chen.


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


Repository: mesos


Description
---

Fixed indentions of HealthCheck files in src/Makefile.am.


Diffs
-

  src/Makefile.am 52d63f26e0455ef31411e964e497a509d96aaf4a 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 49554: Fixed HealthCheck typo in `launcher/executor.cpp`.

[haosdent huang]

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

(Updated July 2, 2016, 3:12 p.m.)


Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Gilbert Song, 
Jie Yu, and Timothy Chen.


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


Repository: mesos


Description
---

Fixed HealthCheck typo in `launcher/executor.cpp`.


Diffs
-

  src/launcher/executor.cpp 63010252ffd6b1dfbe3358a1f1f4447967b824d2 

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


Testing
---


Thanks,

haosdent huang

Review Request 49556: Removed the binary way of HealthCheck.

[haosdent huang]

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

Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Gilbert Song, 
Jie Yu, and Timothy Chen.


Repository: mesos


Description
---

Removed the binary way of HealthCheck.


Diffs
-

  src/Makefile.am 52d63f26e0455ef31411e964e497a509d96aaf4a 
  src/health-check/main.cpp 0b139262ac4f12504202fd5f967724e52ecd1a08 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 49436: Added test cases for tcp health check.

[haosdent huang]

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

(Updated July 2, 2016, 3:10 p.m.)


Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Gilbert Song, 
and Timothy Chen.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Added test cases for tcp health check.


Diffs (updated)
-

  src/tests/health_check_tests.cpp 3e8e41badc33d7fca1e0b19f823a0aa353a64967 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 49435: Added test cases for http health check.

[haosdent huang]

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

(Updated July 2, 2016, 3:08 p.m.)


Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Gilbert Song, 
and Timothy Chen.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Added test cases for http health check.


Diffs (updated)
-

  src/tests/health_check_tests.cpp 3e8e41badc33d7fca1e0b19f823a0aa353a64967 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 49434: Clean up `HealthCheckTest.populateTasks` methods.

[haosdent huang]

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

(Updated July 2, 2016, 3:08 p.m.)


Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Gilbert Song, 
and Timothy Chen.


Changes
---

Rebase.


Repository: mesos


Description
---

Clean up `HealthCheckTest.populateTasks` methods.


Diffs (updated)
-

  src/tests/health_check_tests.cpp 3e8e41badc33d7fca1e0b19f823a0aa353a64967 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 49360: Supported TCP check in health check.

[haosdent huang]

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

(Updated July 2, 2016, 3:08 p.m.)


Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Gilbert Song, 
and Timothy Chen.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Supported TCP check in health check.


Diffs (updated)
-

  include/mesos/mesos.proto 5b15a89bbac0b33c77e305c5b188823f2704a653 
  include/mesos/v1/mesos.proto a1435278e81c8f3179d94cd9d2c3dd8c0ba82d35 
  src/health-check/health_checker.hpp f61e80d8e5eb09f8f71e80b6600f7c5b1548bd62 
  src/health-check/health_checker.cpp 585a0b565d948cfa292bad818a710501a4ce0daf 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 36816: Supported HTTP/HTTPS in health check.

[haosdent huang]

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

(Updated July 2, 2016, 3:07 p.m.)


Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Gilbert Song, 
and Timothy Chen.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Supported HTTP/HTTPS in health check.


Diffs (updated)
-

  include/mesos/mesos.proto 5b15a89bbac0b33c77e305c5b188823f2704a653 
  include/mesos/v1/mesos.proto a1435278e81c8f3179d94cd9d2c3dd8c0ba82d35 
  src/health-check/health_checker.hpp f61e80d8e5eb09f8f71e80b6600f7c5b1548bd62 

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


Testing
---

* Add unit test cases: HealthCheckTest.HealthyTaskViaHttp and 
HealthCheckTest.HealthyTaskNotMatchHttpStatuses
make check


Thanks,

haosdent huang

Re: Review Request 49351: Clean up HealthCheck code in docker executor.

[haosdent huang]

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

(Updated July 2, 2016, 3:07 p.m.)


Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Gilbert Song, 
and Timothy Chen.


Changes
---

Rebase.


Summary (updated)
-

Clean up HealthCheck code in docker executor.


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


Repository: mesos


Description (updated)
---

Clean up HealthCheck code in docker executor.


Diffs (updated)
-

  src/docker/executor.cpp 2089b547408a46c9b4fa91e3ab17b88f7d2d8397 

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


Testing
---


Thanks,

haosdent huang

Review Request 49554: Fixed HealthCheck typo in `launcher/executor.cpp`.

[haosdent huang]

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

Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Gilbert Song, 
Jie Yu, and Timothy Chen.


Repository: mesos


Description
---

Fixed HealthCheck typo in `launcher/executor.cpp`.


Diffs
-

  src/launcher/executor.cpp 63010252ffd6b1dfbe3358a1f1f4447967b824d2 

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


Testing
---


Thanks,

haosdent huang

Review Request 49555: Updated docker executor to use HealthCheck via library way.

[haosdent huang]

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

Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Gilbert Song, 
Jie Yu, and Timothy Chen.


Repository: mesos


Description
---

Updated docker executor to use HealthCheck via library way.


Diffs
-

  src/docker/executor.cpp 2089b547408a46c9b4fa91e3ab17b88f7d2d8397 

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


Testing
---


Thanks,

haosdent huang

Review Request 49553: Fixed indentions of HealthCheck files in src/Makefile.am.

[haosdent huang]

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

Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Gilbert Song, 
Jie Yu, and Timothy Chen.


Repository: mesos


Description
---

Fixed indentions of HealthCheck files in src/Makefile.am.


Diffs
-

  src/Makefile.am 52d63f26e0455ef31411e964e497a509d96aaf4a 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 49426: Added implementation to Appc runtime isolator.

[Mesos ReviewBot]

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



Bad review!

Reviews applied: []

Error:
No reviewers specified. Please find a reviewer by asking on JIRA or the mailing 
list.

- Mesos ReviewBot


On June 30, 2016, 6:01 a.m., Srinivas Brahmaroutu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49426/
> ---
> 
> (Updated June 30, 2016, 6:01 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added implementation to Appc runtime isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/appc/runtime.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/49426/diff/
> 
> 
> Testing
> ---
> 
> Make Check
> 
> 
> Thanks,
> 
> Srinivas Brahmaroutu
> 
>

Re: Review Request 49552: Fixed typo in docker runtime.cpp.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [49552]

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 July 2, 2016, 9:39 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49552/
> ---
> 
> (Updated July 2, 2016, 9:39 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed typo in docker runtime.cpp.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/docker/runtime.cpp 
> 5af0ae93526af41ec54263fd44b5c2b55a7011a4 
> 
> Diff: https://reviews.apache.org/r/49552/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 49414: Added Executor PID in /containers endpoint. Also Added Test Cases.

[Guangya Liu]

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



Seems most comments are style issue, so I would suggest that you go through the 
document here 
https://github.com/apache/mesos/blob/master/docs/c%2B%2B-style-guide.md to get 
more details.


include/mesos/mesos.proto (line 1873)


Period to the end



include/mesos/v1/mesos.proto (line 1872)


ditto



src/slave/containerizer/mesos/launcher.hpp (line 79)


period to the end



src/slave/containerizer/mesos/launcher.cpp (lines 175 - 176)


new line here



src/slave/containerizer/mesos/linux_launcher.cpp (lines 372 - 373)


new line here



src/tests/containerizer/mesos_containerizer_tests.cpp (lines 567 - 568)


period to the end.

Ditto for the following of all the new added comments.


- Guangya Liu


On 六月 30, 2016, 12:51 a.m., Haris Choudhary wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49414/
> ---
> 
> (Updated 六月 30, 2016, 12:51 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-5737
> https://issues.apache.org/jira/browse/MESOS-5737
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added Executor PID in /containers endpoint. Also Added Test Cases.
> 
> In order to greatly simplify the implementation for the Mesos CLI's container 
> plugin, we need the executor PID (Process ID) to be exposed in the 
> /containers endpoint. [Mesos CLI 
> Epic](https://issues.apache.org/jira/browse/MESOS-5676)
> This change will introduce the pid for an executor if it was launched by the 
> mesos containerizer in the /containers endpoint of an agent.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 5b15a89bbac0b33c77e305c5b188823f2704a653 
>   include/mesos/v1/mesos.proto a1435278e81c8f3179d94cd9d2c3dd8c0ba82d35 
>   src/slave/containerizer/mesos/containerizer.cpp 
> a96b382f22886362a1159e1166dfe041072985ba 
>   src/slave/containerizer/mesos/launcher.hpp 
> 05320f462653c31fc2f093d6c67e2182e9c794fa 
>   src/slave/containerizer/mesos/launcher.cpp 
> ff675262af8947b89f8099828665e5e5d86491d8 
>   src/slave/containerizer/mesos/linux_launcher.hpp 
> 89bb2958a41dffe4ade9c2492b9a7412f90a432d 
>   src/slave/containerizer/mesos/linux_launcher.cpp 
> 5028854fa003615f158120e030866b7ec4402b66 
>   src/tests/containerizer/launcher.hpp 
> c352634c4766d289706c7cc738677619d7d02ccd 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> 6c14f6e20b4d14d5ed095670673571739101b0e4 
> 
> Diff: https://reviews.apache.org/r/49414/diff/
> 
> 
> Testing
> ---
> 
> make and make check.
> 
> 
> Thanks,
> 
> Haris Choudhary
> 
>

Re: Review Request 49348: Added implementation to Appc Runtime Isolator.

[Guangya Liu]

> On 七月 2, 2016, 10:19 a.m., Guangya Liu wrote:
> > src/slave/containerizer/mesos/isolators/appc/runtime.cpp, lines 202-205
> > 
> >
> > 1. s/Exec/'Exec'
> > 2. Add period to the end of each comment.
> > 
> > It would be great to add more comments to the end for the commnets by 
> > refering to the row of the table.
> > 
> > Such as:
> > // 1. If 'shell' is false,  and 'value' is not set, use Exec from the 
> > image. (row 1-2)
> > // 2. If 'shell' is false and 'value' is set, ignore Exec from the 
> > image. (row 3-4)
> > // 3. If 'shell' is true and 'value' is not set, return error. (row 5-6)
> > // 4. If 'shell' is true and 'value' is set, ignore Exec from the 
> > image. (row 7-8)

The period should be appended to the end.

// 1. If 'shell' is false,  and 'value' is not set, use Exec from the image 
(row 1-2).
// 2. If 'shell' is false and 'value' is set, ignore Exec from the image (row 
3-4).
// 3. If 'shell' is true and 'value' is not set, return error (row 5-6).
// 4. If 'shell' is true and 'value' is set, ignore Exec from the image (row 
7-8).


- Guangya


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


On 七月 1, 2016, 9:44 p.m., Srinivas Brahmaroutu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49348/
> ---
> 
> (Updated 七月 1, 2016, 9:44 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added implementation to Appc Runtime Isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/appc/runtime.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/49348/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Srinivas Brahmaroutu
> 
>

Re: Review Request 49426: Added implementation to Appc runtime isolator.

[Guangya Liu]

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



Srini, do you want to drop this as this is same with 
https://reviews.apache.org/r/49348/ now.

- Guangya Liu


On 六月 30, 2016, 6:01 a.m., Srinivas Brahmaroutu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49426/
> ---
> 
> (Updated 六月 30, 2016, 6:01 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added implementation to Appc runtime isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/appc/runtime.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/49426/diff/
> 
> 
> Testing
> ---
> 
> Make Check
> 
> 
> Thanks,
> 
> Srinivas Brahmaroutu
> 
>

Re: Review Request 49348: Added implementation to Appc Runtime Isolator.

[Guangya Liu]

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




src/slave/containerizer/mesos/isolators/appc/runtime.cpp (line 80)


As your previous patches are using `Appc` in comments, can you please use 
`Appc` here and other comments as well?



src/slave/containerizer/mesos/isolators/appc/runtime.cpp (lines 87 - 88)


One line



src/slave/containerizer/mesos/isolators/appc/runtime.cpp (line 161)


Can you please add a comment here just like what we did for docker run time 
isolator for how to handle duplicate env vars?

// Keep all environment from runtime isolator. If there exists
// environment variable duplicate cases, it will be overwritten
// in mesos containerizer.



src/slave/containerizer/mesos/isolators/appc/runtime.cpp (line 162)


kill this line



src/slave/containerizer/mesos/isolators/appc/runtime.cpp (line 171)


s/form/from



src/slave/containerizer/mesos/isolators/appc/runtime.cpp (line 192)


kill this line



src/slave/containerizer/mesos/isolators/appc/runtime.cpp (lines 202 - 205)


1. s/Exec/'Exec'
2. Add period to the end of each comment.

It would be great to add more comments to the end for the commnets by 
refering to the row of the table.

Such as:
// 1. If 'shell' is false,  and 'value' is not set, use Exec from the 
image. (row 1-2)
// 2. If 'shell' is false and 'value' is set, ignore Exec from the image. 
(row 3-4)
// 3. If 'shell' is true and 'value' is not set, return error. (row 5-6)
// 4. If 'shell' is true and 'value' is set, ignore Exec from the image. 
(row 7-8)



src/slave/containerizer/mesos/isolators/appc/runtime.cpp (line 243)


I think checking `!command.has_value()` is good enough. The logic here has 
some problem, if the comand does not have value then the check of 
`comand.value()` may core dump here.

if (!command.has_value()) {
  return Error("Shell specified but no command value provided");
}

Or do you mean `if (!command.has_value() || !command.value().empty()) {` 
here.



src/slave/containerizer/mesos/isolators/appc/runtime.cpp (lines 257 - 258)


new line here



src/slave/containerizer/mesos/isolators/appc/runtime.cpp (line 282)


s/WorkingDir/workingDirectory


- Guangya Liu


On 七月 1, 2016, 9:44 p.m., Srinivas Brahmaroutu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49348/
> ---
> 
> (Updated 七月 1, 2016, 9:44 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added implementation to Appc Runtime Isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/appc/runtime.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/49348/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Srinivas Brahmaroutu
> 
>

Review Request 49552: Fixed typo in docker runtime.cpp.

[Guangya Liu]

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

Review request for mesos, Gilbert Song and Jie Yu.


Repository: mesos


Description
---

Fixed typo in docker runtime.cpp.


Diffs
-

  src/slave/containerizer/mesos/isolators/docker/runtime.cpp 
5af0ae93526af41ec54263fd44b5c2b55a7011a4 

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


Testing
---


Thanks,

Guangya Liu

Re: Review Request 49448: Added testcases for LIST_FILES call.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [49301, 49443, 49444, 49445, 49529, 49446, 49447, 49550, 
49551, 49448]

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 July 2, 2016, 7:50 a.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49448/
> ---
> 
> (Updated July 2, 2016, 7:50 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, zhou xing, haosdent huang, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-5514
> https://issues.apache.org/jira/browse/MESOS-5514
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added testcases for LIST_FILES call.
> 
> 
> Diffs
> -
> 
>   src/tests/api_tests.cpp b3127b716480223a6f23b7908bf6bc1808120f80 
> 
> Diff: https://reviews.apache.org/r/49448/diff/
> 
> 
> Testing
> ---
> 
> On Ubuntu 16.04:
> sudo 
> GTEST_FILTER="*AgentAPITest.ListFiles*:FilesTest.BrowseTest:*MasterAPITest.ListFiles*"
>  make -j4 check
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 49524: Fixed a const ref issue in command executor.

[Qian Zhang]

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


Ship it!




Ship It!

- Qian Zhang


On July 2, 2016, 2:02 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49524/
> ---
> 
> (Updated July 2, 2016, 2:02 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Ian Downes, and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-5753
> https://issues.apache.org/jira/browse/MESOS-5753
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed a const ref issue in command executor.
> 
> 
> Diffs
> -
> 
>   src/launcher/posix/executor.hpp e5f3c0a8e2d3687a5330adbf2f667db9106970c7 
>   src/launcher/posix/executor.cpp ab1dd938a7c59f52b44aafe1340c614087460f84 
> 
> Diff: https://reviews.apache.org/r/49524/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 49447: Implemented LIST_FILES Call in v1 agent API.

[Abhishek Dasgupta]

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

(Updated July 2, 2016, 7:55 a.m.)


Review request for mesos, Anand Mazumdar, zhou xing, haosdent huang, and Vinod 
Kone.


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


Repository: mesos


Description
---

Implemented LIST_FILES Call in v1 agent API.


Diffs (updated)
-

  include/mesos/agent/agent.proto 538d12f71df1943f91bafb99650625aa910affaa 
  include/mesos/v1/agent/agent.proto 48f15173fe62b9ce7f648f6b54d74ec62f797c55 
  src/slave/http.cpp 44d8cc98c0c1ada9d5313a3fe5c66029c9c373c6 
  src/slave/slave.hpp 2afd7d152dcd2f5390014cd7bd4e926b62c292d1 

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


Testing
---


Thanks,

Abhishek Dasgupta

Re: Review Request 49529: Removed jsonFileInfo implementation from files.hpp.

[Abhishek Dasgupta]

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

(Updated July 2, 2016, 7:51 a.m.)


Review request for mesos, Anand Mazumdar, zhou xing, haosdent huang, and Vinod 
Kone.


Summary (updated)
-

Removed jsonFileInfo implementation from files.hpp.


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


Repository: mesos


Description
---

Remove jsonFileInfo implementation from files.hpp.


Diffs (updated)
-

  src/files/files.hpp b767d5bc5bee16e3bd98199773a6bc7d30c1c32d 

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


Testing
---


Thanks,

Abhishek Dasgupta

Re: Review Request 49448: Added testcases for LIST_FILES call.

[Abhishek Dasgupta]

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

(Updated July 2, 2016, 7:50 a.m.)


Review request for mesos, Anand Mazumdar, zhou xing, haosdent huang, and Vinod 
Kone.


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


Repository: mesos


Description
---

Added testcases for LIST_FILES call.


Diffs (updated)
-

  src/tests/api_tests.cpp b3127b716480223a6f23b7908bf6bc1808120f80 

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


Testing
---

On Ubuntu 16.04:
sudo 
GTEST_FILTER="*AgentAPITest.ListFiles*:FilesTest.BrowseTest:*MasterAPITest.ListFiles*"
 make -j4 check


Thanks,

Abhishek Dasgupta

Re: Review Request 49448: Added testcases for LIST_FILES call.

[Abhishek Dasgupta]

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

(Updated July 2, 2016, 7:45 a.m.)


Review request for mesos, Anand Mazumdar, zhou xing, haosdent huang, and Vinod 
Kone.


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


Repository: mesos


Description
---

Added testcases for LIST_FILES call.


Diffs (updated)
-

  src/tests/api_tests.cpp b3127b716480223a6f23b7908bf6bc1808120f80 

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


Testing
---

On Ubuntu 16.04:
sudo 
GTEST_FILTER="*AgentAPITest.ListFiles*:FilesTest.BrowseTest:*MasterAPITest.ListFiles*"
 make -j4 check


Thanks,

Abhishek Dasgupta

Review Request 49551: Overloaded equality(==) operator for `FileInfo`.

[Abhishek Dasgupta]

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

Review request for mesos, Anand Mazumdar, zhou xing, haosdent huang, and Vinod 
Kone.


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


Repository: mesos


Description
---

Overloaded equality(==) operator for `FileInfo`.


Diffs
-

  include/mesos/v1/mesos.hpp d220966bb9464595c927474532ccdcaf41690ef6 
  src/v1/mesos.cpp 30788dcc1dd744553ddd93d23b11fcce44502b46 

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


Testing
---


Thanks,

Abhishek Dasgupta

Review Request 49550: Added evolve function for `FileInfo`.

[Abhishek Dasgupta]

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

Review request for mesos, Anand Mazumdar, zhou xing, haosdent huang, and Vinod 
Kone.


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


Repository: mesos


Description
---

Added evolve function for `FileInfo`.


Diffs
-

  src/internal/evolve.hpp 582f3476d0f61c9356cf5aff5f242f3e8103822d 
  src/internal/evolve.cpp bb99cb29db84e5476a820b261ff24348c657bae0 

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


Testing
---


Thanks,

Abhishek Dasgupta

Re: Review Request 49446: Implemented LIST_FILES Call in v1 master API.

[Abhishek Dasgupta]

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

(Updated July 2, 2016, 7:35 a.m.)


Review request for mesos, Anand Mazumdar, zhou xing, haosdent huang, and Vinod 
Kone.


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


Repository: mesos


Description
---

Implemented LIST_FILES Call in v1 master API.


Diffs (updated)
-

  include/mesos/master/master.proto 2e5d6eeb3a960e4f41c382d65321f18bb05ed6be 
  include/mesos/v1/master/master.proto 93157d57dcc53b54fed2ebbc4772c689ddba2119 
  src/master/http.cpp e5acdb8e0bbcd7a2b7e8a8bc7f4bbeaae2c4fea1 
  src/master/master.hpp e2ab2110fe5a287ab16ac9ef4222fed633e02ebe 

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


Testing
---


Thanks,

Abhishek Dasgupta

Re: Review Request 49445: Updated FilesProcess to support List_Files Call in Operator API v1.

[Abhishek Dasgupta]

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

(Updated July 2, 2016, 7:32 a.m.)


Review request for mesos, Anand Mazumdar, zhou xing, haosdent huang, and Vinod 
Kone.


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


Repository: mesos


Description
---

Updated FilesProcess to support List_Files Call in Operator API v1.


Diffs (updated)
-

  src/files/files.hpp b767d5bc5bee16e3bd98199773a6bc7d30c1c32d 
  src/files/files.cpp a5a1b86e14f63e5e3834a2900270252fbe16f586 
  src/tests/files_tests.cpp 31337e280c6224a8c949c8868a53e5a785b4573f 

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


Testing
---


Thanks,

Abhishek Dasgupta