Re: Review Request 46942: Fixed docker volume isolator container mount target.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [46942]

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

- Mesos ReviewBot


On May 3, 2016, 6:10 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46942/
> ---
> 
> (Updated May 3, 2016, 6:10 p.m.)
> 
> 
> Review request for mesos, Guangya Liu, Artem Harutyunyan, Jie Yu, and Timothy 
> Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed docker volume isolator container mount target.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp 
> 9cc9de0d1ba7b98f57cb4ac6a515714e4fcc2f1f 
> 
> Diff: https://reviews.apache.org/r/46942/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Review Request 46942: Fixed docker volume isolator container mount target.

[Gilbert Song]

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

Review request for mesos, Guangya Liu, Artem Harutyunyan, Jie Yu, and Timothy 
Chen.


Repository: mesos


Description
---

Fixed docker volume isolator container mount target.


Diffs
-

  src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp 
9cc9de0d1ba7b98f57cb4ac6a515714e4fcc2f1f 

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


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 46608: Libprocess: Implemented `subprocess_windows.cpp`.

[Daniel Pravat]

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




3rdparty/libprocess/src/subprocess.cpp (line 235)


You should close the thread handle in the `createChildProcess`



3rdparty/libprocess/src/subprocess.cpp (line 235)


You should close the thread handle in the `createChildProcess`


- Daniel Pravat


On May 3, 2016, 5:45 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46608/
> ---
> 
> (Updated May 3, 2016, 5:45 a.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, 
> Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Bugs: MESOS-3637
> https://issues.apache.org/jira/browse/MESOS-3637
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Libprocess: Implemented `subprocess_windows.cpp`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/subprocess_base.hpp PRE-CREATION 
>   3rdparty/libprocess/include/process/windows/subprocess.hpp PRE-CREATION 
>   3rdparty/libprocess/src/io.cpp 83e5f04f246b46880cfc34aa56441046b569b142 
>   3rdparty/libprocess/src/subprocess.cpp 
> bb0fcbcd0dfa455c8700247c5b4ca0473fd163c3 
>   3rdparty/libprocess/src/subprocess_windows.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/46608/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 46942: Fixed docker volume isolator container mount target.

[Jie Yu]

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


Fix it, then Ship it!





src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp (line 334)


Please use path::absolute here.



src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp (line 367)


Can you copy the NOTE as well?


- Jie Yu


On May 3, 2016, 6:10 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46942/
> ---
> 
> (Updated May 3, 2016, 6:10 p.m.)
> 
> 
> Review request for mesos, Guangya Liu, Artem Harutyunyan, Jie Yu, and Timothy 
> Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed docker volume isolator container mount target.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp 
> 9cc9de0d1ba7b98f57cb4ac6a515714e4fcc2f1f 
> 
> Diff: https://reviews.apache.org/r/46942/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 46942: Fixed docker volume isolator container mount target.

[Gilbert Song]

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

(Updated May 3, 2016, 12:50 p.m.)


Review request for mesos, Guangya Liu, Artem Harutyunyan, Jie Yu, and Timothy 
Chen.


Repository: mesos


Description
---

Fixed docker volume isolator container mount target.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp 
9cc9de0d1ba7b98f57cb4ac6a515714e4fcc2f1f 

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


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 46505: Removed MethodNotAllowed error string creation.

[Mesos ReviewBot]

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



Bad patch!

Reviews applied: [46505, 46504]

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

Error:
2016-05-03 06:14:46 URL:https://reviews.apache.org/r/46504/diff/raw/ 
[2082/2082] -> "46504.patch" [1]
Total errors found: 0
Checking 1 files
Error: No line in the commit message summary may exceed 72 characters.

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

- Mesos ReviewBot


On May 3, 2016, 12:20 a.m., Jacob Janco wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46505/
> ---
> 
> (Updated May 3, 2016, 12:20 a.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-4126
> https://issues.apache.org/jira/browse/MESOS-4126
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed hard coding of MethodNotAllowed Reponse body in master and slave.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 4985f24b70a00116caa4bd0335ea51e55d878d19 
>   src/slave/http.cpp 9b558862e025c5caa71e05fc5eeba783c0ad6fd5 
> 
> Diff: https://reviews.apache.org/r/46505/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jacob Janco
> 
>

Re: Review Request 46140: Added test "ROOT_LaunchCommandExecutorNoRootfsWithSingleVolume".

[Gilbert Song]

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




src/tests/containerizer/docker_volume_isolator_tests.cpp (line 143)


I would suggest using an `Owned` pointer here.



src/tests/containerizer/docker_volume_isolator_tests.cpp (line 216)


Let's use `volume` while we dont have sth like volume2.



src/tests/containerizer/docker_volume_isolator_tests.cpp (line 230)


ditto.



src/tests/containerizer/docker_volume_isolator_tests.cpp (line 277)


ditto.


- Gilbert Song


On May 2, 2016, 10:12 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46140/
> ---
> 
> (Updated May 2, 2016, 10:12 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-5266
> https://issues.apache.org/jira/browse/MESOS-5266
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test "ROOT_LaunchCommandExecutorNoRootfsWithSingleVolume".
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 53de98f43629dc94f7619324369caf88407b2f41 
>   src/slave/containerizer/mesos/isolators/docker/volume/driver.hpp 
> 070d52018e82ed3e46fb1b79714ffc4716f6a306 
>   src/tests/containerizer/docker_volume_isolator_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/46140/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 45996: Fixed memory leak of `gc` in `finalize()` in libprocess.

[Neil Conway]

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

(Updated May 3, 2016, 9:41 a.m.)


Review request for mesos and Joris Van Remoortere.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Fixed memory leak of `gc` in `finalize()` in libprocess.


Diffs (updated)
-

  3rdparty/libprocess/include/process/gc.hpp 
799468ebe49f2a49d325f40ffd8acea727abf74c 
  3rdparty/libprocess/src/process.cpp dcaa64633d1eea330649c563635642928164d73c 

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


Testing
---


Thanks,

Neil Conway

Re: Review Request 45995: Fixed memory leak of `Route` in `finalize()` in libprocess.

[Neil Conway]

> On May 2, 2016, 5:53 p.m., haosdent huang wrote:
> > 3rdparty/libprocess/src/process.cpp, line 1007
> > 
> >
> > I am no sure if use `processes_route` style to keep consistent with 
> > `process_manager`.

Good catch!


- Neil


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


On April 11, 2016, 1:39 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45995/
> ---
> 
> (Updated April 11, 2016, 1:39 a.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-5144
> https://issues.apache.org/jira/browse/MESOS-5144
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed memory leak of `Route` in `finalize()` in libprocess.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp 
> 5e9dcfdc52f3a8223bc43af149b8e1f5dbdf5b0a 
> 
> Diff: https://reviews.apache.org/r/45995/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 46146: Fixed libprocess tests to use smart pointers.

[Neil Conway]

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

(Updated May 3, 2016, 9:42 a.m.)


Review request for mesos, Benjamin Bannier and Joris Van Remoortere.


Changes
---

Rebase.


Repository: mesos


Description
---

This ensures that dynamically allocated memory is released when
a test aborts prematurely due to an EXPECT_XXX failure.


Diffs (updated)
-

  3rdparty/libprocess/src/tests/decoder_tests.cpp 
bd990c5eb77e47d7f617199bcc89e9432af7cc51 
  3rdparty/libprocess/src/tests/process_tests.cpp 
3fce6fc41faaa7b6e8a2a957e85de3de973a51ba 
  3rdparty/libprocess/src/tests/sequence_tests.cpp 
566393fbf3f19644b86843f07194d1de36a2015e 

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


Testing
---

make check on OSX and Linux.


Thanks,

Neil Conway

Re: Review Request 46923: Added framework failover timeout validation.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [46923]

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

- Mesos ReviewBot


On May 3, 2016, 3:31 a.m., Jose Guilherme Vanz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46923/
> ---
> 
> (Updated May 3, 2016, 3:31 a.m.)
> 
> 
> Review request for mesos, Kevin Sweeney and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Frameworks were allowed to subscribe themselves with invalid failover
> timeout. For this reason, a validation has been made in the master to
> deny framework subscription if it set a invalid value for the failover
> timeout. MESOS-1575
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp 3e55114ee7866e06513071e86e15608099dae052 
>   src/master/master.cpp 6d3e0f7c634690a35eec1ce827b705e04c3af87e 
>   src/tests/master_tests.cpp 8e00753fcbcd0cae1d08aad387b08aa17c7f63ad 
> 
> Diff: https://reviews.apache.org/r/46923/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jose Guilherme Vanz
> 
>

Re: Review Request 38451: Added test "ROOT_CommandTaskNoRootfsMultipleSameVolumes".

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [46680, 46681, 46682, 46140, 42028, 38451]

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

- Mesos ReviewBot


On May 3, 2016, 8:10 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38451/
> ---
> 
> (Updated May 3, 2016, 8:10 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-5266
> https://issues.apache.org/jira/browse/MESOS-5266
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test "ROOT_CommandTaskNoRootfsMultipleSameVolumes".
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_volume_isolator_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38451/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> GLOG_v=1 ./bin/mesos-tests.sh  
> --gtest_filter="DockerVolumeIsolatorTest.ROOT_CommandTaskNoRootfsMultipleSameVolumes"
>  --verbose
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 46140: Added test "ROOT_CommandTaskNoRootfsSingleVolume".

[Guangya Liu]

> On 五月 3, 2016, 5:28 a.m., Jie Yu wrote:
> > src/tests/containerizer/docker_volume_isolator_tests.cpp, line 204
> > 
> >
> > 
> > `s/ROOT_LaunchCommandExecutorNoRootfsWithSingleVolume/ROOT_CommandTaskNoRootfs/`

I was renaming it to `ROOT_CommandTaskNoRootfsSingleVolume` cause I will have 
other test case with multiple volumes.


> On 五月 3, 2016, 5:28 a.m., Jie Yu wrote:
> > src/tests/containerizer/docker_volume_isolator_tests.cpp, lines 214-215
> > 
> >
> > Is it used?

Yes, it will be called when generating the mount point.


> On 五月 3, 2016, 5:28 a.m., Jie Yu wrote:
> > src/tests/containerizer/docker_volume_isolator_tests.cpp, line 223
> > 
> >
> > is it used?

Yes, it will be called when clean up the container.


> On 五月 3, 2016, 5:28 a.m., Jie Yu wrote:
> > src/tests/containerizer/docker_volume_isolator_tests.cpp, lines 214-227
> > 
> >
> > I think you should move this down right before calling `launchTasks`.
> > 
> > Also, please capture the parameter to the 'mount' and 'unmount' here to 
> > verify that it's the same as specified in ContainerInfo.

OK, moved before `launchTasks`, one question is how can I verify this 
parameters? Did not found a good way to verify here.


- Guangya


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


On 五月 3, 2016, 7:51 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46140/
> ---
> 
> (Updated 五月 3, 2016, 7:51 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-5266
> https://issues.apache.org/jira/browse/MESOS-5266
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test "ROOT_CommandTaskNoRootfsSingleVolume".
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 53de98f43629dc94f7619324369caf88407b2f41 
>   src/slave/containerizer/mesos/isolators/docker/volume/driver.hpp 
> 070d52018e82ed3e46fb1b79714ffc4716f6a306 
>   src/tests/containerizer/docker_volume_isolator_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/46140/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> GLOG_v=1 ./bin/mesos-tests.sh  
> --gtest_filter="DockerVolumeIsolatorTest.ROOT_CommandTaskNoRootfsSingleVolume"
>  --verbose
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 45995: Fixed memory leak of `Route` in `finalize()` in libprocess.

[Neil Conway]

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

(Updated May 3, 2016, 9:40 a.m.)


Review request for mesos and Joris Van Remoortere.


Changes
---

Fix style issue per haosdent


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


Repository: mesos


Description
---

Fixed memory leak of `Route` in `finalize()` in libprocess.


Diffs (updated)
-

  3rdparty/libprocess/src/process.cpp dcaa64633d1eea330649c563635642928164d73c 

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


Testing
---

make check


Thanks,

Neil Conway

Re: Review Request 46140: Added test "ROOT_CommandTaskNoRootfsSingleVolume".

[Guangya Liu]

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

(Updated 五月 3, 2016, 7:51 a.m.)


Review request for mesos, Gilbert Song and Jie Yu.


Summary (updated)
-

Added test "ROOT_CommandTaskNoRootfsSingleVolume".


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


Repository: mesos


Description (updated)
---

Added test "ROOT_CommandTaskNoRootfsSingleVolume".


Diffs (updated)
-

  src/Makefile.am 53de98f43629dc94f7619324369caf88407b2f41 
  src/slave/containerizer/mesos/isolators/docker/volume/driver.hpp 
070d52018e82ed3e46fb1b79714ffc4716f6a306 
  src/tests/containerizer/docker_volume_isolator_tests.cpp PRE-CREATION 

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


Testing (updated)
---

make
make check
GLOG_v=1 ./bin/mesos-tests.sh  
--gtest_filter="DockerVolumeIsolatorTest.ROOT_CommandTaskNoRootfsSingleVolume" 
--verbose


Thanks,

Guangya Liu

Re: Review Request 42028: Added test "ROOT_CommandTaskNoRootfsMultipleVolumes".

[Guangya Liu]

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

(Updated 五月 3, 2016, 8:01 a.m.)


Review request for mesos, Gilbert Song and Jie Yu.


Summary (updated)
-

Added test "ROOT_CommandTaskNoRootfsMultipleVolumes".


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


Repository: mesos


Description (updated)
---

Added test "ROOT_CommandTaskNoRootfsMultipleVolumes".


Diffs (updated)
-

  src/tests/containerizer/docker_volume_isolator_tests.cpp PRE-CREATION 

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


Testing
---

make
make check


Thanks,

Guangya Liu

Re: Review Request 46165: Removed dependency on Boost.Foreach.

[Benjamin Bannier]

> On April 14, 2016, 11:33 a.m., Benjamin Bannier wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/foreach.hpp, line 95
> > 
> >
> > This one is subtle as in principle we could use a moved-from 
> > `FOREACH_ELEM`. This happens for cases where the `begin_expr` returns an 
> > rvalue in violation of general iterator requirements (e.g., that the result 
> > of `*it` is assignable).
> > 
> > Could you explain why we'd need to `forward` here and above in the 
> > first place? I appears we should be able to just `get` directly from the 
> > `FOREACH_ELEM` in scope.

Dropping this one as we wouldn't actually `move` here, but just potentially 
cast to an rvalue.


- Benjamin


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


On April 13, 2016, 11:58 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46165/
> ---
> 
> (Updated April 13, 2016, 11:58 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Hindman, Ben Mahler, and 
> Joris Van Remoortere.
> 
> 
> Bugs: MESOS-3214
> https://issues.apache.org/jira/browse/MESOS-3214
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/foreach.hpp 
> 7fb0044790ee249b69e07b81a26851bd5bfb110f 
> 
> Diff: https://reviews.apache.org/r/46165/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 42028: Added test "ROOT_CommandTaskNoRootfsMultipleVolumes".

[Gilbert Song]

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



I am thinking we should only have one `ROOT_CommandTaskNoRootfsDockerVolumes`, 
and use two volumes in that test case.

- Gilbert Song


On May 3, 2016, 1:01 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42028/
> ---
> 
> (Updated May 3, 2016, 1:01 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-5266
> https://issues.apache.org/jira/browse/MESOS-5266
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test "ROOT_CommandTaskNoRootfsMultipleVolumes".
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_volume_isolator_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/42028/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 38451: Added test "ROOT_CommandTaskNoRootfsMultipleSameVolumes".

[Gilbert Song]

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



I am thinking we should only have one `ROOT_CommandTaskNoRootfsDockerVolumes`.

- Gilbert Song


On May 3, 2016, 1:10 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38451/
> ---
> 
> (Updated May 3, 2016, 1:10 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-5266
> https://issues.apache.org/jira/browse/MESOS-5266
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test "ROOT_CommandTaskNoRootfsMultipleSameVolumes".
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_volume_isolator_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38451/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> GLOG_v=1 ./bin/mesos-tests.sh  
> --gtest_filter="DockerVolumeIsolatorTest.ROOT_CommandTaskNoRootfsMultipleSameVolumes"
>  --verbose
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Review Request 46945: Renamed a shadowing variable.

[Michael Park]

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

Review request for mesos, Benjamin Bannier, Benjamin Hindman, Ben Mahler, and 
Joris Van Remoortere.


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


Repository: mesos


Description
---

The `value` variable declared in the `foreach` shadows the `value` function 
parameter. This seemed to have been ok with the `FOREACH` macro, but this is no 
longer ok with `range-based for`.
This seems to be the only place of violation of this.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/include/stout/json.hpp 
d1f4ae6a1d1e6ccfe55f9f8f78390826dc97d894 

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


Testing
---

`make check` on Ubuntu 14.04 with GCC 4.8


Thanks,

Michael Park

Re: Review Request 38451: Added test "ROOT_CommandTaskNoRootfsMultipleSameVolumes".

[Gilbert Song]

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




src/tests/containerizer/docker_volume_isolator_tests.cpp (line 440)


Did you call the mock methods?


- Gilbert Song


On May 3, 2016, 1:10 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38451/
> ---
> 
> (Updated May 3, 2016, 1:10 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-5266
> https://issues.apache.org/jira/browse/MESOS-5266
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test "ROOT_CommandTaskNoRootfsMultipleSameVolumes".
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_volume_isolator_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38451/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> GLOG_v=1 ./bin/mesos-tests.sh  
> --gtest_filter="DockerVolumeIsolatorTest.ROOT_CommandTaskNoRootfsMultipleSameVolumes"
>  --verbose
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 46140: Added test "ROOT_CommandTaskNoRootfsSingleVolume".

[Gilbert Song]

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




src/tests/containerizer/docker_volume_isolator_tests.cpp (line 112)


Should we use `MNT_DETACH` flag here?



src/tests/containerizer/docker_volume_isolator_tests.cpp (lines 215 - 217)


This test verifies that a docker volume is properly mounted to a container 
without rootfs, and launches a command task that reads files from the mounted 
docker volume.



src/tests/containerizer/docker_volume_isolator_tests.cpp (lines 272 - 273)


Do we need it on this test?



src/tests/containerizer/docker_volume_isolator_tests.cpp (line 276)


Please apply the patch https://reviews.apache.org/r/46942/

and the `container_path` should be a relative path here. E.g., `tmp/foo`. 
This should be corresponding to the the path in your task.



src/tests/containerizer/docker_volume_isolator_tests.cpp (line 277)


One more line above.



src/tests/containerizer/docker_volume_isolator_tests.cpp (line 281)


Are you using this? Please remove if not.



src/tests/containerizer/docker_volume_isolator_tests.cpp (line 288)


Kill this.



src/tests/containerizer/docker_volume_isolator_tests.cpp (line 292)


ditto.



src/tests/containerizer/docker_volume_isolator_tests.cpp (line 294)


Kill this.


- Gilbert Song


On May 3, 2016, 12:51 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46140/
> ---
> 
> (Updated May 3, 2016, 12:51 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-5266
> https://issues.apache.org/jira/browse/MESOS-5266
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test "ROOT_CommandTaskNoRootfsSingleVolume".
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 53de98f43629dc94f7619324369caf88407b2f41 
>   src/slave/containerizer/mesos/isolators/docker/volume/driver.hpp 
> 070d52018e82ed3e46fb1b79714ffc4716f6a306 
>   src/tests/containerizer/docker_volume_isolator_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/46140/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> GLOG_v=1 ./bin/mesos-tests.sh  
> --gtest_filter="DockerVolumeIsolatorTest.ROOT_CommandTaskNoRootfsSingleVolume"
>  --verbose
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 46942: Fixed docker volume isolator container mount target.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [46942]

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

- Mesos ReviewBot


On May 3, 2016, 7:50 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46942/
> ---
> 
> (Updated May 3, 2016, 7:50 p.m.)
> 
> 
> Review request for mesos, Guangya Liu, Artem Harutyunyan, Jie Yu, and Timothy 
> Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed docker volume isolator container mount target.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp 
> 9cc9de0d1ba7b98f57cb4ac6a515714e4fcc2f1f 
> 
> Diff: https://reviews.apache.org/r/46942/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 46165: Removed dependency on Boost.Foreach.

[Michael Park]

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

(Updated May 3, 2016, 8:34 p.m.)


Review request for mesos, Benjamin Bannier, Benjamin Hindman, Ben Mahler, and 
Joris Van Remoortere.


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


Repository: mesos


Description (updated)
---

Removed dependency on Boost.Foreach.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/stout/foreach.hpp 
7fb0044790ee249b69e07b81a26851bd5bfb110f 

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


Testing
---

`make check`


Thanks,

Michael Park

Re: Review Request 46945: Renamed a shadowing variable.

[Benjamin Bannier]

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


Ship it!




Ship It!

- Benjamin Bannier


On May 3, 2016, 10:34 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46945/
> ---
> 
> (Updated May 3, 2016, 10:34 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Hindman, Ben Mahler, and 
> Joris Van Remoortere.
> 
> 
> Bugs: MESOS-3214
> https://issues.apache.org/jira/browse/MESOS-3214
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The `value` variable declared in the `foreach` shadows the `value` function 
> parameter. This seemed to have been ok with the `FOREACH` macro, but this is 
> no longer ok with `range-based for`.
> This seems to be the only place of violation of this.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/json.hpp 
> d1f4ae6a1d1e6ccfe55f9f8f78390826dc97d894 
> 
> Diff: https://reviews.apache.org/r/46945/diff/
> 
> 
> Testing
> ---
> 
> `make check` on Ubuntu 14.04 with GCC 4.8
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 46920: Added validation hook inside Slave::runTask.

[Joseph Wu]

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

(Updated May 3, 2016, 1:50 p.m.)


Review request for mesos, Adam B, Artem Harutyunyan, and Jie Yu.


Changes
---

Added test.


Summary (updated)
-

Added validation hook inside Slave::runTask.


Repository: mesos


Description
---

Adds a new hook `slaveRunTaskValidatorHook`, which allows a module to 
inspect the contents of a task and potentially reject the task with a 
`TASK_ERROR`.


Diffs (updated)
-

  include/mesos/hook.hpp 210ffba09f5acae34ca49b888a781f683777f9ca 
  src/examples/test_hook_module.cpp 4b97f84204934d0e678786fd6cde38b89a6f8f48 
  src/hook/manager.hpp 528674e36639fe78137ba0a4bb004c99730e7a22 
  src/hook/manager.cpp 381807d582998043d73e9b8c9d3c1fddbcf73cf1 
  src/slave/slave.cpp 116ea59b72950db4a7cd721b7ba5bfbb2e1c1454 
  src/tests/hook_tests.cpp 60d52c5849ba555f6f3070883d87aadf105f05b0 

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


Testing (updated)
---

make check


Thanks,

Joseph Wu

Re: Review Request 46165: Removed dependency on Boost.Foreach.

[Michael Park]

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

(Updated May 3, 2016, 8:53 p.m.)


Review request for mesos, Benjamin Bannier, Benjamin Hindman, Ben Mahler, and 
Joris Van Remoortere.


Changes
---

Addressed bbannier's comment.


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


Repository: mesos


Description
---

Removed dependency on Boost.Foreach.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/stout/foreach.hpp 
7fb0044790ee249b69e07b81a26851bd5bfb110f 

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


Testing
---

`make check`


Thanks,

Michael Park

Re: Review Request 46165: Removed dependency on Boost.Foreach.

[Mesos ReviewBot]

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



Bad patch!

Reviews applied: [46165, 46945]

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

Error:
2016-05-03 22:25:32 URL:https://reviews.apache.org/r/46945/diff/raw/ [851/851] 
-> "46945.patch" [1]
Total errors found: 0
Checking 1 files
Error: No line in the commit message summary may exceed 72 characters.

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

- Mesos ReviewBot


On May 3, 2016, 8:53 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46165/
> ---
> 
> (Updated May 3, 2016, 8:53 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Hindman, Ben Mahler, and 
> Joris Van Remoortere.
> 
> 
> Bugs: MESOS-3214
> https://issues.apache.org/jira/browse/MESOS-3214
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed dependency on Boost.Foreach.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/foreach.hpp 
> 7fb0044790ee249b69e07b81a26851bd5bfb110f 
> 
> Diff: https://reviews.apache.org/r/46165/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 46140: Added test "ROOT_CommandTaskNoRootfsSingleVolume".

[Gilbert Song]

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



Could you fix the other patches correspondingly? Thanks, Guangya! We are very 
close.

- Gilbert Song


On May 3, 2016, 12:51 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46140/
> ---
> 
> (Updated May 3, 2016, 12:51 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-5266
> https://issues.apache.org/jira/browse/MESOS-5266
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test "ROOT_CommandTaskNoRootfsSingleVolume".
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 53de98f43629dc94f7619324369caf88407b2f41 
>   src/slave/containerizer/mesos/isolators/docker/volume/driver.hpp 
> 070d52018e82ed3e46fb1b79714ffc4716f6a306 
>   src/tests/containerizer/docker_volume_isolator_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/46140/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> GLOG_v=1 ./bin/mesos-tests.sh  
> --gtest_filter="DockerVolumeIsolatorTest.ROOT_CommandTaskNoRootfsSingleVolume"
>  --verbose
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 46165: Removed dependency on Boost.Foreach.

[Benjamin Bannier]

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


Fix it, then Ship it!





3rdparty/libprocess/3rdparty/stout/include/stout/foreach.hpp (lines 21 - 26)


Could you prefix these with e.g., `STOUT_` to avoid collisions?


- Benjamin Bannier


On May 3, 2016, 10:34 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46165/
> ---
> 
> (Updated May 3, 2016, 10:34 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Hindman, Ben Mahler, and 
> Joris Van Remoortere.
> 
> 
> Bugs: MESOS-3214
> https://issues.apache.org/jira/browse/MESOS-3214
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed dependency on Boost.Foreach.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/foreach.hpp 
> 7fb0044790ee249b69e07b81a26851bd5bfb110f 
> 
> Diff: https://reviews.apache.org/r/46165/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 46920: Added validation hook inside Slave::runTask.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [46920]

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

- Mesos ReviewBot


On May 3, 2016, 8:50 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46920/
> ---
> 
> (Updated May 3, 2016, 8:50 p.m.)
> 
> 
> Review request for mesos, Adam B, Artem Harutyunyan, and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adds a new hook `slaveRunTaskValidatorHook`, which allows a module to 
> inspect the contents of a task and potentially reject the task with a 
> `TASK_ERROR`.
> 
> 
> Diffs
> -
> 
>   include/mesos/hook.hpp 210ffba09f5acae34ca49b888a781f683777f9ca 
>   src/examples/test_hook_module.cpp 4b97f84204934d0e678786fd6cde38b89a6f8f48 
>   src/hook/manager.hpp 528674e36639fe78137ba0a4bb004c99730e7a22 
>   src/hook/manager.cpp 381807d582998043d73e9b8c9d3c1fddbcf73cf1 
>   src/slave/slave.cpp 116ea59b72950db4a7cd721b7ba5bfbb2e1c1454 
>   src/tests/hook_tests.cpp 60d52c5849ba555f6f3070883d87aadf105f05b0 
> 
> Diff: https://reviews.apache.org/r/46920/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 46813: Update distribute to setuptools.

[Bernd Mathiske]

> On April 29, 2016, 9:20 a.m., haosdent huang wrote:
> > 3rdparty/versions.am, line 25
> > 
> >
> > I saw the latest version of setuptools is `20.10.1` in 
> > https://pypi.python.org/pypi/setuptools Do we have any reasons to use 
> > 20.9.0?
> 
> Zhiwei Chen wrote:
> oh, it's latest version is 20.9.0 when I first testing this patch. I will 
> try to upgrade to the latest one.
> 
> Zhiwei Chen wrote:
> I found setuptools now upgrade to 21.0.0, it's version grows too fast[1] 
> and there seems no significant changes[2], so I decided not to catch up the 
> latest version.
> 
> [1]: https://github.com/pypa/setuptools/releases
> [2]: https://pythonhosted.org/setuptools/history.html
> 
> haosdent huang wrote:
> Got it, thank you very much for your explanation!

Thanks for checking! If nobody objects, I will merge this RR today.


- Bernd


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


On April 29, 2016, 2:13 a.m., Zhiwei Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46813/
> ---
> 
> (Updated April 29, 2016, 2:13 a.m.)
> 
> 
> Review request for mesos, Adam B, haosdent huang, Kapil Arya, and Joseph Wu.
> 
> 
> Bugs: MESOS-5244
> https://issues.apache.org/jira/browse/MESOS-5244
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Update distribute to setuptools.
> 
> 
> Diffs
> -
> 
>   3rdparty/Makefile.am 0523151d924a1398334767bd401ae84e41bdceb7 
>   3rdparty/distribute-0.6.26.tar.gz 3a598ccd4bfdfdf250a6d7a28552677394f14a76 
>   3rdparty/setuptools-20.9.0.tar.gz PRE-CREATION 
>   3rdparty/versions.am a94723e213dbbbd1df7e1b784386023412f7b961 
>   LICENSE 19130d977986dc9e23872817251ffcaa5a3c2c7d 
>   configure.ac ae91a07e2bafd97566ac6e6873990302acad14c9 
>   mpi/mpiexec-mesos.in 021dd5b6b9c1c601b81d5bf49fda1bd23c2040ee 
>   src/Makefile.am 53de98f43629dc94f7619324369caf88407b2f41 
>   src/examples/python/test-containerizer.in 
> b8258e75a3cd5f8d18ab8eff8196c5374db4f629 
>   src/examples/python/test-executor.in 
> 6be0ad2d339ff43d074e9688978af1ddf956803c 
>   src/examples/python/test-framework.in 
> 59bab82962c5846c6be288f3d392986e146e9da6 
>   support/coverage.sh 7edfec27d29fa900d35894c7778fcb7cfde513b8 
> 
> Diff: https://reviews.apache.org/r/46813/diff/
> 
> 
> Testing
> ---
> 
> make && make install on Ubuntu 14.04/16.04, RHEL 6.6/7.1 and OS X 10.10.5.
> 
> NOTE: I tested on my RHEL 6.6 and failed, because the package 
> pytz-2010h-2.el6.noarch was installed, when I removed this package verything 
> ok. The default RHEL/CentOS 6.6 has no pytz-2010h-2.el6.noarch, so I think 
> it's ok for this patch.
> 
> 
> Thanks,
> 
> Zhiwei Chen
> 
>

Re: Review Request 39474: Added test "ROOT_CommandTaskNoRootfsSlaveRecovery".

[Guangya Liu]

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

(Updated 五月 3, 2016, 12:19 p.m.)


Review request for mesos, Gilbert Song and Jie Yu.


Summary (updated)
-

Added test "ROOT_CommandTaskNoRootfsSlaveRecovery".


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


Repository: mesos


Description (updated)
---

Added test "ROOT_CommandTaskNoRootfsSlaveRecovery".


Diffs (updated)
-

  src/tests/containerizer/docker_volume_isolator_tests.cpp PRE-CREATION 

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


Testing (updated)
---

make
make check
GLOG_v=1 ./bin/mesos-tests.sh  
--gtest_filter="DockerVolumeIsolatorTest.ROOT_CommandTaskNoRootfsSlaveRecovery" 
--verbose

[==] Running 4 tests from 1 test case.
[--] Global test environment set-up.
[--] 4 tests from DockerVolumeIsolatorTest
[ RUN  ] DockerVolumeIsolatorTest.ROOT_CommandTaskNoRootfsSingleVolume
[   OK ] DockerVolumeIsolatorTest.ROOT_CommandTaskNoRootfsSingleVolume (506 
ms)
[ RUN  ] DockerVolumeIsolatorTest.ROOT_CommandTaskNoRootfsMultipleVolumes
[   OK ] DockerVolumeIsolatorTest.ROOT_CommandTaskNoRootfsMultipleVolumes 
(390 ms)
[ RUN  ] 
DockerVolumeIsolatorTest.ROOT_CommandTaskNoRootfsMultipleSameVolumes
[   OK ] 
DockerVolumeIsolatorTest.ROOT_CommandTaskNoRootfsMultipleSameVolumes (104 ms)
[ RUN  ] DockerVolumeIsolatorTest.ROOT_CommandTaskNoRootfsSlaveRecovery
[   OK ] DockerVolumeIsolatorTest.ROOT_CommandTaskNoRootfsSlaveRecovery 
(2540 ms)
[--] 4 tests from DockerVolumeIsolatorTest (3618 ms total)

[--] Global test environment tear-down
[==] 4 tests from 1 test case ran. (3629 ms total)
[  PASSED  ] 4 tests.


Thanks,

Guangya Liu

Re: Review Request 46923: Added framework failover timeout validation.

[Neil Conway]

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




src/master/master.hpp (line 937)


The preceding comment ("Inner class used to namespace the handling of quota 
requests...") refers to the `QuotaHandler` code below -- you should move this 
function up before the comment.



src/master/master.cpp (line 2255)


Indented too far to the right (two spaces per indent level is Mesos style).

I'd use "failover_timeout", rather than "failover timeout".

Should we include the framework-provided timeout value in the error 
message? I'd think probably.



src/master/master.cpp (line 2476)


We probably want to check `validationError.isNone()` here.



src/master/master.cpp (line 7342)


Whitespace.



src/tests/master_tests.cpp (line 4481)


Can we add a comment describing why this failover_timeout is invalid?


- Neil Conway


On May 3, 2016, 3:31 a.m., Jose Guilherme Vanz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46923/
> ---
> 
> (Updated May 3, 2016, 3:31 a.m.)
> 
> 
> Review request for mesos, Kevin Sweeney and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Frameworks were allowed to subscribe themselves with invalid failover
> timeout. For this reason, a validation has been made in the master to
> deny framework subscription if it set a invalid value for the failover
> timeout. MESOS-1575
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp 3e55114ee7866e06513071e86e15608099dae052 
>   src/master/master.cpp 6d3e0f7c634690a35eec1ce827b705e04c3af87e 
>   src/tests/master_tests.cpp 8e00753fcbcd0cae1d08aad387b08aa17c7f63ad 
> 
> Diff: https://reviews.apache.org/r/46923/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jose Guilherme Vanz
> 
>

Re: Review Request 46813: Update distribute to setuptools.

[Zhiwei Chen]

> On April 30, 2016, 12:20 a.m., haosdent huang wrote:
> > 3rdparty/versions.am, line 25
> > 
> >
> > I saw the latest version of setuptools is `20.10.1` in 
> > https://pypi.python.org/pypi/setuptools Do we have any reasons to use 
> > 20.9.0?
> 
> Zhiwei Chen wrote:
> oh, it's latest version is 20.9.0 when I first testing this patch. I will 
> try to upgrade to the latest one.

I found setuptools now upgrade to 21.0.0, it's version grows too fast[1] and 
there seems no significant changes[2], so I decided not to catch up the latest 
version.

[1]: https://github.com/pypa/setuptools/releases
[2]: https://pythonhosted.org/setuptools/history.html


- Zhiwei


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


On April 29, 2016, 5:13 p.m., Zhiwei Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46813/
> ---
> 
> (Updated April 29, 2016, 5:13 p.m.)
> 
> 
> Review request for mesos, Adam B, haosdent huang, Kapil Arya, and Joseph Wu.
> 
> 
> Bugs: MESOS-5244
> https://issues.apache.org/jira/browse/MESOS-5244
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Update distribute to setuptools.
> 
> 
> Diffs
> -
> 
>   3rdparty/Makefile.am 0523151d924a1398334767bd401ae84e41bdceb7 
>   3rdparty/distribute-0.6.26.tar.gz 3a598ccd4bfdfdf250a6d7a28552677394f14a76 
>   3rdparty/setuptools-20.9.0.tar.gz PRE-CREATION 
>   3rdparty/versions.am a94723e213dbbbd1df7e1b784386023412f7b961 
>   LICENSE 19130d977986dc9e23872817251ffcaa5a3c2c7d 
>   configure.ac ae91a07e2bafd97566ac6e6873990302acad14c9 
>   mpi/mpiexec-mesos.in 021dd5b6b9c1c601b81d5bf49fda1bd23c2040ee 
>   src/Makefile.am 53de98f43629dc94f7619324369caf88407b2f41 
>   src/examples/python/test-containerizer.in 
> b8258e75a3cd5f8d18ab8eff8196c5374db4f629 
>   src/examples/python/test-executor.in 
> 6be0ad2d339ff43d074e9688978af1ddf956803c 
>   src/examples/python/test-framework.in 
> 59bab82962c5846c6be288f3d392986e146e9da6 
>   support/coverage.sh 7edfec27d29fa900d35894c7778fcb7cfde513b8 
> 
> Diff: https://reviews.apache.org/r/46813/diff/
> 
> 
> Testing
> ---
> 
> make && make install on Ubuntu 14.04/16.04, RHEL 6.6/7.1 and OS X 10.10.5.
> 
> NOTE: I tested on my RHEL 6.6 and failed, because the package 
> pytz-2010h-2.el6.noarch was installed, when I removed this package verything 
> ok. The default RHEL/CentOS 6.6 has no pytz-2010h-2.el6.noarch, so I think 
> it's ok for this patch.
> 
> 
> Thanks,
> 
> Zhiwei Chen
> 
>

Re: Review Request 46813: Update distribute to setuptools.

[haosdent huang]

> On April 29, 2016, 4:20 p.m., haosdent huang wrote:
> > 3rdparty/versions.am, line 25
> > 
> >
> > I saw the latest version of setuptools is `20.10.1` in 
> > https://pypi.python.org/pypi/setuptools Do we have any reasons to use 
> > 20.9.0?
> 
> Zhiwei Chen wrote:
> oh, it's latest version is 20.9.0 when I first testing this patch. I will 
> try to upgrade to the latest one.
> 
> Zhiwei Chen wrote:
> I found setuptools now upgrade to 21.0.0, it's version grows too fast[1] 
> and there seems no significant changes[2], so I decided not to catch up the 
> latest version.
> 
> [1]: https://github.com/pypa/setuptools/releases
> [2]: https://pythonhosted.org/setuptools/history.html

Got it, thank you very much for your explanation!


- haosdent


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


On April 29, 2016, 9:13 a.m., Zhiwei Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46813/
> ---
> 
> (Updated April 29, 2016, 9:13 a.m.)
> 
> 
> Review request for mesos, Adam B, haosdent huang, Kapil Arya, and Joseph Wu.
> 
> 
> Bugs: MESOS-5244
> https://issues.apache.org/jira/browse/MESOS-5244
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Update distribute to setuptools.
> 
> 
> Diffs
> -
> 
>   3rdparty/Makefile.am 0523151d924a1398334767bd401ae84e41bdceb7 
>   3rdparty/distribute-0.6.26.tar.gz 3a598ccd4bfdfdf250a6d7a28552677394f14a76 
>   3rdparty/setuptools-20.9.0.tar.gz PRE-CREATION 
>   3rdparty/versions.am a94723e213dbbbd1df7e1b784386023412f7b961 
>   LICENSE 19130d977986dc9e23872817251ffcaa5a3c2c7d 
>   configure.ac ae91a07e2bafd97566ac6e6873990302acad14c9 
>   mpi/mpiexec-mesos.in 021dd5b6b9c1c601b81d5bf49fda1bd23c2040ee 
>   src/Makefile.am 53de98f43629dc94f7619324369caf88407b2f41 
>   src/examples/python/test-containerizer.in 
> b8258e75a3cd5f8d18ab8eff8196c5374db4f629 
>   src/examples/python/test-executor.in 
> 6be0ad2d339ff43d074e9688978af1ddf956803c 
>   src/examples/python/test-framework.in 
> 59bab82962c5846c6be288f3d392986e146e9da6 
>   support/coverage.sh 7edfec27d29fa900d35894c7778fcb7cfde513b8 
> 
> Diff: https://reviews.apache.org/r/46813/diff/
> 
> 
> Testing
> ---
> 
> make && make install on Ubuntu 14.04/16.04, RHEL 6.6/7.1 and OS X 10.10.5.
> 
> NOTE: I tested on my RHEL 6.6 and failed, because the package 
> pytz-2010h-2.el6.noarch was installed, when I removed this package verything 
> ok. The default RHEL/CentOS 6.6 has no pytz-2010h-2.el6.noarch, so I think 
> it's ok for this patch.
> 
> 
> Thanks,
> 
> Zhiwei Chen
> 
>

Re: Review Request 46784: Added authorization of the '/flags' endpoint.

[Jan Schlicht]

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

(Updated May 3, 2016, 2:42 p.m.)


Review request for mesos, Alexander Rukletsov and Benjamin Bannier.


Changes
---

Parameterized the test fixture.


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


Repository: mesos


Description
---

Access to the '/flags' endpoint is authorized using
the 'GET_ENDPOINT_WITH_PATH' action.


Diffs (updated)
-

  src/master/http.cpp 4985f24b70a00116caa4bd0335ea51e55d878d19 
  src/master/master.hpp 3e55114ee7866e06513071e86e15608099dae052 
  src/tests/master_authorization_tests.cpp 
804b39a269c09df9f6c0bbdf6f8b53921ac09ce8 

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


Testing
---

make check


Thanks,

Jan Schlicht

Re: Review Request 46784: Added authorization of the '/flags' endpoint.

[Jan Schlicht]

> On April 29, 2016, 11:20 a.m., Benjamin Bannier wrote:
> > src/tests/master_authorization_tests.cpp, line 1024
> > 
> >
> > This looks very similar to the `SlaveEndpointTest` suite ;) Should we 
> > just also make this parameterized from the start?
> 
> Jan Schlicht wrote:
> I would like to, but unfortunately a parameterized test in GTest needs at 
> least two values, at this point we have only one. It doesn't compile with a 
> single parameter.
> 
> Benjamin Bannier wrote:
> Hmm, it looks like this should work it you give it values of the right 
> type, i.e., pass `std::string` instead of a string literal to 
> `::testing::Values` when instantiating the test case with 
> `INSTANTIATE_TEST_CASE_P`.

Oh, right. Fixed.


- Jan


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


On April 29, 2016, 11:49 a.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46784/
> ---
> 
> (Updated April 29, 2016, 11:49 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Benjamin Bannier.
> 
> 
> Bugs: MESOS-5297
> https://issues.apache.org/jira/browse/MESOS-5297
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Access to the '/flags' endpoint is authorized using
> the 'GET_ENDPOINT_WITH_PATH' action.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 4f489fb9e5bc77b839cc4a44d7ec36af06f73e4a 
>   src/master/master.hpp 3e55114ee7866e06513071e86e15608099dae052 
>   src/tests/master_authorization_tests.cpp 
> 804b39a269c09df9f6c0bbdf6f8b53921ac09ce8 
> 
> Diff: https://reviews.apache.org/r/46784/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 39474: Added test "ROOT_CommandTaskNoRootfsSlaveRecovery".

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [46680, 46681, 46682, 46140, 42028, 38451, 39474]

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

- Mesos ReviewBot


On May 3, 2016, 12:19 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39474/
> ---
> 
> (Updated May 3, 2016, 12:19 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-5266
> https://issues.apache.org/jira/browse/MESOS-5266
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test "ROOT_CommandTaskNoRootfsSlaveRecovery".
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_volume_isolator_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/39474/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> GLOG_v=1 ./bin/mesos-tests.sh  
> --gtest_filter="DockerVolumeIsolatorTest.ROOT_CommandTaskNoRootfsSlaveRecovery"
>  --verbose
> 
> [==] Running 4 tests from 1 test case.
> [--] Global test environment set-up.
> [--] 4 tests from DockerVolumeIsolatorTest
> [ RUN  ] DockerVolumeIsolatorTest.ROOT_CommandTaskNoRootfsSingleVolume
> [   OK ] DockerVolumeIsolatorTest.ROOT_CommandTaskNoRootfsSingleVolume 
> (506 ms)
> [ RUN  ] DockerVolumeIsolatorTest.ROOT_CommandTaskNoRootfsMultipleVolumes
> [   OK ] DockerVolumeIsolatorTest.ROOT_CommandTaskNoRootfsMultipleVolumes 
> (390 ms)
> [ RUN  ] 
> DockerVolumeIsolatorTest.ROOT_CommandTaskNoRootfsMultipleSameVolumes
> [   OK ] 
> DockerVolumeIsolatorTest.ROOT_CommandTaskNoRootfsMultipleSameVolumes (104 ms)
> [ RUN  ] DockerVolumeIsolatorTest.ROOT_CommandTaskNoRootfsSlaveRecovery
> [   OK ] DockerVolumeIsolatorTest.ROOT_CommandTaskNoRootfsSlaveRecovery 
> (2540 ms)
> [--] 4 tests from DockerVolumeIsolatorTest (3618 ms total)
> 
> [--] Global test environment tear-down
> [==] 4 tests from 1 test case ran. (3629 ms total)
> [  PASSED  ] 4 tests.
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 46923: Added framework failover timeout validation.

[Guangya Liu]

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



You can put `MESOS-1575` to the `bugs` column at right.


src/master/master.cpp (line 7341)


s/failoverTimeout_/failoverTimeout



src/master/master.cpp (line 7342)


two spaces is enough


- Guangya Liu


On 五月 3, 2016, 3:31 a.m., Jose Guilherme Vanz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46923/
> ---
> 
> (Updated 五月 3, 2016, 3:31 a.m.)
> 
> 
> Review request for mesos, Kevin Sweeney and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Frameworks were allowed to subscribe themselves with invalid failover
> timeout. For this reason, a validation has been made in the master to
> deny framework subscription if it set a invalid value for the failover
> timeout. MESOS-1575
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp 3e55114ee7866e06513071e86e15608099dae052 
>   src/master/master.cpp 6d3e0f7c634690a35eec1ce827b705e04c3af87e 
>   src/tests/master_tests.cpp 8e00753fcbcd0cae1d08aad387b08aa17c7f63ad 
> 
> Diff: https://reviews.apache.org/r/46923/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jose Guilherme Vanz
> 
>

Review Request 46936: Documented the agent endpoint '/flags'.

[Jan Schlicht]

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

Review request for mesos, Alexander Rukletsov, Alexander Rojas, and Neil Conway.


Repository: mesos


Description
---

See summary.


Diffs
-

  docs/endpoints/slave/flags.md b2740e6a4ce4bb8c25de07071bafbf174adf9137 
  src/slave/http.cpp 9b558862e025c5caa71e05fc5eeba783c0ad6fd5 

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


Testing
---

make check


Thanks,

Jan Schlicht

Re: Review Request 46934: Fixed typo in authentication help.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [46934]

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

- Mesos ReviewBot


On May 3, 2016, 2:17 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46934/
> ---
> 
> (Updated May 3, 2016, 2:17 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Neil Conway.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed typo in authentication help.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/help.hpp 
> 3393b71c484bb8c55c6a4cd2a27dcb798e841a80 
> 
> Diff: https://reviews.apache.org/r/46934/diff/
> 
> 
> Testing
> ---
> 
> make check (OS X, clang-trunk w/o optimizations)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Review Request 38451: Added test "ROOT_CommandTaskNoRootfsMultipleSameVolumes".

[Guangya Liu]

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

Review request for mesos, Gilbert Song and Jie Yu.


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


Repository: mesos


Description
---

Added test "ROOT_CommandTaskNoRootfsMultipleSameVolumes".


Diffs
-

  src/tests/containerizer/docker_volume_isolator_tests.cpp PRE-CREATION 

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


Testing
---

make
make check
GLOG_v=1 ./bin/mesos-tests.sh  
--gtest_filter="DockerVolumeIsolatorTest.ROOT_CommandTaskNoRootfsMultipleSameVolumes"
 --verbose


Thanks,

Guangya Liu

Re: Review Request 46784: Added authorization of the '/flags' endpoint.

[Benjamin Bannier]

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


Ship it!





src/master/http.cpp (line 2882)


I see this is being used also elsewhere, so not your issue, but this that 
downside that it becomes impossible to distinguish an absent principal from one 
called `ANY` when examing logs. @alexr What's the idea behind this?


- Benjamin Bannier


On May 3, 2016, 2:42 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46784/
> ---
> 
> (Updated May 3, 2016, 2:42 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Benjamin Bannier.
> 
> 
> Bugs: MESOS-5297
> https://issues.apache.org/jira/browse/MESOS-5297
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Access to the '/flags' endpoint is authorized using
> the 'GET_ENDPOINT_WITH_PATH' action.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 4985f24b70a00116caa4bd0335ea51e55d878d19 
>   src/master/master.hpp 3e55114ee7866e06513071e86e15608099dae052 
>   src/tests/master_authorization_tests.cpp 
> 804b39a269c09df9f6c0bbdf6f8b53921ac09ce8 
> 
> Diff: https://reviews.apache.org/r/46784/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Review Request 46934: Fixed typo in authentication help.

[Benjamin Bannier]

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

Review request for mesos, Alexander Rukletsov and Neil Conway.


Repository: mesos


Description
---

Fixed typo in authentication help.


Diffs
-

  3rdparty/libprocess/include/process/help.hpp 
3393b71c484bb8c55c6a4cd2a27dcb798e841a80 

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


Testing
---

make check (OS X, clang-trunk w/o optimizations)


Thanks,

Benjamin Bannier

Re: Review Request 46874: Enhanced log message when launch mesos-containerizer.

[Gilbert Song]

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



It seems to me that we have `Starting container..` in launch() and `cloning 
child..` in fork().

- Gilbert Song


On April 30, 2016, 11:24 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46874/
> ---
> 
> (Updated April 30, 2016, 11:24 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The mesos-containerizer will help prepare the rootfs, mount and
> network conf for a container when launch with some flags, it
> would be helpful to print out all of the mesos-containerizer launch
> flags for trouble shooting.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 8d538954d6e1f13e833d75c2eaa37e700278ee0c 
> 
> Diff: https://reviews.apache.org/r/46874/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> Enhanced log message:
> I0501 14:12:17.910006  5991 containerizer.cpp:1175] Launching 
> 'mesos-containerizer' with flags as 
> '--command="{"shell":true,"value":"\/root\/src\/mesos\/m1\/mesos\/build\/src\/mesos-executor"}"
>  --commands="{"commands":[{"shell":true,"value":"#!\/bin\/sh\nset -x 
> -e\n\/root\/src\/mesos\/m1\/mesos\/build\/src\/mesos-containerizer mount 
> --help=\"false\" --operation=\"make-rslave\" --path=\"\/\"\ngrep -E 
> '\/tmp\/mesos\/.+' \/proc\/self\/mountinfo | grep -v 
> 'a2cc264f-72eb-416c-b876-e4a98484a0ab' | cut -d' ' -f5 | xargs 
> --no-run-if-empty umount -l || true \n"}]}" --help="false" --pipe_read="8" 
> --pipe_write="9" 
> --sandbox="/tmp/mesos/slaves/5763efb9-8e12-4bfc-b06b-2a324881608d-S0/frameworks/425afd24-796d-42dd-98c0-593f1d71e246-/executors/test/runs/a2cc264f-72eb-416c-b876-e4a98484a0ab"
>  --user="root"'
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 46165: Removed dependency on Boost.Foreach.

[Ben Mahler]

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


Ship it!




Ship It!

- Ben Mahler


On May 3, 2016, 8:53 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46165/
> ---
> 
> (Updated May 3, 2016, 8:53 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Hindman, Ben Mahler, and 
> Joris Van Remoortere.
> 
> 
> Bugs: MESOS-3214
> https://issues.apache.org/jira/browse/MESOS-3214
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed dependency on Boost.Foreach.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/foreach.hpp 
> 7fb0044790ee249b69e07b81a26851bd5bfb110f 
> 
> Diff: https://reviews.apache.org/r/46165/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 46923: Added framework failover timeout validation.

[Jose Guilherme Vanz]

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

(Updated May 4, 2016, 12:27 a.m.)


Review request for mesos, Kevin Sweeney and Vinod Kone.


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


Repository: mesos


Description
---

Frameworks were allowed to subscribe themselves with invalid failover
timeout. For this reason, a validation has been made in the master to
deny framework subscription if it set a invalid value for the failover
timeout. MESOS-1575


Diffs
-

  src/master/master.hpp 3e55114ee7866e06513071e86e15608099dae052 
  src/master/master.cpp 6d3e0f7c634690a35eec1ce827b705e04c3af87e 
  src/tests/master_tests.cpp 8e00753fcbcd0cae1d08aad387b08aa17c7f63ad 

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


Testing
---


Thanks,

Jose Guilherme Vanz

Re: Review Request 41613: Added `DEFAULT_ROLE` constant to persistent volume tests.

[Michael Park]

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


Fix it, then Ship it!





src/tests/persistent_volume_tests.cpp (line 68)


Perhaps we should consider pulling this out to `src/tests/mesos.hpp`.



src/tests/persistent_volume_tests.cpp (lines 573 - 575)


```cpp
  Resources taskResources =
  Resources::parse("cpus:1;mem:128;disk(" + DEFAULT_ROLE + 
"):32").get() +
  volume;
```



src/tests/persistent_volume_tests.cpp (lines 682 - 684)


Same as above.


- Michael Park


On Jan. 5, 2016, 2:49 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41613/
> ---
> 
> (Updated Jan. 5, 2016, 2:49 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Jie Yu, and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added `DEFAULT_ROLE` constant to persistent volume tests.
> 
> 
> Diffs
> -
> 
>   src/tests/persistent_volume_tests.cpp 
> 2fb57814b2805bc76981d1877603a1a033f29289 
> 
> Diff: https://reviews.apache.org/r/41613/diff/
> 
> 
> Testing
> ---
> 
> `GTEST_FILTER="PersistentVolumeTest*" bin/mesos-tests.sh`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 46923: Added framework failover timeout validation.

[Jose Guilherme Vanz]

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

(Updated May 4, 2016, 12:31 a.m.)


Review request for mesos, Kevin Sweeney and Vinod Kone.


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


Repository: mesos


Description
---

Frameworks were allowed to subscribe themselves with invalid failover
timeout. For this reason, a validation has been made in the master to
deny framework subscription if it set a invalid value for the failover
timeout. MESOS-1575


Diffs (updated)
-

  src/master/master.hpp 3e55114ee7866e06513071e86e15608099dae052 
  src/master/master.cpp 6d3e0f7c634690a35eec1ce827b705e04c3af87e 
  src/tests/master_tests.cpp 8e00753fcbcd0cae1d08aad387b08aa17c7f63ad 

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


Testing
---


Thanks,

Jose Guilherme Vanz

Re: Review Request 46165: Removed dependency on Boost.Foreach.

[Michael Park]

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

(Updated May 4, 2016, 12:31 a.m.)


Review request for mesos, Benjamin Bannier, Benjamin Hindman, Ben Mahler, and 
Joris Van Remoortere.


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


Repository: mesos


Description
---

Removed dependency on Boost.Foreach.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/stout/foreach.hpp 
7fb0044790ee249b69e07b81a26851bd5bfb110f 

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


Testing
---

`make check`


Thanks,

Michael Park

Re: Review Request 46945: Renamed a shadowing variable.

[Michael Park]

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

(Updated May 3, 2016, 10:54 p.m.)


Review request for mesos, Benjamin Bannier, Benjamin Hindman, Ben Mahler, and 
Joris Van Remoortere.


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


Repository: mesos


Description (updated)
---

The `value` variable declared in the `foreach` used to shadow the `value`
function parameter. This seemed to have been ok with the `FOREACH` macro,
butthis is no longer ok with `range-based for`.
This seems to be the only place of violation of this.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/include/stout/json.hpp 
d1f4ae6a1d1e6ccfe55f9f8f78390826dc97d894 

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


Testing
---

`make check` on Ubuntu 14.04 with GCC 4.8


Thanks,

Michael Park

Re: Review Request 46945: Renamed a shadowing variable.

[Ben Mahler]

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


Fix it, then Ship it!





3rdparty/libprocess/3rdparty/stout/include/stout/json.hpp (lines 749 - 751)


Could we avoid the 'elem' name in favor of 'element' or 'v' (full word or 
single letter)?

Per our conversation, do we want to adjust the foreachpair above?


- Ben Mahler


On May 3, 2016, 10:54 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46945/
> ---
> 
> (Updated May 3, 2016, 10:54 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Hindman, Ben Mahler, and 
> Joris Van Remoortere.
> 
> 
> Bugs: MESOS-3214
> https://issues.apache.org/jira/browse/MESOS-3214
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The `value` variable declared in the `foreach` used to shadow the `value`
> function parameter. This seemed to have been ok with the `FOREACH` macro,
> butthis is no longer ok with `range-based for`.
> This seems to be the only place of violation of this.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/json.hpp 
> d1f4ae6a1d1e6ccfe55f9f8f78390826dc97d894 
> 
> Diff: https://reviews.apache.org/r/46945/diff/
> 
> 
> Testing
> ---
> 
> `make check` on Ubuntu 14.04 with GCC 4.8
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 46945: Renamed a shadowing variable.

[Michael Park]

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

(Updated May 4, 2016, 12:30 a.m.)


Review request for mesos, Benjamin Bannier, Benjamin Hindman, Ben Mahler, and 
Joris Van Remoortere.


Changes
---

`s/elem/v/`


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


Repository: mesos


Description (updated)
---

Renamed a shadowing variable.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/stout/json.hpp 
d1f4ae6a1d1e6ccfe55f9f8f78390826dc97d894 

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


Testing
---

`make check` on Ubuntu 14.04 with GCC 4.8


Thanks,

Michael Park

Re: Review Request 46165: Removed dependency on Boost.Foreach.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [46945, 46165]

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

- Mesos ReviewBot


On May 4, 2016, 12:31 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46165/
> ---
> 
> (Updated May 4, 2016, 12:31 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Hindman, Ben Mahler, and 
> Joris Van Remoortere.
> 
> 
> Bugs: MESOS-3214
> https://issues.apache.org/jira/browse/MESOS-3214
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed dependency on Boost.Foreach.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/foreach.hpp 
> 7fb0044790ee249b69e07b81a26851bd5bfb110f 
> 
> Diff: https://reviews.apache.org/r/46165/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 46867: Enabled authorization of libprocess HTTP endpoints (Mesos).

[Kapil Arya]

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




src/common/http.cpp (lines 587 - 588)


Fix continuation.



src/common/http.cpp (lines 590 - 592)


Coalesce?


- Kapil Arya


On May 1, 2016, 6:43 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46867/
> ---
> 
> (Updated May 1, 2016, 6:43 p.m.)
> 
> 
> Review request for mesos, Alexander Rojas and Kapil Arya.
> 
> 
> Bugs: MESOS-5286
> https://issues.apache.org/jira/browse/MESOS-5286
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Code is added to the common headers and the
> master/agent executables which sets authorization
> callbacks for libprocess-level HTTP endpoints.
> This allows these endpoints to be authorized.
> 
> 
> Diffs
> -
> 
>   src/common/http.hpp 6c6f2840bbf49d198b1db876cdf4af5ef49b0e27 
>   src/common/http.cpp ccf386898130c966903cb5aae4eaffbc9b63ca1f 
>   src/master/main.cpp 23149d5511d1556f1a885d01ea9380a9669fa8c5 
>   src/slave/main.cpp fee46bafc88f8cdade868aab8c0fee79b8d2fb6d 
> 
> Diff: https://reviews.apache.org/r/46867/diff/
> 
> 
> Testing
> ---
> 
> `make check` on OSX.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 46876: Fixed tests to work with authorized '/metrics/snapshot'.

[Kapil Arya]

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




src/tests/scheduler_driver_tests.cpp (line 99)


Why this change?


- Kapil Arya


On May 1, 2016, 6:30 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46876/
> ---
> 
> (Updated May 1, 2016, 6:30 p.m.)
> 
> 
> Review request for mesos, Alexander Rojas and Kapil Arya.
> 
> 
> Bugs: MESOS-5286
> https://issues.apache.org/jira/browse/MESOS-5286
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Some tests hit the '/metrics/snapshot' endpoint and
> must be updated to accomodate authorization. In
> particular, an authorizer had to be explicitly
> instantiated and used to initialize the master/agent
> in some tests so that they share the same authorizer.
> 
> 
> Diffs
> -
> 
>   src/tests/master_authorization_tests.cpp 
> 804b39a269c09df9f6c0bbdf6f8b53921ac09ce8 
>   src/tests/mesos.hpp 0f6f541c5d2007a69ad5bd6e884235cd3c0c1be2 
>   src/tests/mesos.cpp 036c589f5aafc8c804b0fb4e5ad62df70e471e88 
>   src/tests/partition_tests.cpp 4ee7f03c7db4fb6cf5b9130de4e03d24100161d1 
>   src/tests/scheduler_driver_tests.cpp 
> 217185780e3576faf633dd9629ae93a275fac284 
>   src/tests/slave_tests.cpp 3f653354869987dce3f5fbc4513b6f3466a718cb 
> 
> Diff: https://reviews.apache.org/r/46876/diff/
> 
> 
> Testing
> ---
> 
> `make check` on OSX.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 46954: Ensuring that id attributes are unique on a Web page.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [46954]

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

- Mesos ReviewBot


On May 4, 2016, 1:48 a.m., Chen Nan Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46954/
> ---
> 
> (Updated May 4, 2016, 1:48 a.m.)
> 
> 
> Review request for mesos and Zhiwei Chen.
> 
> 
> Bugs: MESOS-5201
> https://issues.apache.org/jira/browse/MESOS-5201
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Ensuring that id attributes are unique on a Web page.
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/directives/tableHeader.html 
> 448f67ef20c279fdd542890158fb9f96a10ebebc 
> 
> Diff: https://reviews.apache.org/r/46954/diff/
> 
> 
> Testing
> ---
> 
> Have checked with the patch by firebug, the ID of each table header is 
> different with each other.
> 
> 
> Thanks,
> 
> Chen Nan Li
> 
>

Re: Review Request 46958: Added Ubuntu 16.04 LTS to getting started document.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [46958]

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

- Mesos ReviewBot


On May 4, 2016, 2:50 a.m., Zhiwei Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46958/
> ---
> 
> (Updated May 4, 2016, 2:50 a.m.)
> 
> 
> Review request for mesos, Adam B, haosdent huang, Kapil Arya, and Joseph Wu.
> 
> 
> Bugs: MESOS-5321
> https://issues.apache.org/jira/browse/MESOS-5321
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added Ubuntu 16.04 LTS to getting started document.
> 
> 
> Diffs
> -
> 
>   docs/getting-started.md e6be2818f6e1fa9bfbea5dc977ca0aa978532e8e 
> 
> Diff: https://reviews.apache.org/r/46958/diff/
> 
> 
> Testing
> ---
> 
> ./configure --enable-ssl --enable-libevent && make
> 
> 
> Thanks,
> 
> Zhiwei Chen
> 
>

Review Request 46954: Ensuring that id attributes are unique on a Web page.

[Chen Nan Li]

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

Review request for mesos and Zhiwei Chen.


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


Repository: mesos


Description
---

Ensuring that id attributes are unique on a Web page.


Diffs
-

  src/webui/master/static/directives/tableHeader.html 
448f67ef20c279fdd542890158fb9f96a10ebebc 

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


Testing
---

Have checked with the patch by firebug, the ID of each table header is 
different with each other.


Thanks,

Chen Nan Li

Re: Review Request 46816: Fix the absolute symlink path issue of include/slave.

[Zhiwei Chen]

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

(Updated May 4, 2016, 10:42 a.m.)


Review request for mesos, zhou xing, James Peach, Kapil Arya, and Vinod Kone.


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


Repository: mesos


Description
---

Fix the absolute symlink path issue of include/slave.


Diffs
-

  src/Makefile.am 53de98f43629dc94f7619324369caf88407b2f41 

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


Testing
---

Fixes the slave linked to $RPM_BUILD_ROOT/$PREFIX/include/mesos/agent issue, 
should use relative path.

I am not sure if this copy-template-and-create-symlink should be removed of 
not, since MESOS-3782 & MESOS-5230 already completed. This blocked our daily 
build.


Thanks,

Zhiwei Chen

Re: Review Request 46883: Added authorization callback for '/metrics/snapshot'.

[Kapil Arya]

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




src/common/http.cpp (line 599)


Does non-existing `subject` mean "ANY"?



src/common/http.cpp (line 602)


const?


- Kapil Arya


On May 1, 2016, 8:03 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46883/
> ---
> 
> (Updated May 1, 2016, 8:03 p.m.)
> 
> 
> Review request for mesos, Alexander Rojas and Kapil Arya.
> 
> 
> Bugs: MESOS-5286
> https://issues.apache.org/jira/browse/MESOS-5286
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds a callback to the generating
> function `CreateAuthorizationCallbacks` for the
> '/metrics/snapshot' endpoint.
> 
> 
> Diffs
> -
> 
>   src/common/http.cpp ccf386898130c966903cb5aae4eaffbc9b63ca1f 
> 
> Diff: https://reviews.apache.org/r/46883/diff/
> 
> 
> Testing
> ---
> 
> `make check` on OSX.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 46866: Enabled authorization of libprocess HTTP endpoints (libprocess).

[Kapil Arya]

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




3rdparty/libprocess/include/process/http.hpp (lines 89 - 91)


Is that the correct line-break here? Can we make it better?



3rdparty/libprocess/include/process/http.hpp (lines 89 - 100)


Since this is a public header, let's add comment blocks.



3rdparty/libprocess/src/process.cpp (line 484)


Having non-pod global vars isn't good! Maybe use a pointer instead?


- Kapil Arya


On May 1, 2016, 6:28 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46866/
> ---
> 
> (Updated May 1, 2016, 6:28 p.m.)
> 
> 
> Review request for mesos, Alexander Rojas and Kapil Arya.
> 
> 
> Bugs: MESOS-5286
> https://issues.apache.org/jira/browse/MESOS-5286
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch enables libprocess to store and execute
> authorization callbacks provided by a client
> application.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> 8f4eabcbb71ead1f5c28e1d8a2dd40db7af1f297 
>   3rdparty/libprocess/src/process.cpp 
> dcaa64633d1eea330649c563635642928164d73c 
> 
> Diff: https://reviews.apache.org/r/46866/diff/
> 
> 
> Testing
> ---
> 
> `make check` on OSX.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 46140: Added test "ROOT_CommandTaskNoRootfsSingleVolume".

[Guangya Liu]

> On 五月 3, 2016, 9:05 p.m., Gilbert Song wrote:
> > src/tests/containerizer/docker_volume_isolator_tests.cpp, lines 272-273
> > 
> >
> > Do we need it on this test?

I want to make sure that the option works as I can see that in the log message 
there are options for the mount operation.


- Guangya


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


On 五月 3, 2016, 7:51 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46140/
> ---
> 
> (Updated 五月 3, 2016, 7:51 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-5266
> https://issues.apache.org/jira/browse/MESOS-5266
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test "ROOT_CommandTaskNoRootfsSingleVolume".
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 53de98f43629dc94f7619324369caf88407b2f41 
>   src/slave/containerizer/mesos/isolators/docker/volume/driver.hpp 
> 070d52018e82ed3e46fb1b79714ffc4716f6a306 
>   src/tests/containerizer/docker_volume_isolator_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/46140/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> GLOG_v=1 ./bin/mesos-tests.sh  
> --gtest_filter="DockerVolumeIsolatorTest.ROOT_CommandTaskNoRootfsSingleVolume"
>  --verbose
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 46730: Cleanup syscalls logic.

[Zhiwei Chen]

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


Ship it!




Ship It!

- Zhiwei Chen


On April 29, 2016, 9:03 p.m., Tomasz Janiszewski wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46730/
> ---
> 
> (Updated April 29, 2016, 9:03 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Zhiwei Chen, and haosdent huang.
> 
> 
> Bugs: MESOS-5263
> https://issues.apache.org/jira/browse/MESOS-5263
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Used `` and removed uncessary condition complie.
> 
> 
> Diffs
> -
> 
>   src/linux/fs.cpp 81f040e5c7ed7cbca569f5d43cb5afc5da1b5e64 
> 
> Diff: https://reviews.apache.org/r/46730/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Tomasz Janiszewski
> 
>

Re: Review Request 46923: Added framework failover timeout validation.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [46923]

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

- Mesos ReviewBot


On May 4, 2016, 12:43 a.m., Jose Guilherme Vanz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46923/
> ---
> 
> (Updated May 4, 2016, 12:43 a.m.)
> 
> 
> Review request for mesos, Guangya Liu, Kevin Sweeney, Neil Conway, and Vinod 
> Kone.
> 
> 
> Bugs: MESOS-1575
> https://issues.apache.org/jira/browse/MESOS-1575
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Frameworks were allowed to subscribe themselves with invalid failover
> timeout. For this reason, a validation has been made in the master to
> deny framework subscription if it set a invalid value for the failover
> timeout. MESOS-1575
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp 3e55114ee7866e06513071e86e15608099dae052 
>   src/master/master.cpp 6d3e0f7c634690a35eec1ce827b705e04c3af87e 
>   src/tests/master_tests.cpp 8e00753fcbcd0cae1d08aad387b08aa17c7f63ad 
> 
> Diff: https://reviews.apache.org/r/46923/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jose Guilherme Vanz
> 
>

Review Request 46958: Added Ubuntu 16.04 LTS to getting started document.

[Zhiwei Chen]

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

Review request for mesos, Adam B, haosdent huang, Kapil Arya, and Joseph Wu.


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


Repository: mesos


Description
---

Added Ubuntu 16.04 LTS to getting started document.


Diffs
-

  docs/getting-started.md e6be2818f6e1fa9bfbea5dc977ca0aa978532e8e 

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


Testing
---

./configure --enable-ssl --enable-libevent && make


Thanks,

Zhiwei Chen

Review Request 46960: Remove un-necessary copying of `slave->tasks` in master.

[Anand Mazumdar]

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

Review request for mesos and Ben Mahler.


Repository: mesos


Description
---

It seems that we removed an else block in 0.26 that removed the
`removeTask` call from the `foreachvalue` loop but we still kept
on copying `slave->tasks` though it is not needed anymore.


Diffs
-

  src/master/master.cpp 6d3e0f7c634690a35eec1ce827b705e04c3af87e 

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


Testing
---

make check


Thanks,

Anand Mazumdar

Re: Review Request 46936: Documented the agent endpoint '/flags'.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [46935, 46936]

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

- Mesos ReviewBot


On May 3, 2016, 2:36 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46936/
> ---
> 
> (Updated May 3, 2016, 2:36 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Alexander Rojas, and Neil 
> Conway.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   docs/endpoints/slave/flags.md b2740e6a4ce4bb8c25de07071bafbf174adf9137 
>   src/slave/http.cpp 9b558862e025c5caa71e05fc5eeba783c0ad6fd5 
> 
> Diff: https://reviews.apache.org/r/46936/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 46784: Added authorization of the '/flags' endpoint.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [46784]

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

- Mesos ReviewBot


On May 3, 2016, 12:42 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46784/
> ---
> 
> (Updated May 3, 2016, 12:42 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Benjamin Bannier.
> 
> 
> Bugs: MESOS-5297
> https://issues.apache.org/jira/browse/MESOS-5297
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Access to the '/flags' endpoint is authorized using
> the 'GET_ENDPOINT_WITH_PATH' action.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 4985f24b70a00116caa4bd0335ea51e55d878d19 
>   src/master/master.hpp 3e55114ee7866e06513071e86e15608099dae052 
>   src/tests/master_authorization_tests.cpp 
> 804b39a269c09df9f6c0bbdf6f8b53921ac09ce8 
> 
> Diff: https://reviews.apache.org/r/46784/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Review Request 46935: Documented the agent endpoint '/metrics/snapshot'.

[Benjamin Bannier]

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

Review request for mesos, Alexander Rukletsov, Alexander Rojas, and Neil Conway.


Repository: mesos


Description
---

Documented the agent endpoint '/metrics/snapshot'.


Diffs
-

  src/slave/http.cpp 9b558862e025c5caa71e05fc5eeba783c0ad6fd5 

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


Testing
---

make check (OS X, clang-trunk w/o optimizations)

I also ran `generate-endpoints` and inspected the generated documentation.


Thanks,

Benjamin Bannier

Re: Review Request 46107: Add Appc runtime spec for command, working directory and environment.

[Jojy Varghese]

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




include/mesos/appc/spec.proto (line 71)


I think the message ids should ordered by their order in the spec (see 
`https://github.com/appc/spec/blob/master/spec/aci.md`). So `app` should be 
`5`. But we have `annotation` labeled 5 unfortunately. 

In `App`, lets try to follow the order specified in the spec. So IMO, 
`workingDirectory` should be atleast `6`.



include/mesos/slave/isolator.proto (line 22)


Please sort the include files in alphabetical order. So it would be:
```
import "mesos/appc/spec.proto";

import "mesos/docker/v1.proto";
```



include/mesos/slave/isolator.proto (line 94)


I would place this message above the `Docker` messgae.


- Jojy Varghese


On April 12, 2016, 6:32 p.m., Srinivas Brahmaroutu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46107/
> ---
> 
> (Updated April 12, 2016, 6:32 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add Appc runtime spec for command, working directory and environment.
> 
> 
> Diffs
> -
> 
>   include/mesos/appc/spec.proto 4ef0473748c2f47aff1729c1d969185803fe72d4 
>   include/mesos/slave/isolator.proto 60a9bb637e12593a97ed1a7c510ebccd4e5a9615 
> 
> Diff: https://reviews.apache.org/r/46107/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Srinivas Brahmaroutu
> 
>

Re: Review Request 46934: Fixed typo in authentication help.

[Benjamin Bannier]

> On May 3, 2016, 6:56 p.m., Neil Conway wrote:
> > This isn't a typo; the intended meaning is "if and only if" 
> > (https://en.wikipedia.org/wiki/If_and_only_if). Admittedly that is a little 
> > too clever; I'd be fine with changing it, either to "if" or to "if and only 
> > if". Note we should also rerun the `generate-endpoint-help.py` script.

Oh I see. Yes, this was indeed too clever for me (ESL and never read Freege in 
English). I see we use `iff` in similar contexts in other places in the code 
base.

Looking at the link you gave this appears to be pretty specialized jargon, and 
I feel it might be a good idea to expand this to e.g., `if and only if`, if 
*not only* for the reason that would use a form clearly phonetically different.

Dropping this for now.


- Benjamin


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


On May 3, 2016, 4:17 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46934/
> ---
> 
> (Updated May 3, 2016, 4:17 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Neil Conway.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed typo in authentication help.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/help.hpp 
> 3393b71c484bb8c55c6a4cd2a27dcb798e841a80 
> 
> Diff: https://reviews.apache.org/r/46934/diff/
> 
> 
> Testing
> ---
> 
> make check (OS X, clang-trunk w/o optimizations)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 46934: Fixed typo in authentication help.

[Neil Conway]

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



This isn't a typo; the intended meaning is "if and only if" 
(https://en.wikipedia.org/wiki/If_and_only_if). Admittedly that is a little too 
clever; I'd be fine with changing it, either to "if" or to "if and only if". 
Note we should also rerun the `generate-endpoint-help.py` script.

- Neil Conway


On May 3, 2016, 2:17 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46934/
> ---
> 
> (Updated May 3, 2016, 2:17 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Neil Conway.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed typo in authentication help.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/help.hpp 
> 3393b71c484bb8c55c6a4cd2a27dcb798e841a80 
> 
> Diff: https://reviews.apache.org/r/46934/diff/
> 
> 
> Testing
> ---
> 
> make check (OS X, clang-trunk w/o optimizations)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>