Re: Review Request 47805: Add authorization to GET /weights.

[Adam B]

> On May 27, 2016, 2:39 a.m., Adam B wrote:
> > src/tests/dynamic_weights_tests.cpp, line 88
> > 
> >
> > Neat trick, but I think we're better off just specifying 
> > DEFAULT_CREDENTIAL at those call sites
> 
> zhou xing wrote:
> Adam, not very clear on your suggestions, do you mean that we just put 
> credential = DEFAULT_CREDENTIAL here? but as a macro, it is not allowed to 
> put an expression in this file scope, thanks for your time on looking this.
> 
> zhou xing wrote:
> or do you mean change all the caller side to add the credential param and 
> remove the param default value here?

The latter suggestion exactly. Remove the parameter's default value `= []{ 
return DEFAULT_CREDENTIAL; }()` altogether, and whatever callsites previously 
relied on the default param, you can just pass `DEFAULT_CREDENTIAL` instead.


> On May 27, 2016, 2:39 a.m., Adam B wrote:
> > src/master/weights_handler.cpp, line 85
> > 
> >
> > Technically, this lambda should be indented to align with its fellow 
> > defer() parameter `master->self()`, or (since that looks too jagged) just 
> > indented 4 spaces in from the `.then`.
> > Looks like our convention for the `-> Future` after is 2 spaces in from 
> > the `[=]` like you have.
> 
> zhou xing wrote:
> Thanks for the comments! The indention is a little confusion to me, as 
> somewhere in the code is 4 spaces indention while other places are 2 spaces, 
> do we have any regulation on the indention?

http://mesos.apache.org/documentation/latest/c++-style-guide/ lists our 
exceptions to the Google style guide at 
https://google.github.io/styleguide/cppguide.html (I just noticed the google 
style link on our site is broken; fixing now).
General rules I follow:
Use 2 spaces when
- Beginning the inside of a function/loop/condition or other thing scoped with 
`{`
- Continuing a line of code after wrapping on `=`
Wrap to the same indentation as the previous parameter
- When doing so won't make the code look "jagged"
Use 4 spaces when
- Continuing a line of code otherwise
When in doubt, grep away. When there are conflicts, count (perhaps scoped to 
the same file/folder) to see who wins. 
If you're still not sure, git blame to see which committer's opinion you trust 
more. :)
In this case, the decision wasn't clear to me, so I grepped and blamed.
I'm not personally style-opinionated, but I do like consistency.


- Adam


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


On May 26, 2016, 10:45 p.m., zhou xing wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47805/
> ---
> 
> (Updated May 26, 2016, 10:45 p.m.)
> 
> 
> Review request for mesos and Adam B.
> 
> 
> Bugs: mesos-5335
> https://issues.apache.org/jira/browse/mesos-5335
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add 'GET_WEIGHTS_WITH_ROLE' for the authorization of GET /weights.
> 
> 
> Diffs
> -
> 
>   docs/endpoints/master/weights.md 1e540795c5fb90c8ea957dd444b9e564e0b1ac23 
>   include/mesos/authorizer/acls.proto 
> ace9b698f46e143795c82324a87a0d7827fb 
>   include/mesos/authorizer/authorizer.proto 
> 02d1a01d57cf34b38524f4368187878b03343537 
>   src/authorizer/local/authorizer.cpp 
> 3c7c791bde65cfcbcc4e319c9ccc487ab37d8029 
>   src/master/http.cpp b36b439a1fa07c52146deff2b90728f92676ade3 
>   src/master/master.hpp 1a875c32eddfb6d884e3d0dda7f5716ee53966c3 
>   src/master/weights_handler.cpp 4bc060fdb015df6658194eef92fe11b14aa15c79 
>   src/tests/dynamic_weights_tests.cpp 
> 362c59aae7b305710d5985bfec28f881be3b64b8 
> 
> Diff: https://reviews.apache.org/r/47805/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> zhou xing
> 
>

Re: Review Request 47577: Agent: Added minor changes to various .cpp files to support Windows.

[Alex Clemmer]

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

(Updated May 28, 2016, 5:11 a.m.)


Review request for mesos, Daniel Pravat, Artem Harutyunyan, Joris Van 
Remoortere, and Michael Park.


Repository: mesos


Description
---

Agent: Added minor changes to various .cpp files to support Windows.


Diffs (updated)
-

  src/docker/docker.cpp 19cf424dfd5748ae66a7023840aa2b0652e8f2c0 
  src/docker/executor.cpp f4796e4aa9951c30ada7feb44fb5d037895becce 
  src/files/files.cpp 074d31c116c1b9ff96262c3e0c776f38aea53d16 
  src/health-check/main.cpp 3cf858303ee84f43aedfe5ac71949b786276e463 
  src/logging/logging.cpp 20d2f6341bd39fc5d056f1046d258d006fc602e4 
  src/uri/fetcher.hpp 8af2c9122e0b15fd54f7d3a84779540e7186f566 
  src/uri/fetcher.cpp aa9df5d0256a26b8684934c2bd37b82a069088f7 
  src/uri/fetchers/copy.cpp 2180adfba1f33852d11069eed9d9bca72e2e3b6f 
  src/uri/fetchers/curl.cpp c4420623d718d87776f2eb8e13faf02ef5edb335 
  src/zookeeper/group.cpp 01680899778e554af70b176db82498ca92b51b60 

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


Testing
---


Thanks,

Alex Clemmer

Re: Review Request 47843: Agent: Added Windows support for launcher/fetcher.cpp.

[Alex Clemmer]

> On May 25, 2016, 9:02 p.m., Joris Van Remoortere wrote:
> > src/launcher/fetcher.cpp, lines 76-80
> > 
> >
> > can we use the updated `paths::join` here to simplify this?
> > 
> > Can you add a JIRA to add an OS agnostic helper to quote command line 
> > arguments? This would have simplified this review a LOT :-)

The multiple paths::join is not usable here due to the the added separator.


- Alex


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


On May 28, 2016, 2:19 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47843/
> ---
> 
> (Updated May 28, 2016, 2:19 a.m.)
> 
> 
> Review request for mesos, Daniel Pravat, Artem Harutyunyan, Joris Van 
> Remoortere, and Michael Park.
> 
> 
> Bugs: MESOS-3618
> https://issues.apache.org/jira/browse/MESOS-3618
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Agent: Added Windows support for launcher/fetcher.cpp.
> 
> 
> Diffs
> -
> 
>   src/launcher/fetcher.cpp 0539b0182bd4a7178f1031ab4fee8fc79eda 
> 
> Diff: https://reviews.apache.org/r/47843/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 48004: Windows: Added `src/executor/executor.cpp`.

[Mesos ReviewBot]

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



Bad review!

Reviews applied: [48004, 48003, 48002]

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

- Mesos ReviewBot


On May 28, 2016, 3:16 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48004/
> ---
> 
> (Updated May 28, 2016, 3:16 a.m.)
> 
> 
> Review request for mesos, Daniel Pravat, Artem Harutyunyan, Joris Van 
> Remoortere, and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Added `src/executor/executor.cpp`.
> 
> 
> Diffs
> -
> 
>   src/executor/executor.cpp fd2d0cd64956b5a407e37ddaa1e9e35c1456d57b 
> 
> Diff: https://reviews.apache.org/r/48004/diff/
> 
> 
> Testing
> ---
> 
> Windows: build/run
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Review Request 48004: Windows: Added `src/executor/executor.cpp`.

[Alex Clemmer]

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

Review request for mesos, Daniel Pravat, Artem Harutyunyan, Joris Van 
Remoortere, and Michael Park.


Repository: mesos


Description
---

Windows: Added `src/executor/executor.cpp`.


Diffs
-

  src/executor/executor.cpp fd2d0cd64956b5a407e37ddaa1e9e35c1456d57b 

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


Testing
---

Windows: build/run


Thanks,

Alex Clemmer

Re: Review Request 48000: Windows MVP.

[Alex Clemmer]

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

(Updated May 28, 2016, 3:15 a.m.)


Review request for mesos.


Repository: mesos


Description
---

Windows MVP.


Diffs (updated)
-

  cmake/CompilationConfigure.cmake d3a5daee3c4c90712e920136bfe76fc9ab59ec45 
  src/CMakeLists.txt 9eabd781763adba39b60526c1b4a7d99b3f1 
  src/health-check/CMakeLists.txt PRE-CREATION 
  src/launcher/CMakeLists.txt PRE-CREATION 
  src/slave/CMakeLists.txt PRE-CREATION 
  src/slave/cmake/SlaveConfigure.cmake 187b5cb9d281ea6f3f9c7f81ff2ea9280213b146 
  src/tests/cmake/ContainerizerTestsConfigure.cmake PRE-CREATION 
  src/tests/containerizer/CMakeLists.txt PRE-CREATION 

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


Testing
---


Thanks,

Alex Clemmer

Re: Review Request 47999: Added calico information for CNI.

[Qian Zhang]

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




docs/cni.md (line 332)


s/calico/Calico


- Qian Zhang


On May 28, 2016, 9:27 a.m., Dan Osborne wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47999/
> ---
> 
> (Updated May 28, 2016, 9:27 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added calico information for CNI.
> 
> 
> Diffs
> -
> 
>   docs/cni.md cdc50da9db2f19f27be6a39f4d6e0f54834fb964 
> 
> Diff: https://reviews.apache.org/r/47999/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Dan Osborne
> 
>

Re: Review Request 47973: Updated gc to prevent early exit in case of error.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [47972, 47973]

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

- Mesos ReviewBot


On May 28, 2016, 12:02 a.m., Megha Sharma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47973/
> ---
> 
> (Updated May 28, 2016, 12:02 a.m.)
> 
> 
> Review request for mesos and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-5196
> https://issues.apache.org/jira/browse/MESOS-5196
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> https://issues.apache.org/jira/browse/MESOS-5196
> Updates made to mesos gc to prevent early exit in the event
> of error.
> 
> 
> Diffs
> -
> 
>   src/slave/gc.cpp eedb8ca713d7f5195055bb6417f03ab70731af97 
>   src/tests/gc_tests.cpp 4cb7c2f612984f7f5a9378a7f972f2438bbf28c5 
> 
> Diff: https://reviews.apache.org/r/47973/diff/
> 
> 
> Testing
> ---
> 
> make check.
> Tested with option --gtest_also_run_disabled_tests.
> 
> 
> Thanks,
> 
> Megha Sharma
> 
>

Re: Review Request 47536: Agent: Added Windows isolators.

[Alex Clemmer]

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

(Updated May 28, 2016, 2:22 a.m.)


Review request for mesos, Daniel Pravat, Artem Harutyunyan, Joris Van 
Remoortere, and Michael Park.


Changes
---

master rebased


Bugs: MESOS-3619, MESOS-3620 and MESOS-3683
https://issues.apache.org/jira/browse/MESOS-3619
https://issues.apache.org/jira/browse/MESOS-3620
https://issues.apache.org/jira/browse/MESOS-3683


Repository: mesos


Description
---

Agent: Added Windows isolators.


Diffs (updated)
-

  src/CMakeLists.txt 9eabd781763adba39b60526c1b4a7d99b3f1 
  src/Makefile.am f1bd5e6e5515535c2aaf9ea8b205f4e8c7096ec5 
  src/slave/containerizer/mesos/isolators/filesystem/posix.hpp 
01ab1793d3d5469e6590617326b5e77e56bc7c50 
  src/slave/containerizer/mesos/isolators/filesystem/posix.cpp 
cb42d83a779c63c9ae25bbe72292aba3fe4cb8c9 
  src/slave/containerizer/mesos/isolators/filesystem/windows.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/filesystem/windows.cpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/posix.hpp 
227505a70a440f15e68ac001878bcf25610db45f 
  src/slave/containerizer/mesos/isolators/windows.hpp PRE-CREATION 
  src/usage/main.cpp ab3bb233caf6ba02af1b306dcf31d2b9ba6e322a 
  src/usage/usage.hpp 2e1996a78c617c2559ef882ebede5c8aab6f899f 
  src/usage/usage.cpp 3b19682e67372b81484eacddbab78c2e5eda3c5b 

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


Testing
---


Thanks,

Alex Clemmer

Re: Review Request 47472: Windows: Added support for `fetcher.cpp`.

[Alex Clemmer]

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

(Updated May 28, 2016, 2:22 a.m.)


Review request for mesos, Daniel Pravat, Artem Harutyunyan, Joris Van 
Remoortere, and Michael Park.


Changes
---

master rebased


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


Repository: mesos


Description
---

Windows: Added support for `fetcher.cpp`.


Diffs (updated)
-

  src/slave/containerizer/fetcher.cpp 176d8863d1becd8864218a0012ab45c614f0ad77 

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


Testing
---


Thanks,

Alex Clemmer

Re: Review Request 47577: Agent: Added minor changes to various .cpp files to support Windows.

[Alex Clemmer]

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

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


Review request for mesos, Daniel Pravat, Artem Harutyunyan, Joris Van 
Remoortere, and Michael Park.


Changes
---

master rebased


Repository: mesos


Description
---

Agent: Added minor changes to various .cpp files to support Windows.


Diffs (updated)
-

  src/docker/docker.cpp 19cf424dfd5748ae66a7023840aa2b0652e8f2c0 
  src/docker/executor.cpp f4796e4aa9951c30ada7feb44fb5d037895becce 
  src/files/files.cpp 074d31c116c1b9ff96262c3e0c776f38aea53d16 
  src/health-check/main.cpp 3cf858303ee84f43aedfe5ac71949b786276e463 
  src/logging/logging.cpp 20d2f6341bd39fc5d056f1046d258d006fc602e4 
  src/uri/fetcher.hpp 8af2c9122e0b15fd54f7d3a84779540e7186f566 
  src/uri/fetcher.cpp aa9df5d0256a26b8684934c2bd37b82a069088f7 
  src/uri/fetchers/copy.cpp 2180adfba1f33852d11069eed9d9bca72e2e3b6f 
  src/uri/fetchers/curl.cpp c4420623d718d87776f2eb8e13faf02ef5edb335 
  src/zookeeper/group.cpp 01680899778e554af70b176db82498ca92b51b60 

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


Testing
---


Thanks,

Alex Clemmer

Re: Review Request 47602: Stout:[1/2] Added Windows support for folder `launcher/`.

[Alex Clemmer]

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

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


Review request for mesos, Daniel Pravat, Artem Harutyunyan, Joris Van 
Remoortere, and Michael Park.


Changes
---

master rebased


Repository: mesos


Description
---

Stout:[1/2] Added Windows support for folder `launcher/`.


Diffs (updated)
-

  3rdparty/stout/include/stout/windows.hpp 
a71e2f4965c982331c2e8fe4be70027b373a6516 

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


Testing
---


Thanks,

Alex Clemmer

Re: Review Request 47603: Agent:[2/2] Added Windows support for folder `launcher/`.

[Alex Clemmer]

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

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


Review request for mesos, Daniel Pravat, Artem Harutyunyan, Joris Van 
Remoortere, and Michael Park.


Changes
---

master rebased


Repository: mesos


Description
---

Agent:[2/2] Added Windows support for folder `launcher/`.


Diffs (updated)
-

  src/Makefile.am f1bd5e6e5515535c2aaf9ea8b205f4e8c7096ec5 
  src/exec/exec.cpp 69a1fb256fe3005e814833ecac5a5a642b585510 
  src/launcher/executor.hpp PRE-CREATION 
  src/launcher/executor.cpp e4c3b75b647b54ffecedfdfa34fb3925f686c0c7 
  src/launcher/posix/executor.hpp PRE-CREATION 
  src/launcher/windows/executor.hpp PRE-CREATION 

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


Testing
---


Thanks,

Alex Clemmer

Re: Review Request 47842: Point slave flags at programmatic temp path.

[Alex Clemmer]

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

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


Review request for mesos, Daniel Pravat, Artem Harutyunyan, Joris Van 
Remoortere, and Michael Park.


Changes
---

master rebased


Repository: mesos


Description
---

Point slave flags at programmatic temp path.


Diffs (updated)
-

  src/slave/flags.cpp d30f39c216860c23e24d2d4064470c147c2824d2 

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


Testing
---


Thanks,

Alex Clemmer

Re: Review Request 47843: Agent: Added Windows support for launcher/fetcher.cpp.

[Alex Clemmer]

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

(Updated May 28, 2016, 2:19 a.m.)


Review request for mesos, Daniel Pravat, Artem Harutyunyan, Joris Van 
Remoortere, and Michael Park.


Changes
---

master rebased


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


Repository: mesos


Description
---

Agent: Added Windows support for launcher/fetcher.cpp.


Diffs (updated)
-

  src/launcher/fetcher.cpp 0539b0182bd4a7178f1031ab4fee8fc79eda 

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


Testing
---


Thanks,

Alex Clemmer

Re: Review Request 47943: Stout: Implemented `shell.hpp` on Windows.

[Alex Clemmer]

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

(Updated May 28, 2016, 2:19 a.m.)


Review request for mesos, Daniel Pravat, Artem Harutyunyan, Joris Van 
Remoortere, and Michael Park.


Changes
---

master rebased


Repository: mesos


Description
---

Stout: Implemented `shell.hpp` on Windows.


Diffs (updated)
-

  3rdparty/stout/include/stout/os/windows/shell.hpp 
ea6ae5ab981f422993085e63543d184a8e41524d 

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


Testing
---

Windows: buil/run


Thanks,

Alex Clemmer

Review Request 48002: Windows: Added new Windows.hpp defines.

[Alex Clemmer]

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

Review request for mesos, Daniel Pravat, Artem Harutyunyan, Joris Van 
Remoortere, and Michael Park.


Repository: mesos


Description
---

Windows: Added new Windows.hpp defines.


Diffs
-

  3rdparty/stout/include/stout/windows.hpp 
a71e2f4965c982331c2e8fe4be70027b373a6516 

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


Testing
---


Thanks,

Alex Clemmer

Review Request 48003: Remove SASL dependency from agent tests.

[Alex Clemmer]

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

Review request for mesos.


Repository: mesos


Description
---

Remove SASL dependency from agent tests.


Diffs
-

  configure.ac 875c7d1b13b3739a7abb55f0d16dbcfd17f3defc 
  src/Makefile.am f1bd5e6e5515535c2aaf9ea8b205f4e8c7096ec5 
  src/tests/fault_tolerance_tests.cpp c39a0ccf8fa124b2c230864f3ba1f7e8e05b36dc 
  src/tests/mesos.cpp 629135f0dc59346f0fcddb2cbe65ca5770fad34e 
  src/tests/module.cpp 5b83fd6358ddea4c9d849b8992e1a6040ef74505 
  src/tests/script.cpp ec5f2c3016a6081031c1bd6c9bf35a2d6d953700 

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


Testing
---

osx: make check


Thanks,

Alex Clemmer

Re: Review Request 47576: Agent: Add Windows support to the containerizer.

[Alex Clemmer]

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

(Updated May 28, 2016, 2:17 a.m.)


Review request for mesos, Daniel Pravat, Artem Harutyunyan, Joris Van 
Remoortere, and Michael Park.


Changes
---

master rebased


Bugs: MESOS-3617, MESOS-3618, MESOS-3619, MESOS-3622, MESOS-3623, MESOS-3624, 
MESOS-3681, MESOS-3682, and MESOS-3684
https://issues.apache.org/jira/browse/MESOS-3617
https://issues.apache.org/jira/browse/MESOS-3618
https://issues.apache.org/jira/browse/MESOS-3619
https://issues.apache.org/jira/browse/MESOS-3622
https://issues.apache.org/jira/browse/MESOS-3623
https://issues.apache.org/jira/browse/MESOS-3624
https://issues.apache.org/jira/browse/MESOS-3681
https://issues.apache.org/jira/browse/MESOS-3682
https://issues.apache.org/jira/browse/MESOS-3684


Repository: mesos


Description
---

Agent: Add Windows support to the containerizer.


Diffs (updated)
-

  src/slave/containerizer/docker.cpp 52caf9fe34b1a78efbec2a82d03c04a066690fad 
  src/slave/containerizer/external_containerizer.cpp 
cf4384cce44172a028c890f52f71ceb8ae109383 
  src/slave/containerizer/fetcher.cpp 176d8863d1becd8864218a0012ab45c614f0ad77 
  src/slave/containerizer/mesos/containerizer.cpp 
b154587628a5bf4b1366dbd7a281177e6aa6eb57 
  src/slave/containerizer/mesos/launcher.hpp 
5977c30c0aacc569019f7b34bb0c6577823ec887 
  src/slave/containerizer/mesos/launcher.cpp 
a5c8c31b72773d0bd10b9d02675a01f1d641d41c 

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


Testing
---


Thanks,

Alex Clemmer

Re: Review Request 47469: Agent: Added `launch.cpp` to Windows build.

[Alex Clemmer]

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

(Updated May 28, 2016, 2:16 a.m.)


Review request for mesos, Daniel Pravat, Artem Harutyunyan, Joris Van 
Remoortere, and Michael Park.


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


Repository: mesos


Description
---

Agent: Added `launch.cpp` to Windows build.


Diffs (updated)
-

  src/CMakeLists.txt 9eabd781763adba39b60526c1b4a7d99b3f1 
  src/slave/containerizer/mesos/launch.cpp 
e22106b014c871e2184a15c2ab154a0674874e47 

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


Testing (updated)
---

OSX: make check
Windows: Build/run


Thanks,

Alex Clemmer

Re: Review Request 47470: Stout: Added `os::temp`.

[Alex Clemmer]

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

(Updated May 28, 2016, 2:15 a.m.)


Review request for mesos, Daniel Pravat, Artem Harutyunyan, Joris Van 
Remoortere, and Michael Park.


Changes
---

master rebased


Repository: mesos


Description (updated)
---

Stout: Added `os::temp`.


Diffs (updated)
-

  3rdparty/stout/include/stout/posix/os.hpp 
f08604cdd331f0f4394f64ec06a8461f1df6c888 
  3rdparty/stout/include/stout/windows/os.hpp 
1363be1d4010028d4fc50242c80d91c0dd53e14c 

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


Testing (updated)
---

osx: make check


Thanks,

Alex Clemmer

Re: Review Request 47891: Added RUN_TASK authorization action.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [47891]

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

- Mesos ReviewBot


On May 27, 2016, 9:51 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47891/
> ---
> 
> (Updated May 27, 2016, 9:51 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, Joerg Schad, and Michael 
> Park.
> 
> 
> Bugs: MESOS-5459
> https://issues.apache.org/jira/browse/MESOS-5459
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Authorization requests for RUN_TASK actions can pass `SOME`
> authorization object either in a `FrameworkInfo` holding a user, or a
> `TaskInfo` with optionally a `CommandInfo` which can optionally hold a
> user. If either of these fields is set it will be used as the object;
> otherwise an `ANY` type authorization object will be created.
> 
> `RUN_TASK` aliases `RUN_TASK_WITH_USER` which becomes deprecated with
> 0.29.
> 
> 
> Diffs
> -
> 
>   include/mesos/authorizer/authorizer.proto 
> 3ff67858a99915e0215f3ffb9966f9ac4a3fba8c 
>   src/authorizer/local/authorizer.cpp 
> 7ddb323df09a9b0ea46c6f9543c4af059d184308 
>   src/master/master.cpp 6442762c9fdfa368d5d9d7cd43b97f5addaf7f17 
>   src/tests/authorization_tests.cpp 54bfb46a807677f4a4a2bb88dcb78a358cf5121a 
> 
> Diff: https://reviews.apache.org/r/47891/diff/
> 
> 
> Testing
> ---
> 
> Tested on a range of Linux configurations on internal CI.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 47999: Added calico information for CNI.

[Dan Osborne]

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

(Updated May 28, 2016, 1:27 a.m.)


Review request for mesos.


Repository: mesos


Description
---

Added calico information for CNI.


Diffs (updated)
-

  docs/cni.md cdc50da9db2f19f27be6a39f4d6e0f54834fb964 

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


Testing
---


Thanks,

Dan Osborne

Re: Review Request 47999: Added calico information for CNI.

[Jie Yu]

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



Can you rebase? This patch does not look correct.

- Jie Yu


On May 28, 2016, 1:04 a.m., Dan Osborne wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47999/
> ---
> 
> (Updated May 28, 2016, 1:04 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added calico information for CNI.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/CMakeLists.txt 6312e20ec0102edbef086bf61ce4256109ae8fa0 
>   3rdparty/libprocess/configure.ac c8fcd35241c10829e7b2fa582491898589f0576f 
>   3rdparty/libprocess/include/process/future.hpp 
> 455b750430ae04d16f610c24c0ea0feb8a033708 
>   3rdparty/libprocess/src/tests/CMakeLists.txt 
> 0c659dc00fe4fde78b2b57ebcc49cb47daf162d9 
>   3rdparty/libprocess/src/tests/main.cpp 
> a03dbc4ab46747003d8b11d22f2dc136293264ec 
>   3rdparty/libprocess/src/tests/subprocess_tests.cpp 
> 3f3e21514bd5e2e388165eb64d540764097557ac 
>   3rdparty/stout/configure.ac ada1e22c72875fe9d557f07e4846128be0bcef13 
>   3rdparty/stout/include/Makefile.am 7f31582c176e653873402bd3f19b0c0195503d45 
>   3rdparty/stout/include/stout/os.hpp 
> 53b00932693fba7cf6514da6a519269a904de345 
>   3rdparty/stout/include/stout/os/access.hpp 
> d87762a97152d55c58f6e54a9560c5fa0d051473 
>   3rdparty/stout/include/stout/posix/os.hpp 
> f08604cdd331f0f4394f64ec06a8461f1df6c888 
>   3rdparty/stout/include/stout/windows/os.hpp 
> 1363be1d4010028d4fc50242c80d91c0dd53e14c 
>   CHANGELOG d9b155dbb3beffc782de58b2a9478fd1554184a6 
>   docs/cni.md cdc50da9db2f19f27be6a39f4d6e0f54834fb964 
>   docs/images/architecture-example.jpg 
> e7a010c17779dc6c5cc0e785f3b0dac22da86b76 
>   docs/images/architecture3.jpg aa82a4b42a23ab60dbea2c551df75e92fd375e24 
>   docs/images/containerizer_isolator_api.png 
> b84cd1dd082fc883b7f2b3b12458dc01e933d890 
>   docs/images/ec_orphan_seqdiag.png 06de3a732414767a722e90c031d97a74150b 
>   docs/images/ec_recover_seqdiag.png 08d4ca71a0c28bedc0436f5a01d9c506d1e32e54 
>   docs/images/fetch_components.jpg 8d1ea9d395c4a586161508a78b59899cb41ea2ae 
>   docs/images/networking-architecture.png 
> 4e767d4d8a8b23c73918765483dc1a98e3534926 
>   docs/images/oversubscription-overview.jpg 
> 92ef297392910782a825dc1b3640b91d4dbe908c 
>   docs/persistent-volume.md c70291ad2d58bacaf508dfd22d19c0f1a473d466 
>   docs/powered-by-mesos.md ec02710b2c26b3d91e3b8ae4635c1cf01c881495 
>   docs/release-guide.md dbe6d201d30081c304ea1ad4f750c04548790d6a 
>   docs/reservation.md bbdaf8e3dad605e1fff8a7090865d6a772eb6c92 
>   include/mesos/authorizer/acls.proto 
> b05586ae587edbf9330f1d916340447d157ba80e 
>   include/mesos/authorizer/authorizer.proto 
> 3ff67858a99915e0215f3ffb9966f9ac4a3fba8c 
>   include/mesos/docker/spec.hpp b37c7731590ec144df720d7b28eeed5db76f750e 
>   include/mesos/docker/v1.proto 60d91d7aef5169b84322a6e0cb12bbef4011e1b7 
>   include/mesos/mesos.proto cddfa250e3155966eee4c761e164a5749dd7fc33 
>   include/mesos/type_utils.hpp 0d4dd2f0b1f03c51868247d978a41936cf3faf63 
>   include/mesos/v1/mesos.proto ce187e71003f82852a8addadfdbc7c6f82dfaa2b 
>   site/.gitignore e180964fea36243da61f709d13054c6fff5ef70f 
>   site/Dockerfile 80e28e96be53909a8c3c1eb3c05dbbc7e698a5a5 
>   site/Gemfile 4dcd0a5ad452085e6e3bb152a6547575636cd807 
>   site/Gemfile.lock 1393246f06631155bb052669a86e431778e891bf 
>   site/README.md 91bfd5fff16c7848431bdfc9206f42d18e102ace 
>   site/Rakefile aa777bdd8e459f27ab487b5641595142f4e14c68 
>   site/build.sh 11f15e15621c4d3db1472e88911787b9b3100f97 
>   site/config.rb d43308b1082c64017980c6a37fb7c3be7b7265cf 
>   site/data/releases.yml 0a3ba6ef5afbb3e0ba8eac8260fe63c9f4b31ebb 
>   site/source/assets/css/main.css 8b897d2d91a1d2f4b3bed3cf1a5a1f97ebdc00ef 
>   site/source/blog.html.erb 203c8696581d95ba1f7f04ab3de6656012ea65e9 
>   site/source/community.html.md ea6e88534dbd86db9342f47dcc384f0a533ba39b 
>   site/source/index.html.erb 7d6357c863da18427c04309749688a6b3d20e69c 
>   site/source/layouts/community_section.erb 
> 698d19974b67ed1d1a221c941a68eb1f2c2e8233 
>   site/source/layouts/documentation.erb 
> a91f916a5fb7348b2702c272e7a2059bdf628c66 
>   site/source/layouts/layout.erb c05b1a31c57e600916a62e447bb68a478afb1967 
>   site/source/layouts/post.erb fdb566b01892d87fe6e121c958fe07b29cedf072 
>   src/authorizer/local/authorizer.cpp 
> 7ddb323df09a9b0ea46c6f9543c4af059d184308 
>   src/cli/mesos-cat 778978e4e90e5a2d700699321868ad854e02e615 
>   src/cli/mesos-ps 627795c41005003f0cb89f0ba3a888fce3a037cc 
>   src/cli/mesos-scp a71ab07083465d734613227da7508fb55a258398 
>   src/cli/mesos-tail 0d0fd6b8ebf619f6913928e69982eddf3c48c904 
>   src/common/http.hpp 

Review Request 47999: Added calico information for CNI.

[Dan Osborne]

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

Review request for mesos.


Repository: mesos


Description
---

Added calico information for CNI.


Diffs
-

  3rdparty/libprocess/CMakeLists.txt 6312e20ec0102edbef086bf61ce4256109ae8fa0 
  3rdparty/libprocess/configure.ac c8fcd35241c10829e7b2fa582491898589f0576f 
  3rdparty/libprocess/include/process/future.hpp 
455b750430ae04d16f610c24c0ea0feb8a033708 
  3rdparty/libprocess/src/tests/CMakeLists.txt 
0c659dc00fe4fde78b2b57ebcc49cb47daf162d9 
  3rdparty/libprocess/src/tests/main.cpp 
a03dbc4ab46747003d8b11d22f2dc136293264ec 
  3rdparty/libprocess/src/tests/subprocess_tests.cpp 
3f3e21514bd5e2e388165eb64d540764097557ac 
  3rdparty/stout/configure.ac ada1e22c72875fe9d557f07e4846128be0bcef13 
  3rdparty/stout/include/Makefile.am 7f31582c176e653873402bd3f19b0c0195503d45 
  3rdparty/stout/include/stout/os.hpp 53b00932693fba7cf6514da6a519269a904de345 
  3rdparty/stout/include/stout/os/access.hpp 
d87762a97152d55c58f6e54a9560c5fa0d051473 
  3rdparty/stout/include/stout/posix/os.hpp 
f08604cdd331f0f4394f64ec06a8461f1df6c888 
  3rdparty/stout/include/stout/windows/os.hpp 
1363be1d4010028d4fc50242c80d91c0dd53e14c 
  CHANGELOG d9b155dbb3beffc782de58b2a9478fd1554184a6 
  docs/cni.md cdc50da9db2f19f27be6a39f4d6e0f54834fb964 
  docs/images/architecture-example.jpg e7a010c17779dc6c5cc0e785f3b0dac22da86b76 
  docs/images/architecture3.jpg aa82a4b42a23ab60dbea2c551df75e92fd375e24 
  docs/images/containerizer_isolator_api.png 
b84cd1dd082fc883b7f2b3b12458dc01e933d890 
  docs/images/ec_orphan_seqdiag.png 06de3a732414767a722e90c031d97a74150b 
  docs/images/ec_recover_seqdiag.png 08d4ca71a0c28bedc0436f5a01d9c506d1e32e54 
  docs/images/fetch_components.jpg 8d1ea9d395c4a586161508a78b59899cb41ea2ae 
  docs/images/networking-architecture.png 
4e767d4d8a8b23c73918765483dc1a98e3534926 
  docs/images/oversubscription-overview.jpg 
92ef297392910782a825dc1b3640b91d4dbe908c 
  docs/persistent-volume.md c70291ad2d58bacaf508dfd22d19c0f1a473d466 
  docs/powered-by-mesos.md ec02710b2c26b3d91e3b8ae4635c1cf01c881495 
  docs/release-guide.md dbe6d201d30081c304ea1ad4f750c04548790d6a 
  docs/reservation.md bbdaf8e3dad605e1fff8a7090865d6a772eb6c92 
  include/mesos/authorizer/acls.proto b05586ae587edbf9330f1d916340447d157ba80e 
  include/mesos/authorizer/authorizer.proto 
3ff67858a99915e0215f3ffb9966f9ac4a3fba8c 
  include/mesos/docker/spec.hpp b37c7731590ec144df720d7b28eeed5db76f750e 
  include/mesos/docker/v1.proto 60d91d7aef5169b84322a6e0cb12bbef4011e1b7 
  include/mesos/mesos.proto cddfa250e3155966eee4c761e164a5749dd7fc33 
  include/mesos/type_utils.hpp 0d4dd2f0b1f03c51868247d978a41936cf3faf63 
  include/mesos/v1/mesos.proto ce187e71003f82852a8addadfdbc7c6f82dfaa2b 
  site/.gitignore e180964fea36243da61f709d13054c6fff5ef70f 
  site/Dockerfile 80e28e96be53909a8c3c1eb3c05dbbc7e698a5a5 
  site/Gemfile 4dcd0a5ad452085e6e3bb152a6547575636cd807 
  site/Gemfile.lock 1393246f06631155bb052669a86e431778e891bf 
  site/README.md 91bfd5fff16c7848431bdfc9206f42d18e102ace 
  site/Rakefile aa777bdd8e459f27ab487b5641595142f4e14c68 
  site/build.sh 11f15e15621c4d3db1472e88911787b9b3100f97 
  site/config.rb d43308b1082c64017980c6a37fb7c3be7b7265cf 
  site/data/releases.yml 0a3ba6ef5afbb3e0ba8eac8260fe63c9f4b31ebb 
  site/source/assets/css/main.css 8b897d2d91a1d2f4b3bed3cf1a5a1f97ebdc00ef 
  site/source/blog.html.erb 203c8696581d95ba1f7f04ab3de6656012ea65e9 
  site/source/community.html.md ea6e88534dbd86db9342f47dcc384f0a533ba39b 
  site/source/index.html.erb 7d6357c863da18427c04309749688a6b3d20e69c 
  site/source/layouts/community_section.erb 
698d19974b67ed1d1a221c941a68eb1f2c2e8233 
  site/source/layouts/documentation.erb 
a91f916a5fb7348b2702c272e7a2059bdf628c66 
  site/source/layouts/layout.erb c05b1a31c57e600916a62e447bb68a478afb1967 
  site/source/layouts/post.erb fdb566b01892d87fe6e121c958fe07b29cedf072 
  src/authorizer/local/authorizer.cpp 7ddb323df09a9b0ea46c6f9543c4af059d184308 
  src/cli/mesos-cat 778978e4e90e5a2d700699321868ad854e02e615 
  src/cli/mesos-ps 627795c41005003f0cb89f0ba3a888fce3a037cc 
  src/cli/mesos-scp a71ab07083465d734613227da7508fb55a258398 
  src/cli/mesos-tail 0d0fd6b8ebf619f6913928e69982eddf3c48c904 
  src/common/http.hpp 588a1eb18e2665e71cc4361835d873652c4e100d 
  src/common/http.cpp 27b5d4add218fdc90744407f39d8690b1c87e457 
  src/common/protobuf_utils.cpp 445e9d6dc637480b93943bd6d4aac5fc902bac26 
  src/common/type_utils.cpp 7110d87ba8078a5cc00669c82f8cd36f103c14b3 
  src/docker/spec.cpp 7270d182b8c3a5d5c34cbda2bd736ba489089720 
  src/examples/persistent_volume_framework.cpp 
fe2837cfffb6dd251ccb9c93197f623d0c55fb36 
  src/files/files.hpp a41c203349bac0f12ef574fceb201dd08dd957fc 
  src/files/files.cpp 074d31c116c1b9ff96262c3e0c776f38aea53d16 
  src/master/http.cpp 

Re: Review Request 46428: Fixed the broken Docker Volume Rootfs Test on Centos7.

[Guangya Liu]

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

(Updated 五月 28, 2016, 12:53 a.m.)


Review request for mesos, David vonThenen, Gilbert Song, haosdent huang, and 
Jie Yu.


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


Repository: mesos


Description (updated)
---

Fixed the broken Docker Volume Rootfs Test on Centos7.

This patch is splitting the test case of DockerVolumeIsolatorTest
ROOT_CommandTaskNoRootfsWithVolumes to two cases: one for
absolute path and the other is for relative path. 

The reason that I need to split is that there is no good way to
enable one command with shell as false to execute two commands
to test for both absolute and relative path.


Diffs
-

  src/tests/containerizer/docker_volume_isolator_tests.cpp 
c524f42743bf08ee54f1cbb083d0d3c85a8b70c9 

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


Testing
---

make
make check

[==] Running 2 tests from 1 test case.
[--] Global test environment set-up.
[--] 2 tests from DockerVolumeIsolatorTest
[ RUN  ] 
DockerVolumeIsolatorTest.ROOT_INTERNET_CURL_CommandTaskRootfsWithAbsolutePathVolume
[   OK ] 
DockerVolumeIsolatorTest.ROOT_INTERNET_CURL_CommandTaskRootfsWithAbsolutePathVolume
 (1732 ms)
[ RUN  ] 
DockerVolumeIsolatorTest.ROOT_INTERNET_CURL_CommandTaskRootfsWithRelativeVolume
[   OK ] 
DockerVolumeIsolatorTest.ROOT_INTERNET_CURL_CommandTaskRootfsWithRelativeVolume 
(1708 ms)
[--] 2 tests from DockerVolumeIsolatorTest (3451 ms total)
 
[--] Global test environment tear-down
[==] 2 tests from 1 test case ran. (3460 ms total)
[  PASSED  ] 2 tests.
[root@mesos-24 build]# cat /etc/*release
CentOS Linux release 7.2.1511 (Core) 
NAME="CentOS Linux"
VERSION="7 (Core)"
ID="centos"
ID_LIKE="rhel fedora"
VERSION_ID="7"
PRETTY_NAME="CentOS Linux 7 (Core)"
ANSI_COLOR="0;31"
CPE_NAME="cpe:/o:centos:centos:7"
HOME_URL="https://www.centos.org/;
BUG_REPORT_URL="https://bugs.centos.org/;
 
CENTOS_MANTISBT_PROJECT="CentOS-7"
CENTOS_MANTISBT_PROJECT_VERSION="7"
REDHAT_SUPPORT_PRODUCT="centos"
REDHAT_SUPPORT_PRODUCT_VERSION="7"
 
CentOS Linux release 7.2.1511 (Core) 
CentOS Linux release 7.2.1511 (Core) 
[root@mesos-24 build]#


Thanks,

Guangya Liu

Re: Review Request 46428: Fixed the broken Docker Volume Rootfs Test on Centos7.

[Jie Yu]

> On May 27, 2016, 11:17 p.m., Jie Yu wrote:
> > HUm, i got confused. Why this patch has so many new additions? Do you need 
> > a rebase? It's hard to review this one.
> 
> Guangya Liu wrote:
> This patch is splitting the test case of 
> `DockerVolumeIsolatorTest.ROOT_CommandTaskNoRootfsWithVolumes` to two cases: 
> one for absolute path and the other is for relative path. 
> 
> Just spliting this command 
> https://github.com/apache/mesos/blob/master/src/tests/containerizer/docker_volume_isolator_tests.cpp#L293-L294
>  to two test cases.
> 
> The reason that I need to split is that I did not found a good wayt to 
> enable one `command` with `shell` as false to execute two commands. Comments?

OK, make sense. It'll be helpful to say that in the description of this review.


- Jie


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


On May 20, 2016, 12:20 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46428/
> ---
> 
> (Updated May 20, 2016, 12:20 a.m.)
> 
> 
> Review request for mesos, David vonThenen, Gilbert Song, haosdent huang, and 
> Jie Yu.
> 
> 
> Bugs: MESOS-5351
> https://issues.apache.org/jira/browse/MESOS-5351
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed the broken Docker Volume Rootfs Test on Centos7.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_volume_isolator_tests.cpp 
> c524f42743bf08ee54f1cbb083d0d3c85a8b70c9 
> 
> Diff: https://reviews.apache.org/r/46428/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> [==] Running 2 tests from 1 test case.
> [--] Global test environment set-up.
> [--] 2 tests from DockerVolumeIsolatorTest
> [ RUN  ] 
> DockerVolumeIsolatorTest.ROOT_INTERNET_CURL_CommandTaskRootfsWithAbsolutePathVolume
> [   OK ] 
> DockerVolumeIsolatorTest.ROOT_INTERNET_CURL_CommandTaskRootfsWithAbsolutePathVolume
>  (1732 ms)
> [ RUN  ] 
> DockerVolumeIsolatorTest.ROOT_INTERNET_CURL_CommandTaskRootfsWithRelativeVolume
> [   OK ] 
> DockerVolumeIsolatorTest.ROOT_INTERNET_CURL_CommandTaskRootfsWithRelativeVolume
>  (1708 ms)
> [--] 2 tests from DockerVolumeIsolatorTest (3451 ms total)
>  
> [--] Global test environment tear-down
> [==] 2 tests from 1 test case ran. (3460 ms total)
> [  PASSED  ] 2 tests.
> [root@mesos-24 build]# cat /etc/*release
> CentOS Linux release 7.2.1511 (Core) 
> NAME="CentOS Linux"
> VERSION="7 (Core)"
> ID="centos"
> ID_LIKE="rhel fedora"
> VERSION_ID="7"
> PRETTY_NAME="CentOS Linux 7 (Core)"
> ANSI_COLOR="0;31"
> CPE_NAME="cpe:/o:centos:centos:7"
> HOME_URL="https://www.centos.org/;
> BUG_REPORT_URL="https://bugs.centos.org/;
>  
> CENTOS_MANTISBT_PROJECT="CentOS-7"
> CENTOS_MANTISBT_PROJECT_VERSION="7"
> REDHAT_SUPPORT_PRODUCT="centos"
> REDHAT_SUPPORT_PRODUCT_VERSION="7"
>  
> CentOS Linux release 7.2.1511 (Core) 
> CentOS Linux release 7.2.1511 (Core) 
> [root@mesos-24 build]#
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 47805: Add authorization to GET /weights.

[zhou xing]

> On 五月 27, 2016, 9:39 a.m., Adam B wrote:
> > src/tests/dynamic_weights_tests.cpp, line 88
> > 
> >
> > Neat trick, but I think we're better off just specifying 
> > DEFAULT_CREDENTIAL at those call sites
> 
> zhou xing wrote:
> Adam, not very clear on your suggestions, do you mean that we just put 
> credential = DEFAULT_CREDENTIAL here? but as a macro, it is not allowed to 
> put an expression in this file scope, thanks for your time on looking this.

or do you mean change all the caller side to add the credential param and 
remove the param default value here?


- zhou


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


On 五月 27, 2016, 5:45 a.m., zhou xing wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47805/
> ---
> 
> (Updated 五月 27, 2016, 5:45 a.m.)
> 
> 
> Review request for mesos and Adam B.
> 
> 
> Bugs: mesos-5335
> https://issues.apache.org/jira/browse/mesos-5335
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add 'GET_WEIGHTS_WITH_ROLE' for the authorization of GET /weights.
> 
> 
> Diffs
> -
> 
>   docs/endpoints/master/weights.md 1e540795c5fb90c8ea957dd444b9e564e0b1ac23 
>   include/mesos/authorizer/acls.proto 
> ace9b698f46e143795c82324a87a0d7827fb 
>   include/mesos/authorizer/authorizer.proto 
> 02d1a01d57cf34b38524f4368187878b03343537 
>   src/authorizer/local/authorizer.cpp 
> 3c7c791bde65cfcbcc4e319c9ccc487ab37d8029 
>   src/master/http.cpp b36b439a1fa07c52146deff2b90728f92676ade3 
>   src/master/master.hpp 1a875c32eddfb6d884e3d0dda7f5716ee53966c3 
>   src/master/weights_handler.cpp 4bc060fdb015df6658194eef92fe11b14aa15c79 
>   src/tests/dynamic_weights_tests.cpp 
> 362c59aae7b305710d5985bfec28f881be3b64b8 
> 
> Diff: https://reviews.apache.org/r/47805/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> zhou xing
> 
>

Re: Review Request 47991: Passed `Request` object by ptr instead of copying it.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [47986, 47987, 47988, 47989, 47990, 47991]

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

- Mesos ReviewBot


On May 27, 2016, 10:14 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47991/
> ---
> 
> (Updated May 27, 2016, 10:14 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Michael Park, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change passes the `Request` object by wrapping it in
> `Owned` instead of copying it when forwarding it to `defer`.
> Ideally, this should have been a `unique_ptr`, but our
> `defer` callbacks don't currently support moves.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp 
> 988fb3b81259e7b0a6a002f5799d667892874538 
> 
> Diff: https://reviews.apache.org/r/47991/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 47216: Wired up the new docker environment hook.

[Joseph Wu]

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

(Updated May 27, 2016, 5:17 p.m.)


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


Changes
---

Address `Container*` delete race.


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


Repository: mesos


Description
---

Modifies the code path for docker executors.

Docker command executors are now launched with an additional flag
that is filled in by a hook.  The --task_environment flag tells the
command executor to pass some specified mapping of environment
variables to the task.

Custom executors are launched with the environment variables directly.
It is up to custom executors to pass these variables into tasks.


Diffs (updated)
-

  src/slave/containerizer/docker.hpp fac26a239e9981e5b1c2f2f90b52785819492c2a 
  src/slave/containerizer/docker.cpp 7af14f4472283ceefd73c06dd8df60af4cf6f7e8 

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


Testing
---

sudo make check


Thanks,

Joseph Wu

Re: Review Request 47805: Add authorization to GET /weights.

[zhou xing]

> On May 27, 2016, 9:39 a.m., Adam B wrote:
> > src/master/weights_handler.cpp, line 85
> > 
> >
> > Technically, this lambda should be indented to align with its fellow 
> > defer() parameter `master->self()`, or (since that looks too jagged) just 
> > indented 4 spaces in from the `.then`.
> > Looks like our convention for the `-> Future` after is 2 spaces in from 
> > the `[=]` like you have.

Thanks for the comments! The indention is a little confusion to me, as 
somewhere in the code is 4 spaces indention while other places are 2 spaces, do 
we have any regulation on the indention?


- zhou


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


On May 27, 2016, 5:45 a.m., zhou xing wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47805/
> ---
> 
> (Updated May 27, 2016, 5:45 a.m.)
> 
> 
> Review request for mesos and Adam B.
> 
> 
> Bugs: mesos-5335
> https://issues.apache.org/jira/browse/mesos-5335
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add 'GET_WEIGHTS_WITH_ROLE' for the authorization of GET /weights.
> 
> 
> Diffs
> -
> 
>   docs/endpoints/master/weights.md 1e540795c5fb90c8ea957dd444b9e564e0b1ac23 
>   include/mesos/authorizer/acls.proto 
> ace9b698f46e143795c82324a87a0d7827fb 
>   include/mesos/authorizer/authorizer.proto 
> 02d1a01d57cf34b38524f4368187878b03343537 
>   src/authorizer/local/authorizer.cpp 
> 3c7c791bde65cfcbcc4e319c9ccc487ab37d8029 
>   src/master/http.cpp b36b439a1fa07c52146deff2b90728f92676ade3 
>   src/master/master.hpp 1a875c32eddfb6d884e3d0dda7f5716ee53966c3 
>   src/master/weights_handler.cpp 4bc060fdb015df6658194eef92fe11b14aa15c79 
>   src/tests/dynamic_weights_tests.cpp 
> 362c59aae7b305710d5985bfec28f881be3b64b8 
> 
> Diff: https://reviews.apache.org/r/47805/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> zhou xing
> 
>

Re: Review Request 47150: Implemented new asynchronous docker pre-launch hook.

[Kapil Arya]

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


Ship it!




Ship It!

- Kapil Arya


On May 27, 2016, 6:07 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47150/
> ---
> 
> (Updated May 27, 2016, 6:07 p.m.)
> 
> 
> Review request for mesos, Adam B, Artem Harutyunyan, Jie Yu, and Kapil Arya.
> 
> 
> Bugs: MESOS-5350
> https://issues.apache.org/jira/browse/MESOS-5350
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Introduces, but does not fully wire up a new hook.
> 
> The new hook, "slavePreLaunchDockerEnvironmentDecorator",
> has divergent semantics compared with existing hooks:
> 
> * The hook is asynchronous,
> * can prevent a task from launching if it errors,
> * can overwrite environment variables.
> 
> The new hook is intended to be a strictly-superior 
> replacement for the existing hook "slavePreLaunchDockerHook".
> 
> 
> Diffs
> -
> 
>   include/mesos/hook.hpp 210ffba09f5acae34ca49b888a781f683777f9ca 
>   src/examples/test_hook_module.cpp 4b97f84204934d0e678786fd6cde38b89a6f8f48 
>   src/hook/manager.hpp 528674e36639fe78137ba0a4bb004c99730e7a22 
>   src/hook/manager.cpp 381807d582998043d73e9b8c9d3c1fddbcf73cf1 
>   src/tests/hook_tests.cpp c6b4e8a50534e455cf44f427b09f74eef71177b2 
> 
> Diff: https://reviews.apache.org/r/47150/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 47205: Added optional environment variable argument to mesos-docker-executor.

[Kapil Arya]

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


Ship it!




Ship It!

- Kapil Arya


On May 25, 2016, 10:02 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47205/
> ---
> 
> (Updated May 25, 2016, 10:02 p.m.)
> 
> 
> Review request for mesos, Adam B, Artem Harutyunyan, Jie Yu, and Kapil Arya.
> 
> 
> Bugs: MESOS-5350
> https://issues.apache.org/jira/browse/MESOS-5350
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This flag opens up a way for hooks to specify environment variables for
> docker tasks.  Existing hooks can only affect the environment variables
> of docker executors.
> 
> 
> Diffs
> -
> 
>   src/docker/executor.hpp 798ca3d4e261854a3b911d59929f2ca2afeb9ac8 
>   src/docker/executor.cpp 04f44044a80e1b355580ee3d1d8d301ac3baf565 
> 
> Diff: https://reviews.apache.org/r/47205/diff/
> 
> 
> Testing
> ---
> 
> See later reviews in chain.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 47805: Add authorization to GET /weights.

[zhou xing]

> On May 27, 2016, 9:39 a.m., Adam B wrote:
> > src/tests/dynamic_weights_tests.cpp, line 88
> > 
> >
> > Neat trick, but I think we're better off just specifying 
> > DEFAULT_CREDENTIAL at those call sites

Adam, not very clear on your suggestions, do you mean that we just put 
credential = DEFAULT_CREDENTIAL here? but as a macro, it is not allowed to put 
an expression in this file scope, thanks for your time on looking this.


- zhou


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


On May 27, 2016, 5:45 a.m., zhou xing wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47805/
> ---
> 
> (Updated May 27, 2016, 5:45 a.m.)
> 
> 
> Review request for mesos and Adam B.
> 
> 
> Bugs: mesos-5335
> https://issues.apache.org/jira/browse/mesos-5335
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add 'GET_WEIGHTS_WITH_ROLE' for the authorization of GET /weights.
> 
> 
> Diffs
> -
> 
>   docs/endpoints/master/weights.md 1e540795c5fb90c8ea957dd444b9e564e0b1ac23 
>   include/mesos/authorizer/acls.proto 
> ace9b698f46e143795c82324a87a0d7827fb 
>   include/mesos/authorizer/authorizer.proto 
> 02d1a01d57cf34b38524f4368187878b03343537 
>   src/authorizer/local/authorizer.cpp 
> 3c7c791bde65cfcbcc4e319c9ccc487ab37d8029 
>   src/master/http.cpp b36b439a1fa07c52146deff2b90728f92676ade3 
>   src/master/master.hpp 1a875c32eddfb6d884e3d0dda7f5716ee53966c3 
>   src/master/weights_handler.cpp 4bc060fdb015df6658194eef92fe11b14aa15c79 
>   src/tests/dynamic_weights_tests.cpp 
> 362c59aae7b305710d5985bfec28f881be3b64b8 
> 
> Diff: https://reviews.apache.org/r/47805/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> zhou xing
> 
>

Re: Review Request 47973: Updated gc to prevent early exit in case of error.

[Megha Sharma]

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

(Updated May 28, 2016, 12:02 a.m.)


Review request for mesos and Jiang Yan Xu.


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


Repository: mesos


Description (updated)
---

https://issues.apache.org/jira/browse/MESOS-5196
Updates made to mesos gc to prevent early exit in the event
of error.


Diffs (updated)
-

  src/slave/gc.cpp eedb8ca713d7f5195055bb6417f03ab70731af97 
  src/tests/gc_tests.cpp 4cb7c2f612984f7f5a9378a7f972f2438bbf28c5 

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


Testing (updated)
---

make check.
Tested with option --gtest_also_run_disabled_tests.


Thanks,

Megha Sharma

Re: Review Request 47972: Updated rmdir to continue deletion on error.

[Megha Sharma]

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

(Updated May 28, 2016, 12:01 a.m.)


Review request for mesos and Jiang Yan Xu.


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


Repository: mesos


Description
---

https://issues.apache.org/jira/browse/MESOS-5196
Updates made to rmdir to prevent early exit in the event of error.


Diffs
-

  3rdparty/stout/include/stout/os/posix/rmdir.hpp 
a6d3b578762d5c570542b0497578c330820b821a 
  3rdparty/stout/include/stout/os/windows/rmdir.hpp 
772b86dd111d28aefbeeba5f829ffa377fd12efb 
  3rdparty/stout/tests/os/rmdir_tests.cpp 
a11bfc9f9e6cbb05f3e9ce0ea48297b8f88fe53f 

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


Testing (updated)
---

make check.

Tested with option --gtest_also_run_disabled_tests.


Thanks,

Megha Sharma

Re: Review Request 47972: Updated rmdir to continue deletion on error.

[Megha Sharma]

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

(Updated May 28, 2016, 12:01 a.m.)


Review request for mesos and Jiang Yan Xu.


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


Repository: mesos


Description (updated)
---

https://issues.apache.org/jira/browse/MESOS-5196
Updates made to rmdir to prevent early exit in the event of error.


Diffs (updated)
-

  3rdparty/stout/include/stout/os/posix/rmdir.hpp 
a6d3b578762d5c570542b0497578c330820b821a 
  3rdparty/stout/include/stout/os/windows/rmdir.hpp 
772b86dd111d28aefbeeba5f829ffa377fd12efb 
  3rdparty/stout/tests/os/rmdir_tests.cpp 
a11bfc9f9e6cbb05f3e9ce0ea48297b8f88fe53f 

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


Testing
---

make check.


Thanks,

Megha Sharma

Re: Review Request 46428: Fixed the broken Docker Volume Rootfs Test on Centos7.

[Guangya Liu]

> On 五月 27, 2016, 11:17 p.m., Jie Yu wrote:
> > HUm, i got confused. Why this patch has so many new additions? Do you need 
> > a rebase? It's hard to review this one.

This patch is splitting the test case of 
`DockerVolumeIsolatorTest.ROOT_CommandTaskNoRootfsWithVolumes` to two cases: 
one for absolute path and the other is for relative path. 

Just spliting this command 
https://github.com/apache/mesos/blob/master/src/tests/containerizer/docker_volume_isolator_tests.cpp#L293-L294
 to two test cases.

The reason that I need to split is that I did not found a good wayt to enable 
one `command` with `shell` as false to execute two commands. Comments?


- Guangya


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


On 五月 20, 2016, 12:20 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46428/
> ---
> 
> (Updated 五月 20, 2016, 12:20 a.m.)
> 
> 
> Review request for mesos, David vonThenen, Gilbert Song, haosdent huang, and 
> Jie Yu.
> 
> 
> Bugs: MESOS-5351
> https://issues.apache.org/jira/browse/MESOS-5351
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed the broken Docker Volume Rootfs Test on Centos7.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_volume_isolator_tests.cpp 
> c524f42743bf08ee54f1cbb083d0d3c85a8b70c9 
> 
> Diff: https://reviews.apache.org/r/46428/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> [==] Running 2 tests from 1 test case.
> [--] Global test environment set-up.
> [--] 2 tests from DockerVolumeIsolatorTest
> [ RUN  ] 
> DockerVolumeIsolatorTest.ROOT_INTERNET_CURL_CommandTaskRootfsWithAbsolutePathVolume
> [   OK ] 
> DockerVolumeIsolatorTest.ROOT_INTERNET_CURL_CommandTaskRootfsWithAbsolutePathVolume
>  (1732 ms)
> [ RUN  ] 
> DockerVolumeIsolatorTest.ROOT_INTERNET_CURL_CommandTaskRootfsWithRelativeVolume
> [   OK ] 
> DockerVolumeIsolatorTest.ROOT_INTERNET_CURL_CommandTaskRootfsWithRelativeVolume
>  (1708 ms)
> [--] 2 tests from DockerVolumeIsolatorTest (3451 ms total)
>  
> [--] Global test environment tear-down
> [==] 2 tests from 1 test case ran. (3460 ms total)
> [  PASSED  ] 2 tests.
> [root@mesos-24 build]# cat /etc/*release
> CentOS Linux release 7.2.1511 (Core) 
> NAME="CentOS Linux"
> VERSION="7 (Core)"
> ID="centos"
> ID_LIKE="rhel fedora"
> VERSION_ID="7"
> PRETTY_NAME="CentOS Linux 7 (Core)"
> ANSI_COLOR="0;31"
> CPE_NAME="cpe:/o:centos:centos:7"
> HOME_URL="https://www.centos.org/;
> BUG_REPORT_URL="https://bugs.centos.org/;
>  
> CENTOS_MANTISBT_PROJECT="CentOS-7"
> CENTOS_MANTISBT_PROJECT_VERSION="7"
> REDHAT_SUPPORT_PRODUCT="centos"
> REDHAT_SUPPORT_PRODUCT_VERSION="7"
>  
> CentOS Linux release 7.2.1511 (Core) 
> CentOS Linux release 7.2.1511 (Core) 
> [root@mesos-24 build]#
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 47216: Wired up the new docker environment hook.

[Jie Yu]

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


Fix it, then Ship it!





src/slave/containerizer/docker.cpp (lines 1048 - 1049)


I would put them into one line here:
```
f = HookManager::slavePreLaunchDockerEnvironmentDecorator(
taskInfo,
...);
```



src/slave/containerizer/docker.cpp (lines 1056 - 1058)


`container` might be deleted before the callback in `then` is called. You 
should:
```
.then(defer(self(), [=](...) {
  if (!containers_.contains(containerId)) {
return Failure("...");
  }
  
  ...
});
```


- Jie Yu


On May 27, 2016, 10:10 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47216/
> ---
> 
> (Updated May 27, 2016, 10:10 p.m.)
> 
> 
> Review request for mesos, Adam B, Artem Harutyunyan, Jie Yu, and Kapil Arya.
> 
> 
> Bugs: MESOS-5350
> https://issues.apache.org/jira/browse/MESOS-5350
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Modifies the code path for docker executors.
> 
> Docker command executors are now launched with an additional flag
> that is filled in by a hook.  The --task_environment flag tells the
> command executor to pass some specified mapping of environment
> variables to the task.
> 
> Custom executors are launched with the environment variables directly.
> It is up to custom executors to pass these variables into tasks.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.hpp fac26a239e9981e5b1c2f2f90b52785819492c2a 
>   src/slave/containerizer/docker.cpp 7af14f4472283ceefd73c06dd8df60af4cf6f7e8 
> 
> Diff: https://reviews.apache.org/r/47216/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 46618: Enhanced the log message when there are duplicate volumes.

[Guangya Liu]

> On 五月 27, 2016, 11:13 p.m., Jie Yu wrote:
> > The container Id is already printed. Why print it again?

I see, will discard this. Thanks.


- Guangya


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


On 五月 6, 2016, 5:33 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46618/
> ---
> 
> (Updated 五月 6, 2016, 5:33 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added containerId to failure message when there are duplicate volumes
> in one container.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp 
> 76d0fa183c9099584da6a4ee1e5d474b871310c3 
> 
> Diff: https://reviews.apache.org/r/46618/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> GLOG_v=2 ./bin/mesos-tests.sh  
> --gtest_filter="DockerVolumeIsolatorTest.ROOT_CommandTaskNoRootfsFailedWithSameVolumes"
>  --verbose
> 
> E0506 13:28:57.268867 22904 slave.cpp:3995] Container 
> '9d3f864f-8907-47d5-b0b1-657bf06ed12f' for executor 
> '59d6e794-7d6a-479d-9d15-5026661eb51d' of framework 
> a81cc2e7-e802-458b-9ba2-8c7f7eab83d0- failed to start: Found duplicate 
> docker volume with driver 'driver1' and name 'name1' for container 
> 9d3f864f-8907-47d5-b0b1-657bf06ed12f
> 
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 47633: Checked if IPv6 module was loaded before disabling it.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On May 23, 2016, 7:55 a.m., Zhengju Sha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47633/
> ---
> 
> (Updated May 23, 2016, 7:55 a.m.)
> 
> 
> Review request for mesos, haosdent huang, Jie Yu, and Cong Wang.
> 
> 
> Bugs: MESOS-5381
> https://issues.apache.org/jira/browse/MESOS-5381
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Checked if IPv6 module was loaded before disabling it.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
> ad792def2bb3a1614d21ca28d858e400d2e3ede1 
> 
> Diff: https://reviews.apache.org/r/47633/diff/
> 
> 
> Testing
> ---
> 
> Enniornment and steps:
> 1. Enable mesos-slave --isolation=network/port_mapping on CentOS7.2 with 
> kernel version: 3.10.0-327.10.1.el7.x86_64
> 2. Create application on marathon framework with commands such as "echo 
> hello" using MesosContainerizer
> 3. Load IPv6 module by removing "ipv6.disable=1" of GRUB_CMDLINE_LINUX in 
> /etc/default/grub
> 4. Disable IPv6 module by adding "ipv6.disable=1" of GRUB_CMDLINE_LINUX in 
> /etc/default/grub
> 
> Now mesos can run both of the testcases successfully.
> 
> 
> Thanks,
> 
> Zhengju Sha
> 
>

Re: Review Request 47216: Wired up the new docker environment hook.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [47149, 47205, 47212, 47215, 47150, 47216]

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

- Mesos ReviewBot


On May 27, 2016, 10:10 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47216/
> ---
> 
> (Updated May 27, 2016, 10:10 p.m.)
> 
> 
> Review request for mesos, Adam B, Artem Harutyunyan, Jie Yu, and Kapil Arya.
> 
> 
> Bugs: MESOS-5350
> https://issues.apache.org/jira/browse/MESOS-5350
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Modifies the code path for docker executors.
> 
> Docker command executors are now launched with an additional flag
> that is filled in by a hook.  The --task_environment flag tells the
> command executor to pass some specified mapping of environment
> variables to the task.
> 
> Custom executors are launched with the environment variables directly.
> It is up to custom executors to pass these variables into tasks.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.hpp fac26a239e9981e5b1c2f2f90b52785819492c2a 
>   src/slave/containerizer/docker.cpp 7af14f4472283ceefd73c06dd8df60af4cf6f7e8 
> 
> Diff: https://reviews.apache.org/r/47216/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 46428: Fixed the broken Docker Volume Rootfs Test on Centos7.

[Jie Yu]

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



HUm, i got confused. Why this patch has so many new additions? Do you need a 
rebase? It's hard to review this one.

- Jie Yu


On May 20, 2016, 12:20 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46428/
> ---
> 
> (Updated May 20, 2016, 12:20 a.m.)
> 
> 
> Review request for mesos, David vonThenen, Gilbert Song, haosdent huang, and 
> Jie Yu.
> 
> 
> Bugs: MESOS-5351
> https://issues.apache.org/jira/browse/MESOS-5351
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed the broken Docker Volume Rootfs Test on Centos7.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_volume_isolator_tests.cpp 
> c524f42743bf08ee54f1cbb083d0d3c85a8b70c9 
> 
> Diff: https://reviews.apache.org/r/46428/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> [==] Running 2 tests from 1 test case.
> [--] Global test environment set-up.
> [--] 2 tests from DockerVolumeIsolatorTest
> [ RUN  ] 
> DockerVolumeIsolatorTest.ROOT_INTERNET_CURL_CommandTaskRootfsWithAbsolutePathVolume
> [   OK ] 
> DockerVolumeIsolatorTest.ROOT_INTERNET_CURL_CommandTaskRootfsWithAbsolutePathVolume
>  (1732 ms)
> [ RUN  ] 
> DockerVolumeIsolatorTest.ROOT_INTERNET_CURL_CommandTaskRootfsWithRelativeVolume
> [   OK ] 
> DockerVolumeIsolatorTest.ROOT_INTERNET_CURL_CommandTaskRootfsWithRelativeVolume
>  (1708 ms)
> [--] 2 tests from DockerVolumeIsolatorTest (3451 ms total)
>  
> [--] Global test environment tear-down
> [==] 2 tests from 1 test case ran. (3460 ms total)
> [  PASSED  ] 2 tests.
> [root@mesos-24 build]# cat /etc/*release
> CentOS Linux release 7.2.1511 (Core) 
> NAME="CentOS Linux"
> VERSION="7 (Core)"
> ID="centos"
> ID_LIKE="rhel fedora"
> VERSION_ID="7"
> PRETTY_NAME="CentOS Linux 7 (Core)"
> ANSI_COLOR="0;31"
> CPE_NAME="cpe:/o:centos:centos:7"
> HOME_URL="https://www.centos.org/;
> BUG_REPORT_URL="https://bugs.centos.org/;
>  
> CENTOS_MANTISBT_PROJECT="CentOS-7"
> CENTOS_MANTISBT_PROJECT_VERSION="7"
> REDHAT_SUPPORT_PRODUCT="centos"
> REDHAT_SUPPORT_PRODUCT_VERSION="7"
>  
> CentOS Linux release 7.2.1511 (Core) 
> CentOS Linux release 7.2.1511 (Core) 
> [root@mesos-24 build]#
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 47447: Some cleanup for watchdog logic in subprocess.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On May 17, 2016, 8:39 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47447/
> ---
> 
> (Updated May 17, 2016, 8:39 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Some cleanup for watchdog logic in subprocess.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/docker/volume/driver.cpp 
> 19799644990132d15d3e6543112742dca92c1f67 
>   src/slave/containerizer/mesos/isolators/posix/disk.cpp 
> dde958b252eae75563003ec15087b4231beb285a 
> 
> Diff: https://reviews.apache.org/r/47447/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 46618: Enhanced the log message when there are duplicate volumes.

[Jie Yu]

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



The container Id is already printed. Why print it again?

- Jie Yu


On May 6, 2016, 5:33 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46618/
> ---
> 
> (Updated May 6, 2016, 5:33 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added containerId to failure message when there are duplicate volumes
> in one container.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp 
> 76d0fa183c9099584da6a4ee1e5d474b871310c3 
> 
> Diff: https://reviews.apache.org/r/46618/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> GLOG_v=2 ./bin/mesos-tests.sh  
> --gtest_filter="DockerVolumeIsolatorTest.ROOT_CommandTaskNoRootfsFailedWithSameVolumes"
>  --verbose
> 
> E0506 13:28:57.268867 22904 slave.cpp:3995] Container 
> '9d3f864f-8907-47d5-b0b1-657bf06ed12f' for executor 
> '59d6e794-7d6a-479d-9d15-5026661eb51d' of framework 
> a81cc2e7-e802-458b-9ba2-8c7f7eab83d0- failed to start: Found duplicate 
> docker volume with driver 'driver1' and name 'name1' for container 
> 9d3f864f-8907-47d5-b0b1-657bf06ed12f
> 
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 45377: Made "driver" as optional for DockerVolume.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On May 26, 2016, 8:33 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45377/
> ---
> 
> (Updated May 26, 2016, 8:33 p.m.)
> 
> 
> Review request for mesos, David vonThenen, Gilbert Song, haosdent huang, and 
> Jie Yu.
> 
> 
> Bugs: MESOS-5354
> https://issues.apache.org/jira/browse/MESOS-5354
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Made "driver" as optional for DockerVolume.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto cddfa250e3155966eee4c761e164a5749dd7fc33 
>   include/mesos/v1/mesos.proto ce187e71003f82852a8addadfdbc7c6f82dfaa2b 
>   src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp 
> 8ab2cf39a6bc3dd841b51639942b3e5258b7292e 
> 
> Diff: https://reviews.apache.org/r/45377/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 47895: Renamed 'posix/disk' isolator as 'disk/du'.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On May 26, 2016, 3:08 p.m., Jiang Yan Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47895/
> ---
> 
> (Updated May 26, 2016, 3:08 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This better summarizes how the isolator works and is more consistent
> with the 'disk/*' isolator naming pattern.
> 
> 'posix/disk' is still allowed but a deprecation warning is logged.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md ae9067d65535ff26e8fd77088748299c91511087 
>   docs/mesos-containerizer.md 22dffe00b315148f4ca158b0b606f254b9ee53c2 
>   docs/multiple-disk.md 2b272f0101908af116f868b4e57145d39b45de92 
>   docs/sandbox.md 5acddff914ccc2aba5735c2dbf418117560e17c7 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 75e5a32a3e70ec60a6800e21a621673184ea0956 
>   src/slave/flags.cpp d3543a019e57658cf60879f40c5f0834a1b8e836 
>   src/tests/containerizer/filesystem_isolator_tests.cpp 
> 4293416ac8434e9eb7e80724480a54936a2fe24a 
>   src/tests/containerizer/xfs_quota_tests.cpp 
> ce16006f1a370b9b48837abee729669b84a885a2 
>   src/tests/disk_quota_tests.cpp 1564f70854ff5630da1d7348a034f726d67b1757 
> 
> Diff: https://reviews.apache.org/r/47895/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>

Re: Review Request 47894: Renamed 'xfs/disk' isolator to 'disk/xfs'.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On May 26, 2016, 3:05 p.m., Jiang Yan Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47894/
> ---
> 
> (Updated May 26, 2016, 3:05 p.m.)
> 
> 
> Review request for mesos, Jie Yu and James Peach.
> 
> 
> Bugs: MESOS-5394
> https://issues.apache.org/jira/browse/MESOS-5394
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Renamed 'xfs/disk' isolator to 'disk/xfs'.
> 
> 
> Diffs
> -
> 
>   CHANGELOG d9b155dbb3beffc782de58b2a9478fd1554184a6 
>   docs/mesos-containerizer.md 22dffe00b315148f4ca158b0b606f254b9ee53c2 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 75e5a32a3e70ec60a6800e21a621673184ea0956 
>   src/tests/containerizer/xfs_quota_tests.cpp 
> ce16006f1a370b9b48837abee729669b84a885a2 
> 
> Diff: https://reviews.apache.org/r/47894/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>

Re: Review Request 47971: Remove subnet prefix length from IP address.

[Jie Yu]

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




src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (line 1086)


For comments, please refer to our style. It should be a sentense 
(capitalize the first word and add a period to the end). I'll fix it for you.


- Jie Yu


On May 27, 2016, 8:42 p.m., Dan Osborne wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47971/
> ---
> 
> (Updated May 27, 2016, 8:42 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-5453
> https://issues.apache.org/jira/browse/MESOS-5453
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Remove the prefix length included in the IP address string
> that is returned by the CNI plugin before storing the value
> in NetworkInfo.IPAddress.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
> 2ba3d62041379f5175b58f8c1ba3c2eae73edcf6 
> 
> Diff: https://reviews.apache.org/r/47971/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Dan Osborne
> 
>

Review Request 47991: Passed `Request` object by ptr instead of copying it.

[Anand Mazumdar]

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

Review request for mesos, Benjamin Mahler, Michael Park, and Vinod Kone.


Repository: mesos


Description
---

This change passes the `Request` object by wrapping it in
`Owned` instead of copying it when forwarding it to `defer`.
Ideally, this should have been a `unique_ptr`, but our
`defer` callbacks don't currently support moves.


Diffs
-

  3rdparty/libprocess/src/process.cpp 988fb3b81259e7b0a6a002f5799d667892874538 

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


Testing
---

make check


Thanks,

Anand Mazumdar

Review Request 47988: Added move constructor/assignment to `Try`.

[Anand Mazumdar]

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

Review request for mesos, Michael Park and Vinod Kone.


Repository: mesos


Description
---

Added move constructor/assignment to `Try`.


Diffs
-

  3rdparty/stout/include/stout/try.hpp 89dedec0c818263f17f220a2e71ed470bf75a42f 

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


Testing
---

make check


Thanks,

Anand Mazumdar

Review Request 47989: Added move semantics to `Future::set`.

[Anand Mazumdar]

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

Review request for mesos, Michael Park and Vinod Kone.


Repository: mesos


Description
---

See summary.


Diffs
-

  3rdparty/libprocess/include/process/future.hpp 
455b750430ae04d16f610c24c0ea0feb8a033708 

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


Testing
---

make check


Thanks,

Anand Mazumdar

Review Request 47987: Constrained constructible types constructor for `Result`.

[Anand Mazumdar]

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

Review request for mesos, Michael Park and Vinod Kone.


Repository: mesos


Description
---

Constrained constructible types constructor for `Result`.


Diffs
-

  3rdparty/stout/include/stout/result.hpp 
5d93ee03248249117f8ce259ebb1b126803e1a5c 

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


Testing
---

make check


Thanks,

Anand Mazumdar

Review Request 47990: Added move semantics to `Pipe::write()`.

[Anand Mazumdar]

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

Review request for mesos, Benjamin Mahler, Michael Park, and Vinod Kone.


Repository: mesos


Description
---

Instead of having separate rvalue overload for `write`,
this change takes the sink argument by value
and then does a move. This has the additional small
overhead of performing an extra move.


Diffs
-

  3rdparty/libprocess/include/process/http.hpp 
10f6fb90e436300bc47f4d5d2ad4beabd19026ba 
  3rdparty/libprocess/src/http.cpp b7839b1402d0660a2461085f5a1db191296e3308 

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


Testing
---

make check


Thanks,

Anand Mazumdar

Re: Review Request 47891: Added RUN_TASK authorization action.

[Joerg Schad]

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




src/authorizer/local/authorizer.cpp (line 141)


command is required in ExecutorInfo.



src/authorizer/local/authorizer.cpp (line 152)


Note that at least with the other Actions non-existent `value` implies `ANY`


- Joerg Schad


On May 27, 2016, 9:51 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47891/
> ---
> 
> (Updated May 27, 2016, 9:51 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, Joerg Schad, and Michael 
> Park.
> 
> 
> Bugs: MESOS-5459
> https://issues.apache.org/jira/browse/MESOS-5459
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Authorization requests for RUN_TASK actions can pass `SOME`
> authorization object either in a `FrameworkInfo` holding a user, or a
> `TaskInfo` with optionally a `CommandInfo` which can optionally hold a
> user. If either of these fields is set it will be used as the object;
> otherwise an `ANY` type authorization object will be created.
> 
> `RUN_TASK` aliases `RUN_TASK_WITH_USER` which becomes deprecated with
> 0.29.
> 
> 
> Diffs
> -
> 
>   include/mesos/authorizer/authorizer.proto 
> 3ff67858a99915e0215f3ffb9966f9ac4a3fba8c 
>   src/authorizer/local/authorizer.cpp 
> 7ddb323df09a9b0ea46c6f9543c4af059d184308 
>   src/master/master.cpp 6442762c9fdfa368d5d9d7cd43b97f5addaf7f17 
>   src/tests/authorization_tests.cpp 54bfb46a807677f4a4a2bb88dcb78a358cf5121a 
> 
> Diff: https://reviews.apache.org/r/47891/diff/
> 
> 
> Testing
> ---
> 
> Tested on a range of Linux configurations on internal CI.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Review Request 47986: Added move constructor/assignment operator to `Result`.

[Anand Mazumdar]

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

Review request for mesos, Michael Park and Vinod Kone.


Repository: mesos


Description
---

Added move constructor/assignment operator to `Result`.
Note that `Some` still makes a copy and would be fixed in
a separate patch.


Diffs
-

  3rdparty/stout/include/stout/result.hpp 
5d93ee03248249117f8ce259ebb1b126803e1a5c 

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


Testing
---

make check


Thanks,

Anand Mazumdar

Re: Review Request 47216: Wired up the new docker environment hook.

[Joseph Wu]

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

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


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


Changes
---

Added a modicum of support for custom executors.


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


Repository: mesos


Description (updated)
---

Modifies the code path for docker executors.

Docker command executors are now launched with an additional flag
that is filled in by a hook.  The --task_environment flag tells the
command executor to pass some specified mapping of environment
variables to the task.

Custom executors are launched with the environment variables directly.
It is up to custom executors to pass these variables into tasks.


Diffs (updated)
-

  src/slave/containerizer/docker.hpp fac26a239e9981e5b1c2f2f90b52785819492c2a 
  src/slave/containerizer/docker.cpp 7af14f4472283ceefd73c06dd8df60af4cf6f7e8 

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


Testing
---

sudo make check


Thanks,

Joseph Wu

Re: Review Request 47150: Implemented new asynchronous docker pre-launch hook.

[Joseph Wu]

> On May 25, 2016, 6:50 p.m., Jie Yu wrote:
> > include/mesos/hook.hpp, lines 108-116
> > 
> >
> > Any reason why all the fields here are needed? For instance, 
> > 'containerInfo' is part of either taskInfo or executorInfo, why bother 
> > passing this duplicated information? Similarily, 'resources' can be derived 
> > as well.
> 
> Joseph Wu wrote:
> I haven't dug too deeply into what each value could be in the various 
> DockerContainerizer paths, but the interface is meant to be almost identical 
> to `slavePreLaunchDockerHook`, except for the return type.

Removed these arguments because they can be derived from the other fields:

const ContainerInfo& containerInfo,
const CommandInfo& commandInfo,
const Option& resources,


- Joseph


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


On May 27, 2016, 3:07 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47150/
> ---
> 
> (Updated May 27, 2016, 3:07 p.m.)
> 
> 
> Review request for mesos, Adam B, Artem Harutyunyan, Jie Yu, and Kapil Arya.
> 
> 
> Bugs: MESOS-5350
> https://issues.apache.org/jira/browse/MESOS-5350
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Introduces, but does not fully wire up a new hook.
> 
> The new hook, "slavePreLaunchDockerEnvironmentDecorator",
> has divergent semantics compared with existing hooks:
> 
> * The hook is asynchronous,
> * can prevent a task from launching if it errors,
> * can overwrite environment variables.
> 
> The new hook is intended to be a strictly-superior 
> replacement for the existing hook "slavePreLaunchDockerHook".
> 
> 
> Diffs
> -
> 
>   include/mesos/hook.hpp 210ffba09f5acae34ca49b888a781f683777f9ca 
>   src/examples/test_hook_module.cpp 4b97f84204934d0e678786fd6cde38b89a6f8f48 
>   src/hook/manager.hpp 528674e36639fe78137ba0a4bb004c99730e7a22 
>   src/hook/manager.cpp 381807d582998043d73e9b8c9d3c1fddbcf73cf1 
>   src/tests/hook_tests.cpp c6b4e8a50534e455cf44f427b09f74eef71177b2 
> 
> Diff: https://reviews.apache.org/r/47150/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 47150: Implemented new asynchronous docker pre-launch hook.

[Joseph Wu]

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

(Updated May 27, 2016, 3:07 p.m.)


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


Changes
---

Renamed hook.  Removed 3 arguments from interface.  Reworked some comments.  
Tweaked test.


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


Repository: mesos


Description (updated)
---

Introduces, but does not fully wire up a new hook.

The new hook, "slavePreLaunchDockerEnvironmentDecorator",
has divergent semantics compared with existing hooks:

* The hook is asynchronous,
* can prevent a task from launching if it errors,
* can overwrite environment variables.

The new hook is intended to be a strictly-superior 
replacement for the existing hook "slavePreLaunchDockerHook".


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/tests/hook_tests.cpp c6b4e8a50534e455cf44f427b09f74eef71177b2 

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


Testing
---

make check


Thanks,

Joseph Wu

Re: Review Request 47971: Remove subnet prefix length from IP address.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [47971]

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

- Mesos ReviewBot


On May 27, 2016, 8:42 p.m., Dan Osborne wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47971/
> ---
> 
> (Updated May 27, 2016, 8:42 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-5453
> https://issues.apache.org/jira/browse/MESOS-5453
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Remove the prefix length included in the IP address string
> that is returned by the CNI plugin before storing the value
> in NetworkInfo.IPAddress.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
> 2ba3d62041379f5175b58f8c1ba3c2eae73edcf6 
> 
> Diff: https://reviews.apache.org/r/47971/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Dan Osborne
> 
>

Re: Review Request 47891: Added RUN_TASK authorization action.

[Benjamin Bannier]

> On May 27, 2016, 10:52 a.m., Adam B wrote:
> > include/mesos/authorizer/authorizer.proto, line 56
> > 
> >
> > Is this deprecated or unused now?

I changed `RUN_TASK` to alias `RUN_TASK_WITH_USER` and added a deprecation 
comment to `RUN_TASK_WITH_USER`. I also removed all other references to it from 
the code base and made the (trivial) changes from `RUN_TASK_WITH_USER` to 
`RUN_TASK`.


> On May 27, 2016, 10:52 a.m., Adam B wrote:
> > include/mesos/authorizer/authorizer.proto, line 86
> > 
> >
> > I wonder if we should just alias RUN_TASK to the same enum value as 
> > RUN_TASK_WITH_USER.. There shouldn't be any backwards compatibility issues 
> > since these are only used in-memory, and modules have to recompile anyway.

Good idea. I changed the proto message definitions to that effect. As a note, 
it seems we usually don't use aliasing fields as this will be the only enum 
using proto's `allow_alias` setting.


> On May 27, 2016, 10:52 a.m., Adam B wrote:
> > src/tests/authorization_tests.cpp, line 203
> > 
> >
> > But principal "foo" could run as any other user, e.g. "bar", right? 
> > That'd be worth testing.

Let's not mix this into this patch.


> On May 27, 2016, 10:52 a.m., Adam B wrote:
> > src/tests/authorization_tests.cpp, line 529
> > 
> >
> > Would be better to test that "bar" cannot run a "user1", since we've 
> > shown previously that somebody else ("foo") can.

Let's not mix this into this patch.


- Benjamin


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


On May 27, 2016, 11:51 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47891/
> ---
> 
> (Updated May 27, 2016, 11:51 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, Joerg Schad, and Michael 
> Park.
> 
> 
> Bugs: MESOS-5459
> https://issues.apache.org/jira/browse/MESOS-5459
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Authorization requests for RUN_TASK actions can pass `SOME`
> authorization object either in a `FrameworkInfo` holding a user, or a
> `TaskInfo` with optionally a `CommandInfo` which can optionally hold a
> user. If either of these fields is set it will be used as the object;
> otherwise an `ANY` type authorization object will be created.
> 
> `RUN_TASK` aliases `RUN_TASK_WITH_USER` which becomes deprecated with
> 0.29.
> 
> 
> Diffs
> -
> 
>   include/mesos/authorizer/authorizer.proto 
> 3ff67858a99915e0215f3ffb9966f9ac4a3fba8c 
>   src/authorizer/local/authorizer.cpp 
> 7ddb323df09a9b0ea46c6f9543c4af059d184308 
>   src/master/master.cpp 6442762c9fdfa368d5d9d7cd43b97f5addaf7f17 
>   src/tests/authorization_tests.cpp 54bfb46a807677f4a4a2bb88dcb78a358cf5121a 
> 
> Diff: https://reviews.apache.org/r/47891/diff/
> 
> 
> Testing
> ---
> 
> Tested on a range of Linux configurations on internal CI.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 47891: Added RUN_TASK authorization action.

[Benjamin Bannier]

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

(Updated May 27, 2016, 11:51 p.m.)


Review request for mesos, Adam B, Alexander Rojas, Joerg Schad, and Michael 
Park.


Changes
---

Addressed Adam's comments, rebased.


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


Repository: mesos


Description (updated)
---

Authorization requests for RUN_TASK actions can pass `SOME`
authorization object either in a `FrameworkInfo` holding a user, or a
`TaskInfo` with optionally a `CommandInfo` which can optionally hold a
user. If either of these fields is set it will be used as the object;
otherwise an `ANY` type authorization object will be created.

`RUN_TASK` aliases `RUN_TASK_WITH_USER` which becomes deprecated with
0.29.


Diffs (updated)
-

  include/mesos/authorizer/authorizer.proto 
3ff67858a99915e0215f3ffb9966f9ac4a3fba8c 
  src/authorizer/local/authorizer.cpp 7ddb323df09a9b0ea46c6f9543c4af059d184308 
  src/master/master.cpp 6442762c9fdfa368d5d9d7cd43b97f5addaf7f17 
  src/tests/authorization_tests.cpp 54bfb46a807677f4a4a2bb88dcb78a358cf5121a 

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


Testing
---

Tested on a range of Linux configurations on internal CI.


Thanks,

Benjamin Bannier

Re: Review Request 47894: Renamed 'xfs/disk' isolator to 'disk/xfs'.

[Jiang Yan Xu]

> On May 27, 2016, 2:21 p.m., James Peach wrote:
> > I guess not many people are using this, but consider whether you should 
> > keep the old name at well for backwards compatibility.

This is pre-release feature so I think we shuold be fine if this rename lands 
before 0.29.0 is cut. For /r/47895/ we should totally maintain 
backwards-compatibility but for this I don't think it's expected.


- Jiang Yan


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


On May 26, 2016, 8:05 a.m., Jiang Yan Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47894/
> ---
> 
> (Updated May 26, 2016, 8:05 a.m.)
> 
> 
> Review request for mesos, Jie Yu and James Peach.
> 
> 
> Bugs: MESOS-5394
> https://issues.apache.org/jira/browse/MESOS-5394
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Renamed 'xfs/disk' isolator to 'disk/xfs'.
> 
> 
> Diffs
> -
> 
>   CHANGELOG d9b155dbb3beffc782de58b2a9478fd1554184a6 
>   docs/mesos-containerizer.md 22dffe00b315148f4ca158b0b606f254b9ee53c2 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 75e5a32a3e70ec60a6800e21a621673184ea0956 
>   src/tests/containerizer/xfs_quota_tests.cpp 
> ce16006f1a370b9b48837abee729669b84a885a2 
> 
> Diff: https://reviews.apache.org/r/47894/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>

Re: Review Request 47894: Renamed 'xfs/disk' isolator to 'disk/xfs'.

[James Peach]

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



I guess not many people are using this, but consider whether you should keep 
the old name at well for backwards compatibility.

- James Peach


On May 26, 2016, 3:05 p.m., Jiang Yan Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47894/
> ---
> 
> (Updated May 26, 2016, 3:05 p.m.)
> 
> 
> Review request for mesos, Jie Yu and James Peach.
> 
> 
> Bugs: MESOS-5394
> https://issues.apache.org/jira/browse/MESOS-5394
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Renamed 'xfs/disk' isolator to 'disk/xfs'.
> 
> 
> Diffs
> -
> 
>   CHANGELOG d9b155dbb3beffc782de58b2a9478fd1554184a6 
>   docs/mesos-containerizer.md 22dffe00b315148f4ca158b0b606f254b9ee53c2 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 75e5a32a3e70ec60a6800e21a621673184ea0956 
>   src/tests/containerizer/xfs_quota_tests.cpp 
> ce16006f1a370b9b48837abee729669b84a885a2 
> 
> Diff: https://reviews.apache.org/r/47894/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>

Re: Review Request 47977: Added tests for VIEW_* authorization actions.

[Mesos ReviewBot]

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



Bad patch!

Reviews applied: [47977, 47736, 47704, 47559, 47558, 47875, 46613, 47069, 
47490, 47453]

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

Error:
2016-05-27 20:44:57 URL:https://reviews.apache.org/r/47558/diff/raw/ 
[28602/28602] -> "47558.patch" [1]
error: patch failed: src/authorizer/local/authorizer.cpp:248
error: src/authorizer/local/authorizer.cpp: patch does not apply

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

- Mesos ReviewBot


On May 27, 2016, 8:04 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47977/
> ---
> 
> (Updated May 27, 2016, 8:04 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> As the new VIEW_* actions support multiple optional
> fields in the actions we need a number of different
> combinations here. Note that testing the request based
> interface also tests the same request via new
> authorizer interface implicitly.
> 
> 
> Diffs
> -
> 
>   src/tests/authorization_tests.cpp 54bfb46a807677f4a4a2bb88dcb78a358cf5121a 
> 
> Diff: https://reviews.apache.org/r/47977/diff/
> 
> 
> Testing
> ---
> 
> make check (OSx).
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 47971: Remove subnet prefix length from IP address.

[Dan Osborne]

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

(Updated May 27, 2016, 8:42 p.m.)


Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

Remove the prefix length included in the IP address string
that is returned by the CNI plugin before storing the value
in NetworkInfo.IPAddress.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
2ba3d62041379f5175b58f8c1ba3c2eae73edcf6 

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


Testing
---


Thanks,

Dan Osborne

Re: Review Request 47921: Enabled authorization for Mesos log access.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [47794, 47795, 47921]

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

- Mesos ReviewBot


On May 27, 2016, 6:08 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47921/
> ---
> 
> (Updated May 27, 2016, 6:08 p.m.)
> 
> 
> Review request for mesos, Adam B, Joerg Schad, and Michael Park.
> 
> 
> Bugs: MESOS-5153
> https://issues.apache.org/jira/browse/MESOS-5153
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Uses the authorization primitives in `mesos::internal::Files` to add
> protection of the Mesos logs on both master and agents.
> 
> 
> Diffs
> -
> 
>   include/mesos/authorizer/acls.proto 
> b05586ae587edbf9330f1d916340447d157ba80e 
>   include/mesos/authorizer/authorizer.proto 
> 3ff67858a99915e0215f3ffb9966f9ac4a3fba8c 
>   src/authorizer/local/authorizer.cpp 
> 7ddb323df09a9b0ea46c6f9543c4af059d184308 
>   src/master/master.hpp 1a875c32eddfb6d884e3d0dda7f5716ee53966c3 
>   src/master/master.cpp 6442762c9fdfa368d5d9d7cd43b97f5addaf7f17 
>   src/slave/slave.hpp f48e4b6bbb87d1b8b03176f68fe8d5ea6c109652 
>   src/slave/slave.cpp 9fcf334a69ae96ff8180df50aab571fac99b6fad 
> 
> Diff: https://reviews.apache.org/r/47921/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> and the script
> 
> ```bash
> #! /usr/bin/env bash
> 
> rm -rf /tmp/mesos/*
> 
> cat < /tmp/credentials.txt
> foo bar
> baz bar
> EOF
> 
> cat < /tmp/acls.json
> {
>   "permissive": false,
>   "access_mesos_log" : [
> {
>   "principals" : { "values" : ["foo"] },
>   "logs" : { "type" : "ANY" }
> }
>   ]
> }
> EOF
> 
> ./bin/mesos-master.sh --work_dir=/tmp/mesos/master \
>   --authenticate_http \
>   --credentials=file:///tmp/credentials.txt \
>   --acls=file:///tmp/acls.json \
>   --log_dir=/tmp/mesos/logs/master &
> ./bin/mesos-slave.sh --work_dir=/tmp/mesos/agent \
>  --master=127.0.0.1:5050 \
>  --authenticate_http \
>  --http_credentials=file:///tmp/credentials.txt \
>  --acls=file:///tmp/acls.json \
>  --log_dir=/tmp/mesos/logs/agent &
> 
> # This should yield a 200 OK response
> http GET http://127.0.0.1:5051/files/download?path=/slave/log -a foo:bar
> 
> # This should yield a 200 OK response
> http GET http://127.0.0.1:5050/files/download?path=/master/log/ -a foo:bar
> 
> 
> # This shold yield a 403 Forbidden response
> http GET http://127.0.0.1:5051/files/download?path=/slave/log -a baz:bar
> 
> # This shold yield a 403 Forbidden response
> http GET http://127.0.0.1:5050/files/download?path=/master/log/ -a baz:bar
> ```
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 47736: Used TaskObjectAllower to filter /tasks endpoint.

[Joerg Schad]

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

(Updated May 27, 2016, 8:13 p.m.)


Review request for mesos, Adam B and Michael Park.


Summary (updated)
-

Used TaskObjectAllower to filter /tasks endpoint.


Repository: mesos


Description (updated)
---

Used TaskObjectAllower to filter /tasks endpoint.


Diffs (updated)
-

  src/master/http.cpp b36b439a1fa07c52146deff2b90728f92676ade3 

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


Testing
---

make check (OSX)


Thanks,

Joerg Schad

Review Request 47977: Added tests for VIEW_* authorization actions.

[Joerg Schad]

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

Review request for mesos and Michael Park.


Repository: mesos


Description
---

As the new VIEW_* actions support multiple optional
fields in the actions we need a number of different
combinations here. Note that testing the request based
interface also tests the same request via new
authorizer interface implicitly.


Diffs
-

  src/tests/authorization_tests.cpp 54bfb46a807677f4a4a2bb88dcb78a358cf5121a 

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


Testing
---

make check (OSx).


Thanks,

Joerg Schad

Re: Review Request 47971: Remove subnet prefix length from IP address.

[Jie Yu]

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




src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (line 1065)


We use camelCase for variables in Mesos. `s/ip_address/ipAddress`



src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (lines 1065 - 1067)


I would move this down after we get `ip`. No need to do this if `parse` 
below fails, right?



src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (lines 1088 - 1090)


Hum, you're parsing an ipv6 address using AF_INET? I don't think 
IPNetwork::parse supports AF_INET6 yet.

You should either add IPV6 support to `parse`, or add a TODO here.


- Jie Yu


On May 27, 2016, 5:19 p.m., Dan Osborne wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47971/
> ---
> 
> (Updated May 27, 2016, 5:19 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-5453
> https://issues.apache.org/jira/browse/MESOS-5453
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Remove the prefix length included in the IP address string
> that is returned by the CNI plugin before storing the value
> in NetworkInfo.IPAddress.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
> 2ba3d62041379f5175b58f8c1ba3c2eae73edcf6 
> 
> Diff: https://reviews.apache.org/r/47971/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Dan Osborne
> 
>

Re: Review Request 47921: Enabled authorization for Mesos log access.

[Alexander Rojas]

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

(Updated May 27, 2016, 8:07 p.m.)


Review request for mesos, Adam B, Joerg Schad, and Michael Park.


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


Repository: mesos


Description (updated)
---

Uses the authorization primitives in `mesos::internal::Files` to add
protection of the Mesos logs on both master and agents.


Diffs (updated)
-

  include/mesos/authorizer/acls.proto b05586ae587edbf9330f1d916340447d157ba80e 
  include/mesos/authorizer/authorizer.proto 
3ff67858a99915e0215f3ffb9966f9ac4a3fba8c 
  src/authorizer/local/authorizer.cpp 7ddb323df09a9b0ea46c6f9543c4af059d184308 
  src/master/master.hpp 1a875c32eddfb6d884e3d0dda7f5716ee53966c3 
  src/master/master.cpp 6442762c9fdfa368d5d9d7cd43b97f5addaf7f17 
  src/slave/slave.hpp f48e4b6bbb87d1b8b03176f68fe8d5ea6c109652 
  src/slave/slave.cpp 9fcf334a69ae96ff8180df50aab571fac99b6fad 

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


Testing
---

`make check`

and the script

```bash
#! /usr/bin/env bash

rm -rf /tmp/mesos/*

cat < /tmp/credentials.txt
foo bar
baz bar
EOF

cat < /tmp/acls.json
{
  "permissive": false,
  "access_mesos_log" : [
{
  "principals" : { "values" : ["foo"] },
  "logs" : { "type" : "ANY" }
}
  ]
}
EOF

./bin/mesos-master.sh --work_dir=/tmp/mesos/master \
  --authenticate_http \
  --credentials=file:///tmp/credentials.txt \
  --acls=file:///tmp/acls.json \
  --log_dir=/tmp/mesos/logs/master &
./bin/mesos-slave.sh --work_dir=/tmp/mesos/agent \
 --master=127.0.0.1:5050 \
 --authenticate_http \
 --http_credentials=file:///tmp/credentials.txt \
 --acls=file:///tmp/acls.json \
 --log_dir=/tmp/mesos/logs/agent &

# This should yield a 200 OK response
http GET http://127.0.0.1:5051/files/download?path=/slave/log -a foo:bar

# This should yield a 200 OK response
http GET http://127.0.0.1:5050/files/download?path=/master/log/ -a foo:bar


# This shold yield a 403 Forbidden response
http GET http://127.0.0.1:5051/files/download?path=/slave/log -a baz:bar

# This shold yield a 403 Forbidden response
http GET http://127.0.0.1:5050/files/download?path=/master/log/ -a baz:bar
```


Thanks,

Alexander Rojas

Re: Review Request 47921: Enabled authorization for Mesos log access.

[Alexander Rojas]

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

(Updated May 27, 2016, 8:08 p.m.)


Review request for mesos, Adam B, Joerg Schad, and Michael Park.


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


Repository: mesos


Description (updated)
---

Uses the authorization primitives in `mesos::internal::Files` to add
protection of the Mesos logs on both master and agents.


Diffs
-

  include/mesos/authorizer/acls.proto b05586ae587edbf9330f1d916340447d157ba80e 
  include/mesos/authorizer/authorizer.proto 
3ff67858a99915e0215f3ffb9966f9ac4a3fba8c 
  src/authorizer/local/authorizer.cpp 7ddb323df09a9b0ea46c6f9543c4af059d184308 
  src/master/master.hpp 1a875c32eddfb6d884e3d0dda7f5716ee53966c3 
  src/master/master.cpp 6442762c9fdfa368d5d9d7cd43b97f5addaf7f17 
  src/slave/slave.hpp f48e4b6bbb87d1b8b03176f68fe8d5ea6c109652 
  src/slave/slave.cpp 9fcf334a69ae96ff8180df50aab571fac99b6fad 

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


Testing
---

`make check`

and the script

```bash
#! /usr/bin/env bash

rm -rf /tmp/mesos/*

cat < /tmp/credentials.txt
foo bar
baz bar
EOF

cat < /tmp/acls.json
{
  "permissive": false,
  "access_mesos_log" : [
{
  "principals" : { "values" : ["foo"] },
  "logs" : { "type" : "ANY" }
}
  ]
}
EOF

./bin/mesos-master.sh --work_dir=/tmp/mesos/master \
  --authenticate_http \
  --credentials=file:///tmp/credentials.txt \
  --acls=file:///tmp/acls.json \
  --log_dir=/tmp/mesos/logs/master &
./bin/mesos-slave.sh --work_dir=/tmp/mesos/agent \
 --master=127.0.0.1:5050 \
 --authenticate_http \
 --http_credentials=file:///tmp/credentials.txt \
 --acls=file:///tmp/acls.json \
 --log_dir=/tmp/mesos/logs/agent &

# This should yield a 200 OK response
http GET http://127.0.0.1:5051/files/download?path=/slave/log -a foo:bar

# This should yield a 200 OK response
http GET http://127.0.0.1:5050/files/download?path=/master/log/ -a foo:bar


# This shold yield a 403 Forbidden response
http GET http://127.0.0.1:5051/files/download?path=/slave/log -a baz:bar

# This shold yield a 403 Forbidden response
http GET http://127.0.0.1:5050/files/download?path=/master/log/ -a baz:bar
```


Thanks,

Alexander Rojas

Review Request 47971: Remove subnet prefix length from IP address.

[Dan Osborne]

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

Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

Remove the prefix length included in the IP address string
that is returned by the CNI plugin before storing the value
in NetworkInfo.IPAddress.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
2ba3d62041379f5175b58f8c1ba3c2eae73edcf6 

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


Testing
---


Thanks,

Dan Osborne

Review Request 47973: Updated gc to prevent early exit in case of error.

[Megha Sharma]

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

Review request for mesos.


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


Repository: mesos


Description
---

Updates made to mesos gc to prevent early exit in the event
of error.


Diffs
-

  src/slave/gc.cpp eedb8ca713d7f5195055bb6417f03ab70731af97 
  src/tests/gc_tests.cpp 4cb7c2f612984f7f5a9378a7f972f2438bbf28c5 

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


Testing
---

make check.


Thanks,

Megha Sharma

Review Request 47972: Updated rmdir to continue deletion on error.

[Megha Sharma]

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

Review request for mesos and Jiang Yan Xu.


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


Repository: mesos


Description
---

Updates made to rmdir to prevent early exit in the event of error.


Diffs
-

  3rdparty/stout/include/stout/os/posix/rmdir.hpp 
a6d3b578762d5c570542b0497578c330820b821a 
  3rdparty/stout/include/stout/os/windows/rmdir.hpp 
772b86dd111d28aefbeeba5f829ffa377fd12efb 
  3rdparty/stout/tests/os/rmdir_tests.cpp 
a11bfc9f9e6cbb05f3e9ce0ea48297b8f88fe53f 

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


Testing
---

make check.


Thanks,

Megha Sharma

Re: Review Request 47952: Added a stout test for missing required flags.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [46620, 46814, 46621, 47078, 46003, 46004, 45562, 47952]

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

- Mesos ReviewBot


On May 27, 2016, 11:56 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47952/
> ---
> 
> (Updated May 27, 2016, 11:56 a.m.)
> 
> 
> Review request for mesos, Michael Park and Vinod Kone.
> 
> 
> Bugs: MESOS-5064
> https://issues.apache.org/jira/browse/MESOS-5064
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a stout test for missing required flags.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/tests/flags_tests.cpp 
> ecf65b59c2662f893d843e7e49bc788607241bb6 
> 
> Diff: https://reviews.apache.org/r/47952/diff/
> 
> 
> Testing
> ---
> 
> `make check` on OSX.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 37989: Enhanced log message when launching mesos docker executor.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [37989]

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

- Mesos ReviewBot


On May 27, 2016, 9:15 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37989/
> ---
> 
> (Updated May 27, 2016, 9:15 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, haosdent huang, Jie Yu, and Joseph Wu.
> 
> 
> Bugs: MESOS-5348
> https://issues.apache.org/jira/browse/MESOS-5348
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enhanced log message when launching mesos docker executor.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.cpp 7af14f4472283ceefd73c06dd8df60af4cf6f7e8 
> 
> Diff: https://reviews.apache.org/r/37989/diff/
> 
> 
> Testing
> ---
> 
> I0511 11:26:18.176844 29178 docker.cpp:1236] Launching 
> 'mesos-docker-executor' with flags 
> '--container="mesos-7f581417-e295-474f-b038-3900ee0479dc-S1.b19cbf40-d69f-4507-9019-dbe61b9eff6b"
>  --docker="docker" --docker_socket="/var/run/docker.sock" --help="false" 
> --initialize_driver_logging="true" 
> --launcher_dir="/root/src/mesos/m1/mesos/build/src" --logbufsecs="0" 
> --logging_level="INFO" --mapped_directory="/mnt/mesos/sandbox" 
> --quiet="false" 
> --sandbox_directory="/tmp/mesos/slaves/7f581417-e295-474f-b038-3900ee0479dc-S1/frameworks/7f581417-e295-474f-b038-3900ee0479dc-0001/executors/test_mesos/runs/b19cbf40-d69f-4507-9019-dbe61b9eff6b"
>  --stop_timeout="0ns"'
> 
> Start agent without VLOG_v=1, once the docker container is started, check 
> sandbox log:
> root@mesos002:/tmp/mesos/slaves/fb930730-03f9-4027-96cb-81d612e8e35f-S0/frameworks/fb930730-03f9-4027-96cb-81d612e8e35f-/executors/test/runs/latest#
>  cat stderr
> I0526 22:25:59.805173 11422 logging.cpp:195] Logging to STDERR
> I0526 22:25:59.808446 11422 process.cpp:1057] libprocess is initialized on 
> 192.168.56.12:36164 with 8 worker threads
> I0526 22:25:59.809834 11422 exec.cpp:150] Version: 0.29.0
> I0526 22:25:59.813784 11445 exec.cpp:200] Executor started at: 
> executor(1)@192.168.56.12:36164 with pid 11422
> I0526 22:25:59.818483 11446 exec.cpp:225] Executor registered on agent 
> fb930730-03f9-4027-96cb-81d612e8e35f-S0
> I0526 22:25:59.819725 11446 exec.cpp:237] Executor::registered took 272279ns
> I0526 22:25:59.820055 11446 exec.cpp:312] Executor asked to run task 'test'
> I0526 22:25:59.820132 11446 exec.cpp:321] Executor::launchTask took 55859ns
> I0526 22:25:59.820344 11446 docker.cpp:677] Running docker -H 
> unix:///var/run/docker.sock run --cpu-shares 1024 --memory 134217728 -e 
> MESOS_SANDBOX=/mnt/mesos/sandbox -e 
> MESOS_CONTAINER_NAME=mesos-fb930730-03f9-4027-96cb-81d612e8e35f-S0.7accfa4d-909b-4e46-bdf7-6bcdf505f6d6
>  -v 
> /tmp/mesos/slaves/fb930730-03f9-4027-96cb-81d612e8e35f-S0/frameworks/fb930730-03f9-4027-96cb-81d612e8e35f-/executors/test/runs/7accfa4d-909b-4e46-bdf7-6bcdf505f6d6:/mnt/mesos/sandbox
>  --net host --entrypoint /bin/sh --name 
> mesos-fb930730-03f9-4027-96cb-81d612e8e35f-S0.7accfa4d-909b-4e46-bdf7-6bcdf505f6d6
>  ubuntu:14.04 -c sleep 10
> I0526 22:25:59.822684 11446 docker.cpp:824] Running docker -H 
> unix:///var/run/docker.sock inspect 
> mesos-fb930730-03f9-4027-96cb-81d612e8e35f-S0.7accfa4d-909b-4e46-bdf7-6bcdf505f6d6
> WARNING: Your kernel does not support swap limit capabilities, memory limited 
> without swap.
> I0526 22:25:59.924623 11445 docker.cpp:871] Retrying inspect with non-zero 
> status code. cmd: 'docker -H unix:///var/run/docker.sock inspect 
> mesos-fb930730-03f9-4027-96cb-81d612e8e35f-S0.7accfa4d-909b-4e46-bdf7-6bcdf505f6d6',
>  interval: 500ms
> I0526 22:26:00.425278 11449 docker.cpp:824] Running docker -H 
> unix:///var/run/docker.sock inspect 
> mesos-fb930730-03f9-4027-96cb-81d612e8e35f-S0.7accfa4d-909b-4e46-bdf7-6bcdf505f6d6
> I0526 22:26:00.544518 11443 exec.cpp:535] Executor sending status update 
> TASK_RUNNING (UUID: b686d7d1-110f-4f42-a392-b9ed9a854d31) for task test of 
> framework fb930730-03f9-4027-96cb-81d612e8e35f-
> I0526 22:26:00.549468 11442 exec.cpp:358] Executor received status update 
> acknowledgement b686d7d1-110f-4f42-a392-b9ed9a854d31 for task test of 
> framework fb930730-03f9-4027-96cb-81d612e8e35f-
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Review Request 47952: Added a stout test for missing required flags.

[Greg Mann]

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

Review request for mesos, Michael Park and Vinod Kone.


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


Repository: mesos


Description
---

Added a stout test for missing required flags.


Diffs
-

  3rdparty/stout/tests/flags_tests.cpp ecf65b59c2662f893d843e7e49bc788607241bb6 

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


Testing
---

`make check` on OSX.


Thanks,

Greg Mann

Re: Review Request 45562: Edited `--work_dir` configuration docs.

[Greg Mann]

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

(Updated May 27, 2016, 11:52 a.m.)


Review request for mesos and Michael Park.


Changes
---

Addressed comments.


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


Repository: mesos


Description
---

Some users have encountered difficulty when running the Mesos agent
with the `work_dir` located in a subdirectory of `/tmp`. This patch
adds language to the `work_dir` help strings and configuration docs
advising users to avoid the use of this location in production.


Diffs (updated)
-

  docs/configuration.md ae9067d65535ff26e8fd77088748299c91511087 

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


Testing
---

Viewed with the Mesos website container: 
https://github.com/mesosphere/mesos-website-container


Thanks,

Greg Mann

Re: Review Request 46004: Updated master '--work_dir' help string.

[Greg Mann]

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

(Updated May 27, 2016, 11:51 a.m.)


Review request for mesos and Michael Park.


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


Repository: mesos


Description
---

Adds language to the master's `--work_dir` help string which advises
users to avoid locations in '/tmp' when setting the work directory.


Diffs (updated)
-

  src/master/flags.cpp ceb4bd4f863d3857eede8287041a53d66f1c9e4c 

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


Testing
---

`sudo make check`


Thanks,

Greg Mann

Re: Review Request 46003: Removed the default value for agent work_dir.

[Greg Mann]

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

(Updated May 27, 2016, 11:47 a.m.)


Review request for mesos, Michael Park and Vinod Kone.


Changes
---

Addressed comments.


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


Repository: mesos


Description
---

The default value for the agent's `--work_dir` flag was removed, the
type of the parameter was changed to `Option`, and code was
added to explicitly set the work directory when necessary.


Diffs (updated)
-

  src/local/local.cpp c0c2766c9317b53a27eb50f6df1eb6ffd556a4cc 
  src/slave/flags.cpp d3543a019e57658cf60879f40c5f0834a1b8e836 
  src/tests/containerizer/mesos_containerizer_tests.cpp 
09742ff21513dc2570684d384b257868dd57a9ce 

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


Testing
---

`sudo make check`


Thanks,

Greg Mann

Re: Review Request 47078: Added checking for required flags to FlagsBase.

[Greg Mann]

> On May 11, 2016, 10:39 p.m., Vinod Kone wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp, line 149
> > 
> >
> > make this `const Option&` instead?

Per our discussion, I'm going to stick with `T2*` due to problems passing 
string literals when using `Option`. I added a comment to this effect, and 
linked a JIRA.


- Greg


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


On May 27, 2016, 11:29 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47078/
> ---
> 
> (Updated May 27, 2016, 11:29 a.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-5064
> https://issues.apache.org/jira/browse/MESOS-5064
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added checking for required flags to FlagsBase.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/flags/flag.hpp 
> 9be35c61c4f40b4bb9e7bcb29e07ae8b35f6e20b 
>   3rdparty/stout/include/stout/flags/flags.hpp 
> 91542f56431579be8a3da5ec6e3e58f68e7dfcbe 
> 
> Diff: https://reviews.apache.org/r/47078/diff/
> 
> 
> Testing
> ---
> 
> `make check` on OSX.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 47078: Added checking for required flags to FlagsBase.

[Greg Mann]

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

(Updated May 27, 2016, 11:29 a.m.)


Review request for mesos and Michael Park.


Changes
---

Addressed comments.


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


Repository: mesos


Description
---

Added checking for required flags to FlagsBase.


Diffs (updated)
-

  3rdparty/stout/include/stout/flags/flag.hpp 
9be35c61c4f40b4bb9e7bcb29e07ae8b35f6e20b 
  3rdparty/stout/include/stout/flags/flags.hpp 
91542f56431579be8a3da5ec6e3e58f68e7dfcbe 

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


Testing
---

`make check` on OSX.


Thanks,

Greg Mann

Re: Review Request 47521: Added creator principal to '/create-volumes' tests.

[Bernd Mathiske]

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


Ship it!




Ship It!

- Bernd Mathiske


On May 27, 2016, 3:55 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47521/
> ---
> 
> (Updated May 27, 2016, 3:55 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, and Neil Conway.
> 
> 
> Bugs: MESOS-5005
> https://issues.apache.org/jira/browse/MESOS-5005
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> A creator principal is added to the persistent volumes
> used in the PersistentVolumeEndpointsTests.
> 
> 
> Diffs
> -
> 
>   src/tests/persistent_volume_endpoints_tests.cpp 
> a57461d881b2bf0175f83b50b0a46167acd5bd3e 
> 
> Diff: https://reviews.apache.org/r/47521/diff/
> 
> 
> Testing
> ---
> 
> `make check` was used to test on OSX.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 47944: Set timestamp when updating task status from long lived framework.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [47944]

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

- Mesos ReviewBot


On May 27, 2016, 7:17 a.m., zhou xing wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47944/
> ---
> 
> (Updated May 27, 2016, 7:17 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: mesos-5290
> https://issues.apache.org/jira/browse/mesos-5290
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add the step of setting the status update timestamp for long
> lived framework executor.
> 
> 
> Diffs
> -
> 
>   src/examples/long_lived_executor.cpp 
> 770aae2fcc7a11db9abab31262cf81967796bb23 
> 
> Diff: https://reviews.apache.org/r/47944/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> Launch a master and an agent
> Run long lived framework against master
> 
> Open browser, goto 127.0.0.1:5050 and switch to frameworks tab -> long lived 
> framework
> the page shall show the active and finished tasks of long lived framework, 
> instead of showing "46 years ago", it should show "just now" or the current 
> timestamp
> 
> 
> Thanks,
> 
> zhou xing
> 
>

Re: Review Request 47521: Added creator principal to '/create-volumes' tests.

[Greg Mann]

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

(Updated May 27, 2016, 10:55 a.m.)


Review request for mesos, Bernd Mathiske, Joerg Schad, and Neil Conway.


Changes
---

Removed unnecessary reserver principals.


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


Repository: mesos


Description
---

A creator principal is added to the persistent volumes
used in the PersistentVolumeEndpointsTests.


Diffs (updated)
-

  src/tests/persistent_volume_endpoints_tests.cpp 
a57461d881b2bf0175f83b50b0a46167acd5bd3e 

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


Testing
---

`make check` was used to test on OSX.


Thanks,

Greg Mann

Re: Review Request 47521: Added creator principal to '/create-volumes' tests.

[Greg Mann]

> On May 27, 2016, 9:36 a.m., Bernd Mathiske wrote:
> > src/tests/persistent_volume_endpoints_tests.cpp, line 1011
> > 
> >
> > In the above case, this part is within the scope block, here it is not. 
> > Please move the opening curly brace up.

The volume is left out of the scope here because it gets used much later in the 
test. If we scoped it, we would have to extend the scope nearly to the end of 
the test. What do you think?


- Greg


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


On May 27, 2016, 5:37 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47521/
> ---
> 
> (Updated May 27, 2016, 5:37 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, and Neil Conway.
> 
> 
> Bugs: MESOS-5005
> https://issues.apache.org/jira/browse/MESOS-5005
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> A creator principal is added to the persistent volumes
> used in the PersistentVolumeEndpointsTests.
> 
> 
> Diffs
> -
> 
>   src/tests/persistent_volume_endpoints_tests.cpp 
> a57461d881b2bf0175f83b50b0a46167acd5bd3e 
> 
> Diff: https://reviews.apache.org/r/47521/diff/
> 
> 
> Testing
> ---
> 
> `make check` was used to test on OSX.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 47522: Added creator principal to persistent volume tests.

[Greg Mann]

> On May 27, 2016, 9:47 a.m., Bernd Mathiske wrote:
> > src/tests/persistent_volume_tests.cpp, line 1448
> > 
> >
> > As we have "volume and subsequent statements in a scope block above, 
> > can we have the same here, please?

This instance doesn't turn out quite as elegant as the previous one, since this 
`volume` gets used for the remainder of the test. If we scoped it, the scope 
would need to extend nearly to the end of the test. What do you think?


- Greg


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


On May 27, 2016, 6:25 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47522/
> ---
> 
> (Updated May 27, 2016, 6:25 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, and Neil Conway.
> 
> 
> Bugs: MESOS-5005
> https://issues.apache.org/jira/browse/MESOS-5005
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds a creator principal to the
> persistent volumes used in the PersistentVolumeTests.
> 
> 
> Diffs
> -
> 
>   src/tests/persistent_volume_tests.cpp 
> d246f35046fff469b847c908de2b305ae629212f 
> 
> Diff: https://reviews.apache.org/r/47522/diff/
> 
> 
> Testing
> ---
> 
> `make check` was used to test on OSX at the end of this review chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 47603: Agent:[2/2] Added Windows support for folder `launcher/`.

[Mesos ReviewBot]

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



Bad patch!

Reviews applied: [47603, 47602, 47576, 47536, 47472, 47470, 47469, 47942, 
47412, 47411, 47410, 47409, 47404, 47403, 47391, 47390, 47389, 47388, 47387, 
47386, 47169, 47168, 41632, 47054, 47221, 47053, 47052]

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

Error:
2016-05-27 09:54:01 URL:https://reviews.apache.org/r/47576/diff/raw/ 
[14272/14272] -> "47576.patch" [1]
error: patch failed: src/slave/containerizer/mesos/containerizer.cpp:1182
error: src/slave/containerizer/mesos/containerizer.cpp: patch does not apply

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

- Mesos ReviewBot


On May 27, 2016, 6:57 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47603/
> ---
> 
> (Updated May 27, 2016, 6:57 a.m.)
> 
> 
> Review request for mesos, Daniel Pravat, Artem Harutyunyan, Joris Van 
> Remoortere, and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Agent:[2/2] Added Windows support for folder `launcher/`.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 710e1644e2f0a8e9b87cc997b2211291f4e055fd 
>   src/exec/exec.cpp a2e6d86fd0b1f3d688d17296151db74bcb9b3418 
>   src/launcher/executor.hpp PRE-CREATION 
>   src/launcher/executor.cpp fa4a89e97b0c427b4595a6adebc397aeb5bfaaa5 
>   src/launcher/posix/executor.hpp PRE-CREATION 
>   src/launcher/windows/executor.hpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/47603/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 47516: Added creator principal to tests.

[Bernd Mathiske]

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


Ship it!




Ship It!

- Bernd Mathiske


On May 26, 2016, 11:28 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47516/
> ---
> 
> (Updated May 26, 2016, 11:28 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, and Neil Conway.
> 
> 
> Bugs: MESOS-5005
> https://issues.apache.org/jira/browse/MESOS-5005
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds a creator principal to the persistent
> volumes used in various tests.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> f43165388f29513ab8be6594ab6647e8f9eb5a93 
>   src/tests/containerizer/filesystem_isolator_tests.cpp 
> 4293416ac8434e9eb7e80724480a54936a2fe24a 
>   src/tests/disk_quota_tests.cpp 1564f70854ff5630da1d7348a034f726d67b1757 
>   src/tests/reservation_tests.cpp 2d7fb21e2fe153c2b62dfd60bbaccb350a157391 
>   src/tests/role_tests.cpp 24959d6e0f83ef7b62b0586be18661aa3cac91dd 
> 
> Diff: https://reviews.apache.org/r/47516/diff/
> 
> 
> Testing
> ---
> 
> `make check` was used to test on OSX at the end of this review chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 47522: Added creator principal to persistent volume tests.

[Bernd Mathiske]

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




src/tests/persistent_volume_tests.cpp (line 1448)


As we have "volume and subsequent statements in a scope block above, can we 
have the same here, please?



src/tests/persistent_volume_tests.cpp (line 1665)


Same here: above block around "volume" with braces, this one without.


- Bernd Mathiske


On May 26, 2016, 11:25 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47522/
> ---
> 
> (Updated May 26, 2016, 11:25 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, and Neil Conway.
> 
> 
> Bugs: MESOS-5005
> https://issues.apache.org/jira/browse/MESOS-5005
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds a creator principal to the
> persistent volumes used in the PersistentVolumeTests.
> 
> 
> Diffs
> -
> 
>   src/tests/persistent_volume_tests.cpp 
> d246f35046fff469b847c908de2b305ae629212f 
> 
> Diff: https://reviews.apache.org/r/47522/diff/
> 
> 
> Testing
> ---
> 
> `make check` was used to test on OSX at the end of this review chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 47608: Add Labels from TaskInfo into TaskStatus message.

[Guangya Liu]

> On 五月 25, 2016, 4:41 p.m., Shuai Lin wrote:
> > src/slave/slave.cpp, line 3454
> > 
> >
> > Should we also search in in `termiantedTasks`?
> > 
> > ```cpp
> >   // Terminated but pending updates.
> >   LinkedHashMap terminatedTasks;
> > ```

The Executor::updateTaskState now only update status for launchedTasks, why do 
you want to update labels for termiantedTasks? What is the use of labeles for 
termiantedTasks?


- Guangya


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


On 五月 20, 2016, 6:38 p.m., Srinivas Brahmaroutu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47608/
> ---
> 
> (Updated 五月 20, 2016, 6:38 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jojy Varghese.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> WIP: Add Labels from TaskInfo into TaskStatus message.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp ce0e7b1f1d17c3b82d835b0a6296ed7b1e9eeac1 
>   src/tests/hook_tests.cpp c6b4e8a50534e455cf44f427b09f74eef71177b2 
>   src/tests/slave_tests.cpp e1f5bfe074c357f46403887365b3f9ae554000b4 
> 
> Diff: https://reviews.apache.org/r/47608/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Srinivas Brahmaroutu
> 
>

Re: Review Request 47608: Add Labels from TaskInfo into TaskStatus message.

[Guangya Liu]

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



Srini, please add MESOS-4135 to `Bugs` filed at right-top of the page.


src/slave/slave.cpp (line 3454)


The `Executor::updateTaskState` now only update status for `launchedTasks`, 
why do you want to update `labels` for `termiantedTasks`? What is the use of 
`labeles` for `termiantedTasks`?



src/slave/slave.cpp (line 3455)


I think you can update `Executor::updateTaskState` directly 
https://github.com/apache/mesos/blob/master/src/slave/slave.cpp#L6132-L6144


- Guangya Liu


On 五月 20, 2016, 6:38 p.m., Srinivas Brahmaroutu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47608/
> ---
> 
> (Updated 五月 20, 2016, 6:38 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jojy Varghese.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> WIP: Add Labels from TaskInfo into TaskStatus message.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp ce0e7b1f1d17c3b82d835b0a6296ed7b1e9eeac1 
>   src/tests/hook_tests.cpp c6b4e8a50534e455cf44f427b09f74eef71177b2 
>   src/tests/slave_tests.cpp e1f5bfe074c357f46403887365b3f9ae554000b4 
> 
> Diff: https://reviews.apache.org/r/47608/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Srinivas Brahmaroutu
> 
>

Re: Review Request 47805: Add authorization to GET /weights.

[Adam B]

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



Great job! Fix these nits and we should be ready to ship.


include/mesos/authorizer/acls.proto (line 271)


s/GetWeights/GetWeight/ since the other ACL messages are singular. Note 
that `get_weights` is still correct though.



include/mesos/authorizer/authorizer.proto (line 85)


Since this action applies to a single role, and a single weight, let's do 
`GET_WEIGHT_WITH_ROLE`



src/master/http.cpp (line 1262)


s/weights/weight/ since a given role only has a single weight.



src/master/master.hpp (line 1058)


If this only takes a single role, then it should be called 
`authorizeGetWeight`.



src/master/master.hpp (line 1068)


Do you need the "mesos::" here?



src/master/weights_handler.cpp (line 77)


This is not a list of roles that have been authorized (`authorizedRoles`), 
but a list of authorizations of various roles, so I'd suggest renaming this 
`roleAuthorizations`.



src/master/weights_handler.cpp (line 84)


Technically, this lambda should be indented to align with its fellow 
defer() parameter `master->self()`, or (since that looks too jagged) just 
indented 4 spaces in from the `.then`.
Looks like our convention for the `-> Future` after is 2 spaces in from the 
`[=]` like you have.



src/tests/dynamic_weights_tests.cpp (line 88)


Neat trick, but I think we're better off just specifying DEFAULT_CREDENTIAL 
at those call sites


- Adam B


On May 26, 2016, 10:45 p.m., zhou xing wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47805/
> ---
> 
> (Updated May 26, 2016, 10:45 p.m.)
> 
> 
> Review request for mesos and Adam B.
> 
> 
> Bugs: mesos-5335
> https://issues.apache.org/jira/browse/mesos-5335
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add 'GET_WEIGHTS_WITH_ROLE' for the authorization of GET /weights.
> 
> 
> Diffs
> -
> 
>   docs/endpoints/master/weights.md 1e540795c5fb90c8ea957dd444b9e564e0b1ac23 
>   include/mesos/authorizer/acls.proto 
> ace9b698f46e143795c82324a87a0d7827fb 
>   include/mesos/authorizer/authorizer.proto 
> 02d1a01d57cf34b38524f4368187878b03343537 
>   src/authorizer/local/authorizer.cpp 
> 3c7c791bde65cfcbcc4e319c9ccc487ab37d8029 
>   src/master/http.cpp b36b439a1fa07c52146deff2b90728f92676ade3 
>   src/master/master.hpp 1a875c32eddfb6d884e3d0dda7f5716ee53966c3 
>   src/master/weights_handler.cpp 4bc060fdb015df6658194eef92fe11b14aa15c79 
>   src/tests/dynamic_weights_tests.cpp 
> 362c59aae7b305710d5985bfec28f881be3b64b8 
> 
> Diff: https://reviews.apache.org/r/47805/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> zhou xing
> 
>

Re: Review Request 47805: Add authorization to GET /weights.

[Adam B]

> On May 26, 2016, 3:30 a.m., Adam B wrote:
> > src/master/weights_handler.cpp, line 85
> > 
> >
> > s/=/request, weightInfos/
> > s/list/list&/
> 
> zhou xing wrote:
> Adam, this function also requires to capture 'this', which will be used 
> in the invocation of '_get()', can we keep using '=' here?

Yeah, ok. I'm usually in favor of being explicit when we can, but others favor 
readability. Dropping.


> On May 26, 2016, 3:30 a.m., Adam B wrote:
> > src/tests/dynamic_weights_tests.cpp, lines 492-494
> > 
> >
> > Seems like `checkWithGetRequest` should take the credential as a 
> > parameter, rather than hiding that credential-selection logic inside the 
> > check method.
> 
> zhou xing wrote:
> as in checkWithGetRequest, we are using the constant strings to identify 
> the result of get weights, so for 'get weights' specific test cases, I 
> suggest keep these two new constant strings. I will add a new credential 
> parameter to checkWithGetRequest method and remove the credential-selection 
> logic in it.

Thanks, but let's remove the default value too, so the credential must always 
be specified.


> On May 26, 2016, 3:30 a.m., Adam B wrote:
> > src/tests/dynamic_weights_tests.cpp, line 512
> > 
> >
> > Since this test already uses a principal, why not only allow that 
> > principal to get the weights?
> > Then you can also test that a get request without a principal (or with 
> > CREDENTIAL_2) will see no roles.
> 
> zhou xing wrote:
> Adam, yes, I will only allow DEFAULT_CREDENTIAL in these test cases, but 
> for the get check, as I already added a new test case to test "get weights 
> with role" only, I suggest not to add a new get request test in this test 
> case. Or we can merge the get weights test and update weights test together 
> into one, please let me know your comments, thanks.

If it's already covered by another test, we can leave it at that. Dropping.


- Adam


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


On May 26, 2016, 10:45 p.m., zhou xing wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47805/
> ---
> 
> (Updated May 26, 2016, 10:45 p.m.)
> 
> 
> Review request for mesos and Adam B.
> 
> 
> Bugs: mesos-5335
> https://issues.apache.org/jira/browse/mesos-5335
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add 'GET_WEIGHTS_WITH_ROLE' for the authorization of GET /weights.
> 
> 
> Diffs
> -
> 
>   docs/endpoints/master/weights.md 1e540795c5fb90c8ea957dd444b9e564e0b1ac23 
>   include/mesos/authorizer/acls.proto 
> ace9b698f46e143795c82324a87a0d7827fb 
>   include/mesos/authorizer/authorizer.proto 
> 02d1a01d57cf34b38524f4368187878b03343537 
>   src/authorizer/local/authorizer.cpp 
> 3c7c791bde65cfcbcc4e319c9ccc487ab37d8029 
>   src/master/http.cpp b36b439a1fa07c52146deff2b90728f92676ade3 
>   src/master/master.hpp 1a875c32eddfb6d884e3d0dda7f5716ee53966c3 
>   src/master/weights_handler.cpp 4bc060fdb015df6658194eef92fe11b14aa15c79 
>   src/tests/dynamic_weights_tests.cpp 
> 362c59aae7b305710d5985bfec28f881be3b64b8 
> 
> Diff: https://reviews.apache.org/r/47805/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> zhou xing
> 
>

Re: Review Request 47521: Added creator principal to '/create-volumes' tests.

[Bernd Mathiske]

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




src/tests/persistent_volume_endpoints_tests.cpp (line 660)


I don't think the reservation principal should be necessary here. Should 
not have any effect either way, right? But the documentation for this 
recommends not setting a reservation principal unless there is a reservation. 
Also I think we can avoid confusing by using None() here instead, so it is 
clear which parameter triggers the expected response.



src/tests/persistent_volume_endpoints_tests.cpp (line 718)


Same here.



src/tests/persistent_volume_endpoints_tests.cpp (line 1010)


In the above case, this part is within the scope block, here it is not. 
Please move the opening curly brace up.


- Bernd Mathiske


On May 26, 2016, 10:37 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47521/
> ---
> 
> (Updated May 26, 2016, 10:37 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, and Neil Conway.
> 
> 
> Bugs: MESOS-5005
> https://issues.apache.org/jira/browse/MESOS-5005
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> A creator principal is added to the persistent volumes
> used in the PersistentVolumeEndpointsTests.
> 
> 
> Diffs
> -
> 
>   src/tests/persistent_volume_endpoints_tests.cpp 
> a57461d881b2bf0175f83b50b0a46167acd5bd3e 
> 
> Diff: https://reviews.apache.org/r/47521/diff/
> 
> 
> Testing
> ---
> 
> `make check` was used to test on OSX.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 47516: Added creator principal to tests.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [47515, 47528, 47519, 47520, 47521, 47522, 47516]

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

- Mesos ReviewBot


On May 27, 2016, 6:28 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47516/
> ---
> 
> (Updated May 27, 2016, 6:28 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, and Neil Conway.
> 
> 
> Bugs: MESOS-5005
> https://issues.apache.org/jira/browse/MESOS-5005
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds a creator principal to the persistent
> volumes used in various tests.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> f43165388f29513ab8be6594ab6647e8f9eb5a93 
>   src/tests/containerizer/filesystem_isolator_tests.cpp 
> 4293416ac8434e9eb7e80724480a54936a2fe24a 
>   src/tests/disk_quota_tests.cpp 1564f70854ff5630da1d7348a034f726d67b1757 
>   src/tests/reservation_tests.cpp 2d7fb21e2fe153c2b62dfd60bbaccb350a157391 
>   src/tests/role_tests.cpp 24959d6e0f83ef7b62b0586be18661aa3cac91dd 
> 
> Diff: https://reviews.apache.org/r/47516/diff/
> 
> 
> Testing
> ---
> 
> `make check` was used to test on OSX at the end of this review chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 37989: Enhanced log message when launching mesos docker executor.

[Guangya Liu]

> On 五月 27, 2016, 8:41 a.m., haosdent huang wrote:
> > src/slave/containerizer/docker.cpp, line 1195
> > 
> >
> > How about change those VLOG to LOG(INFO) instead of add a tricky hack 
> > here?
> > 
> > We have already LOG(INFO) any http requests, I think LOG(INFO) docker 
> > running commands is also acceptable?
> 
> Guangya Liu wrote:
> @haosdent, you can take a look at Joseph's comments on MESOS-5197.
> 
> --
> The src/docker/docker.(hpp|cpp) files are shared between the agent 
> (docker containerizer) and docker-command-executor. The INFO logging is 
> useful for the docker-command-executor, but is not so useful for the docker 
> containerizer.
> 
> haosdent huang wrote:
> Yes, last time @yongtang posted his patch and I saw that. I think this 
> should be a tradeoff issue. I agree should log dockerFlags, but seems add 
> `environment["GLOG_v"] = "1";` still not nice enough.

I think that customer cared most is tasks, so we should make sure the log in 
sandbox is clear enough; But for logs in docker containerizer, it is managed by 
mesos, customer can enable it via some agent flags. I'm adding Joseph here 
also, hope can we can get more comments. Thanks.


- Guangya


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


On 五月 27, 2016, 8:16 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37989/
> ---
> 
> (Updated 五月 27, 2016, 8:16 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-5348
> https://issues.apache.org/jira/browse/MESOS-5348
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enhanced log message when launching mesos docker executor.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.cpp 7af14f4472283ceefd73c06dd8df60af4cf6f7e8 
> 
> Diff: https://reviews.apache.org/r/37989/diff/
> 
> 
> Testing
> ---
> 
> I0511 11:26:18.176844 29178 docker.cpp:1236] Launching 
> 'mesos-docker-executor' with flags 
> '--container="mesos-7f581417-e295-474f-b038-3900ee0479dc-S1.b19cbf40-d69f-4507-9019-dbe61b9eff6b"
>  --docker="docker" --docker_socket="/var/run/docker.sock" --help="false" 
> --initialize_driver_logging="true" 
> --launcher_dir="/root/src/mesos/m1/mesos/build/src" --logbufsecs="0" 
> --logging_level="INFO" --mapped_directory="/mnt/mesos/sandbox" 
> --quiet="false" 
> --sandbox_directory="/tmp/mesos/slaves/7f581417-e295-474f-b038-3900ee0479dc-S1/frameworks/7f581417-e295-474f-b038-3900ee0479dc-0001/executors/test_mesos/runs/b19cbf40-d69f-4507-9019-dbe61b9eff6b"
>  --stop_timeout="0ns"'
> 
> Start agent without VLOG_v=1, once the docker container is started, check 
> sandbox log:
> root@mesos002:/tmp/mesos/slaves/fb930730-03f9-4027-96cb-81d612e8e35f-S0/frameworks/fb930730-03f9-4027-96cb-81d612e8e35f-/executors/test/runs/latest#
>  cat stderr
> I0526 22:25:59.805173 11422 logging.cpp:195] Logging to STDERR
> I0526 22:25:59.808446 11422 process.cpp:1057] libprocess is initialized on 
> 192.168.56.12:36164 with 8 worker threads
> I0526 22:25:59.809834 11422 exec.cpp:150] Version: 0.29.0
> I0526 22:25:59.813784 11445 exec.cpp:200] Executor started at: 
> executor(1)@192.168.56.12:36164 with pid 11422
> I0526 22:25:59.818483 11446 exec.cpp:225] Executor registered on agent 
> fb930730-03f9-4027-96cb-81d612e8e35f-S0
> I0526 22:25:59.819725 11446 exec.cpp:237] Executor::registered took 272279ns
> I0526 22:25:59.820055 11446 exec.cpp:312] Executor asked to run task 'test'
> I0526 22:25:59.820132 11446 exec.cpp:321] Executor::launchTask took 55859ns
> I0526 22:25:59.820344 11446 docker.cpp:677] Running docker -H 
> unix:///var/run/docker.sock run --cpu-shares 1024 --memory 134217728 -e 
> MESOS_SANDBOX=/mnt/mesos/sandbox -e 
> MESOS_CONTAINER_NAME=mesos-fb930730-03f9-4027-96cb-81d612e8e35f-S0.7accfa4d-909b-4e46-bdf7-6bcdf505f6d6
>  -v 
> /tmp/mesos/slaves/fb930730-03f9-4027-96cb-81d612e8e35f-S0/frameworks/fb930730-03f9-4027-96cb-81d612e8e35f-/executors/test/runs/7accfa4d-909b-4e46-bdf7-6bcdf505f6d6:/mnt/mesos/sandbox
>  --net host --entrypoint /bin/sh --name 
> mesos-fb930730-03f9-4027-96cb-81d612e8e35f-S0.7accfa4d-909b-4e46-bdf7-6bcdf505f6d6
>  ubuntu:14.04 -c sleep 10
> I0526 22:25:59.822684 11446 docker.cpp:824] Running docker -H 
> unix:///var/run/docker.sock inspect 
> mesos-fb930730-03f9-4027-96cb-81d612e8e35f-S0.7accfa4d-909b-4e46-bdf7-6bcdf505f6d6
> WARNING: Your kernel does not support swap limit capabilities, memory limited 
> without swap.
> I0526 22:25:59.924623 11445 docker.cpp:871] Retrying inspect with non-zero 
> status code. cmd: 

Re: Review Request 47071: Added framework/task filtering to /state and /tasks endpoint.

[Adam B]

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



Shall we discard this patch in favor of https://reviews.apache.org/r/47736/ ?

- Adam B


On May 9, 2016, 7:47 a.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47071/
> ---
> 
> (Updated May 9, 2016, 7:47 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The /state and /task endpoints output framework and task level data.
> Hence we need to authorize both frameworks and tasks.
> 
> 
> Diffs
> -
> 
>   src/authorizer/local/authorizer.cpp 
> e59c11269670a7ed72b780913971b421ee17f33f 
>   src/master/http.cpp 4985f24b70a00116caa4bd0335ea51e55d878d19 
>   src/master/master.hpp 3e55114ee7866e06513071e86e15608099dae052 
> 
> Diff: https://reviews.apache.org/r/47071/diff/
> 
> 
> Testing
> ---
> 
> make check (osx)
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 47891: Added RUN_TASK authorization action.

[Adam B]

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



Please update `Master::authorizeTask` to call your new/aliased action with 
additional FwkInfo/ExecInfo, plus value=user for those authorization modules 
that didn't want to modify any code yet. Please also update the user extraction 
code to include taskInfo.executorInfo, and order them as suggested. Thanks!


include/mesos/authorizer/authorizer.proto (line 56)


Is this deprecated or unused now?



include/mesos/authorizer/authorizer.proto (line 85)


s/or TaskInfp/and TaskInfo/



include/mesos/authorizer/authorizer.proto (line 86)


I wonder if we should just alias RUN_TASK to the same enum value as 
RUN_TASK_WITH_USER.. There shouldn't be any backwards compatibility issues 
since these are only used in-memory, and modules have to recompile anyway.



src/authorizer/local/authorizer.cpp (line 128)


This case will currently never be called. See Master::authorizeTask for 
where we call the authorizer with RUN_TASK_WITH_USER. We'll need to set the new 
action, and fill in the new Object metadata (FwkInfo, TaskInfo)



src/authorizer/local/authorizer.cpp (line 304)


I'd rather we didn't put all this RunTask/user-specific logic in the 
general LocalAuthorizer::authorized() method.
See how `authorization::ACCESS_SANDBOX` handles this with its `realRequest` 
(although maybe `specificRequest` would be a better name), in 
https://reviews.apache.org/r/47795/ (soon to be committed).



src/authorizer/local/authorizer.cpp (lines 307 - 311)


For RUN_TASK, I would actually reverse the order of precedence (and add 
executorInfo.commandInfo.user).
Ideally, I find a user in the taskInfo/executorInfo (only one can have a 
CommandInfo). If the task/executor doesn't have a user set, then default to the 
framework info.
FrameworkInfo must have a user, but if the FrameworkInfo (and TaskInfo) 
weren't set, then I'd rely on the value string.
task_info.executor_info.command.user
task_info.command.user
framework_info.user
value



src/tests/authorization_tests.cpp (lines 117 - 144)


Redundant. Please remove.



src/tests/authorization_tests.cpp (line 203)


But principal "foo" could run as any other user, e.g. "bar", right? That'd 
be worth testing.



src/tests/authorization_tests.cpp (line 529)


Would be better to test that "bar" cannot run a "user1", since we've shown 
previously that somebody else ("foo") can.


- Adam B


On May 26, 2016, 2:17 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47891/
> ---
> 
> (Updated May 26, 2016, 2:17 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, Joerg Schad, and Michael 
> Park.
> 
> 
> Bugs: MESOS-5459
> https://issues.apache.org/jira/browse/MESOS-5459
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Authorization requests for RUN_TASK actions can pass `SOME`
> authorization object either in a `FrameworkInfo` holding a user, or a
> `TaskInfo` with optionally a `CommandInfo` which can optionally hold a
> user. If either of these fields is set it will be used as the object;
> otherwise an `ANY` type authorization object will be created.
> 
> 
> Diffs
> -
> 
>   include/mesos/authorizer/authorizer.proto 
> 02d1a01d57cf34b38524f4368187878b03343537 
>   src/authorizer/local/authorizer.cpp 
> 3c7c791bde65cfcbcc4e319c9ccc487ab37d8029 
>   src/tests/authorization_tests.cpp 54bfb46a807677f4a4a2bb88dcb78a358cf5121a 
> 
> Diff: https://reviews.apache.org/r/47891/diff/
> 
> 
> Testing
> ---
> 
> Tested on a range of Linux configurations on internal CI.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>