Re: Review Request 42016: Windows:[2/2] Use ZK in Windows build.

[M Lawindi]

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

(Updated Jan. 27, 2016, 10:29 p.m.)


Review request for mesos, Alex Naparu, Daniel Pravat, Alex Clemmer, M Lawindi, 
and Yi Sun.


Summary (updated)
-

Windows:[2/2] Use ZK in Windows build.


Repository: mesos


Description (updated)
---

Windows:[2/2] Use ZK in Windows build.


Diffs (updated)
-

  3rdparty/CMakeLists.txt ac5c25a8797a687e84384682975ab99fb3e30448 
  3rdparty/cmake/Mesos3rdpartyConfigure.cmake 
34e61ff90eca0ffdddb6b6b8e2f8e552691637fa 
  3rdparty/patch.exe.manifest PRE-CREATION 
  CMakeLists.txt 9b7044b7860fd64b854ac27b28a48d297dfdeae8 
  src/slave/cmake/SlaveConfigure.cmake cf378a27297474b2a9f338e0c832612370f7302a 

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


Testing
---


Thanks,

M Lawindi

Re: Review Request 40851: Windows:[1/2] Add patch for Windows ZK version.

[Alex Clemmer]

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


Ship it!




Ship It!

- Alex Clemmer


On Jan. 27, 2016, 10:29 p.m., M Lawindi wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40851/
> ---
> 
> (Updated Jan. 27, 2016, 10:29 p.m.)
> 
> 
> Review request for mesos, Alex Naparu, Dario Bazan, Daniel Pravat, Alex 
> Clemmer, and M Lawindi.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows:[1/2] Add patch for Windows ZK version.
> 
> 
> Diffs
> -
> 
>   3rdparty/zookeeper-06d3f3f.patch PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/40851/diff/
> 
> 
> Testing
> ---
> 
> - Applied patch to original zookeeper 3.4.5
> - Successfully opened 2015 solution in VS2015
> - Cli and Zookeeper build successfully
> 
> 
> Thanks,
> 
> M Lawindi
> 
>

Review Request 42888: Used absolute paths for excludes paths in posix disk isolator.

[Jie Yu]

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

Review request for mesos, Artem Harutyunyan and Joris Van Remoortere.


Repository: mesos


Description
---

Used absolute paths for excludes paths in posix disk isolator.

We should consistently use absolute paths for excludes paths in disk isolator, 
otherwise, it's possible that some other paths under the directory matches the 
relative path, causing incorrect disk usage being reported.


Diffs
-

  src/slave/containerizer/mesos/isolators/posix/disk.hpp 
5607c0f43084f491e4927f5c6ec283210aa5f355 
  src/slave/containerizer/mesos/isolators/posix/disk.cpp 
d5bf8c10b9cfb360a7856144d59ce89060de5c99 

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


Testing
---

sudo make check


Thanks,

Jie Yu

Re: Review Request 42890: Allocator Performance: Simplified Sorter's 'CalculateShare'.

[Joris Van Remoortere]

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

(Updated Jan. 28, 2016, 1:44 a.m.)


Review request for mesos, Jie Yu and Michael Park.


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


Repository: mesos


Description
---

Used existing functions to aggregate Scalars in the 'Resources' object.


Diffs
-

  src/master/allocator/sorter/drf/sorter.cpp 
1f39f6d12cb2673ef4b75c3efb23ca5f7dbbf29a 

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


Testing
---

valgrind on allocator benchmark: 2x


Thanks,

Joris Van Remoortere

Re: Review Request 42794: URL query string order is defined.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [42794]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export 
COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Jan. 27, 2016, 7:07 p.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42794/
> ---
> 
> (Updated Jan. 27, 2016, 7:07 p.m.)
> 
> 
> Review request for mesos, Kapil Arya and Jan Schlicht.
> 
> 
> Bugs: MESOS-3317
> https://issues.apache.org/jira/browse/MESOS-3317
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> URL query string order is defined.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/http.cpp 762da9a 
>   3rdparty/libprocess/src/tests/http_tests.cpp 66d185e 
> 
> Diff: https://reviews.apache.org/r/42794/diff/
> 
> 
> Testing
> ---
> 
> The following test cases in http_test.cpp have been modified -
> HTTPTest.QueryEncodeDecode to check generated query strings in URL to be 
> according alphabatic order.
> URLTest.Stringification to check generated query strings in URL to be 
> according alphabatic order.
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Review Request 42876: Disallowed non-`const` iteration over `Resources`.

[Michael Park]

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

Review request for mesos, Jie Yu and Joris Van Remoortere.


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


Repository: mesos


Description
---

See summary.


Diffs
-

  include/mesos/resources.hpp cc8fef9470d779078aa408ed03e747e5a492deaa 
  include/mesos/v1/resources.hpp f4892977f8d7b0439db6e9cf7921334f606a496c 
  src/master/allocator/mesos/hierarchical.cpp 
dbc77a2d0ce797eea2287b0242f5d2da81c16455 
  src/master/http.cpp 12c1fe5a514903f657911302e8770e9b245fdbb7 
  src/slave/resource_estimators/fixed.cpp 
aedd08df8db59be49b374e61eb268a947d35acea 
  src/tests/resources_tests.cpp 54a4fa88bfdcff3c0a7e89cbf3a1674c954b7f23 

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


Testing
---

`make check`


Thanks,

Michael Park

Re: Review Request 42866: Disabled the test RegistryClientTest.BadTokenServerAddress.

[Anand Mazumdar]

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


Ship it!




Ship It!

- Anand Mazumdar


On Jan. 27, 2016, 8:38 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42866/
> ---
> 
> (Updated Jan. 27, 2016, 8:38 p.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> It has shown flaky behavior and needs further investigation. Disabling now as
> its not a very important test for functionality.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> 33df60065903228749833bbad20449ba8784594a 
> 
> Diff: https://reviews.apache.org/r/42866/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 42881: Updated release guide to include running the benchmarks.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On Jan. 28, 2016, 12:11 a.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42881/
> ---
> 
> (Updated Jan. 28, 2016, 12:11 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> It would be good to run benchmarks before we make a release to ensure
> that there have been no performance regressions since the last release.
> 
> This will also ensure that we write benchmarks that don't take an
> unreasonable amount of time to run to completion. This will be
> important if we ever want to run benchmarks in CI.
> 
> 
> Diffs
> -
> 
>   docs/release-guide.md 7c9ee140b394d61581f736f2413969d650f18fa4 
> 
> Diff: https://reviews.apache.org/r/42881/diff/
> 
> 
> Testing
> ---
> 
> N/A
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Re: Review Request 42872: Relaxed the subsystem check for net_cls.

[Jie Yu]

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


Fix it, then Ship it!





src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp (line 75)


You want to update the comments here?



src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp (lines 82 - 83)


Could you please mentioned that on some systems, net_cls is co-mounted with 
net_prio in the same hierarchy.


- Jie Yu


On Jan. 27, 2016, 10:12 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42872/
> ---
> 
> (Updated Jan. 27, 2016, 10:12 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4530
> https://issues.apache.org/jira/browse/MESOS-4530
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Relaxed the subsystem check for net_cls. The check ensures that only the 
> net_cls and net_prio subsystem can be mounted in the same hierarchy.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp 
> 03a488e0025da2f0a835cdc39379c901a83b78b0 
> 
> Diff: https://reviews.apache.org/r/42872/diff/
> 
> 
> Testing
> ---
> 
> make and make check on Debian 8
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 42603: Added an http::Authenticator factory.

[Benjamin Bannier]

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


Fix it, then Ship it!





include/mesos/authentication/http/basic_authenticator_factory.hpp (line 35)


Not originally yours, but this has to be `virtual` for e.g., 
`CRAMMD5Authenticator`.

Also, I believe we should prefer just `default`ing instead of adding an 
empty body.



include/mesos/authentication/http/basic_authenticator_factory.hpp (line 42)


Not originally yours, but should probably also be `default`ed.



src/examples/test_http_authenticator_module.cpp (line 28)


Not yours, but since you are touching most of the code below mind changing 
this to `namespace mesos { ...` (after moving it below the other using decls)?


- Benjamin Bannier


On Jan. 27, 2016, 10:22 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42603/
> ---
> 
> (Updated Jan. 27, 2016, 10:22 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Bannier, Jan 
> Schlicht, and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Move the code required to instanciate instances of 
> `process::http::authentication::Authenticator`
> in `src/master/main.cpp` to its own factory.
> 
> 
> Diffs
> -
> 
>   include/mesos/authentication/http/basic_authenticator_factory.hpp 
> c11bb47c8e02f2e8645cf387d18eb64d1c8cb604 
>   src/Makefile.am bdb34020043f64c07721b3e8b29b221e5b99 
>   src/authentication/http/authenticator.cpp PRE-CREATION 
>   src/authentication/http/basic_authenticator_factory.cpp 
> 6eb1c5bd09b136d3bc20481ddcc65cb8bd153682 
>   src/examples/test_http_authenticator_module.cpp 
> acf51a6deb8e7dc4ab6ac0cf70380ddbb1839906 
>   src/master/master.cpp 9ee56277c8a472be9e683d5db505becfb5f7c422 
>   src/tests/http_authentication_tests.cpp 
> bd622576973648e0dfeae1453a5ce631e4171352 
> 
> Diff: https://reviews.apache.org/r/42603/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 42865: Fixed LogrotateContainerLogger's FD ownership.

[Benjamin Hindman]

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




src/slave/container_loggers/lib_logrotate.cpp (line 107)


Can we get a comment on why we need to `cloexec` please?



src/slave/container_loggers/lib_logrotate.cpp (line 149)


Is this supposed to be `outfds` or `errfds`? Do we need the `cloexec` at 
all? If just the post-fork function can we simply put a comment there that 
explains it? Using `cloexec` is better though because the fork solution won't 
work on Windows.


- Benjamin Hindman


On Jan. 27, 2016, 11:20 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42865/
> ---
> 
> (Updated Jan. 27, 2016, 11:20 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Artem Harutyunyan.
> 
> 
> Bugs: MESOS-4535
> https://issues.apache.org/jira/browse/MESOS-4535
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Changes the logrotate container logger to manually construct and deal with 
> pipes.  Specifically, both read and write ends of the pipe must end up in the 
> child processes (read -> logger executables, write -> container).  
> 
> If ownership is not transferred, the pipe's FDs may be closed (again) when 
> `Subprocess` is destructed, which may unexpectedly close random FDs belonging 
> to other threads.
> 
> 
> Diffs
> -
> 
>   src/slave/container_loggers/lib_logrotate.cpp 
> bfc7cade2eed98f21fc1c364c104ad5583648c63 
> 
> Diff: https://reviews.apache.org/r/42865/diff/
> 
> 
> Testing
> ---
> 
> make check (OSX)
> 
> ---
> 
> NOTE: I'll add an integration to codify the manual testing below.
> This discarded review has the gist of the needed test:
> https://reviews.apache.org/r/42864/
> 
> ---
> 
> Manual testing:
> 
> ```
>   ./mesos-master.sh --work_dir=/tmp/master --ip=127.0.0.1
>   ./mesos-slave.sh --master=127.0.0.1:5050 
> --modules=file:///path/to/modules.json 
> --container_logger="org_apache_mesos_LogrotateContainerLogger" 
> --work_dir=/tmp/agent
>   
>   # Repeatedly trigger executor terminations.
>   ./mesos-execute --master=127.0.0.1:5050 --name="Repeater" --command="while 
> true; do /Users/josephwu/mesos/build/src/mesos-execute 
> --master=127.0.0.1:5050 --name=Repeat --command=\"echo Wheee; sleep 0.4\"; 
> done"
> ```
> 
> "modules.json" follows the template:
> ```
> {
>   "libraries": [
> {
>   "file": "/path/to/liblogrotate_container_logger.la",
>   "modules": [
> {
>   "name": "org_apache_mesos_LogrotateContainerLogger",
>   "parameters": [
> {
>   "key": "launcher_dir",
>   "value": "/path/to/mesos/build/src/"
> }, {
>   "key": "logrotate_stdout_options",
>   "value": "rotate 2"
> }, {
>   "key": "logrotate_stderr_options",
>   "value": "rotate 2"
> }
>   ]
> }
>   ]
> }
>   ]
> }
> ```
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 42865: Fixed LogrotateContainerLogger's FD ownership.

[Joseph Wu]

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

(Updated Jan. 27, 2016, 4:11 p.m.)


Review request for mesos, Benjamin Hindman and Artem Harutyunyan.


Changes
---

Addressed cloexec-ing vs setup functions.  (Fixed typo in code)


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


Repository: mesos


Description
---

Changes the logrotate container logger to manually construct and deal with 
pipes.  Specifically, both read and write ends of the pipe must end up in the 
child processes (read -> logger executables, write -> container).  

If ownership is not transferred, the pipe's FDs may be closed (again) when 
`Subprocess` is destructed, which may unexpectedly close random FDs belonging 
to other threads.


Diffs (updated)
-

  src/slave/container_loggers/lib_logrotate.cpp 
bfc7cade2eed98f21fc1c364c104ad5583648c63 

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


Testing
---

make check (OSX)

---

NOTE: I'll add an integration to codify the manual testing below.
This discarded review has the gist of the needed test:
https://reviews.apache.org/r/42864/

---

Manual testing:

```
  ./mesos-master.sh --work_dir=/tmp/master --ip=127.0.0.1
  ./mesos-slave.sh --master=127.0.0.1:5050 
--modules=file:///path/to/modules.json 
--container_logger="org_apache_mesos_LogrotateContainerLogger" 
--work_dir=/tmp/agent
  
  # Repeatedly trigger executor terminations.
  ./mesos-execute --master=127.0.0.1:5050 --name="Repeater" --command="while 
true; do /Users/josephwu/mesos/build/src/mesos-execute --master=127.0.0.1:5050 
--name=Repeat --command=\"echo Wheee; sleep 0.4\"; done"
```

"modules.json" follows the template:
```
{
  "libraries": [
{
  "file": "/path/to/liblogrotate_container_logger.la",
  "modules": [
{
  "name": "org_apache_mesos_LogrotateContainerLogger",
  "parameters": [
{
  "key": "launcher_dir",
  "value": "/path/to/mesos/build/src/"
}, {
  "key": "logrotate_stdout_options",
  "value": "rotate 2"
}, {
  "key": "logrotate_stderr_options",
  "value": "rotate 2"
}
  ]
}
  ]
}
  ]
}
```


Thanks,

Joseph Wu

Review Request 42881: Updated release guide to include running the benchmarks.

[Ben Mahler]

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

Review request for mesos and Vinod Kone.


Repository: mesos


Description
---

It would be good to run benchmarks before we make a release to ensure
that there have been no performance regressions since the last release.

This will also ensure that we write benchmarks that don't take an
unreasonable amount of time to run to completion. This will be
important if we ever want to run benchmarks in CI.


Diffs
-

  docs/release-guide.md 7c9ee140b394d61581f736f2413969d650f18fa4 

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


Testing
---

N/A


Thanks,

Ben Mahler

Re: Review Request 42866: Disabled the test RegistryClientTest.BadTokenServerAddress.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [42866]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export 
COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Jan. 27, 2016, 8:38 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42866/
> ---
> 
> (Updated Jan. 27, 2016, 8:38 p.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> It has shown flaky behavior and needs further investigation. Disabling now as
> its not a very important test for functionality.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> 33df60065903228749833bbad20449ba8784594a 
> 
> Diff: https://reviews.apache.org/r/42866/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Review Request 42887: Fixed a flaky test in disk quota tests.

[Jie Yu]

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

Review request for mesos, Artem Harutyunyan and Joris Van Remoortere.


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


Repository: mesos


Description
---

Fixed a flaky test in disk quota tests.


Diffs
-

  src/tests/disk_quota_tests.cpp eb93c2fc4a791fdb8a61e4c539fa85ece7c5604e 

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


Testing
---

make check

verified on Neil's box.


Thanks,

Jie Yu

Review Request 42873: Enhanced the NetClsIsolatorTest filter.

[Avinash sridharan]

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

Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

Enhanced the NetClsIsolatorTest filter. Added a check to make sure that the 
only other subsystem allowed to be mounted under the same hierarchy as the 
net_cls subsystem is net_prio.


Diffs
-

  src/tests/environment.cpp e112270b68d402bb9b01445af552500fb3929e52 

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


Testing
---


Thanks,

Avinash sridharan

Re: Review Request 40851: Windows:[1/2] Add patch for Windows ZK version.

[M Lawindi]

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

(Updated Jan. 27, 2016, 10:29 p.m.)


Review request for mesos, Alex Naparu, Dario Bazan, Daniel Pravat, Alex 
Clemmer, and M Lawindi.


Summary (updated)
-

Windows:[1/2] Add patch for Windows ZK version.


Repository: mesos


Description (updated)
---

Windows:[1/2] Add patch for Windows ZK version.


Diffs (updated)
-

  3rdparty/zookeeper-06d3f3f.patch PRE-CREATION 

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


Testing
---

- Applied patch to original zookeeper 3.4.5
- Successfully opened 2015 solution in VS2015
- Cli and Zookeeper build successfully


Thanks,

M Lawindi

Re: Review Request 42773: Added utility function to compute SHA512 digest.

[Jojy Varghese]

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

(Updated Jan. 27, 2016, 10:29 p.m.)


Review request for mesos.


Changes
---

simplified the method.


Repository: mesos


Description
---

Added utility function to compute SHA512 digest.


Diffs (updated)
-

  src/common/command_utils.hpp PRE-CREATION 
  src/common/command_utils.cpp PRE-CREATION 
  src/tests/common/command_utils_tests.cpp PRE-CREATION 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 42865: Fixed LogrotateContainerLogger's FD ownership.

[Joseph Wu]

> On Jan. 27, 2016, 4:02 p.m., Benjamin Hindman wrote:
> > src/slave/container_loggers/lib_logrotate.cpp, line 149
> > 
> >
> > Is this supposed to be `outfds` or `errfds`? Do we need the `cloexec` 
> > at all? If just the post-fork function can we simply put a comment there 
> > that explains it? Using `cloexec` is better though because the fork 
> > solution won't work on Windows.

Looks like only the cloexec is needed.  (Removed the setup functions).


- Joseph


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


On Jan. 27, 2016, 4:11 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42865/
> ---
> 
> (Updated Jan. 27, 2016, 4:11 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Artem Harutyunyan.
> 
> 
> Bugs: MESOS-4535
> https://issues.apache.org/jira/browse/MESOS-4535
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Changes the logrotate container logger to manually construct and deal with 
> pipes.  Specifically, both read and write ends of the pipe must end up in the 
> child processes (read -> logger executables, write -> container).  
> 
> If ownership is not transferred, the pipe's FDs may be closed (again) when 
> `Subprocess` is destructed, which may unexpectedly close random FDs belonging 
> to other threads.
> 
> 
> Diffs
> -
> 
>   src/slave/container_loggers/lib_logrotate.cpp 
> bfc7cade2eed98f21fc1c364c104ad5583648c63 
> 
> Diff: https://reviews.apache.org/r/42865/diff/
> 
> 
> Testing
> ---
> 
> make check (OSX)
> 
> ---
> 
> NOTE: I'll add an integration to codify the manual testing below.
> This discarded review has the gist of the needed test:
> https://reviews.apache.org/r/42864/
> 
> ---
> 
> Manual testing:
> 
> ```
>   ./mesos-master.sh --work_dir=/tmp/master --ip=127.0.0.1
>   ./mesos-slave.sh --master=127.0.0.1:5050 
> --modules=file:///path/to/modules.json 
> --container_logger="org_apache_mesos_LogrotateContainerLogger" 
> --work_dir=/tmp/agent
>   
>   # Repeatedly trigger executor terminations.
>   ./mesos-execute --master=127.0.0.1:5050 --name="Repeater" --command="while 
> true; do /Users/josephwu/mesos/build/src/mesos-execute 
> --master=127.0.0.1:5050 --name=Repeat --command=\"echo Wheee; sleep 0.4\"; 
> done"
> ```
> 
> "modules.json" follows the template:
> ```
> {
>   "libraries": [
> {
>   "file": "/path/to/liblogrotate_container_logger.la",
>   "modules": [
> {
>   "name": "org_apache_mesos_LogrotateContainerLogger",
>   "parameters": [
> {
>   "key": "launcher_dir",
>   "value": "/path/to/mesos/build/src/"
> }, {
>   "key": "logrotate_stdout_options",
>   "value": "rotate 2"
> }, {
>   "key": "logrotate_stderr_options",
>   "value": "rotate 2"
> }
>   ]
> }
>   ]
> }
>   ]
> }
> ```
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 42865: Fixed LogrotateContainerLogger's FD ownership.

[Benjamin Hindman]

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


Ship it!




Ship It!

- Benjamin Hindman


On Jan. 28, 2016, 12:11 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42865/
> ---
> 
> (Updated Jan. 28, 2016, 12:11 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Artem Harutyunyan.
> 
> 
> Bugs: MESOS-4535
> https://issues.apache.org/jira/browse/MESOS-4535
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Changes the logrotate container logger to manually construct and deal with 
> pipes.  Specifically, both read and write ends of the pipe must end up in the 
> child processes (read -> logger executables, write -> container).  
> 
> If ownership is not transferred, the pipe's FDs may be closed (again) when 
> `Subprocess` is destructed, which may unexpectedly close random FDs belonging 
> to other threads.
> 
> 
> Diffs
> -
> 
>   src/slave/container_loggers/lib_logrotate.cpp 
> bfc7cade2eed98f21fc1c364c104ad5583648c63 
> 
> Diff: https://reviews.apache.org/r/42865/diff/
> 
> 
> Testing
> ---
> 
> make check (OSX)
> 
> ---
> 
> NOTE: I'll add an integration to codify the manual testing below.
> This discarded review has the gist of the needed test:
> https://reviews.apache.org/r/42864/
> 
> ---
> 
> Manual testing:
> 
> ```
>   ./mesos-master.sh --work_dir=/tmp/master --ip=127.0.0.1
>   ./mesos-slave.sh --master=127.0.0.1:5050 
> --modules=file:///path/to/modules.json 
> --container_logger="org_apache_mesos_LogrotateContainerLogger" 
> --work_dir=/tmp/agent
>   
>   # Repeatedly trigger executor terminations.
>   ./mesos-execute --master=127.0.0.1:5050 --name="Repeater" --command="while 
> true; do /Users/josephwu/mesos/build/src/mesos-execute 
> --master=127.0.0.1:5050 --name=Repeat --command=\"echo Wheee; sleep 0.4\"; 
> done"
> ```
> 
> "modules.json" follows the template:
> ```
> {
>   "libraries": [
> {
>   "file": "/path/to/liblogrotate_container_logger.la",
>   "modules": [
> {
>   "name": "org_apache_mesos_LogrotateContainerLogger",
>   "parameters": [
> {
>   "key": "launcher_dir",
>   "value": "/path/to/mesos/build/src/"
> }, {
>   "key": "logrotate_stdout_options",
>   "value": "rotate 2"
> }, {
>   "key": "logrotate_stderr_options",
>   "value": "rotate 2"
> }
>   ]
> }
>   ]
> }
>   ]
> }
> ```
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 42615: mesos: Cleaned up usage of namespace-qualified identifiers.

[Neil Conway]

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

(Updated Jan. 28, 2016, 12:56 a.m.)


Review request for mesos and Michael Park.


Changes
---

Rebase.


Repository: mesos


Description
---

For example, avoid using `std::string` in a .cpp file that already does
`using std::string`.


Diffs (updated)
-

  src/authentication/http/basic_authenticator_factory.cpp 
6eb1c5bd09b136d3bc20481ddcc65cb8bd153682 
  src/cli/execute.cpp 0add77558e07ff49f0134ddb9085509501c61aab 
  src/common/http.cpp b7e71eb79ccb8ef1d0bbd168d77d4d4591ecb09b 
  src/common/protobuf_utils.cpp 53324ab569751924f25290641d1a70da790c2104 
  src/docker/docker.cpp 4d2f1fa14f4450b8fa3401081bf52807d2e79a7e 
  src/examples/balloon_framework.cpp 3c17676c92f72d41c501b2435a7017e3c9f01278 
  src/examples/test_container_logger_module.cpp 
6b1f4dbd63145afe4de17830c0a2a6202a896be7 
  src/examples/test_http_authenticator_module.cpp 
acf51a6deb8e7dc4ab6ac0cf70380ddbb1839906 
  src/examples/test_qos_controller_module.cpp 
f8aa1d730f6cdf5ab3f11789e9699682cc08ddd5 
  src/files/files.cpp dd64976ca454786152a8e29f590e8c1ed1df3b54 
  src/health-check/main.cpp 0beaed575ec865d81e6e3d83d8a0c894613acba4 
  src/hook/manager.cpp 6ee93038dce1247cffbc82c736a1c7a1ecb84bb0 
  src/launcher/fetcher.cpp 902e927ce0cbcf70e41041375e61987752629957 
  src/linux/routing/queueing/fq_codel.cpp 
d840e383f28481ba58d8269217a8a194c4e87db5 
  src/linux/routing/queueing/ingress.cpp 
c25a93966caca5c5a9eef91e6caa5d9e29091bee 
  src/linux/systemd.cpp d3f4a63d833a5bc599494e925d709ba0cc70d9ec 
  src/local/local.cpp 582d4a10444831b0753d20650698e5d3b51cca9f 
  src/log/tool/initialize.cpp c9706d1e33659f7b0e375419197b207cb96d4ca9 
  src/log/tool/read.cpp 9abf5a82338c554920283e0329cd83e4c787c103 
  src/log/tool/replica.cpp 8baf79755da420cf594d706f31aa561b5d665051 
  src/master/allocator/sorter/drf/sorter.cpp 
1f39f6d12cb2673ef4b75c3efb23ca5f7dbbf29a 
  src/master/main.cpp 0d5ac49121491152d8426cd764a1f1a0f37483ae 
  src/master/maintenance.cpp df7cd6ca5a097611e5ca5a46cc988ef636da096b 
  src/master/master.cpp 9ee56277c8a472be9e683d5db505becfb5f7c422 
  src/module/manager.cpp a53f71b9965f7ab85aadb6c0c7af18de958faf38 
  src/slave/containerizer/docker.cpp 2887cb4a01febbbf276026e584ffc466289e10c9 
  src/slave/containerizer/fetcher.cpp f7e3f7de6d0ebc253d2ccfe4a66b10ed2eca2c11 
  src/slave/containerizer/mesos/linux_launcher.cpp 
61801fffe9bac1a995a57d8b4e8c004d624bbd63 
  src/slave/containerizer/mesos/provisioner/docker/paths.cpp 
9c5c24992f9ea36159271691cca0147851240088 
  src/slave/containerizer/mesos/provisioner/docker/registry_client.cpp 
b74c760e90b12f5bf45b6e626e2b661f841e5a8d 
  src/slave/containerizer/mesos/provisioner/docker/store.cpp 
03425daaf015cf231920b7eda1d5424895a41286 
  src/slave/qos_controllers/load.cpp 52520d6220784423c09001b8bd59e090ebf787ec 
  src/slave/state.cpp fae7738a1cb7168abc0cfe35b075bbc73306b820 
  src/tests/attributes_tests.cpp 3f3dde1301566f279c3fc7dc24e3ef67039a4c14 
  src/tests/common/http_tests.cpp 0ea06341b092cd6ad278075b12dd970b84c84464 
  src/tests/containerizer/cgroups_isolator_tests.cpp 
0d24809701c42dc036bbd051791545f4742d1e9a 
  src/tests/containerizer/external_containerizer_test.cpp 
da52860f4a1d5363d7b61b9a8bb6abad02d89736 
  src/tests/containerizer/provisioner_docker_tests.cpp 
33df60065903228749833bbad20449ba8784594a 
  src/tests/credentials_tests.cpp 6e3725c61a55a2c1d183ee278cf3d72be16a6e40 
  src/tests/fetcher_cache_tests.cpp 2747b72ba49c9fde04e556b649601b037517283e 
  src/tests/gc_tests.cpp ef5544b627ac4a9f9c13fe13b0c51b21d5b98a12 
  src/tests/hierarchical_allocator_tests.cpp 
f18e6eb10572b0f5b8bbff338384d9406f6ad62b 
  src/tests/hook_tests.cpp 1a1bab49469485887e0a6a8fa1837bd70262d3b5 
  src/tests/http_authentication_tests.cpp 
bd622576973648e0dfeae1453a5ce631e4171352 
  src/tests/master_quota_tests.cpp 04efcf3362d3594e0ad8077793fa1f32536dd658 
  src/tests/mesos.cpp 18d0d8f8037ebc27c87bcb0f1ce9f143e7505ec8 
  src/tests/monitor_tests.cpp 2226458b59b4a279a92e1353bd61457a0018d2a9 
  src/tests/registrar_tests.cpp 58b3fccf5450d20032c5c6d20d76dbd868c424a3 
  src/tests/registrar_zookeeper_tests.cpp 
3861dcf8341bfa520f94fd244b4814827299b861 
  src/tests/scheduler_driver_tests.cpp f35c4957b08803dba924ebf1a5f0af9daac9d0c5 
  src/tests/slave_tests.cpp e943ab99baf3d74679a5da89a89f6a4b7ead 
  src/tests/values_tests.cpp a4eb68ad13407f471a07a9a923ed31c7890da9f7 
  src/watcher/whitelist_watcher.cpp 14d7de751884d4734942e315e61a94c29868ff4b 
  src/zookeeper/detector.cpp a3d68c12e3800805a35f9bb05e7689830eedbae6 

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


Testing
---

"make check" on OSX and Arch Linux.


Thanks,

Neil Conway

Re: Review Request 42617: stout: Cleaned up usage of namespace-qualified identifiers.

[Neil Conway]

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

(Updated Jan. 28, 2016, 12:56 a.m.)


Review request for mesos and Michael Park.


Changes
---

Rebase.


Repository: mesos


Description
---

For example, avoid using `std::string` in a .cpp file that already does
`using std::string`.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/tests/flags_tests.cpp 
5a10f59f3698a6ad85b7c2386def9686af8a23a6 
  3rdparty/libprocess/3rdparty/stout/tests/linkedhashmap_tests.cpp 
21a98b85277d946b887cb23c4ebb4d32cfcb0c6b 
  3rdparty/libprocess/3rdparty/stout/tests/os/filesystem_tests.cpp 
c5ffd017f83149a58a2dee8c87461cd06bb43bed 
  3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp 
4b7d1347f93261df7a7d512721146edca2703b34 
  3rdparty/libprocess/3rdparty/stout/tests/some_tests.cpp 
c0be974ea2dc74c63ec3561403b3f4d3722a8df3 
  3rdparty/libprocess/3rdparty/stout/tests/strings_tests.cpp 
7715fa4baa150f370bdd0fd5c2f98dc4c6f5fc55 

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


Testing
---

"make check" on OSX and Arch Linux.


Thanks,

Neil Conway

Re: Review Request 42888: Used absolute paths for excludes paths in posix disk isolator.

[Jie Yu]

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

(Updated Jan. 28, 2016, 1:14 a.m.)


Review request for mesos, Artem Harutyunyan and Joris Van Remoortere.


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


Repository: mesos


Description
---

Used absolute paths for excludes paths in posix disk isolator.

We should consistently use absolute paths for excludes paths in disk isolator, 
otherwise, it's possible that some other paths under the directory matches the 
relative path, causing incorrect disk usage being reported.


Diffs
-

  src/slave/containerizer/mesos/isolators/posix/disk.hpp 
5607c0f43084f491e4927f5c6ec283210aa5f355 
  src/slave/containerizer/mesos/isolators/posix/disk.cpp 
d5bf8c10b9cfb360a7856144d59ce89060de5c99 

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


Testing
---

sudo make check


Thanks,

Jie Yu

Re: Review Request 42016: Windows:[2/2] Use ZK in Windows build.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [39850, 39851, 39852, 39888, 39889, 40102, 40851, 42016]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export 
COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Jan. 27, 2016, 10:29 p.m., M Lawindi wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42016/
> ---
> 
> (Updated Jan. 27, 2016, 10:29 p.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Alex Clemmer, M 
> Lawindi, and Yi Sun.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows:[2/2] Use ZK in Windows build.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt ac5c25a8797a687e84384682975ab99fb3e30448 
>   3rdparty/cmake/Mesos3rdpartyConfigure.cmake 
> 34e61ff90eca0ffdddb6b6b8e2f8e552691637fa 
>   3rdparty/patch.exe.manifest PRE-CREATION 
>   CMakeLists.txt 9b7044b7860fd64b854ac27b28a48d297dfdeae8 
>   src/slave/cmake/SlaveConfigure.cmake 
> cf378a27297474b2a9f338e0c832612370f7302a 
> 
> Diff: https://reviews.apache.org/r/42016/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> M Lawindi
> 
>

Re: Review Request 42878: Fixed the NetClsIsolatorTest to correctly learn the net_cls hierarchy.

[Avinash sridharan]

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

(Updated Jan. 28, 2016, 1:48 a.m.)


Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

Fixed the NetClsIsolatorTest to correctly learn the net_cls hierarchy. The test 
relied on the flags.cgroups_hierarchy to learn the net_cls hierarchy, instead 
it should be making a call to cgroups::hierarchy to learn the hierarchy.


Diffs
-

  src/tests/containerizer/isolator_tests.cpp 
7033ac1aa7ae8a3cf06d4f717580ce5a5afb648a 

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


Testing
---

make and make check on Debian 8

Also ran the patch on CentOS 6 with Greg's help. He was using 
(boxcutter/centos66) vagrant image to test this patch. (boxcutter/centos66) is 
where this test was failing.


Thanks,

Avinash sridharan

Re: Review Request 42876: Disallowed non-`const` iteration over `Resources`.

[Klaus Ma]

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


Ship it!




Ship It!

- Klaus Ma


On Jan. 28, 2016, 6:50 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42876/
> ---
> 
> (Updated Jan. 28, 2016, 6:50 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-4534
> https://issues.apache.org/jira/browse/MESOS-4534
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   include/mesos/resources.hpp cc8fef9470d779078aa408ed03e747e5a492deaa 
>   include/mesos/v1/resources.hpp f4892977f8d7b0439db6e9cf7921334f606a496c 
>   src/master/allocator/mesos/hierarchical.cpp 
> dbc77a2d0ce797eea2287b0242f5d2da81c16455 
>   src/master/http.cpp 12c1fe5a514903f657911302e8770e9b245fdbb7 
>   src/slave/resource_estimators/fixed.cpp 
> aedd08df8db59be49b374e61eb268a947d35acea 
>   src/tests/resources_tests.cpp 54a4fa88bfdcff3c0a7e89cbf3a1674c954b7f23 
> 
> Diff: https://reviews.apache.org/r/42876/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 42876: Disallowed non-`const` iteration over `Resources`.

[Michael Park]

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

(Updated Jan. 27, 2016, 10:50 p.m.)


Review request for mesos, Jie Yu and Joris Van Remoortere.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  include/mesos/resources.hpp cc8fef9470d779078aa408ed03e747e5a492deaa 
  include/mesos/v1/resources.hpp f4892977f8d7b0439db6e9cf7921334f606a496c 
  src/master/allocator/mesos/hierarchical.cpp 
dbc77a2d0ce797eea2287b0242f5d2da81c16455 
  src/master/http.cpp 12c1fe5a514903f657911302e8770e9b245fdbb7 
  src/slave/resource_estimators/fixed.cpp 
aedd08df8db59be49b374e61eb268a947d35acea 
  src/tests/resources_tests.cpp 54a4fa88bfdcff3c0a7e89cbf3a1674c954b7f23 

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


Testing
---

`make check`


Thanks,

Michael Park

Re: Review Request 42880: Add test for LogrotateContainerLogger's FD management.

[Benjamin Hindman]

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


Fix it, then Ship it!





src/tests/container_logger_tests.cpp (line 555)


s/open_fds/fds/


- Benjamin Hindman


On Jan. 27, 2016, 11:53 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42880/
> ---
> 
> (Updated Jan. 27, 2016, 11:53 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Artem Harutyunyan.
> 
> 
> Bugs: MESOS-4535
> https://issues.apache.org/jira/browse/MESOS-4535
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adds a test which checks for erroneous calls to `os::close` by the 
> LogrotateContainerLogger.  This may happen by accident if the container 
> logger module uses `Subprocess::PIPE` when launching child processes; as 
> libprocess will track these FDs and close them (possibly even if they've 
> already been closed) when the child processes exit.
> 
> 
> Diffs
> -
> 
>   src/tests/container_logger_tests.cpp 
> 5fe9cce97ee83d6a9e272879ec0395b5ace4a491 
> 
> Diff: https://reviews.apache.org/r/42880/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> Test failed (expected) when I reverted the previous patch and ran `make check`
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 42876: Disallowed non-`const` iteration over `Resources`.

[Joris Van Remoortere]

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


Ship it!




Ship It!

- Joris Van Remoortere


On Jan. 27, 2016, 10:50 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42876/
> ---
> 
> (Updated Jan. 27, 2016, 10:50 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-4534
> https://issues.apache.org/jira/browse/MESOS-4534
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   include/mesos/resources.hpp cc8fef9470d779078aa408ed03e747e5a492deaa 
>   include/mesos/v1/resources.hpp f4892977f8d7b0439db6e9cf7921334f606a496c 
>   src/master/allocator/mesos/hierarchical.cpp 
> dbc77a2d0ce797eea2287b0242f5d2da81c16455 
>   src/master/http.cpp 12c1fe5a514903f657911302e8770e9b245fdbb7 
>   src/slave/resource_estimators/fixed.cpp 
> aedd08df8db59be49b374e61eb268a947d35acea 
>   src/tests/resources_tests.cpp 54a4fa88bfdcff3c0a7e89cbf3a1674c954b7f23 
> 
> Diff: https://reviews.apache.org/r/42876/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 42880: Add test for LogrotateContainerLogger's FD management.

[Joseph Wu]

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

(Updated Jan. 27, 2016, 4:09 p.m.)


Review request for mesos, Benjamin Hindman and Artem Harutyunyan.


Changes
---

Renamed variable.


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


Repository: mesos


Description
---

Adds a test which checks for erroneous calls to `os::close` by the 
LogrotateContainerLogger.  This may happen by accident if the container logger 
module uses `Subprocess::PIPE` when launching child processes; as libprocess 
will track these FDs and close them (possibly even if they've already been 
closed) when the child processes exit.


Diffs (updated)
-

  src/tests/container_logger_tests.cpp 5fe9cce97ee83d6a9e272879ec0395b5ace4a491 

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


Testing
---

make check

Test failed (expected) when I reverted the previous patch and ran `make check`


Thanks,

Joseph Wu

Re: Review Request 40553: Enable mesos tests installation.

[Benjamin Bannier]

> On Jan. 27, 2016, 8:44 p.m., Benjamin Bannier wrote:
> > Looks mostly good to me. A few things were unclear to me:
> > 
> > * Would it make sense to add an `installcheck` target? My expectation for 
> > that would be for it to invoke the installed tests and adding the 
> > (in)correct `builddir` magic to make sure only stuff from `$PREFIX` is used.
> > * Why don't we install stout or libprocess tests? I would have thought that 
> > especially the libprocess tests could be useful to diagnose low-level 
> > incompatibilites.
> 
> James Peach wrote:
> I added an ``installcheck`` target.
> 
> I didn't try to deal with stout or libprocess at the same time as this. 
> Maybe it makes sense to also install them, but the Mesos suite is going to 
> give the most benefit. If we want to go down that path, I'd like to do that 
> as separate Jira tickets.

Nice. I've filed MESOS-4538 to activate `--enable-tests-install` for CI 
distcheck so these tests don't rot away. I've also filed MESOS-4537 for 
installing stout and libprocess tests.


- Benjamin


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


On Jan. 27, 2016, 10:03 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40553/
> ---
> 
> (Updated Jan. 27, 2016, 10:03 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Till Toenshoff.
> 
> 
> Bugs: MESOS-3608
> https://issues.apache.org/jira/browse/MESOS-3608
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch enables the installation mesos-tests and its dependencies
> and helper tool. The goal is to allow operators to build a separate
> test package that can be run at deployment time to verify that Mesos
> works in the deployment environment.
> 
> Since the build directory is searched first, to run it on a host
> that has a build tree, you need to specify a non-existent tree:
> 
> ~ $ $PREFIX/libexec/mesos/tests/mesos-tests --build_dir=/none
> 
> - Add --enable-tests-install
> - Fix mesos-tests gmock dependencies
> - Optionally install tests, helpers and test modules
> - Add utility helpers to find various test resources
> 
> 
> Diffs
> -
> 
>   Makefile.am fbd4e5a6356e90c867ba47c48c86fc9161ddd98e 
>   configure.ac 70d16871d5888ac1d9d5fb0ba0b453799b00f320 
>   src/Makefile.am 8657a869f931aa7482fbb09f2c6df95b6a8c50c6 
>   src/examples/test_framework.cpp ff7b00543eea1a7cbf52c7abfba81600868bfbab 
>   src/tests/balloon_framework_test.sh 
> 25a19cfde87a3fd2d7d3e780700d342d08bd0a91 
>   src/tests/containerizer/launch_tests.cpp 
> c7ebe2606e4ff99ced90342dd16e0b4bf02bc504 
>   src/tests/containerizer/memory_test_helper.cpp 
> 4a3de2e3c887aa6afc604588850e1386f92d8c11 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> a45ed1b2175b7dc16d621a44fbccfb8f957ae2b5 
>   src/tests/containerizer/ns_tests.cpp 
> 603e54b7303c5aa15e2c5715dc7a2f7e7d39541b 
>   src/tests/containerizer/port_mapping_tests.cpp 
> 182fe9217a5da9af603d6f9c203a1689eff4ca1b 
>   src/tests/environment.cpp e112270b68d402bb9b01445af552500fb3929e52 
>   src/tests/event_call_framework_test.sh 
> 9d1211552734afbf15b376f8c4629bae8a2065af 
>   src/tests/fetcher_tests.cpp 1831d896ca8a52bec4adf87a67b6af845079796c 
>   src/tests/health_check_tests.cpp 65e8fe25ed7feb1080ad833ba98e6b462bd3152c 
>   src/tests/mesos.cpp 18d0d8f8037ebc27c87bcb0f1ce9f143e7505ec8 
>   src/tests/module.cpp 246f3a402d4fe3b273c459f6e02c009f3de65f3e 
>   src/tests/module_tests.cpp a3271a3267647e0964dd3decb3ca8384417dd559 
>   src/tests/no_executor_framework_test.sh 
> aebdc8c380abb2d041d6fc74dfac5a111c15267e 
>   src/tests/oversubscription_tests.cpp 
> 6f43103e81303015fb614653e3bfece55009d1bf 
>   src/tests/persistent_volume_framework_test.sh 
> 84f02847a8d89400512d8a5714d33fb29cf5b03a 
>   src/tests/script.cpp ee44fef29fb40e414d7507168091ee5cd0d15736 
>   src/tests/slave_tests.cpp e943ab99baf3d74679a5da89a89f6a4b7ead 
>   src/tests/test_framework_test.sh 409e80994f63448115ea8ac34b4fd5c6cf88aa22 
>   src/tests/utils.hpp a6cca472f4dfab12cd6eccab6972206d842177aa 
>   src/tests/utils.cpp 22bf3a85da5261fcfcc8b6aa9626aacdc8391ad4 
> 
> Diff: https://reviews.apache.org/r/40553/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 40553: Enable mesos tests installation.

[Benjamin Bannier]

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


Fix it, then Ship it!





src/Makefile.am (line 1999)


Sorry for being unclear, of course `s/installcheck/installcheck-local`.


- Benjamin Bannier


On Jan. 27, 2016, 10:03 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40553/
> ---
> 
> (Updated Jan. 27, 2016, 10:03 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Till Toenshoff.
> 
> 
> Bugs: MESOS-3608
> https://issues.apache.org/jira/browse/MESOS-3608
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch enables the installation mesos-tests and its dependencies
> and helper tool. The goal is to allow operators to build a separate
> test package that can be run at deployment time to verify that Mesos
> works in the deployment environment.
> 
> Since the build directory is searched first, to run it on a host
> that has a build tree, you need to specify a non-existent tree:
> 
> ~ $ $PREFIX/libexec/mesos/tests/mesos-tests --build_dir=/none
> 
> - Add --enable-tests-install
> - Fix mesos-tests gmock dependencies
> - Optionally install tests, helpers and test modules
> - Add utility helpers to find various test resources
> 
> 
> Diffs
> -
> 
>   Makefile.am fbd4e5a6356e90c867ba47c48c86fc9161ddd98e 
>   configure.ac 70d16871d5888ac1d9d5fb0ba0b453799b00f320 
>   src/Makefile.am 8657a869f931aa7482fbb09f2c6df95b6a8c50c6 
>   src/examples/test_framework.cpp ff7b00543eea1a7cbf52c7abfba81600868bfbab 
>   src/tests/balloon_framework_test.sh 
> 25a19cfde87a3fd2d7d3e780700d342d08bd0a91 
>   src/tests/containerizer/launch_tests.cpp 
> c7ebe2606e4ff99ced90342dd16e0b4bf02bc504 
>   src/tests/containerizer/memory_test_helper.cpp 
> 4a3de2e3c887aa6afc604588850e1386f92d8c11 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> a45ed1b2175b7dc16d621a44fbccfb8f957ae2b5 
>   src/tests/containerizer/ns_tests.cpp 
> 603e54b7303c5aa15e2c5715dc7a2f7e7d39541b 
>   src/tests/containerizer/port_mapping_tests.cpp 
> 182fe9217a5da9af603d6f9c203a1689eff4ca1b 
>   src/tests/environment.cpp e112270b68d402bb9b01445af552500fb3929e52 
>   src/tests/event_call_framework_test.sh 
> 9d1211552734afbf15b376f8c4629bae8a2065af 
>   src/tests/fetcher_tests.cpp 1831d896ca8a52bec4adf87a67b6af845079796c 
>   src/tests/health_check_tests.cpp 65e8fe25ed7feb1080ad833ba98e6b462bd3152c 
>   src/tests/mesos.cpp 18d0d8f8037ebc27c87bcb0f1ce9f143e7505ec8 
>   src/tests/module.cpp 246f3a402d4fe3b273c459f6e02c009f3de65f3e 
>   src/tests/module_tests.cpp a3271a3267647e0964dd3decb3ca8384417dd559 
>   src/tests/no_executor_framework_test.sh 
> aebdc8c380abb2d041d6fc74dfac5a111c15267e 
>   src/tests/oversubscription_tests.cpp 
> 6f43103e81303015fb614653e3bfece55009d1bf 
>   src/tests/persistent_volume_framework_test.sh 
> 84f02847a8d89400512d8a5714d33fb29cf5b03a 
>   src/tests/script.cpp ee44fef29fb40e414d7507168091ee5cd0d15736 
>   src/tests/slave_tests.cpp e943ab99baf3d74679a5da89a89f6a4b7ead 
>   src/tests/test_framework_test.sh 409e80994f63448115ea8ac34b4fd5c6cf88aa22 
>   src/tests/utils.hpp a6cca472f4dfab12cd6eccab6972206d842177aa 
>   src/tests/utils.cpp 22bf3a85da5261fcfcc8b6aa9626aacdc8391ad4 
> 
> Diff: https://reviews.apache.org/r/40553/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 42877: Cleaned up MesosSchedulerDriver shutdown in unit tests.

[Klaus Ma]

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


Ship it!




Ship It!

- Klaus Ma


On Jan. 28, 2016, 6:56 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42877/
> ---
> 
> (Updated Jan. 28, 2016, 6:56 a.m.)
> 
> 
> Review request for mesos, Greg Mann and Joris Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> For consistency, we should do `driver.stop(); driver.join();` if a
> `MesosSchedulerDriver` has been started by the unit test. The destructor for
> `MesosSchedulerDriver` will do something _similar_, so the consequence of
> omitting these calls is not dire, but it seems safer to be consistent between
> all the test cases.
> 
> 
> Diffs
> -
> 
>   src/tests/executor_http_api_tests.cpp 
> 36a042ed103271ca873450236f39a8152fbbf07e 
>   src/tests/oversubscription_tests.cpp 
> 6f43103e81303015fb614653e3bfece55009d1bf 
>   src/tests/reservation_endpoints_tests.cpp 
> 35c093567b07a11ca6eee85e2ff91894a460a7af 
>   src/tests/scheduler_event_call_tests.cpp 
> bd8920fa9d5475e5f6533c8424ebff1588bfe645 
>   src/tests/scheduler_http_api_tests.cpp 
> 9eb1de7d9541395b92b951f0fe0ddbb2f219fe30 
>   src/tests/slave_tests.cpp e943ab99baf3d74679a5da89a89f6a4b7ead 
> 
> Diff: https://reviews.apache.org/r/42877/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> Note that adding the `driver.stop(); driver.join();` calls to two of the 
> slave tests requires adding an extra shutdown expectation to avoid a GMock 
> warning.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 42662: Added common command utils file.

[Klaus Ma]

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




src/common/command_utils.cpp (line 17)


Move to line 28.



src/common/command_utils.cpp (lines 91 - 92)


Move to one line if less than 80.



src/tests/common/command_utils_tests.cpp (line 71)


Add a case for `tar/untar` failure.


- Klaus Ma


On Jan. 28, 2016, 4:29 a.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42662/
> ---
> 
> (Updated Jan. 28, 2016, 4:29 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This common file is a good place to add common command line utilities like 
> tar,
> digests(sha256, sha512, etc). Currently this functionality is spread in the 
> code
> base and this change would enable all those call sites to be replaced with a
> common code.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 47d0a7c0fe73b9297cd7dde6086b5e6e9e1f9e4e 
>   src/Makefile.am 19bf3a7c2e43ca04ed6e6d506e052de5537f7c2f 
>   src/common/command_utils.hpp PRE-CREATION 
>   src/common/command_utils.cpp PRE-CREATION 
>   src/tests/common/command_utils_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/42662/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Review Request 42890: Allocator Performance: Simplified Sorter's 'CalculateShare'.

[Joris Van Remoortere]

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

Review request for mesos, Jie Yu and Michael Park.


Repository: mesos


Description
---

Used existing functions to aggregate Scalars in the 'Resources' object.


Diffs
-

  src/master/allocator/sorter/drf/sorter.cpp 
1f39f6d12cb2673ef4b75c3efb23ca5f7dbbf29a 

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


Testing
---

valgrind on allocator benchmark: 2x


Thanks,

Joris Van Remoortere

Review Request 42872: Relaxed the subsystem check for net_cls.

[Avinash sridharan]

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

Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

Relaxed the subsystem check for net_cls. The check ensures that only the 
net_cls and net_prio subsystem can be mounted in the same hierarchy.


Diffs
-

  src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp 
03a488e0025da2f0a835cdc39379c901a83b78b0 

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


Testing
---

make and make check on Debian 8


Thanks,

Avinash sridharan

Re: Review Request 42877: Cleaned up MesosSchedulerDriver shutdown in unit tests.

[Neil Conway]

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

(Updated Jan. 27, 2016, 10:56 p.m.)


Review request for mesos, Greg Mann and Joris Van Remoortere.


Changes
---

Add now-required shutdown mocks.


Repository: mesos


Description
---

For consistency, we should do `driver.stop(); driver.join();` if a
`MesosSchedulerDriver` has been started by the unit test. The destructor for
`MesosSchedulerDriver` will do something _similar_, so the consequence of
omitting these calls is not dire, but it seems safer to be consistent between
all the test cases.


Diffs (updated)
-

  src/tests/executor_http_api_tests.cpp 
36a042ed103271ca873450236f39a8152fbbf07e 
  src/tests/oversubscription_tests.cpp 6f43103e81303015fb614653e3bfece55009d1bf 
  src/tests/reservation_endpoints_tests.cpp 
35c093567b07a11ca6eee85e2ff91894a460a7af 
  src/tests/scheduler_event_call_tests.cpp 
bd8920fa9d5475e5f6533c8424ebff1588bfe645 
  src/tests/scheduler_http_api_tests.cpp 
9eb1de7d9541395b92b951f0fe0ddbb2f219fe30 
  src/tests/slave_tests.cpp e943ab99baf3d74679a5da89a89f6a4b7ead 

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


Testing (updated)
---

make check

Note that adding the `driver.stop(); driver.join();` calls to two of the slave 
tests requires adding an extra shutdown expectation to avoid a GMock warning.


Thanks,

Neil Conway

Re: Review Request 40553: Enable mesos tests installation.

[James Peach]

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

(Updated Jan. 28, 2016, 12:49 a.m.)


Review request for mesos, Benjamin Bannier and Till Toenshoff.


Changes
---

Switch to installcheck-local.


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


Repository: mesos


Description
---

This patch enables the installation mesos-tests and its dependencies
and helper tool. The goal is to allow operators to build a separate
test package that can be run at deployment time to verify that Mesos
works in the deployment environment.

Since the build directory is searched first, to run it on a host
that has a build tree, you need to specify a non-existent tree:

~ $ $PREFIX/libexec/mesos/tests/mesos-tests --build_dir=/none

- Add --enable-tests-install
- Fix mesos-tests gmock dependencies
- Optionally install tests, helpers and test modules
- Add utility helpers to find various test resources


Diffs (updated)
-

  Makefile.am fbd4e5a6356e90c867ba47c48c86fc9161ddd98e 
  configure.ac 70d16871d5888ac1d9d5fb0ba0b453799b00f320 
  src/Makefile.am 8657a869f931aa7482fbb09f2c6df95b6a8c50c6 
  src/examples/test_framework.cpp ff7b00543eea1a7cbf52c7abfba81600868bfbab 
  src/tests/balloon_framework_test.sh 25a19cfde87a3fd2d7d3e780700d342d08bd0a91 
  src/tests/containerizer/launch_tests.cpp 
c7ebe2606e4ff99ced90342dd16e0b4bf02bc504 
  src/tests/containerizer/memory_test_helper.cpp 
4a3de2e3c887aa6afc604588850e1386f92d8c11 
  src/tests/containerizer/mesos_containerizer_tests.cpp 
a45ed1b2175b7dc16d621a44fbccfb8f957ae2b5 
  src/tests/containerizer/ns_tests.cpp 603e54b7303c5aa15e2c5715dc7a2f7e7d39541b 
  src/tests/containerizer/port_mapping_tests.cpp 
182fe9217a5da9af603d6f9c203a1689eff4ca1b 
  src/tests/environment.cpp e112270b68d402bb9b01445af552500fb3929e52 
  src/tests/event_call_framework_test.sh 
9d1211552734afbf15b376f8c4629bae8a2065af 
  src/tests/fetcher_tests.cpp 1831d896ca8a52bec4adf87a67b6af845079796c 
  src/tests/health_check_tests.cpp 65e8fe25ed7feb1080ad833ba98e6b462bd3152c 
  src/tests/mesos.cpp 18d0d8f8037ebc27c87bcb0f1ce9f143e7505ec8 
  src/tests/module.cpp 246f3a402d4fe3b273c459f6e02c009f3de65f3e 
  src/tests/module_tests.cpp a3271a3267647e0964dd3decb3ca8384417dd559 
  src/tests/no_executor_framework_test.sh 
aebdc8c380abb2d041d6fc74dfac5a111c15267e 
  src/tests/oversubscription_tests.cpp 6f43103e81303015fb614653e3bfece55009d1bf 
  src/tests/persistent_volume_framework_test.sh 
84f02847a8d89400512d8a5714d33fb29cf5b03a 
  src/tests/script.cpp ee44fef29fb40e414d7507168091ee5cd0d15736 
  src/tests/slave_tests.cpp e943ab99baf3d74679a5da89a89f6a4b7ead 
  src/tests/test_framework_test.sh 409e80994f63448115ea8ac34b4fd5c6cf88aa22 
  src/tests/utils.hpp a6cca472f4dfab12cd6eccab6972206d842177aa 
  src/tests/utils.cpp 22bf3a85da5261fcfcc8b6aa9626aacdc8391ad4 

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


Testing
---


Thanks,

James Peach

Re: Review Request 42887: Fixed a flaky test in disk quota tests.

[Joris Van Remoortere]

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


Ship it!




Ship It!

- Joris Van Remoortere


On Jan. 28, 2016, 1:06 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42887/
> ---
> 
> (Updated Jan. 28, 2016, 1:06 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-4533
> https://issues.apache.org/jira/browse/MESOS-4533
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed a flaky test in disk quota tests.
> 
> 
> Diffs
> -
> 
>   src/tests/disk_quota_tests.cpp eb93c2fc4a791fdb8a61e4c539fa85ece7c5604e 
> 
> Diff: https://reviews.apache.org/r/42887/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> verified on Neil's box.
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Review Request 42878: Fixed the NetClsIsolatorTest to correctly learn the net_cls hierarchy.

[Avinash sridharan]

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

Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

Fixed the NetClsIsolatorTest to correctly learn the net_cls hierarchy. The test 
relied on the flags.cgroups_hierarchy to learn the net_cls hierarchy, instead 
it should be making a call to cgroups::hierarchy to learn the hierarchy.


Diffs
-

  src/tests/containerizer/isolator_tests.cpp 
7033ac1aa7ae8a3cf06d4f717580ce5a5afb648a 

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


Testing
---

make and make check on Debian 8

Also ran the patch on CentOS 6 with Greg's help. He was using 
(boxcutter/centos66) vagrant image to test this patch. (boxcutter/centos66) is 
where this test was failing.


Thanks,

Avinash sridharan

Re: Review Request 42648: Moved http authenticator initialization to main.

[Benjamin Bannier]

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




src/master/main.cpp (line 112)


Personally I find this adds more confusion. There are both 
`process::http::authentication` and `mesos::http::authentication`, and here we 
introduce an alias for only of of them (and (sadly) we don't seem to be too 
fond of namespace aliases anyway). I personally would prefer to drop this here 
and just going full verbose (since we cannot use `auto`).



src/tests/cluster.cpp (lines 213 - 245)


Could this be factored out into a separate function we'd call from both 
`main` and here? The `EXIT` certainly wouldn't make this a nice function to 
call, but this is a lot of repeated, not 100% trivial code ...


- Benjamin Bannier


On Jan. 27, 2016, 10:23 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42648/
> ---
> 
> (Updated Jan. 27, 2016, 10:23 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Bannier, Jan 
> Schlicht, and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Moves the code which initializes instances of 
> `process::http::authentication::Authenticator` from `mesos::internal::Master` 
> to the master main file.
> 
> The change is needed in order to show that libprocess http authentication is 
> turned at the system process level and not at the libprocess process level.
> 
> It also adds the same initialization as in main to the test function 
> `StartMaster`.
> 
> 
> Diffs
> -
> 
>   src/master/main.cpp 0d5ac49121491152d8426cd764a1f1a0f37483ae 
>   src/master/master.cpp 9ee56277c8a472be9e683d5db505becfb5f7c422 
>   src/tests/cluster.cpp 1a3a038e78999bca2fc3f08725f4ffaf4290bcb9 
> 
> Diff: https://reviews.apache.org/r/42648/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> manual tests.
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 42878: Fixed the NetClsIsolatorTest to correctly learn the net_cls hierarchy.

[Jie Yu]

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


Fix it, then Ship it!





src/tests/containerizer/isolator_tests.cpp (lines 866 - 867)


Can you move this down right above `cgroups::processes`?



src/tests/containerizer/isolator_tests.cpp (line 928)


Sorry, I missed that during review. Why snake_case here?


- Jie Yu


On Jan. 27, 2016, 11:16 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42878/
> ---
> 
> (Updated Jan. 27, 2016, 11:16 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4530
> https://issues.apache.org/jira/browse/MESOS-4530
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed the NetClsIsolatorTest to correctly learn the net_cls hierarchy. The 
> test relied on the flags.cgroups_hierarchy to learn the net_cls hierarchy, 
> instead it should be making a call to cgroups::hierarchy to learn the 
> hierarchy.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/isolator_tests.cpp 
> 7033ac1aa7ae8a3cf06d4f717580ce5a5afb648a 
> 
> Diff: https://reviews.apache.org/r/42878/diff/
> 
> 
> Testing
> ---
> 
> make and make check on Debian 8
> 
> Also ran the patch on CentOS 6 with Greg's help. He was using 
> (boxcutter/centos66) vagrant image to test this patch. (boxcutter/centos66) 
> is where this test was failing.
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 42890: Allocator Performance: Simplified Sorter's 'CalculateShare'.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On Jan. 28, 2016, 1:44 a.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42890/
> ---
> 
> (Updated Jan. 28, 2016, 1:44 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Michael Park.
> 
> 
> Bugs: MESOS-4505
> https://issues.apache.org/jira/browse/MESOS-4505
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Used existing functions to aggregate Scalars in the 'Resources' object.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/sorter/drf/sorter.cpp 
> 1f39f6d12cb2673ef4b75c3efb23ca5f7dbbf29a 
> 
> Diff: https://reviews.apache.org/r/42890/diff/
> 
> 
> Testing
> ---
> 
> valgrind on allocator benchmark: 2x
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 42662: Added common command utils file.

[Jie Yu]

> On Jan. 28, 2016, 12:52 a.m., Klaus Ma wrote:
> > src/common/command_utils.cpp, line 17
> > 
> >
> > Move to line 28.
> 
> Jojy Varghese wrote:
> According to google style guide 
> (https://google.github.io/styleguide/cppguide.html#Names_and_Order_of_Includes),
>  the header file for an implementation should be included first. I thought we 
> follow google style guide unless something is overridden.

+1


> On Jan. 28, 2016, 12:52 a.m., Klaus Ma wrote:
> > src/common/command_utils.cpp, lines 91-92
> > 
> >
> > Move to one line if less than 80.

+1


> On Jan. 28, 2016, 12:52 a.m., Klaus Ma wrote:
> > src/tests/common/command_utils_tests.cpp, line 71
> > 
> >
> > Add a case for `tar/untar` failure.
> 
> Jojy Varghese wrote:
> Will add a TODO.

Maybe in a subsequent review. This patch is getting too big. @jojy, can you 
follow up with that?


- Jie


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


On Jan. 27, 2016, 8:29 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42662/
> ---
> 
> (Updated Jan. 27, 2016, 8:29 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This common file is a good place to add common command line utilities like 
> tar,
> digests(sha256, sha512, etc). Currently this functionality is spread in the 
> code
> base and this change would enable all those call sites to be replaced with a
> common code.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 47d0a7c0fe73b9297cd7dde6086b5e6e9e1f9e4e 
>   src/Makefile.am 19bf3a7c2e43ca04ed6e6d506e052de5537f7c2f 
>   src/common/command_utils.hpp PRE-CREATION 
>   src/common/command_utils.cpp PRE-CREATION 
>   src/tests/common/command_utils_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/42662/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 42860: Add paths::sameParent to find out the same root of a path list.

[haosdent huang]

> On Jan. 27, 2016, 9:06 p.m., Benjamin Bannier wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp, line 68
> > 
> >
> > I am extremely confused about the expected semantics of this function 
> > (e.g. `EXPECT_SOME_EQ("/", path::sameParent({"/usr", "/", "/tmp"}));`?). I 
> > feel developing an intuition would be easier would this function take just 
> > two args (but am unsure); also, the name seems to suggest this returning 
> > some `bool` type which it doesn't.
> > 
> > Looking at the implementation isn't too helpful either due to the 
> > complicated control flow. It should in principle be possible to simplify 
> > this by separating extracting path components and their comparisons, but am 
> > unsure if this is something we need for the intended use case from the 
> > follow-up patch.
> 
> haosdent huang wrote:
> Thank you very much for your review. Do you mean change `sameParent` to 
> other names here?
> 
> Benjamin Bannier wrote:
> If you can, yes, but I am unsure this is a useful enough primitive for 
> `path`s. You should find a shepherd and discuss this with him/her.
> 
> I'd imagine that if you'd provide something to e.g. find a common prefix 
> for a list of `path`s one might be able to reuse that functionality 
> elsewhere; to get the functionality you need you could then check if that 
> common prefix is already in the given list (that would be in your next patch, 
> not here in stout).

oh, yes. I want implement a util funtion similar to os.path.commonprefix in 
python here.


- haosdent


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


On Jan. 28, 2016, 2:12 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42860/
> ---
> 
> (Updated Jan. 28, 2016, 2:12 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-4521
> https://issues.apache.org/jira/browse/MESOS-4521
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add paths::sameParent to find out the same root of a path list.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp 
> ef538045a8b7a1e3d8962c869317d86a85e0259f 
>   3rdparty/libprocess/3rdparty/stout/tests/path_tests.cpp 
> 6dff5e76e0e15098c5a262adc50bfcb65f933697 
> 
> Diff: https://reviews.apache.org/r/42860/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 42617: stout: Cleaned up usage of namespace-qualified identifiers.

[Mesos ReviewBot]

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



Bad patch!

Reviews applied: [42615]

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

Error:
 ..
2016-01-28 05:53:38 URL:https://reviews.apache.org/r/42615/diff/raw/ 
[40106/40106] -> "42615.patch" [1]
Total errors found: 0
Checking 54 files
Error: Commit message summary (the first line) must start with a capital letter.

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

- Mesos ReviewBot


On Jan. 28, 2016, 12:56 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42617/
> ---
> 
> (Updated Jan. 28, 2016, 12:56 a.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> For example, avoid using `std::string` in a .cpp file that already does
> `using std::string`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/tests/flags_tests.cpp 
> 5a10f59f3698a6ad85b7c2386def9686af8a23a6 
>   3rdparty/libprocess/3rdparty/stout/tests/linkedhashmap_tests.cpp 
> 21a98b85277d946b887cb23c4ebb4d32cfcb0c6b 
>   3rdparty/libprocess/3rdparty/stout/tests/os/filesystem_tests.cpp 
> c5ffd017f83149a58a2dee8c87461cd06bb43bed 
>   3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp 
> 4b7d1347f93261df7a7d512721146edca2703b34 
>   3rdparty/libprocess/3rdparty/stout/tests/some_tests.cpp 
> c0be974ea2dc74c63ec3561403b3f4d3722a8df3 
>   3rdparty/libprocess/3rdparty/stout/tests/strings_tests.cpp 
> 7715fa4baa150f370bdd0fd5c2f98dc4c6f5fc55 
> 
> Diff: https://reviews.apache.org/r/42617/diff/
> 
> 
> Testing
> ---
> 
> "make check" on OSX and Arch Linux.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 42890: Allocator Performance: Simplified Sorter's 'CalculateShare'.

[Jie Yu]

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




src/master/allocator/sorter/drf/sorter.cpp (line 358)


This should be '='?


- Jie Yu


On Jan. 28, 2016, 1:44 a.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42890/
> ---
> 
> (Updated Jan. 28, 2016, 1:44 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Michael Park.
> 
> 
> Bugs: MESOS-4505
> https://issues.apache.org/jira/browse/MESOS-4505
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Used existing functions to aggregate Scalars in the 'Resources' object.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/sorter/drf/sorter.cpp 
> 1f39f6d12cb2673ef4b75c3efb23ca5f7dbbf29a 
> 
> Diff: https://reviews.apache.org/r/42890/diff/
> 
> 
> Testing
> ---
> 
> valgrind on allocator benchmark: 2x
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 42662: Added common command utils file.

[Jojy Varghese]

> On Jan. 28, 2016, 12:52 a.m., Klaus Ma wrote:
> > src/tests/common/command_utils_tests.cpp, line 71
> > 
> >
> > Add a case for `tar/untar` failure.
> 
> Jojy Varghese wrote:
> Will add a TODO.
> 
> Jie Yu wrote:
> Maybe in a subsequent review. This patch is getting too big. @jojy, can 
> you follow up with that?

@jie will do.


- Jojy


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


On Jan. 28, 2016, 2:21 a.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42662/
> ---
> 
> (Updated Jan. 28, 2016, 2:21 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4360
> https://issues.apache.org/jira/browse/MESOS-4360
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This common file is a good place to add common command line utilities like 
> tar,
> digests(sha256, sha512, etc). Currently this functionality is spread in the 
> code
> base and this change would enable all those call sites to be replaced with a
> common code.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 47d0a7c0fe73b9297cd7dde6086b5e6e9e1f9e4e 
>   src/Makefile.am 19bf3a7c2e43ca04ed6e6d506e052de5537f7c2f 
>   src/common/command_utils.hpp PRE-CREATION 
>   src/common/command_utils.cpp PRE-CREATION 
>   src/tests/common/command_utils_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/42662/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 42860: Add paths::sameParent to find out the same root of a path list.

[haosdent huang]

> On Jan. 27, 2016, 9:06 p.m., Benjamin Bannier wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp, line 68
> > 
> >
> > I am extremely confused about the expected semantics of this function 
> > (e.g. `EXPECT_SOME_EQ("/", path::sameParent({"/usr", "/", "/tmp"}));`?). I 
> > feel developing an intuition would be easier would this function take just 
> > two args (but am unsure); also, the name seems to suggest this returning 
> > some `bool` type which it doesn't.
> > 
> > Looking at the implementation isn't too helpful either due to the 
> > complicated control flow. It should in principle be possible to simplify 
> > this by separating extracting path components and their comparisons, but am 
> > unsure if this is something we need for the intended use case from the 
> > follow-up patch.

Thank you very much for your review. Do you mean change `sameParent` to other 
names here?


- haosdent


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


On Jan. 27, 2016, 6:36 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42860/
> ---
> 
> (Updated Jan. 27, 2016, 6:36 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-4521
> https://issues.apache.org/jira/browse/MESOS-4521
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add paths::sameParent to find out the same root of a path list.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp 
> ef538045a8b7a1e3d8962c869317d86a85e0259f 
>   3rdparty/libprocess/3rdparty/stout/tests/path_tests.cpp 
> 6dff5e76e0e15098c5a262adc50bfcb65f933697 
> 
> Diff: https://reviews.apache.org/r/42860/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 40731: Added a fixture to test the floating point precision during CPU resource allocation.

[Avinash sridharan]

> On Jan. 19, 2016, 10:37 a.m., Bernd Mathiske wrote:
> > src/tests/reservation_tests.cpp, line 253
> > 
> >
> > Could we check for 0.2 CPU here?
> 
> Avinash sridharan wrote:
> This makes sense. Will modify the expectation, test and upload a change 
> to reflect this.

Modified the expectation on the final offer to check for the following reserved 
resources in the final offer:
Resources reserved = Resources::parse("cpus:0.2;mem:512").get();


- Avinash


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


On Jan. 28, 2016, 2:20 a.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40731/
> ---
> 
> (Updated Jan. 28, 2016, 2:20 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Klaus Ma, Mandeep Chadha, and Neil 
> Conway.
> 
> 
> Bugs: MESOS-3552
> https://issues.apache.org/jira/browse/MESOS-3552
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a fixture to test the floating point precision during CPU resource 
> allocation. This patch is based on the commit submitted by : Mandeep 
> (@mchadha) https://reviews.apache.org/r/39056/
> 
> 
> Diffs
> -
> 
>   src/tests/reservation_tests.cpp d0c88560a5618ebec39fc4e0539ddb2d9ca554a2 
> 
> Diff: https://reviews.apache.org/r/40731/diff/
> 
> 
> Testing
> ---
> 
> Ran make check after adding Mandeep's test case to the GTEST framework.
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 40731: Added a fixture to test the floating point precision during CPU resource allocation.

[Avinash sridharan]

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

(Updated Jan. 28, 2016, 2:20 a.m.)


Review request for mesos, Bernd Mathiske, Klaus Ma, Mandeep Chadha, and Neil 
Conway.


Summary (updated)
-

Added a fixture to test the floating point precision during CPU resource 
allocation.


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


Repository: mesos


Description
---

Added a fixture to test the floating point precision during CPU resource 
allocation. This patch is based on the commit submitted by : Mandeep (@mchadha) 
https://reviews.apache.org/r/39056/


Diffs (updated)
-

  src/tests/reservation_tests.cpp d0c88560a5618ebec39fc4e0539ddb2d9ca554a2 

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


Testing
---

Ran make check after adding Mandeep's test case to the GTEST framework.


Thanks,

Avinash sridharan

Re: Review Request 42662: Added common command utils file.

[Jojy Varghese]

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

(Updated Jan. 28, 2016, 2:21 a.m.)


Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

This common file is a good place to add common command line utilities like tar,
digests(sha256, sha512, etc). Currently this functionality is spread in the code
base and this change would enable all those call sites to be replaced with a
common code.


Diffs
-

  src/CMakeLists.txt 47d0a7c0fe73b9297cd7dde6086b5e6e9e1f9e4e 
  src/Makefile.am 19bf3a7c2e43ca04ed6e6d506e052de5537f7c2f 
  src/common/command_utils.hpp PRE-CREATION 
  src/common/command_utils.cpp PRE-CREATION 
  src/tests/common/command_utils_tests.cpp PRE-CREATION 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 42860: Add paths::sameParent to find out the same root of a path list.

[Benjamin Bannier]

> On Jan. 27, 2016, 10:06 p.m., Benjamin Bannier wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp, line 68
> > 
> >
> > I am extremely confused about the expected semantics of this function 
> > (e.g. `EXPECT_SOME_EQ("/", path::sameParent({"/usr", "/", "/tmp"}));`?). I 
> > feel developing an intuition would be easier would this function take just 
> > two args (but am unsure); also, the name seems to suggest this returning 
> > some `bool` type which it doesn't.
> > 
> > Looking at the implementation isn't too helpful either due to the 
> > complicated control flow. It should in principle be possible to simplify 
> > this by separating extracting path components and their comparisons, but am 
> > unsure if this is something we need for the intended use case from the 
> > follow-up patch.
> 
> haosdent huang wrote:
> Thank you very much for your review. Do you mean change `sameParent` to 
> other names here?

If you can, yes, but I am unsure this is a useful enough primitive for `path`s. 
You should find a shepherd and discuss this with him/her.

I'd imagine that if you'd provide something to e.g. find a common prefix for a 
list of `path`s one might be able to reuse that functionality elsewhere; to get 
the functionality you need you could then check if that common prefix is 
already in the given list (that would be in your next patch, not here in stout).


- Benjamin


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


On Jan. 28, 2016, 3:12 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42860/
> ---
> 
> (Updated Jan. 28, 2016, 3:12 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-4521
> https://issues.apache.org/jira/browse/MESOS-4521
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add paths::sameParent to find out the same root of a path list.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp 
> ef538045a8b7a1e3d8962c869317d86a85e0259f 
>   3rdparty/libprocess/3rdparty/stout/tests/path_tests.cpp 
> 6dff5e76e0e15098c5a262adc50bfcb65f933697 
> 
> Diff: https://reviews.apache.org/r/42860/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 42888: Used absolute paths for excludes paths in posix disk isolator.

[Joris Van Remoortere]

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


Ship it!




Ship It!

- Joris Van Remoortere


On Jan. 28, 2016, 1:14 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42888/
> ---
> 
> (Updated Jan. 28, 2016, 1:14 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-4539
> https://issues.apache.org/jira/browse/MESOS-4539
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Used absolute paths for excludes paths in posix disk isolator.
> 
> We should consistently use absolute paths for excludes paths in disk 
> isolator, otherwise, it's possible that some other paths under the directory 
> matches the relative path, causing incorrect disk usage being reported.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/posix/disk.hpp 
> 5607c0f43084f491e4927f5c6ec283210aa5f355 
>   src/slave/containerizer/mesos/isolators/posix/disk.cpp 
> d5bf8c10b9cfb360a7856144d59ce89060de5c99 
> 
> Diff: https://reviews.apache.org/r/42888/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 42890: Allocator Performance: Simplified Sorter's 'CalculateShare'.

[Michael Park]

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


Ship it!




Ship It!

- Michael Park


On Jan. 28, 2016, 2:59 a.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42890/
> ---
> 
> (Updated Jan. 28, 2016, 2:59 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Michael Park.
> 
> 
> Bugs: MESOS-4505
> https://issues.apache.org/jira/browse/MESOS-4505
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Used existing functions to aggregate Scalars in the 'Resources' object.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/sorter/drf/sorter.cpp 
> 1f39f6d12cb2673ef4b75c3efb23ca5f7dbbf29a 
> 
> Diff: https://reviews.apache.org/r/42890/diff/
> 
> 
> Testing
> ---
> 
> valgrind on allocator benchmark: 2x
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 42589: Added test case for allocator recover with Quota.

[Klaus Ma]

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



ping AlexR/Joris, can you help to review this test case for allocator recover 
with Quota?

- Klaus Ma


On Jan. 21, 2016, 2:27 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42589/
> ---
> 
> (Updated Jan. 21, 2016, 2:27 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Ben Mahler, and Joris Van 
> Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test case for allocator recover with Quota.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> 953712149bd951789beb29c72779c4ac65aa48dc 
> 
> Diff: https://reviews.apache.org/r/42589/diff/
> 
> 
> Testing
> ---
> 
> make
> make check GTEST_FILTER="HierarchicalAllocatorTest.RecoverWithQuota" (CHECK() 
> failed without fix)
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 42890: Allocator Performance: Simplified Sorter's 'CalculateShare'.

[Joris Van Remoortere]

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

(Updated Jan. 28, 2016, 2:59 a.m.)


Review request for mesos, Jie Yu and Michael Park.


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


Repository: mesos


Description
---

Used existing functions to aggregate Scalars in the 'Resources' object.


Diffs (updated)
-

  src/master/allocator/sorter/drf/sorter.cpp 
1f39f6d12cb2673ef4b75c3efb23ca5f7dbbf29a 

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


Testing
---

valgrind on allocator benchmark: 2x


Thanks,

Joris Van Remoortere

Re: Review Request 40851: Windows:[1/2] Add patch for Windows ZK version.

[M Lawindi]

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

(Updated Jan. 28, 2016, 4:59 a.m.)


Review request for Alex Naparu, Dario Bazan, Daniel Pravat, Alex Clemmer, and M 
Lawindi.


Repository: mesos


Description
---

Windows:[1/2] Add patch for Windows ZK version.


Diffs (updated)
-

  3rdparty/zookeeper-06d3f3f.patch PRE-CREATION 

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


Testing
---

- Applied patch to original zookeeper 3.4.5
- Successfully opened 2015 solution in VS2015
- Cli and Zookeeper build successfully


Thanks,

M Lawindi

Re: Review Request 42016: Windows:[2/2] Use ZK in Windows build.

[M Lawindi]

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

(Updated Jan. 28, 2016, 4:58 a.m.)


Review request for mesos, Alex Naparu, Daniel Pravat, Alex Clemmer, M Lawindi, 
and Yi Sun.


Repository: mesos


Description
---

Windows:[2/2] Use ZK in Windows build.


Diffs (updated)
-

  3rdparty/CMakeLists.txt ac5c25a8797a687e84384682975ab99fb3e30448 
  3rdparty/cmake/Mesos3rdpartyConfigure.cmake 
34e61ff90eca0ffdddb6b6b8e2f8e552691637fa 
  3rdparty/patch.exe.manifest PRE-CREATION 
  CMakeLists.txt 9b7044b7860fd64b854ac27b28a48d297dfdeae8 
  src/slave/cmake/SlaveConfigure.cmake cf378a27297474b2a9f338e0c832612370f7302a 

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


Testing
---


Thanks,

M Lawindi

Re: Review Request 42861: Ensure two Mount Disk resources do not have the same root path.

[haosdent huang]

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

(Updated Jan. 28, 2016, 2:12 a.m.)


Review request for mesos, Jie Yu and Joris Van Remoortere.


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


Repository: mesos


Description
---

Ensure two Mount Disk resources do not have the same root path.


Diffs (updated)
-

  src/slave/slave.cpp 1f4c8368feb0ce19963577582ce745acfb21aa9f 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 42860: Add paths::sameParent to find out the same root of a path list.

[haosdent huang]

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

(Updated Jan. 28, 2016, 2:12 a.m.)


Review request for mesos, Jie Yu and Joris Van Remoortere.


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


Repository: mesos


Description
---

Add paths::sameParent to find out the same root of a path list.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp 
ef538045a8b7a1e3d8962c869317d86a85e0259f 
  3rdparty/libprocess/3rdparty/stout/tests/path_tests.cpp 
6dff5e76e0e15098c5a262adc50bfcb65f933697 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 42835: Updated the allocator to not offer unreserved resources beyond quota.

[Qian Zhang]

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




src/master/allocator/mesos/hierarchical.cpp (line 1341)


Should `any more non-revocable` be `any more non-reserved`?


- Qian Zhang


On Jan. 27, 2016, 9:31 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42835/
> ---
> 
> (Updated Jan. 27, 2016, 9:31 a.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Eventually, we will also want to offer unreserved resources as revocable 
> beyond
> the role's quota. In the short term, we do not offer unreserved, non-revocable
> resources beyond a role's quota.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> fa9939700ff44911b9d149a391677b3eb07577ae 
>   src/tests/hierarchical_allocator_tests.cpp 
> 7d5eed4dd4ad2d23e925d5cac73675752b016086 
> 
> Diff: https://reviews.apache.org/r/42835/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 42662: Added common command utils file.

[Jojy Varghese]

> On Jan. 28, 2016, 12:52 a.m., Klaus Ma wrote:
> > src/common/command_utils.cpp, line 17
> > 
> >
> > Move to line 28.

According to google style guide 
(https://google.github.io/styleguide/cppguide.html#Names_and_Order_of_Includes),
 the header file for an implementation should be included first. I thought we 
follow google style guide unless something is overridden.


> On Jan. 28, 2016, 12:52 a.m., Klaus Ma wrote:
> > src/tests/common/command_utils_tests.cpp, line 71
> > 
> >
> > Add a case for `tar/untar` failure.

Will add a TODO.


- Jojy


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


On Jan. 27, 2016, 8:29 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42662/
> ---
> 
> (Updated Jan. 27, 2016, 8:29 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This common file is a good place to add common command line utilities like 
> tar,
> digests(sha256, sha512, etc). Currently this functionality is spread in the 
> code
> base and this change would enable all those call sites to be replaced with a
> common code.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 47d0a7c0fe73b9297cd7dde6086b5e6e9e1f9e4e 
>   src/Makefile.am 19bf3a7c2e43ca04ed6e6d506e052de5537f7c2f 
>   src/common/command_utils.hpp PRE-CREATION 
>   src/common/command_utils.cpp PRE-CREATION 
>   src/tests/common/command_utils_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/42662/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 42362: Added persistent volume endpoint test without authentication.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [42530, 42362]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export 
COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Jan. 27, 2016, 11:04 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42362/
> ---
> 
> (Updated Jan. 27, 2016, 11:04 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Kapil Arya, and Neil Conway.
> 
> 
> Bugs: MESOS-4395
> https://issues.apache.org/jira/browse/MESOS-4395
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added persistent volume endpoint tests with HTTP authentication disabled.
> 
> The persistent volume endpoint tests allow volume creation and destruction 
> when HTTP authentication is disabled; this patch introduces a test for this 
> scenario: `PersistentVolumeEndpointsTest.NoAuthentication`.
> 
> 
> Diffs
> -
> 
>   src/tests/persistent_volume_endpoints_tests.cpp 
> 22e18758ee91a649486725473d9e50fae9d43b01 
> 
> Diff: https://reviews.apache.org/r/42362/diff/
> 
> 
> Testing
> ---
> 
> A new test, `PersistentVolumeEndpointsTest.NoAuthentication`, was added to 
> the persistent volume endpoint tests.
> 
> `make check` was used to test, and the new test was run with 
> `--gtest_repeat=1000 -gtest_break_on_failure=1`.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 40553: Enable mesos tests installation.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [39780, 39781, 39782, 40553]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export 
COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Jan. 28, 2016, 12:49 a.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40553/
> ---
> 
> (Updated Jan. 28, 2016, 12:49 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Till Toenshoff.
> 
> 
> Bugs: MESOS-3608
> https://issues.apache.org/jira/browse/MESOS-3608
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch enables the installation mesos-tests and its dependencies
> and helper tool. The goal is to allow operators to build a separate
> test package that can be run at deployment time to verify that Mesos
> works in the deployment environment.
> 
> Since the build directory is searched first, to run it on a host
> that has a build tree, you need to specify a non-existent tree:
> 
> ~ $ $PREFIX/libexec/mesos/tests/mesos-tests --build_dir=/none
> 
> - Add --enable-tests-install
> - Fix mesos-tests gmock dependencies
> - Optionally install tests, helpers and test modules
> - Add utility helpers to find various test resources
> 
> 
> Diffs
> -
> 
>   Makefile.am fbd4e5a6356e90c867ba47c48c86fc9161ddd98e 
>   configure.ac 70d16871d5888ac1d9d5fb0ba0b453799b00f320 
>   src/Makefile.am 8657a869f931aa7482fbb09f2c6df95b6a8c50c6 
>   src/examples/test_framework.cpp ff7b00543eea1a7cbf52c7abfba81600868bfbab 
>   src/tests/balloon_framework_test.sh 
> 25a19cfde87a3fd2d7d3e780700d342d08bd0a91 
>   src/tests/containerizer/launch_tests.cpp 
> c7ebe2606e4ff99ced90342dd16e0b4bf02bc504 
>   src/tests/containerizer/memory_test_helper.cpp 
> 4a3de2e3c887aa6afc604588850e1386f92d8c11 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> a45ed1b2175b7dc16d621a44fbccfb8f957ae2b5 
>   src/tests/containerizer/ns_tests.cpp 
> 603e54b7303c5aa15e2c5715dc7a2f7e7d39541b 
>   src/tests/containerizer/port_mapping_tests.cpp 
> 182fe9217a5da9af603d6f9c203a1689eff4ca1b 
>   src/tests/environment.cpp e112270b68d402bb9b01445af552500fb3929e52 
>   src/tests/event_call_framework_test.sh 
> 9d1211552734afbf15b376f8c4629bae8a2065af 
>   src/tests/fetcher_tests.cpp 1831d896ca8a52bec4adf87a67b6af845079796c 
>   src/tests/health_check_tests.cpp 65e8fe25ed7feb1080ad833ba98e6b462bd3152c 
>   src/tests/mesos.cpp 18d0d8f8037ebc27c87bcb0f1ce9f143e7505ec8 
>   src/tests/module.cpp 246f3a402d4fe3b273c459f6e02c009f3de65f3e 
>   src/tests/module_tests.cpp a3271a3267647e0964dd3decb3ca8384417dd559 
>   src/tests/no_executor_framework_test.sh 
> aebdc8c380abb2d041d6fc74dfac5a111c15267e 
>   src/tests/oversubscription_tests.cpp 
> 6f43103e81303015fb614653e3bfece55009d1bf 
>   src/tests/persistent_volume_framework_test.sh 
> 84f02847a8d89400512d8a5714d33fb29cf5b03a 
>   src/tests/script.cpp ee44fef29fb40e414d7507168091ee5cd0d15736 
>   src/tests/slave_tests.cpp e943ab99baf3d74679a5da89a89f6a4b7ead 
>   src/tests/test_framework_test.sh 409e80994f63448115ea8ac34b4fd5c6cf88aa22 
>   src/tests/utils.hpp a6cca472f4dfab12cd6eccab6972206d842177aa 
>   src/tests/utils.cpp 22bf3a85da5261fcfcc8b6aa9626aacdc8391ad4 
> 
> Diff: https://reviews.apache.org/r/40553/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 42016: Windows:[2/2] Use ZK in Windows build.

[Alex Clemmer]

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


Ship it!




Ship It!

- Alex Clemmer


On Jan. 27, 2016, 10:29 p.m., M Lawindi wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42016/
> ---
> 
> (Updated Jan. 27, 2016, 10:29 p.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Alex Clemmer, M 
> Lawindi, and Yi Sun.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows:[2/2] Use ZK in Windows build.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt ac5c25a8797a687e84384682975ab99fb3e30448 
>   3rdparty/cmake/Mesos3rdpartyConfigure.cmake 
> 34e61ff90eca0ffdddb6b6b8e2f8e552691637fa 
>   3rdparty/patch.exe.manifest PRE-CREATION 
>   CMakeLists.txt 9b7044b7860fd64b854ac27b28a48d297dfdeae8 
>   src/slave/cmake/SlaveConfigure.cmake 
> cf378a27297474b2a9f338e0c832612370f7302a 
> 
> Diff: https://reviews.apache.org/r/42016/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> M Lawindi
> 
>

Re: Review Request 42362: Added persistent volume endpoint test without authentication.

[Greg Mann]

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

(Updated Jan. 27, 2016, 11:03 p.m.)


Review request for mesos, Alexander Rukletsov, Michael Park, and Neil Conway.


Changes
---

Edited comments.


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


Repository: mesos


Description
---

Added persistent volume endpoint tests with HTTP authentication disabled.

The persistent volume endpoint tests allow volume creation and destruction when 
HTTP authentication is disabled; this patch introduces a test for this 
scenario: `PersistentVolumeEndpointsTest.NoAuthentication`.


Diffs (updated)
-

  src/tests/persistent_volume_endpoints_tests.cpp 
22e18758ee91a649486725473d9e50fae9d43b01 

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


Testing
---

A new test, `PersistentVolumeEndpointsTest.NoAuthentication`, was added to the 
persistent volume endpoint tests.

`make check` was used to test, and the new test was run with 
`--gtest_repeat=1000 -gtest_break_on_failure=1`.


Thanks,

Greg Mann

Re: Review Request 42865: Fixed LogrotateContainerLogger's FD ownership.

[Joseph Wu]

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

(Updated Jan. 27, 2016, 3:20 p.m.)


Review request for mesos, Benjamin Hindman and Artem Harutyunyan.


Changes
---

Changed to a different approach, putting all the code change into the module.


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


Repository: mesos


Description (updated)
---

Changes the logrotate container logger to manually construct and deal with 
pipes.  Specifically, both read and write ends of the pipe must end up in the 
child processes (read -> logger executables, write -> container).  

If ownership is not transferred, the pipe's FDs may be closed (again) when 
`Subprocess` is destructed, which may unexpectedly close random FDs belonging 
to other threads.


Diffs (updated)
-

  src/slave/container_loggers/lib_logrotate.cpp 
bfc7cade2eed98f21fc1c364c104ad5583648c63 

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


Testing (updated)
---

make check (OSX)

---

NOTE: I'll add an integration to codify the manual testing below.
This discarded review has the gist of the needed test:
https://reviews.apache.org/r/42864/

---

Manual testing:

```
  ./mesos-master.sh --work_dir=/tmp/master --ip=127.0.0.1
  ./mesos-slave.sh --master=127.0.0.1:5050 
--modules=file:///path/to/modules.json 
--container_logger="org_apache_mesos_LogrotateContainerLogger" 
--work_dir=/tmp/agent
  
  # Repeatedly trigger executor terminations.
  ./mesos-execute --master=127.0.0.1:5050 --name="Repeater" --command="while 
true; do /Users/josephwu/mesos/build/src/mesos-execute --master=127.0.0.1:5050 
--name=Repeat --command=\"echo Wheee; sleep 0.4\"; done"
```

"modules.json" follows the template:
```
{
  "libraries": [
{
  "file": "/path/to/liblogrotate_container_logger.la",
  "modules": [
{
  "name": "org_apache_mesos_LogrotateContainerLogger",
  "parameters": [
{
  "key": "launcher_dir",
  "value": "/path/to/mesos/build/src/"
}, {
  "key": "logrotate_stdout_options",
  "value": "rotate 2"
}, {
  "key": "logrotate_stderr_options",
  "value": "rotate 2"
}
  ]
}
  ]
}
  ]
}
```


Thanks,

Joseph Wu

Re: Review Request 42362: Added persistent volume endpoint test without authentication.

[Anand Mazumdar]

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


Fix it, then Ship it!




LGTM.


src/tests/persistent_volume_endpoints_tests.cpp (line 1112)


Nit: Can we omit the part about not setting the authentication headers 
since it's self-explanatory. 

How about just:

```
// Try a request to create a volume without authentication headers. 
Additionally, authorization is not enabled, so any principal, including 
`None()`, can create and destroy volumes.
```

What do you think?



src/tests/persistent_volume_endpoints_tests.cpp (line 1116)


Can we wrap both of these in blocks i.e.

```
{
  Future response = blah blah;
  ...
}

{
  Future response = blah blah part 2;
  ...
}
```

Also,
s/createResponse/response
s/destroyResponse/response thereafter.


- Anand Mazumdar


On Jan. 27, 2016, 11:04 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42362/
> ---
> 
> (Updated Jan. 27, 2016, 11:04 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Kapil Arya, and Neil Conway.
> 
> 
> Bugs: MESOS-4395
> https://issues.apache.org/jira/browse/MESOS-4395
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added persistent volume endpoint tests with HTTP authentication disabled.
> 
> The persistent volume endpoint tests allow volume creation and destruction 
> when HTTP authentication is disabled; this patch introduces a test for this 
> scenario: `PersistentVolumeEndpointsTest.NoAuthentication`.
> 
> 
> Diffs
> -
> 
>   src/tests/persistent_volume_endpoints_tests.cpp 
> 22e18758ee91a649486725473d9e50fae9d43b01 
> 
> Diff: https://reviews.apache.org/r/42362/diff/
> 
> 
> Testing
> ---
> 
> A new test, `PersistentVolumeEndpointsTest.NoAuthentication`, was added to 
> the persistent volume endpoint tests.
> 
> `make check` was used to test, and the new test was run with 
> `--gtest_repeat=1000 -gtest_break_on_failure=1`.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Review Request 42880: Add test for LogrotateContainerLogger's FD management.

[Joseph Wu]

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

Review request for mesos, Benjamin Hindman and Artem Harutyunyan.


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


Repository: mesos


Description
---

Adds a test which checks for erroneous calls to `os::close` by the 
LogrotateContainerLogger.  This may happen by accident if the container logger 
module uses `Subprocess::PIPE` when launching child processes; as libprocess 
will track these FDs and close them (possibly even if they've already been 
closed) when the child processes exit.


Diffs
-

  src/tests/container_logger_tests.cpp 5fe9cce97ee83d6a9e272879ec0395b5ace4a491 

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


Testing
---

make check

Test failed (expected) when I reverted the previous patch and ran `make check`


Thanks,

Joseph Wu

Re: Review Request 42603: Added an http::Authenticator factory.

[Alexander Rojas]

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

(Updated Jan. 27, 2016, 4:38 p.m.)


Review request for mesos, Alexander Rukletsov, Benjamin Bannier, Jan Schlicht, 
and Till Toenshoff.


Repository: mesos


Description
---

Move the code required to instanciate instances of 
`process::http::authentication::Authenticator`
in `src/master/main.cpp` to its own factory.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/stout/strings.hpp 
c20e2ffe128059fe63ac2630a2c73e512fc6 
  3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp 
4b7d1347f93261df7a7d512721146edca2703b34 
  3rdparty/libprocess/include/process/gmock.hpp 
22a0eac37b16f2ed6afd249f5ea911d81970ba6b 
  CHANGELOG 78e7796dd8d3727cc456649ddeaf825ffb48808a 
  CMakeLists.txt d828b754f25803bc3f654aeafa0aec1722dff98b 
  configure.ac cb39c7f7681eb7f5e3c379dc9a096eca1ffcca93 
  docs/allocation-module.md 3bbb9045dea2c0e50c4791cc3e220cfe383e0b7b 
  docs/app-framework-development-guide.md 
e0f40adacf96bdf0c510b3400eb0ed0cd964ab9d 
  docs/authentication.md b40b09eaceddc8e5498b54b3e8ef7a5dbd7b9dc2 
  docs/c++-style-guide.md 5e2d88d5a7ae06191346bf672f58dbe27b2bccee 
  docs/clang-format.md 5e720e016ae2635c6a3e1825c0a7fc36f1ef07b7 
  docs/committers.md 94ee9a6553ad5cd894db6f71825f46a7a3239eb8 
  docs/containerizer-internals.md 20bf2d16d4f06994ee766a0c15f3528513751efe 
  docs/containerizer.md cd23cf9c006b862e6e2cf4e215706eff03cd07f8 
  docs/deploy-scripts.md 87f8eb6254dbf00dc537a180b8d3e0084165d17c 
  docs/documentation-guide.md b7e861468e8abe1b8e83c115d353e78e601fb41f 
  docs/effective-code-reviewing.md 5a633bc324290bc1f57884bab550ee9f246f9d0f 
  docs/fetcher-cache-internals.md 1ccb1c2cbb0a80771f79e98c0bdcc8ee8464ea30 
  docs/fetcher.md cb4f3c33fb67f7ac65dbe7ebe1347d2599c43f37 
  docs/high-availability.md 3e9802bd3a2a6f75fff3aea3cfb319adce3271cf 
  docs/home.md dea6ec2605662dbd4b10d69b2bf8f35af50389ec 
  docs/operational-guide.md 4680ee3397d081acd6f82499703de4456e7ca4f4 
  docs/quota.md 2762557ad1cec0655a27c4dda4016e07c7f63cd9 
  docs/sandbox.md 276e1126f495e7af15afd4f4ad8c63beb40db739 
  docs/tools.md 6234e38c017d8284b2d1a31de63110598c3c60c7 
  docs/upgrades.md 4f30d725c6ed28c09a1c5528fd4193c3f06b2d93 
  docs/versioning.md cc31fd53d5bd33168fed0a7c3408616f7c1e9858 
  include/mesos/authentication/http/basic_authenticator_factory.hpp 
c11bb47c8e02f2e8645cf387d18eb64d1c8cb604 
  include/mesos/docker/spec.hpp 6033e824c6f776dc744caf11066b68d3629bb8ca 
  include/mesos/executor/executor.proto 
e905a12e88e9a9e1382830974f7b4f82d83be51e 
  include/mesos/quota/quota.proto 78e12d56c4f41613ec6b731d86093f48a23b4509 
  include/mesos/scheduler.hpp 14c7ff964aa7b94f439d16e605380661d2279d54 
  include/mesos/scheduler/scheduler.proto 
4aa32e6422a458221b7829ec6f5223e6a013feae 
  include/mesos/v1/executor.hpp adca287be3bb88c8b3298705740cb6bcbb3a09f0 
  include/mesos/v1/executor/executor.hpp 
6e7b41288f44cc1b74b9863407885139f20a379d 
  include/mesos/v1/executor/executor.proto 
dbbc2656461e32b47f5c9e2d3767ebdfe343368e 
  include/mesos/v1/scheduler/scheduler.proto 
5d0e6c7dfcdbe004a8629361328287a88d53a1b6 
  site/Rakefile 0ce4b7975f95ab6930f0b2674191930df9ab5b20 
  src/Makefile.am bdb34020043f64c07721b3e8b29b221e5b99 
  src/authentication/http/authenticator.cpp PRE-CREATION 
  src/authentication/http/basic_authenticator_factory.cpp 
6eb1c5bd09b136d3bc20481ddcc65cb8bd153682 
  src/common/resources.cpp 588a279c3cdebbeb58047bfbff5e78a42f53fc13 
  src/common/roles.cpp 65c71931a5645d346439c3cdff1c5c8cc3ee01a3 
  src/examples/docker_no_executor_framework.cpp 
cf7b4e1b4a27a3a6d484b1e205b5ca1c3ec583e1 
  src/examples/load_generator_framework.cpp 
7d64b6617564f43ef383ee60d92da92b2c958c47 
  src/examples/test_http_authenticator_module.cpp 
acf51a6deb8e7dc4ab6ac0cf70380ddbb1839906 
  src/executor/executor.cpp 92334ffb8af83b1c86c44bd406147893c7c6daa3 
  src/jvm/jvm.cpp 909d34a0112b219456dce76126029c603077e66d 
  src/master/allocator/mesos/hierarchical.hpp 
1f0c440565ee0f9926d168734df30fd740a53f88 
  src/master/allocator/mesos/hierarchical.cpp 
dbc77a2d0ce797eea2287b0242f5d2da81c16455 
  src/master/allocator/sorter/drf/sorter.hpp 
4669149b81de39b4bb921ef7cd6787aa583f6e40 
  src/master/allocator/sorter/drf/sorter.cpp 
1f39f6d12cb2673ef4b75c3efb23ca5f7dbbf29a 
  src/master/allocator/sorter/sorter.hpp 
a0a779b81f6d048271f15256b38ff907ae144b83 
  src/master/master.cpp 9ee56277c8a472be9e683d5db505becfb5f7c422 
  src/master/quota.hpp 3cb194aa9f032d5d18cb2dcc09c672b11e8d56eb 
  src/master/quota.cpp e7ce0f1fc1c83b2ec30185db005d2671a93baed6 
  src/master/quota_handler.cpp a41c91f10bc0eedc754425b4de1b3e184c4ffb08 
  src/sched/sched.cpp 8e51752536041b896e42d44e6e9eae909b0a1d68 
  src/slave/container_loggers/lib_logrotate.hpp 
8c5602da3e5ff7bcf758da61723b7a0ea00a6a6e 
  src/slave/container_loggers/logrotate.hpp 

Re: Review Request 42603: Added an http::Authenticator factory.

[Benjamin Bannier]

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



Could you separate changes due to `http::Authenticator` and for updating to 
0.28?

- Benjamin Bannier


On Jan. 27, 2016, 4:38 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42603/
> ---
> 
> (Updated Jan. 27, 2016, 4:38 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Bannier, Jan 
> Schlicht, and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Move the code required to instanciate instances of 
> `process::http::authentication::Authenticator`
> in `src/master/main.cpp` to its own factory.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/strings.hpp 
> c20e2ffe128059fe63ac2630a2c73e512fc6 
>   3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp 
> 4b7d1347f93261df7a7d512721146edca2703b34 
>   3rdparty/libprocess/include/process/gmock.hpp 
> 22a0eac37b16f2ed6afd249f5ea911d81970ba6b 
>   CHANGELOG 78e7796dd8d3727cc456649ddeaf825ffb48808a 
>   CMakeLists.txt d828b754f25803bc3f654aeafa0aec1722dff98b 
>   configure.ac cb39c7f7681eb7f5e3c379dc9a096eca1ffcca93 
>   docs/allocation-module.md 3bbb9045dea2c0e50c4791cc3e220cfe383e0b7b 
>   docs/app-framework-development-guide.md 
> e0f40adacf96bdf0c510b3400eb0ed0cd964ab9d 
>   docs/authentication.md b40b09eaceddc8e5498b54b3e8ef7a5dbd7b9dc2 
>   docs/c++-style-guide.md 5e2d88d5a7ae06191346bf672f58dbe27b2bccee 
>   docs/clang-format.md 5e720e016ae2635c6a3e1825c0a7fc36f1ef07b7 
>   docs/committers.md 94ee9a6553ad5cd894db6f71825f46a7a3239eb8 
>   docs/containerizer-internals.md 20bf2d16d4f06994ee766a0c15f3528513751efe 
>   docs/containerizer.md cd23cf9c006b862e6e2cf4e215706eff03cd07f8 
>   docs/deploy-scripts.md 87f8eb6254dbf00dc537a180b8d3e0084165d17c 
>   docs/documentation-guide.md b7e861468e8abe1b8e83c115d353e78e601fb41f 
>   docs/effective-code-reviewing.md 5a633bc324290bc1f57884bab550ee9f246f9d0f 
>   docs/fetcher-cache-internals.md 1ccb1c2cbb0a80771f79e98c0bdcc8ee8464ea30 
>   docs/fetcher.md cb4f3c33fb67f7ac65dbe7ebe1347d2599c43f37 
>   docs/high-availability.md 3e9802bd3a2a6f75fff3aea3cfb319adce3271cf 
>   docs/home.md dea6ec2605662dbd4b10d69b2bf8f35af50389ec 
>   docs/operational-guide.md 4680ee3397d081acd6f82499703de4456e7ca4f4 
>   docs/quota.md 2762557ad1cec0655a27c4dda4016e07c7f63cd9 
>   docs/sandbox.md 276e1126f495e7af15afd4f4ad8c63beb40db739 
>   docs/tools.md 6234e38c017d8284b2d1a31de63110598c3c60c7 
>   docs/upgrades.md 4f30d725c6ed28c09a1c5528fd4193c3f06b2d93 
>   docs/versioning.md cc31fd53d5bd33168fed0a7c3408616f7c1e9858 
>   include/mesos/authentication/http/basic_authenticator_factory.hpp 
> c11bb47c8e02f2e8645cf387d18eb64d1c8cb604 
>   include/mesos/docker/spec.hpp 6033e824c6f776dc744caf11066b68d3629bb8ca 
>   include/mesos/executor/executor.proto 
> e905a12e88e9a9e1382830974f7b4f82d83be51e 
>   include/mesos/quota/quota.proto 78e12d56c4f41613ec6b731d86093f48a23b4509 
>   include/mesos/scheduler.hpp 14c7ff964aa7b94f439d16e605380661d2279d54 
>   include/mesos/scheduler/scheduler.proto 
> 4aa32e6422a458221b7829ec6f5223e6a013feae 
>   include/mesos/v1/executor.hpp adca287be3bb88c8b3298705740cb6bcbb3a09f0 
>   include/mesos/v1/executor/executor.hpp 
> 6e7b41288f44cc1b74b9863407885139f20a379d 
>   include/mesos/v1/executor/executor.proto 
> dbbc2656461e32b47f5c9e2d3767ebdfe343368e 
>   include/mesos/v1/scheduler/scheduler.proto 
> 5d0e6c7dfcdbe004a8629361328287a88d53a1b6 
>   site/Rakefile 0ce4b7975f95ab6930f0b2674191930df9ab5b20 
>   src/Makefile.am bdb34020043f64c07721b3e8b29b221e5b99 
>   src/authentication/http/authenticator.cpp PRE-CREATION 
>   src/authentication/http/basic_authenticator_factory.cpp 
> 6eb1c5bd09b136d3bc20481ddcc65cb8bd153682 
>   src/common/resources.cpp 588a279c3cdebbeb58047bfbff5e78a42f53fc13 
>   src/common/roles.cpp 65c71931a5645d346439c3cdff1c5c8cc3ee01a3 
>   src/examples/docker_no_executor_framework.cpp 
> cf7b4e1b4a27a3a6d484b1e205b5ca1c3ec583e1 
>   src/examples/load_generator_framework.cpp 
> 7d64b6617564f43ef383ee60d92da92b2c958c47 
>   src/examples/test_http_authenticator_module.cpp 
> acf51a6deb8e7dc4ab6ac0cf70380ddbb1839906 
>   src/executor/executor.cpp 92334ffb8af83b1c86c44bd406147893c7c6daa3 
>   src/jvm/jvm.cpp 909d34a0112b219456dce76126029c603077e66d 
>   src/master/allocator/mesos/hierarchical.hpp 
> 1f0c440565ee0f9926d168734df30fd740a53f88 
>   src/master/allocator/mesos/hierarchical.cpp 
> dbc77a2d0ce797eea2287b0242f5d2da81c16455 
>   src/master/allocator/sorter/drf/sorter.hpp 
> 4669149b81de39b4bb921ef7cd6787aa583f6e40 
>   src/master/allocator/sorter/drf/sorter.cpp 
> 

Review Request 42900: Fixed some typos.

[Neil Conway]

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

Review request for mesos and Jie Yu.


Repository: mesos


Description
---

Fixed some typos.


Diffs
-

  docs/reservation.md 8d2d33a6518c73542cbfb3a5ee36da1c00c6ff1a 
  src/slave/containerizer/mesos/isolators/posix/disk.cpp 
fd9b3abfe2c502c69ba7b2f0b2015fe90888d5bc 

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


Testing
---


Thanks,

Neil Conway

Review Request 42901: Fixed a few typos in the HA framework guide.

[Neil Conway]

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

Review request for mesos and Vinod Kone.


Repository: mesos


Description
---

Fixed a few typos in the HA framework guide.


Diffs
-

  docs/high-availability-framework-guide.md 
3d429d85edce695f608320cab05cdafdafa4a42e 

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


Testing
---


Thanks,

Neil Conway

Re: Review Request 42861: Ensure two Mount Disk resources do not have the same root path.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [42860, 42861]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export 
COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Jan. 28, 2016, 2:12 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42861/
> ---
> 
> (Updated Jan. 28, 2016, 2:12 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-4521
> https://issues.apache.org/jira/browse/MESOS-4521
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Ensure two Mount Disk resources do not have the same root path.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp 1f4c8368feb0ce19963577582ce745acfb21aa9f 
> 
> Diff: https://reviews.apache.org/r/42861/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Review Request 42860: Add paths::sameParent to find out the same root of a path list.

[haosdent huang]

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

Review request for mesos, Jie Yu and Joris Van Remoortere.


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


Repository: mesos


Description
---

Add paths::sameParent to find out the same root of a path list.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp 
ef538045a8b7a1e3d8962c869317d86a85e0259f 
  3rdparty/libprocess/3rdparty/stout/tests/path_tests.cpp 
6dff5e76e0e15098c5a262adc50bfcb65f933697 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 42362: Added persistent volume endpoint test without authentication.

[Greg Mann]

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

(Updated Jan. 27, 2016, 6:24 p.m.)


Review request for mesos, Alexander Rukletsov, Michael Park, and Neil Conway.


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


Repository: mesos


Description (updated)
---

Added persistent volume endpoint tests with HTTP authentication disabled.

The persistent volume endpoint tests allow volume creation and destruction when 
HTTP authentication is disabled; this patch introduces a test for this 
scenario: `PersistentVolumeEndpointsTest.NoAuthentication`.


Diffs
-

  src/tests/persistent_volume_endpoints_tests.cpp 
22e18758ee91a649486725473d9e50fae9d43b01 

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


Testing (updated)
---

A new test, `PersistentVolumeEndpointsTest.NoAuthentication`, was added to the 
persistent volume endpoint tests.

`make check` was used to test, and the new test was run with 
`--gtest_repeat=1000 -gtest_break_on_failure=1`.


Thanks,

Greg Mann

Re: Review Request 42839: WIP: Appc cache redesign.

[Jojy Varghese]

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

(Updated Jan. 27, 2016, 5:01 p.m.)


Review request for mesos and Jie Yu.


Changes
---

removed CachedImage


Repository: mesos


Description
---

WIP: Appc cache redesign.


Diffs (updated)
-

  src/slave/containerizer/mesos/provisioner/appc/image_cache.hpp PRE-CREATION 
  src/slave/containerizer/mesos/provisioner/appc/image_cache.cpp PRE-CREATION 
  src/slave/containerizer/mesos/provisioner/appc/store.cpp 
73c4df858a70da3d4cc4a1cb15092165f6ff8fe4 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 42841: WIP: Introducing appc image fetcher.

[Jojy Varghese]

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

(Updated Jan. 27, 2016, 5:02 p.m.)


Review request for mesos and Jie Yu.


Changes
---

rebased after cache change.


Repository: mesos


Description
---

WIP: Introducing appc image fetcher.


Diffs (updated)
-

  src/CMakeLists.txt 47d0a7c0fe73b9297cd7dde6086b5e6e9e1f9e4e 
  src/Makefile.am 19bf3a7c2e43ca04ed6e6d506e052de5537f7c2f 
  src/slave/containerizer/mesos/provisioner/appc/image_fetcher.hpp PRE-CREATION 
  src/slave/containerizer/mesos/provisioner/appc/image_fetcher.cpp PRE-CREATION 
  src/slave/flags.hpp 23ec158f4a95fa76d657d9eb6a4a6fe30d57e5b6 
  src/slave/flags.cpp 75d7429a4e3e1d6259296257c0ace1ade365ac2b 

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


Testing
---

make check; local image server


Thanks,

Jojy Varghese

Review Request 42861: Ensure two Mount Disk resources do not have the same root path.

[haosdent huang]

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

Review request for mesos, Jie Yu and Joris Van Remoortere.


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


Repository: mesos


Description
---

Ensure two Mount Disk resources do not have the same root path.


Diffs
-

  src/slave/slave.cpp 1f4c8368feb0ce19963577582ce745acfb21aa9f 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 42841: WIP: Introducing appc image fetcher.

[Mesos ReviewBot]

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



Bad review!

Reviews applied: []

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

- Mesos ReviewBot


On Jan. 27, 2016, 5:02 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42841/
> ---
> 
> (Updated Jan. 27, 2016, 5:02 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> WIP: Introducing appc image fetcher.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 47d0a7c0fe73b9297cd7dde6086b5e6e9e1f9e4e 
>   src/Makefile.am 19bf3a7c2e43ca04ed6e6d506e052de5537f7c2f 
>   src/slave/containerizer/mesos/provisioner/appc/image_fetcher.hpp 
> PRE-CREATION 
>   src/slave/containerizer/mesos/provisioner/appc/image_fetcher.cpp 
> PRE-CREATION 
>   src/slave/flags.hpp 23ec158f4a95fa76d657d9eb6a4a6fe30d57e5b6 
>   src/slave/flags.cpp 75d7429a4e3e1d6259296257c0ace1ade365ac2b 
> 
> Diff: https://reviews.apache.org/r/42841/diff/
> 
> 
> Testing
> ---
> 
> make check; local image server
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 41892: DockerContinerizer infers hostPath for persistent volumes.

[Zhitao Li]

> On Jan. 15, 2016, 12:03 a.m., Zhitao Li wrote:
> > src/docker/docker.cpp, lines 410-420
> > 
> >
> > (Sorry I just got time to come back to this).
> > 
> > I don't exactly understand your suggestion about "add additional 
> > containerInfo.volumes() in 
> > DockerContainerizerProcess::Container::create()", because the latter 
> > function actually does not pass a `ContainerInfo`.
> > 
> > Because the volume is actually included in 
> > `TaskInfo::ContainerInfo::volumes`, we would need to mutate the `TaskInfo` 
> > before passing it to the `DockerExecutorProcess::launchTask()` function, 
> > which I don't really know how to do it, and I even doubt whether it's a 
> > good idea for logging/clarity purpose.
> > 
> > Please let me know if I didn't understand your suggestion, or if you 
> > think we should explore the other alternative (passing `hostPath` earlier 
> > in resource offer).
> 
> Jie Yu wrote:
> Sorry for being late on this.
> 
> My sugguestion is that we add some logics in 
> DockerContainerProcess::Container::create(), the 'create' function has both 
> TaskINfo and ExecutorInfo. So, you should be able to get the ContainerInfo 
> from them. When you generate DockerContainerizerProcess::Container, you 
> should be able to generate a new ContainerInfo that has persistent volumes in 
> it. Let me know if you still don't understand.
> 
> FYI, docker just merged its mount propagation support for volumes. That 
> means we can have full persistent volume support. We need to mount the 
> sandbox as a shared mount into the docker container. ANy mounts under the 
> sandbox mount will be automatically propergated to the docker container.

@jieyu, I tried your recommendation and managed to generate a different 
`ContainerInfo` object inside `DockerContainerProcess::Container::create()`. 
However, I don't think it could work either because the actual docker container 
is created by the call sequence `DockerExecutor::launchTask()` -> 
`Docker::run()` in the forked executor subprocess, and it seems impossible to 
properly pass the modified `ContainerInfo` to that.

My proposal is:
- 1) keep the `persistent_volumes_root` flag to the executor;
- 2) implement this logic inside `DockerExecutor::launchTask()` (see my other 
inline comment for exact location) so we don't need to change `Docker` class;
- 3) for testing, create a test case in docker_containerizer_tests.cpp which 
creates persistent volume and launch an executor.

I'll rebase and update this diff with the above proposal.


- Zhitao


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


On Jan. 4, 2016, 9:15 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41892/
> ---
> 
> (Updated Jan. 4, 2016, 9:15 p.m.)
> 
> 
> Review request for mesos, haosdent huang and Jie Yu.
> 
> 
> Bugs: MESOS-3413
> https://issues.apache.org/jira/browse/MESOS-3413
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This diff makes DockerContainerizer infers hostPath for persistent volumes 
> from resources passed-in.
> 
> This allows current DockerContainerizer to use persistent volumes w/o hard 
> code slave's work_dir. I also checked that the inferred directory actually 
> exists to avoid framework messing up role or persistent id.
> 
> Note that some validation on the resource part should be done before we start 
> the container, which seems to belong to slave.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp dde2b29deda7e40929e0169935a5bafdd43136b1 
>   src/docker/docker.cpp e93280735f1c5f66c765fcbabfd3e50b46c024e8 
>   src/docker/executor.hpp abbc419533ab40312e917931a2fc2ce78b38da41 
>   src/docker/executor.cpp 7512d07de6c8324340e6b5f3e5162ef00efc47fc 
>   src/slave/containerizer/docker.cpp aacf90f2cb6c08f94340936d29b2df513ac9825a 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> cb58b7183c36d96b9ac4803c63980c278a50c97b 
>   src/tests/containerizer/docker_tests.cpp 
> 83eceacaddc38d0ccfc42e65e700a09406d8df36 
>   src/tests/mesos.hpp a4811b0d8dee33ff2ca4968f532ce64b7ea95249 
>   src/tests/mesos.cpp f4b0f82449c4b6a2b4b7b7f14518714485d5a13a 
> 
> Diff: https://reviews.apache.org/r/41892/diff/
> 
> 
> Testing
> ---
> 
> New unit test.
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 41892: DockerContinerizer infers hostPath for persistent volumes.

[Zhitao Li]

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




src/docker/executor.cpp (line 136)


@jieyu, this would be a better place to implement the inferring of host 
paths for persistent volumes, comparing to the current draft.


- Zhitao Li


On Jan. 4, 2016, 9:15 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41892/
> ---
> 
> (Updated Jan. 4, 2016, 9:15 p.m.)
> 
> 
> Review request for mesos, haosdent huang and Jie Yu.
> 
> 
> Bugs: MESOS-3413
> https://issues.apache.org/jira/browse/MESOS-3413
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This diff makes DockerContainerizer infers hostPath for persistent volumes 
> from resources passed-in.
> 
> This allows current DockerContainerizer to use persistent volumes w/o hard 
> code slave's work_dir. I also checked that the inferred directory actually 
> exists to avoid framework messing up role or persistent id.
> 
> Note that some validation on the resource part should be done before we start 
> the container, which seems to belong to slave.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp dde2b29deda7e40929e0169935a5bafdd43136b1 
>   src/docker/docker.cpp e93280735f1c5f66c765fcbabfd3e50b46c024e8 
>   src/docker/executor.hpp abbc419533ab40312e917931a2fc2ce78b38da41 
>   src/docker/executor.cpp 7512d07de6c8324340e6b5f3e5162ef00efc47fc 
>   src/slave/containerizer/docker.cpp aacf90f2cb6c08f94340936d29b2df513ac9825a 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> cb58b7183c36d96b9ac4803c63980c278a50c97b 
>   src/tests/containerizer/docker_tests.cpp 
> 83eceacaddc38d0ccfc42e65e700a09406d8df36 
>   src/tests/mesos.hpp a4811b0d8dee33ff2ca4968f532ce64b7ea95249 
>   src/tests/mesos.cpp f4b0f82449c4b6a2b4b7b7f14518714485d5a13a 
> 
> Diff: https://reviews.apache.org/r/41892/diff/
> 
> 
> Testing
> ---
> 
> New unit test.
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 40553: Enable mesos tests installation.

[James Peach]

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

(Updated Jan. 27, 2016, 5:54 p.m.)


Review request for mesos, Benjamin Bannier and Till Toenshoff.


Changes
---

Rebased onto master and added support for new tests.

All tests are passing except the python and Java example frameworks and 
ZooKeeper tests, since those tests intrinsically depend on artifacts that are 
only found in the source tree.


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


Repository: mesos


Description (updated)
---

This patch enables the installation mesos-tests and its dependencies
and helper tool. The goal is to allow operators to build a separate
test package that can be run at deployment time to verify that Mesos
works in the deployment environment.

Since the build directory is searched first, to run it on a host
that has a build tree, you need to specify a non-existent tree:

~ $ $PREFIX/libexec/mesos/tests/mesos-tests --build_dir=/none

- Add --enable-tests-install
- Fix mesos-tests gmock dependencies
- Optionally install tests, helpers and test modules
- Add utility helpers to find various test resources


Diffs (updated)
-

  Makefile.am fbd4e5a6356e90c867ba47c48c86fc9161ddd98e 
  configure.ac 70d16871d5888ac1d9d5fb0ba0b453799b00f320 
  src/Makefile.am 8657a869f931aa7482fbb09f2c6df95b6a8c50c6 
  src/examples/test_framework.cpp ff7b00543eea1a7cbf52c7abfba81600868bfbab 
  src/tests/balloon_framework_test.sh 25a19cfde87a3fd2d7d3e780700d342d08bd0a91 
  src/tests/containerizer/launch_tests.cpp 
c7ebe2606e4ff99ced90342dd16e0b4bf02bc504 
  src/tests/containerizer/memory_test_helper.cpp 
4a3de2e3c887aa6afc604588850e1386f92d8c11 
  src/tests/containerizer/mesos_containerizer_tests.cpp 
a45ed1b2175b7dc16d621a44fbccfb8f957ae2b5 
  src/tests/containerizer/ns_tests.cpp 603e54b7303c5aa15e2c5715dc7a2f7e7d39541b 
  src/tests/containerizer/port_mapping_tests.cpp 
182fe9217a5da9af603d6f9c203a1689eff4ca1b 
  src/tests/environment.cpp e112270b68d402bb9b01445af552500fb3929e52 
  src/tests/event_call_framework_test.sh 
9d1211552734afbf15b376f8c4629bae8a2065af 
  src/tests/fetcher_tests.cpp 1831d896ca8a52bec4adf87a67b6af845079796c 
  src/tests/health_check_tests.cpp 65e8fe25ed7feb1080ad833ba98e6b462bd3152c 
  src/tests/mesos.cpp 18d0d8f8037ebc27c87bcb0f1ce9f143e7505ec8 
  src/tests/module.cpp 246f3a402d4fe3b273c459f6e02c009f3de65f3e 
  src/tests/module_tests.cpp a3271a3267647e0964dd3decb3ca8384417dd559 
  src/tests/no_executor_framework_test.sh 
aebdc8c380abb2d041d6fc74dfac5a111c15267e 
  src/tests/oversubscription_tests.cpp 6f43103e81303015fb614653e3bfece55009d1bf 
  src/tests/persistent_volume_framework_test.sh 
84f02847a8d89400512d8a5714d33fb29cf5b03a 
  src/tests/script.cpp ee44fef29fb40e414d7507168091ee5cd0d15736 
  src/tests/slave_tests.cpp e943ab99baf3d74679a5da89a89f6a4b7ead 
  src/tests/test_framework_test.sh 409e80994f63448115ea8ac34b4fd5c6cf88aa22 
  src/tests/utils.hpp a6cca472f4dfab12cd6eccab6972206d842177aa 
  src/tests/utils.cpp 22bf3a85da5261fcfcc8b6aa9626aacdc8391ad4 

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


Testing
---


Thanks,

James Peach

Re: Review Request 42860: Add paths::sameParent to find out the same root of a path list.

[haosdent huang]

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

(Updated Jan. 27, 2016, 6:36 p.m.)


Review request for mesos, Jie Yu and Joris Van Remoortere.


Changes
---

Rebase


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


Repository: mesos


Description
---

Add paths::sameParent to find out the same root of a path list.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/stout/path.hpp 
ef538045a8b7a1e3d8962c869317d86a85e0259f 
  3rdparty/libprocess/3rdparty/stout/tests/path_tests.cpp 
6dff5e76e0e15098c5a262adc50bfcb65f933697 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 39780: Update OversubscriptionTest to not assume dynamic dlopen search.

[James Peach]

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

(Updated Jan. 27, 2016, 6:47 p.m.)


Review request for mesos, Benjamin Bannier, Kapil Arya, Niklas Nielsen, and 
Till Toenshoff.


Changes
---

Update.


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


Repository: mesos


Description
---

Update OversubscriptionTest to not assume dynamic dlopen search.


Diffs (updated)
-

  src/tests/oversubscription_tests.cpp 6f43103e81303015fb614653e3bfece55009d1bf 

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


Testing
---

make check


Thanks,

James Peach

Re: Review Request 42861: Ensure two Mount Disk resources do not have the same root path.

[haosdent huang]

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

(Updated Jan. 27, 2016, 6:36 p.m.)


Review request for mesos, Jie Yu and Joris Van Remoortere.


Changes
---

Rebase


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


Repository: mesos


Description
---

Ensure two Mount Disk resources do not have the same root path.


Diffs (updated)
-

  src/slave/slave.cpp 1f4c8368feb0ce19963577582ce745acfb21aa9f 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 39782: Add a comment for os::libraries::setPaths.

[James Peach]

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

(Updated Jan. 27, 2016, 6:44 p.m.)


Review request for mesos, Benjamin Bannier, Kapil Arya, Niklas Nielsen, and 
Till Toenshoff.


Changes
---

Updated.


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


Repository: mesos


Description
---

Add a comment for os::libraries::setPaths.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 
ed3bb2c5ab035d8c0bbf9f44baa2b0b8fb1f79b6 

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


Testing
---

No code changes.


Thanks,

James Peach

Re: Review Request 39781: Update ModuleTest to not assume dynamic dlopen search.

[James Peach]

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

(Updated Jan. 27, 2016, 6:44 p.m.)


Review request for mesos, Benjamin Bannier, Kapil Arya, Niklas Nielsen, and 
Till Toenshoff.


Changes
---

Updated.


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


Repository: mesos


Description
---

Update ModuleTest to not assume dynamic dlopen search.


Diffs (updated)
-

  src/tests/module_tests.cpp a3271a3267647e0964dd3decb3ca8384417dd559 

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


Testing
---

make check


Thanks,

James Peach

Re: Review Request 42648: Moved http authenticator initialization to main.

[Alexander Rojas]

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

(Updated Jan. 27, 2016, 7:50 p.m.)


Review request for mesos, Alexander Rukletsov, Benjamin Bannier, Jan Schlicht, 
and Till Toenshoff.


Changes
---

Actually removes functionality from `mesos::Master`


Repository: mesos


Description (updated)
---

Moves the code which initializes instances of 
`process::http::authentication::Authenticator` from `mesos::internal::Master` 
to the master main file.

The change is needed in order to show that libprocess http authentication is 
turned at the system process level and not at the libprocess process level.

It also adds the same initialization as in main to the test function 
`StartMaster`.


Diffs (updated)
-

  src/master/main.cpp 0d5ac49121491152d8426cd764a1f1a0f37483ae 
  src/master/master.cpp 9ee56277c8a472be9e683d5db505becfb5f7c422 
  src/tests/cluster.cpp 1a3a038e78999bca2fc3f08725f4ffaf4290bcb9 

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


Testing
---

make check

manual tests.


Thanks,

Alexander Rojas

Re: Review Request 42794: URL query string order is defined.

[Abhishek Dasgupta]

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

(Updated Jan. 27, 2016, 7:07 p.m.)


Review request for mesos, Kapil Arya and Jan Schlicht.


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


Repository: mesos


Description
---

URL query string order is defined.


Diffs (updated)
-

  3rdparty/libprocess/src/http.cpp 762da9a 
  3rdparty/libprocess/src/tests/http_tests.cpp 66d185e 

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


Testing
---

The following test cases in http_test.cpp have been modified -
HTTPTest.QueryEncodeDecode to check generated query strings in URL to be 
according alphabatic order.
URLTest.Stringification to check generated query strings in URL to be according 
alphabatic order.


Thanks,

Abhishek Dasgupta

Re: Review Request 41491: Exposed docker image manifest to mesos containerizer.

[Jie Yu]

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


Fix it, then Ship it!





src/slave/containerizer/mesos/provisioner/docker/store.cpp (line 219)


CHECK_LT(0u, image.layer_ids_size());



src/slave/containerizer/mesos/provisioner/docker/store.cpp (lines 238 - 244)


I think we already support parsing v1 manifest from string. No need for the 
JSON parsing part.



src/tests/containerizer/provisioner_docker_tests.cpp (line 1255)


Can you add a TODO here. We need a helper to create test layers which will 
allow us to set manifest so that we can add checks here.


- Jie Yu


On Jan. 26, 2016, 5 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41491/
> ---
> 
> (Updated Jan. 26, 2016, 5 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, Qian Zhang, and Timothy 
> Chen.
> 
> 
> Bugs: MESOS-4225
> https://issues.apache.org/jira/browse/MESOS-4225
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Exposed docker image manifest to mesos containerizer.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> 03425daaf015cf231920b7eda1d5424895a41286 
>   src/slave/containerizer/mesos/provisioner/provisioner.hpp 
> 4202f5552174ad3ab595ab3f16ab91d0854951f0 
>   src/slave/containerizer/mesos/provisioner/provisioner.cpp 
> 023f7363f33c4a927fae11037a408f6747fc3c39 
>   src/slave/containerizer/mesos/provisioner/store.hpp 
> aec725f789f7aeb92abfcc6718c2e6e2f1f37981 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> 33df60065903228749833bbad20449ba8784594a 
> 
> Diff: https://reviews.apache.org/r/41491/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 42662: Added common command utils file.

[Jie Yu]

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


Fix it, then Ship it!




This is looking good! Some nits and we're good to go.


src/common/command_utils.hpp (line 48)


Why untar has a default for 'directory' while tar does not?



src/common/command_utils.hpp (line 56)


Please update the comments accordingly.



src/common/command_utils.hpp (line 59)


s/inputFilePath/input/



src/common/command_utils.cpp (line 19)


is it used? Please make sure to remove headers that are not used.



src/common/command_utils.cpp (line 31)


is it used?



src/common/command_utils.cpp (line 45)


s/command/path/ to be consistent with subprocess.



src/common/command_utils.cpp (lines 55 - 56)


Instead of saving a lambda, can we just save the string that we want to log?

```
string command = strings::join(
", ",
path,
strings::join(", ", argv));
```



src/common/command_utils.cpp (lines 63 - 73)


You should be able to use strings::join here.



src/common/command_utils.cpp (line 143)


I don't think we need to check if 'directory' is empty or not. Otherwise, 
do you need to check if 'input' or 'output' is empty or not?



src/common/command_utils.cpp (line 161)


Please align '{' here accordingly.



src/common/command_utils.cpp (line 183)


Ditto on empty check.



src/tests/common/command_utils_tests.cpp (line 23)


Why do you need this? Please remove headers that are not needed please.



src/tests/common/command_utils_tests.cpp (line 33)


we try to avoid using 'using namespace' clause. For this test, I think it's 
pretty clean and readable to use command::tar, command::untar (i.e., no need 
for this clause).



src/tests/common/command_utils_tests.cpp (line 43)


I would suggest to rename it to TarTest. Two reasons: 1) we might have 'ar' 
util in the future 2) it's better to keep the name consistent with the command 
name to readability and searchability.



src/tests/common/command_utils_tests.cpp (line 46)


s/createTestFileInDir/createTestFile/

InDir is implied by the parameter 'directory'



src/tests/common/command_utils_tests.cpp (line 47)


s/filePath/file/

Path is the type, no need for the redundent Path in variable name.



src/tests/common/command_utils_tests.cpp (line 50)


s/testFilePath/testFile/



src/tests/common/command_utils_tests.cpp (line 54)


Do you need this temp variable?



src/tests/common/command_utils_tests.cpp (lines 93 - 95)


You can use EXPECT_SOME_EQ("test", os::read(testFile));



src/tests/common/command_utils_tests.cpp (line 103)


s/testFilePath/testFile/



src/tests/common/command_utils_tests.cpp (lines 122 - 124)


Ditto on using EXPECT_SOME_EQ



src/tests/common/command_utils_tests.cpp (line 135)


s/testFilePath/testFile/



src/tests/common/command_utils_tests.cpp (lines 157 - 159)


Ditto on using EXPECT_SOME_EQ



src/tests/common/command_utils_tests.cpp (lines 190 - 192)


Ditto.



src/tests/common/command_utils_tests.cpp (line 205)


s/testFilePath/testFile/



src/tests/common/command_utils_tests.cpp (lines 212 - 213)


Can you do the indentation like the following so that it's consistent with 
your style in BZIP2CompressFile
```
AWAIT_ASSERT_READY(tar(
testDir,
outputTarFile,
topDir,
Compression::GZIP));
```



src/tests/common/command_utils_tests.cpp (lines 229 - 231)

Re: Review Request 40553: Enable mesos tests installation.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [39780, 39781, 39782, 40553]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export 
COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Jan. 27, 2016, 5:54 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40553/
> ---
> 
> (Updated Jan. 27, 2016, 5:54 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Till Toenshoff.
> 
> 
> Bugs: MESOS-3608
> https://issues.apache.org/jira/browse/MESOS-3608
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch enables the installation mesos-tests and its dependencies
> and helper tool. The goal is to allow operators to build a separate
> test package that can be run at deployment time to verify that Mesos
> works in the deployment environment.
> 
> Since the build directory is searched first, to run it on a host
> that has a build tree, you need to specify a non-existent tree:
> 
> ~ $ $PREFIX/libexec/mesos/tests/mesos-tests --build_dir=/none
> 
> - Add --enable-tests-install
> - Fix mesos-tests gmock dependencies
> - Optionally install tests, helpers and test modules
> - Add utility helpers to find various test resources
> 
> 
> Diffs
> -
> 
>   Makefile.am fbd4e5a6356e90c867ba47c48c86fc9161ddd98e 
>   configure.ac 70d16871d5888ac1d9d5fb0ba0b453799b00f320 
>   src/Makefile.am 8657a869f931aa7482fbb09f2c6df95b6a8c50c6 
>   src/examples/test_framework.cpp ff7b00543eea1a7cbf52c7abfba81600868bfbab 
>   src/tests/balloon_framework_test.sh 
> 25a19cfde87a3fd2d7d3e780700d342d08bd0a91 
>   src/tests/containerizer/launch_tests.cpp 
> c7ebe2606e4ff99ced90342dd16e0b4bf02bc504 
>   src/tests/containerizer/memory_test_helper.cpp 
> 4a3de2e3c887aa6afc604588850e1386f92d8c11 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> a45ed1b2175b7dc16d621a44fbccfb8f957ae2b5 
>   src/tests/containerizer/ns_tests.cpp 
> 603e54b7303c5aa15e2c5715dc7a2f7e7d39541b 
>   src/tests/containerizer/port_mapping_tests.cpp 
> 182fe9217a5da9af603d6f9c203a1689eff4ca1b 
>   src/tests/environment.cpp e112270b68d402bb9b01445af552500fb3929e52 
>   src/tests/event_call_framework_test.sh 
> 9d1211552734afbf15b376f8c4629bae8a2065af 
>   src/tests/fetcher_tests.cpp 1831d896ca8a52bec4adf87a67b6af845079796c 
>   src/tests/health_check_tests.cpp 65e8fe25ed7feb1080ad833ba98e6b462bd3152c 
>   src/tests/mesos.cpp 18d0d8f8037ebc27c87bcb0f1ce9f143e7505ec8 
>   src/tests/module.cpp 246f3a402d4fe3b273c459f6e02c009f3de65f3e 
>   src/tests/module_tests.cpp a3271a3267647e0964dd3decb3ca8384417dd559 
>   src/tests/no_executor_framework_test.sh 
> aebdc8c380abb2d041d6fc74dfac5a111c15267e 
>   src/tests/oversubscription_tests.cpp 
> 6f43103e81303015fb614653e3bfece55009d1bf 
>   src/tests/persistent_volume_framework_test.sh 
> 84f02847a8d89400512d8a5714d33fb29cf5b03a 
>   src/tests/script.cpp ee44fef29fb40e414d7507168091ee5cd0d15736 
>   src/tests/slave_tests.cpp e943ab99baf3d74679a5da89a89f6a4b7ead 
>   src/tests/test_framework_test.sh 409e80994f63448115ea8ac34b4fd5c6cf88aa22 
>   src/tests/utils.hpp a6cca472f4dfab12cd6eccab6972206d842177aa 
>   src/tests/utils.cpp 22bf3a85da5261fcfcc8b6aa9626aacdc8391ad4 
> 
> Diff: https://reviews.apache.org/r/40553/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 42839: WIP: Appc cache redesign.

[Jojy Varghese]

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

(Updated Jan. 27, 2016, 8:25 p.m.)


Review request for mesos and Jie Yu.


Changes
---

added defensive check


Repository: mesos


Description
---

WIP: Appc cache redesign.


Diffs (updated)
-

  src/slave/containerizer/mesos/provisioner/appc/image_cache.hpp PRE-CREATION 
  src/slave/containerizer/mesos/provisioner/appc/image_cache.cpp PRE-CREATION 
  src/slave/containerizer/mesos/provisioner/appc/store.cpp 
73c4df858a70da3d4cc4a1cb15092165f6ff8fe4 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 42841: WIP: Introducing appc image fetcher.

[Jojy Varghese]

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

(Updated Jan. 27, 2016, 8:26 p.m.)


Review request for Jie Yu.


Changes
---

rebased


Repository: mesos


Description
---

WIP: Introducing appc image fetcher.


Diffs (updated)
-

  src/CMakeLists.txt 47d0a7c0fe73b9297cd7dde6086b5e6e9e1f9e4e 
  src/Makefile.am 19bf3a7c2e43ca04ed6e6d506e052de5537f7c2f 
  src/slave/containerizer/mesos/provisioner/appc/image_fetcher.hpp PRE-CREATION 
  src/slave/containerizer/mesos/provisioner/appc/image_fetcher.cpp PRE-CREATION 
  src/slave/flags.hpp 23ec158f4a95fa76d657d9eb6a4a6fe30d57e5b6 
  src/slave/flags.cpp 75d7429a4e3e1d6259296257c0ace1ade365ac2b 

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


Testing
---

make check; local image server


Thanks,

Jojy Varghese

Re: Review Request 42662: Added common command utils file.

[Jojy Varghese]

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

(Updated Jan. 27, 2016, 8:29 p.m.)


Review request for mesos and Jie Yu.


Changes
---

addressed review.


Repository: mesos


Description
---

This common file is a good place to add common command line utilities like tar,
digests(sha256, sha512, etc). Currently this functionality is spread in the code
base and this change would enable all those call sites to be replaced with a
common code.


Diffs (updated)
-

  src/CMakeLists.txt 47d0a7c0fe73b9297cd7dde6086b5e6e9e1f9e4e 
  src/Makefile.am 19bf3a7c2e43ca04ed6e6d506e052de5537f7c2f 
  src/common/command_utils.hpp PRE-CREATION 
  src/common/command_utils.cpp PRE-CREATION 
  src/tests/common/command_utils_tests.cpp PRE-CREATION 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Review Request 42866: Disabled the test RegistryClientTest.BadTokenServerAddress.

[Jojy Varghese]

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

Review request for mesos and Timothy Chen.


Repository: mesos


Description
---

It has shown flaky behavior and needs further investigation. Disabling now as
its not a very important test for functionality.


Diffs
-

  src/tests/containerizer/provisioner_docker_tests.cpp 
33df60065903228749833bbad20449ba8784594a 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 40553: Enable mesos tests installation.

[Benjamin Bannier]

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



Looks mostly good to me. A few things were unclear to me:

* Would it make sense to add an `installcheck` target? My expectation for that 
would be for it to invoke the installed tests and adding the (in)correct 
`builddir` magic to make sure only stuff from `$PREFIX` is used.
* Why don't we install stout or libprocess tests? I would have thought that 
especially the libprocess tests could be useful to diagnose low-level 
incompatibilites.


src/Makefile.am (lines 1684 - 1688)


Does the reasoning here need an update?



src/Makefile.am (lines 1693 - 1694)


nit-pick: remove spaces before tabs here.



src/Makefile.am (line 1719)


This seems to be fixed now, right? If yes, probably a good idea to revisit 
(and close?) MESOS-1940.



src/Makefile.am (line 1962)


nit-pick: remove space before tabs here.


- Benjamin Bannier


On Jan. 27, 2016, 6:54 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40553/
> ---
> 
> (Updated Jan. 27, 2016, 6:54 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Till Toenshoff.
> 
> 
> Bugs: MESOS-3608
> https://issues.apache.org/jira/browse/MESOS-3608
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch enables the installation mesos-tests and its dependencies
> and helper tool. The goal is to allow operators to build a separate
> test package that can be run at deployment time to verify that Mesos
> works in the deployment environment.
> 
> Since the build directory is searched first, to run it on a host
> that has a build tree, you need to specify a non-existent tree:
> 
> ~ $ $PREFIX/libexec/mesos/tests/mesos-tests --build_dir=/none
> 
> - Add --enable-tests-install
> - Fix mesos-tests gmock dependencies
> - Optionally install tests, helpers and test modules
> - Add utility helpers to find various test resources
> 
> 
> Diffs
> -
> 
>   Makefile.am fbd4e5a6356e90c867ba47c48c86fc9161ddd98e 
>   configure.ac 70d16871d5888ac1d9d5fb0ba0b453799b00f320 
>   src/Makefile.am 8657a869f931aa7482fbb09f2c6df95b6a8c50c6 
>   src/examples/test_framework.cpp ff7b00543eea1a7cbf52c7abfba81600868bfbab 
>   src/tests/balloon_framework_test.sh 
> 25a19cfde87a3fd2d7d3e780700d342d08bd0a91 
>   src/tests/containerizer/launch_tests.cpp 
> c7ebe2606e4ff99ced90342dd16e0b4bf02bc504 
>   src/tests/containerizer/memory_test_helper.cpp 
> 4a3de2e3c887aa6afc604588850e1386f92d8c11 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> a45ed1b2175b7dc16d621a44fbccfb8f957ae2b5 
>   src/tests/containerizer/ns_tests.cpp 
> 603e54b7303c5aa15e2c5715dc7a2f7e7d39541b 
>   src/tests/containerizer/port_mapping_tests.cpp 
> 182fe9217a5da9af603d6f9c203a1689eff4ca1b 
>   src/tests/environment.cpp e112270b68d402bb9b01445af552500fb3929e52 
>   src/tests/event_call_framework_test.sh 
> 9d1211552734afbf15b376f8c4629bae8a2065af 
>   src/tests/fetcher_tests.cpp 1831d896ca8a52bec4adf87a67b6af845079796c 
>   src/tests/health_check_tests.cpp 65e8fe25ed7feb1080ad833ba98e6b462bd3152c 
>   src/tests/mesos.cpp 18d0d8f8037ebc27c87bcb0f1ce9f143e7505ec8 
>   src/tests/module.cpp 246f3a402d4fe3b273c459f6e02c009f3de65f3e 
>   src/tests/module_tests.cpp a3271a3267647e0964dd3decb3ca8384417dd559 
>   src/tests/no_executor_framework_test.sh 
> aebdc8c380abb2d041d6fc74dfac5a111c15267e 
>   src/tests/oversubscription_tests.cpp 
> 6f43103e81303015fb614653e3bfece55009d1bf 
>   src/tests/persistent_volume_framework_test.sh 
> 84f02847a8d89400512d8a5714d33fb29cf5b03a 
>   src/tests/script.cpp ee44fef29fb40e414d7507168091ee5cd0d15736 
>   src/tests/slave_tests.cpp e943ab99baf3d74679a5da89a89f6a4b7ead 
>   src/tests/test_framework_test.sh 409e80994f63448115ea8ac34b4fd5c6cf88aa22 
>   src/tests/utils.hpp a6cca472f4dfab12cd6eccab6972206d842177aa 
>   src/tests/utils.cpp 22bf3a85da5261fcfcc8b6aa9626aacdc8391ad4 
> 
> Diff: https://reviews.apache.org/r/40553/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 41491: Exposed docker image manifest to mesos containerizer.

[Gilbert Song]

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

(Updated Jan. 27, 2016, 11:55 a.m.)


Review request for mesos, Artem Harutyunyan, Jie Yu, Qian Zhang, and Timothy 
Chen.


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


Repository: mesos


Description
---

Exposed docker image manifest to mesos containerizer.


Diffs (updated)
-

  src/slave/containerizer/mesos/provisioner/docker/store.cpp 
03425daaf015cf231920b7eda1d5424895a41286 
  src/slave/containerizer/mesos/provisioner/provisioner.hpp 
4202f5552174ad3ab595ab3f16ab91d0854951f0 
  src/slave/containerizer/mesos/provisioner/provisioner.cpp 
023f7363f33c4a927fae11037a408f6747fc3c39 
  src/slave/containerizer/mesos/provisioner/store.hpp 
aec725f789f7aeb92abfcc6718c2e6e2f1f37981 
  src/tests/containerizer/provisioner_docker_tests.cpp 
33df60065903228749833bbad20449ba8784594a 

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


Testing
---

make check


Thanks,

Gilbert Song