Re: Review Request 59145: Used a helper to create ExecutorInfo in some nested container tests.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [58847, 58262, 58718, 58817, 58818, 58819, 58820, 58263, 
58821, 58967, 59145]

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 10, 2017, 2:28 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59145/
> ---
> 
> (Updated May 10, 2017, 2:28 p.m.)
> 
> 
> Review request for mesos and Gastón Kleiman.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/nested_mesos_containerizer_tests.cpp 
> c163b882fb2fc463537d6906c5a47b24a9a756c4 
> 
> 
> Diff: https://reviews.apache.org/r/59145/diff/1/
> 
> 
> Testing
> ---
> 
> make check on several Linux distros.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 59116: Windows: Updated `support/windows-build.bat`.

[Li Li]

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


Ship it!




Ship It!

- Li Li


On May 10, 2017, 8:40 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59116/
> ---
> 
> (Updated May 10, 2017, 8:40 p.m.)
> 
> 
> Review request for mesos, Jeff Coffler, John Kordich, Joseph Wu, and Li Li.
> 
> 
> Bugs: MESOS-7343
> https://issues.apache.org/jira/browse/MESOS-7343
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> It now understands the environment variable `CMAKE_GENERATOR` so that
> users can change the generator; the default is still Visual Studio 14.
> 
> Direct usage of `msbuild` was replaced with `cmake --build`.
> 
> We now build `mesos-tests` in the same pattern as `stout-tests` and
> `libprocess-tests`. If admin priveleges are missing, we skip running the
> tests, but still proceed to build the general target.
> 
> We set the CMake toolset instead of an environment variable for
> `PreferredToolArchitecture`.
> 
> 
> Diffs
> -
> 
>   support/windows-build.bat 88e177ced912b61f8169b58a50d1cf6ba0c4c0c7 
> 
> 
> Diff: https://reviews.apache.org/r/59116/diff/1/
> 
> 
> Testing
> ---
> 
> Built and tested script with clean build on Windows.
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 59155: CMake: Enable `/debug:fastlink` on Windows.

[Li Li]

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


Ship it!




Ship It!

- Li Li


On May 10, 2017, 8:59 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59155/
> ---
> 
> (Updated May 10, 2017, 8:59 p.m.)
> 
> 
> Review request for mesos, Jeff Coffler, John Kordich, Joseph Wu, and Li Li.
> 
> 
> Bugs: MESOS-7496
> https://issues.apache.org/jira/browse/MESOS-7496
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> A previous commit attempted to enable this option, but it was not
> actually being used by the linker. This patch correctly adds the option
> to each linker flag variable for shared libraries, modules, and
> executables, and only for the debug configuration. This results in much
> faster link times.
> 
> Also, the `/zc:inline` option was removed, as it is only relevant in
> optimized builds (i.e. release configurations).
> 
> 
> Diffs
> -
> 
>   cmake/CompilationConfigure.cmake 7b2669f0c54abf9b5e0fd60f8af01e97e1f0f86a 
> 
> 
> Diff: https://reviews.apache.org/r/59155/diff/2/
> 
> 
> Testing
> ---
> 
> Built and tested on Windows with VS Code.
> 
> NOTE: With `/debug:fastlink` properly enabled, the current VS Code C++ 
> debugger does not work without a manual update. See [this 
> thread](https://github.com/Microsoft/vscode-cpptools/issues/695#issuecomment-299554289)
>  for the work-around.
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 59155: CMake: Enable `/debug:fastlink` on Windows.

[Li Li]

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


Ship it!




Ship It!

- Li Li


On May 10, 2017, 8:59 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59155/
> ---
> 
> (Updated May 10, 2017, 8:59 p.m.)
> 
> 
> Review request for mesos, Jeff Coffler, John Kordich, Joseph Wu, and Li Li.
> 
> 
> Bugs: MESOS-7496
> https://issues.apache.org/jira/browse/MESOS-7496
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> A previous commit attempted to enable this option, but it was not
> actually being used by the linker. This patch correctly adds the option
> to each linker flag variable for shared libraries, modules, and
> executables, and only for the debug configuration. This results in much
> faster link times.
> 
> Also, the `/zc:inline` option was removed, as it is only relevant in
> optimized builds (i.e. release configurations).
> 
> 
> Diffs
> -
> 
>   cmake/CompilationConfigure.cmake 7b2669f0c54abf9b5e0fd60f8af01e97e1f0f86a 
> 
> 
> Diff: https://reviews.apache.org/r/59155/diff/2/
> 
> 
> Testing
> ---
> 
> Built and tested on Windows with VS Code.
> 
> NOTE: With `/debug:fastlink` properly enabled, the current VS Code C++ 
> debugger does not work without a manual update. See [this 
> thread](https://github.com/Microsoft/vscode-cpptools/issues/695#issuecomment-299554289)
>  for the work-around.
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 59116: Windows: Updated `support/windows-build.bat`.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [59155, 59156, 59157, 59116]

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 10, 2017, 8:40 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59116/
> ---
> 
> (Updated May 10, 2017, 8:40 p.m.)
> 
> 
> Review request for mesos, Jeff Coffler, John Kordich, Joseph Wu, and Li Li.
> 
> 
> Bugs: MESOS-7343
> https://issues.apache.org/jira/browse/MESOS-7343
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> It now understands the environment variable `CMAKE_GENERATOR` so that
> users can change the generator; the default is still Visual Studio 14.
> 
> Direct usage of `msbuild` was replaced with `cmake --build`.
> 
> We now build `mesos-tests` in the same pattern as `stout-tests` and
> `libprocess-tests`. If admin priveleges are missing, we skip running the
> tests, but still proceed to build the general target.
> 
> We set the CMake toolset instead of an environment variable for
> `PreferredToolArchitecture`.
> 
> 
> Diffs
> -
> 
>   support/windows-build.bat 88e177ced912b61f8169b58a50d1cf6ba0c4c0c7 
> 
> 
> Diff: https://reviews.apache.org/r/59116/diff/1/
> 
> 
> Testing
> ---
> 
> Built and tested script with clean build on Windows.
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 58874: Stripped spaces between directory elements in WebUI.

[haosdent huang]

> On May 11, 2017, 12:38 a.m., Benjamin Mahler wrote:
> > src/webui/master/static/browse.html
> > Lines 17-20 (original), 17-20 (patched)
> > 
> >
> > I suspect, if my understanding is correct, you could do the following 
> > format:
> > 
> > ```
> >   
> > {{dir}}
> > /
> >   
> > ```
> > 
> > Does that also insert a space?

Yes, this still insert a space

```
')">
https://reviews.apache.org/r/58874/#review174590
---


On May 8, 2017, 8:01 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58874/
> ---
> 
> (Updated May 8, 2017, 8:01 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Tomasz Janiszewski.
> 
> 
> Bugs: MESOS-7468
> https://issues.apache.org/jira/browse/MESOS-7468
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Stripped spaces between directory elements in WebUI.
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/browse.html 
> b9849197227b06df348789a49348e2b5d4cfd2ae 
>   src/webui/master/static/css/mesos.css 
> 9f3de5427071fc61d3791c4bc2a660368c2cd3c2 
> 
> 
> Diff: https://reviews.apache.org/r/58874/diff/3/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> strip_space.gif
>   
> https://reviews.apache.org/media/uploaded/files/2017/05/03/79afd0b1-5fb1-437c-a91c-732009af8fe3__strip_space.gif
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 58874: Stripped spaces between directory elements in WebUI.

[haosdent huang]

> On May 11, 2017, 12:38 a.m., Benjamin Mahler wrote:
> > Ok, I understand now what's going on in this change. I gave some 
> > suggestions for comments / naming to clarify this.
> > 
> > It feels like a hack however, since I would expect the breadcrumb '/' 
> > characters to be getting copied. Do you understand why they're not getting 
> > copied? If not, please leave a TODO to look into that.

Thx @bmahler, 

> why they're not getting copied?

As I mentioned in https://issues.apache.org/jira/browse/MESOS-7468 , this is 
expected here because the definition of breadcrumb in bootstrap is

```
.breadcrumb > li + li:before {
content: "/";
}
```

`:before` selector make that content not getter copied here.


- haosdent


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


On May 8, 2017, 8:01 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58874/
> ---
> 
> (Updated May 8, 2017, 8:01 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Tomasz Janiszewski.
> 
> 
> Bugs: MESOS-7468
> https://issues.apache.org/jira/browse/MESOS-7468
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Stripped spaces between directory elements in WebUI.
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/browse.html 
> b9849197227b06df348789a49348e2b5d4cfd2ae 
>   src/webui/master/static/css/mesos.css 
> 9f3de5427071fc61d3791c4bc2a660368c2cd3c2 
> 
> 
> Diff: https://reviews.apache.org/r/58874/diff/3/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> strip_space.gif
>   
> https://reviews.apache.org/media/uploaded/files/2017/05/03/79afd0b1-5fb1-437c-a91c-732009af8fe3__strip_space.gif
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 55903: Update XFS disk isolator documentation.

[Jiang Yan Xu]

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




docs/mesos-containerizer.md
Lines 98-102 (patched)


Updates needed here regarding the implementation of the accounting mode.



docs/mesos-containerizer.md
Lines 113-115 (original), 119-121 (patched)


XFS isolator now does support accounting mode now. :)



docs/mesos-containerizer.md
Line 121 (original), 127 (patched)


I guess you meant s/1000/1/ in your previous version? Should we do that 
because [5000-1] is the default project ID range so the user likely can 
just paste this command to get what they want if they use the isolator.


- Jiang Yan Xu


On April 25, 2017, 3:38 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55903/
> ---
> 
> (Updated April 25, 2017, 3:38 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-5116
> https://issues.apache.org/jira/browse/MESOS-5116
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Update XFS disk isolator documentation.
> 
> 
> Diffs
> -
> 
>   docs/mesos-containerizer.md d0b9d762ab83bff1cf2bd98ed566376b7bd566c6 
> 
> 
> Diff: https://reviews.apache.org/r/55903/diff/4/
> 
> 
> Testing
> ---
> 
> None.
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 55897: Add support for not enforcing XFS quotas.

[Jiang Yan Xu]

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


Fix it, then Ship it!





src/slave/containerizer/mesos/isolators/xfs/disk.cpp
Lines 331-332 (original), 338-339 (patched)


Align the start of the 2nd line with `"` which is *inside* the `(`.



src/slave/containerizer/mesos/isolators/xfs/disk.cpp
Lines 351 (patched)


Ditto.


- Jiang Yan Xu


On May 2, 2017, 10:23 a.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55897/
> ---
> 
> (Updated May 2, 2017, 10:23 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-5116
> https://issues.apache.org/jira/browse/MESOS-5116
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add XFS disk isolator support for not enforcing disk quotas on
> containers. While there is a global filesystem configuration option
> to turn off quota enforcement, we should not automatically toggle
> that because we don't know why the operator might have changed that
> configuration. Instead, we just apply an unlimited (0) quota, which
> engages XFS space accounting without enforcing any limit.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/xfs/disk.hpp 
> 52f0459421a45b01ce38b17c689633301cd97982 
>   src/slave/containerizer/mesos/isolators/xfs/disk.cpp 
> 40f1049358ad99d3f213289e36def81c580f07f3 
>   src/slave/containerizer/mesos/isolators/xfs/utils.hpp 
> eddd4c37fb42339ca21ecb392dea47acf6b277bb 
>   src/tests/containerizer/xfs_quota_tests.cpp 
> 7beb60b059910a0d4451b1ace895a35dc974a043 
> 
> 
> Diff: https://reviews.apache.org/r/55897/diff/6/
> 
> 
> Testing
> ---
> 
> sudo make check (Fedora 25)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 58099: Added authorization for frameworks in `GetRoles` v1 API.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [58095, 58096, 58097, 58099]

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 10, 2017, 6:52 a.m., Jay Guo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58099/
> ---
> 
> (Updated May 10, 2017, 6:52 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, and Benjamin Mahler.
> 
> 
> Bugs: MESOS-7260
> https://issues.apache.org/jira/browse/MESOS-7260
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added authorization for frameworks in `GetRoles` v1 API.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp e2590a17044ac019b24a24629428d4ec8adc0c31 
> 
> 
> Diff: https://reviews.apache.org/r/58099/diff/6/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jay Guo
> 
>

Re: Review Request 59018: Supported Image::Secret in docker URI fetcher plugin.

[Vinod Kone]

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




src/uri/fetchers/docker.cpp
Line 492 (original), 502 (patched)


unrelated to this review but this should probably be plural 
"getAuthHeaders" given this is returning `http::Headers`. maybe update in a 
different review?



src/uri/fetchers/docker.cpp
Line 492 (original), 502 (patched)


unrelated to this review but this should probably be plural 
"getAuthHeaders" given this is returning `http::Headers`. maybe update in a 
different review?



src/uri/fetchers/docker.cpp
Line 505 (original), 516 (patched)


As discussed offline passing headers and secrets here is very confusing.

We should add some comments on the semantics of http headers and secret 
value, which is used when, auth assumptions regarding manifest server and blob 
servers etc, for posterity.


- Vinod Kone


On May 10, 2017, 12:48 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59018/
> ---
> 
> (Updated May 10, 2017, 12:48 p.m.)
> 
> 
> Review request for mesos, Adam B, Chun-Hung Hsiao, Jie Yu, Kapil Arya, Till 
> Toenshoff, and Vinod Kone.
> 
> 
> Bugs: MESOS-7088
> https://issues.apache.org/jira/browse/MESOS-7088
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Supported Image::Secret in docker URI fetcher plugin.
> 
> 
> Diffs
> -
> 
>   src/uri/fetchers/docker.cpp 44169bf5f22f0ffd9fad7bb3b8f7d2a4989c6415 
> 
> 
> Diff: https://reviews.apache.org/r/59018/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> Manually tested using mesos-execute with task group. Verified that pulling 
> separate private image from different registries using different image 
> secrets works correctly.
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 58874: Stripped spaces between directory elements in WebUI.

[Benjamin Mahler]

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


Fix it, then Ship it!




Ok, I understand now what's going on in this change. I gave some suggestions 
for comments / naming to clarify this.

It feels like a hack however, since I would expect the breadcrumb '/' 
characters to be getting copied. Do you understand why they're not getting 
copied? If not, please leave a TODO to look into that.


src/webui/master/static/browse.html
Lines 15-16 (original), 15-16 (patched)


This comment seems misleading, since we're not doing any stripping here, 
are we?

It seems that like Tomasz said, the spaces are being removed because now 
there are no spaces or newlines between the closing `` and `` tags?

How about this?

```

```

Is this accurate?



src/webui/master/static/browse.html
Lines 17-20 (original), 17-20 (patched)


I suspect, if my understanding is correct, you could do the following 
format:

```
  
{{dir}}
/
  
```

Does that also insert a space?



src/webui/master/static/css/mesos.css
Lines 179-181 (patched)


Ok I think I understand now, in that this is just making the text invisible 
using a 0 size font..?

If so, how about calling this `hidden-text`?

```
// The `hidden-text` class hides text by making the font size 0.
// This can be used, for example, to insert text that will be
// added when the user highlights and copies, while leaving
// the text hidden from the user.
```

This still feels like a hack to me, have you tried looking into whether we 
can just update the `breadcrumb` class to have the slashes show up when copying?


- Benjamin Mahler


On May 8, 2017, 8:01 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58874/
> ---
> 
> (Updated May 8, 2017, 8:01 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Tomasz Janiszewski.
> 
> 
> Bugs: MESOS-7468
> https://issues.apache.org/jira/browse/MESOS-7468
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Stripped spaces between directory elements in WebUI.
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/browse.html 
> b9849197227b06df348789a49348e2b5d4cfd2ae 
>   src/webui/master/static/css/mesos.css 
> 9f3de5427071fc61d3791c4bc2a660368c2cd3c2 
> 
> 
> Diff: https://reviews.apache.org/r/58874/diff/3/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> strip_space.gif
>   
> https://reviews.apache.org/media/uploaded/files/2017/05/03/79afd0b1-5fb1-437c-a91c-732009af8fe3__strip_space.gif
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 59074: Updated all tests that use Containerizer::launch(...).

[Joseph Wu]

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

(Updated May 10, 2017, 5:27 p.m.)


Review request for mesos and Jie Yu.


Changes
---

Pulled out fetcher test changes, as those have some functional changes that 
should be called out.


Bugs: MESOS-7304 and MESOS-7449
https://issues.apache.org/jira/browse/MESOS-7304
https://issues.apache.org/jira/browse/MESOS-7449


Repository: mesos


Description (updated)
---

This sweeps through the unit tests and translates all existing
tests that use the two variants of Containerizer::launch(...)
(i.e. nested and non-nested) and translates the arguments to the
new interface.  Some of the fetcher interface changes are also
addressed in this sweep.


Diffs (updated)
-

  src/tests/api_tests.cpp faf3242f9c86d866c4bb5e457fcfe47c1063cc09 
  src/tests/check_tests.cpp ec0d5ee78a94991c68f38174d09b41e8f2e4be35 
  src/tests/command_executor_tests.cpp da4b653303d436309f6cb190ad6b600b8d934678 
  src/tests/container_logger_tests.cpp 28436b6b67c1251d877064751e695c6696725a23 
  src/tests/containerizer/cgroups_isolator_tests.cpp 
4e1d027bde7f0ab51e778bfd3bac8797fc7f7f1b 
  src/tests/containerizer/cni_isolator_tests.cpp 
565e58ae75e918453e4386f5e35a5a844a8b15f8 
  src/tests/containerizer/composing_containerizer_tests.cpp 
d7fd6216080f2f437503b898bff6b1046317933e 
  src/tests/containerizer/cpu_isolator_tests.cpp 
3c6f7481de2d6d1f6abf9850bd33cea44931480a 
  src/tests/containerizer/docker_volume_isolator_tests.cpp 
b47a6b5081a63ac474ac4634701b1a572eb58137 
  src/tests/containerizer/isolator_tests.cpp 
355e15ff69ca6bdd94821f6566fd09a280d03b47 
  src/tests/containerizer/linux_filesystem_isolator_tests.cpp 
70a0dce0f0166fcb3c15a03a89151152954abbac 
  src/tests/containerizer/memory_isolator_tests.cpp 
0df4aa892b531eb305eb3012862ad25527344ab6 
  src/tests/containerizer/memory_pressure_tests.cpp 
c4ad779906ee31d07fdd8a6fc0e51e1edd1cbe85 
  src/tests/containerizer/port_mapping_tests.cpp 
a528382e8b4831b9c7e8dcc877a5e242909f0cd5 
  src/tests/containerizer/volume_image_isolator_tests.cpp 
ad1884459e87ece767e1019adcf702a2f2ff8f1c 
  src/tests/containerizer/xfs_quota_tests.cpp 
7beb60b059910a0d4451b1ace895a35dc974a043 
  src/tests/disk_quota_tests.cpp f758bee07c448afe09dc380e99c4a81e2dd9f650 
  src/tests/health_check_tests.cpp 6c1b9a0ead6edb34c20cf4973fffcff913c5d7ad 
  src/tests/hook_tests.cpp 02d8f800c3eb9b1e617a14c78c2ef1e45d1c72bb 
  src/tests/master_tests.cpp ceee2f4a5d38e0f4200f444769e058d2173de821 
  src/tests/slave_recovery_tests.cpp 52e78b6b6280a159233b402ce2849448204d4f11 
  src/tests/slave_tests.cpp 8c97dc6d088708d301dc3ccf90d413fd785b782f 


Diff: https://reviews.apache.org/r/59074/diff/2/

Changes: https://reviews.apache.org/r/59074/diff/1-2/


Testing
---

make check

Currently fails a few of the fetcher tests, which might need tweaking (these 
tests assume the fetcher caches things in the current directory):
```
[  FAILED  ] FetcherTest.RelativeFilePath
[  FAILED  ] FetcherCacheTest.SimpleEviction
[  FAILED  ] FetcherCacheTest.FallbackFromEviction
[  FAILED  ] FetcherCacheTest.RemoveLRUCacheEntries
```


Thanks,

Joseph Wu

Review Request 59164: Updated fetcher tests to reflect changed interfaces.

[Joseph Wu]

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

Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

There are two primary changes to the tests:
  * The arguments passed to the fetcher's methods no longer contain
the Flags or SlaveID.  So a couple of the mock expectations need
to be tweaked.
  * Flags are now passed into the fetcher at construction.  Some of
the tests modify the flags after construction, so the order of
operations needed to be flipped.


Diffs
-

  src/tests/fetcher_cache_tests.cpp 3bd63ed0a66493829a82c542ad05ebe0f7828d1a 
  src/tests/fetcher_tests.cpp 27ea7244c713934ef5a721ec133804a006dcb26e 


Diff: https://reviews.apache.org/r/59164/diff/1/


Testing
---

See last patch in chain.


Thanks,

Joseph Wu

Re: Review Request 59073: Updated Docker containerizer tests to use the old naming scheme.

[Joseph Wu]

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

(Updated May 10, 2017, 5:26 p.m.)


Review request for mesos and Jie Yu.


Changes
---

Fixed a few root tests that didn't pass the same flags to the containerizer and 
agent.


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


Repository: mesos


Description
---

This changes test expectations of Docker container names back to the
0.22 scheme and simultaneously updates the tests to the new
containerizer interface.


Diffs (updated)
-

  src/tests/containerizer/docker_containerizer_tests.cpp 
4eef399b05f6e35d75c7c23992f0ccda04576277 


Diff: https://reviews.apache.org/r/59073/diff/3/

Changes: https://reviews.apache.org/r/59073/diff/2-3/


Testing
---

See last patch in chain.


Thanks,

Joseph Wu

Re: Review Request 58910: Addressed a TODO about checkpointing in tests.

[Joseph Wu]

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

(Updated May 10, 2017, 5:26 p.m.)


Review request for mesos and Jie Yu.


Changes
---

Fixed up some typos.


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


Repository: mesos


Description
---

Checkpointing only plays a role in tests when the containerizer
is destroyed and subsequently recovered.  This commit updates a
couple of test files to the new containerizer interface and
also turns checkpointing _off_ in tests which do not perform any
containerizer recovery.


Diffs (updated)
-

  src/tests/containerizer/io_switchboard_tests.cpp 
b19961ee91af3c174d4377c2514465d6c99cf331 
  src/tests/containerizer/mesos_containerizer_tests.cpp 
13e0f7e603a3ffdd0965b253d7abfe6a069cd2b4 
  src/tests/containerizer/nested_mesos_containerizer_tests.cpp 
c163b882fb2fc463537d6906c5a47b24a9a756c4 
  src/tests/containerizer/volume_sandbox_path_isolator_tests.cpp 
5f4e382baf80439b9a61f22c1912cadeba1109b7 


Diff: https://reviews.apache.org/r/58910/diff/3/

Changes: https://reviews.apache.org/r/58910/diff/2-3/


Testing
---

See last patch in chain.


Thanks,

Joseph Wu

Re: Review Request 58898: Send task kill for non-Partition Aware frameworks.

[Neil Conway]

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



My apologies for the delay in reviewing this.

High-level comments:

(a) Can we improve the description of the problem in the commit summary? It 
took me quite a while to figure out what is actually going on here. My 
understanding is:

(1) Agent re-registers
(2) Master sends `ShutdownFrameworkMessage` for non-PA frameworks on the agent
(3) Master offers agent resources to framework
(4) Framework launches new task on agent _before the agent has finished 
shutting down the framework_
(5) Agent ignores launch task because it believes the framework is still 
terminating.

This is a race condition, because if the agent finished shutting down the 
framework (in #4) before the launch task was received, there would not be a 
problem. Is my understanding correct?

(2) I'd prefer a new unit test that constructs exactly this scenario, rather 
than changing existing unit tests.

(3) The new behavior is that the framework will receive `TASK_KILLED` for any 
non-PA tasks running on a partitioned agent that re-registers. This does not 
seem ideal, because `TASK_KILLED` _normally_ corresponds to a 
framework-initiated `killTask` operation.

(4) Thinking out loud, what if a custom executor ignores the `killTask` 
request? When shutting down a framework, it seems we forcibly destroy the 
container (via `containerizer->destroy()`), if the executor doesn't exit 
promptly upon receiving the framework shutdown request. AFAIK we don't have 
similar logic for `killTask` requests.

3 + 4 suggests that maybe we want a different way to terminate the task on the 
agent -- let's discuss.


src/master/master.cpp
Lines 6034 (patched)


`tasksToKill`



src/master/master.cpp
Lines 6057 (patched)


"Keep"



src/master/master.cpp
Lines 6058 (patched)


"separate data structure"



src/master/master.cpp
Line 6097 (original), 6102 (patched)


"launched by"



src/tests/partition_tests.cpp
Lines 693 (patched)


Why did you move this to before the agent re-registers? The point of doing 
explicit reconciliation is to check the master's state after the agent 
re-registers.

There's no harm in _also_ reconciling before the agent re-registers, but if 
you want to add that, let's do it in a separate review.



src/tests/partition_tests.cpp
Line 754 (original), 786 (patched)


`EXPECT_FALSE`.



src/tests/partition_tests.cpp
Line 794 (original), 826 (patched)


The point of this part of the test is to check that the agent's metrics are 
correct after re-registration (and before starting any additional tasks). I'd 
prefer to check the metrics, then start another task on the agent afterward.



src/tests/partition_tests.cpp
Line 2049 (original)


Removing this results in two gmock warnings:

```
GMOCK WARNING:
Uninteresting mock function call - returning directly.
Function call: killTask(0x7fbcf1c5fa20, @0x7fbcf1c54640 1)
```

```
GMOCK WARNING:
Uninteresting mock function call - returning directly.
Function call: shutdown(0x7fbcf1c5fa20)
```


- Neil Conway


On May 1, 2017, 11:58 p.m., Megha Sharma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58898/
> ---
> 
> (Updated May 1, 2017, 11:58 p.m.)
> 
> 
> Review request for mesos, Neil Conway and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-7215
> https://issues.apache.org/jira/browse/MESOS-7215
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Mesos is now sending ShutdownFrameworkMessages to the agent for all
> non-partition-aware frameworks (including the ones that are still
> registered). This is problematic. The offer from this agent can
> still go to the same framework which can then launch new tasks.
> The agent then receives tasks of the same framework and ignores
> them because it thinks the framework is shutting down. The framework
> is not shutting down of course, so from the master and the scheduler's
> perspective the task is pending in STAGING forever until the next agent
> reregistration, which could happen much later. This commit fixes
> the problem by sending a task kill instead of ShutdownFrameworkMessage,
> when the agent re-registers after being unreachable, for non-partition
> 

Review Request 59162: Added a clearefying comment to the appc spec.proto.

[Till Toenshoff]

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

Review request for mesos, Gilbert Song and Jie Yu.


Repository: mesos


Description
---

Adds a comment and removes a blank line.


Diffs
-

  include/mesos/appc/spec.proto 7be2a6fa867d78866263ef4d40088c9d6c8b9ac1 


Diff: https://reviews.apache.org/r/59162/diff/1/


Testing
---

no functional change done.


Thanks,

Till Toenshoff

Re: Review Request 59141: Added Secret::Value to the URI fetcher interface.

[Chun-Hung Hsiao]

> On May 10, 2017, 7:36 p.m., Chun-Hung Hsiao wrote:
> > include/mesos/uri/fetcher.hpp
> > Lines 78 (patched)
> > 
> >
> > I'm thinking about what is a proper interface extension for this 
> > fetcher plugin. Instead of having a `Secret::Value`, how about a more 
> > general `config` or some session info parameter for the fetcher, which may 
> > or may not come from a secret?
> 
> Gilbert Song wrote:
> This new parameter in URI fetcher should be used for authentication, no 
> matter which plugin it is. And it means this parameter represents sensitive 
> information. By introducing an optinal field secretValue, we are expecting 
> this sensitive information is always from our new secret resolver infterface 
> (Secret::Value).
> 
> So I think the assumption that the URI fetcher is expecting a resolved 
> secret sounds reasonable.
> 
> ```
>   virtual process::Future resolve(
>   const Secret& secret) const = 0;
> ```
> 
> Thoughts?

I was wondering if there would be a scenario that we might what to have a 
plugin that could use some extra dynamic input for more than just 
authentication in the future. This is more of a design choice about how 
flexible this plugin interface could/should be.


- Chun-Hung


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


On May 10, 2017, 12:48 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59141/
> ---
> 
> (Updated May 10, 2017, 12:48 p.m.)
> 
> 
> Review request for mesos, Adam B, Chun-Hung Hsiao, Jie Yu, Kapil Arya, Till 
> Toenshoff, and Vinod Kone.
> 
> 
> Bugs: MESOS-7088
> https://issues.apache.org/jira/browse/MESOS-7088
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added Secret::Value to the URI fetcher interface.
> 
> 
> Diffs
> -
> 
>   include/mesos/uri/fetcher.hpp ebf86c78b794a6ef46332df788a1317bbec5d983 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
> 6db788dd0c582deadf3e91c4d21bb9c20cf94e6b 
>   src/uri/fetcher.cpp f4d1a4c69e910260dc536aa42ae03fd17403b060 
>   src/uri/fetchers/copy.hpp f4a2fb3d5156e5ebfdf7c4202f8dc9b1cd1d6ac7 
>   src/uri/fetchers/copy.cpp 5e1470503f4fa0e543680c93b2ad3e36351afc1c 
>   src/uri/fetchers/curl.hpp 083f155092d159cd83069bfdfd905d679e9ab57c 
>   src/uri/fetchers/curl.cpp 24b53c77946170cba45152c458d85b6fddfce9f8 
>   src/uri/fetchers/docker.hpp 65e01cba1d41688a8ee5da73d1d6f57515fbc7f5 
>   src/uri/fetchers/docker.cpp 44169bf5f22f0ffd9fad7bb3b8f7d2a4989c6415 
>   src/uri/fetchers/hadoop.hpp 4923dc6491d7cc6efc6ced4f5356af9f945ba5d2 
>   src/uri/fetchers/hadoop.cpp 3c5ffe607c92ea1ab66ba261bd70031f2907cea6 
> 
> 
> Diff: https://reviews.apache.org/r/59141/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 59015: Implemented passing docker config depended methods.

[Vinod Kone]

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




src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp
Lines 87 (patched)


s/Secret::Value/JSON::Object/

This file doesn't need to know about secrets AFAICT?


- Vinod Kone


On May 10, 2017, 12:47 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59015/
> ---
> 
> (Updated May 10, 2017, 12:47 p.m.)
> 
> 
> Review request for mesos, Adam B, Chun-Hung Hsiao, Jie Yu, Kapil Arya, Till 
> Toenshoff, and Vinod Kone.
> 
> 
> Bugs: MESOS-7088
> https://issues.apache.org/jira/browse/MESOS-7088
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented passing docker config depended methods.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
> 6db788dd0c582deadf3e91c4d21bb9c20cf94e6b 
> 
> 
> Diff: https://reviews.apache.org/r/59015/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 59012: Implemented passing the secret resolver to registry puller.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On May 10, 2017, 12:47 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59012/
> ---
> 
> (Updated May 10, 2017, 12:47 p.m.)
> 
> 
> Review request for mesos, Adam B, Chun-Hung Hsiao, Jie Yu, Kapil Arya, Till 
> Toenshoff, and Vinod Kone.
> 
> 
> Bugs: MESOS-7088
> https://issues.apache.org/jira/browse/MESOS-7088
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented passing the secret resolver to registry puller.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> b58baed64480e22f640a4852537f85922ed382ae 
>   src/slave/containerizer/mesos/provisioner/appc/store.hpp 
> 15c79e9401a821e445fbd32b34503e4fb0014d42 
>   src/slave/containerizer/mesos/provisioner/appc/store.cpp 
> 09a40a5835454bb7519d11bae4a851337a89b935 
>   src/slave/containerizer/mesos/provisioner/docker/puller.hpp 
> 6dacdb1403433a37dd31a93d4ff2e37d4685eb87 
>   src/slave/containerizer/mesos/provisioner/docker/puller.cpp 
> ac9dae8ecbb897b8ff942d11ac70281a63e06831 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.hpp 
> 62ddb7a332030f3116477408d8b16c19e434c159 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
> 6db788dd0c582deadf3e91c4d21bb9c20cf94e6b 
>   src/slave/containerizer/mesos/provisioner/docker/store.hpp 
> e1abff19c8877ad07900bb2f44deca1cdda41f58 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> 68ce265a5d0a61f8d9ed55dd14c630dcf893a7d2 
>   src/slave/containerizer/mesos/provisioner/provisioner.hpp 
> 7d6c1b93a2c0e265b9344a0fc27f1cf4ed5325f2 
>   src/slave/containerizer/mesos/provisioner/provisioner.cpp 
> be45fc59027f176b43b767e9441fd8089ceec7b4 
>   src/slave/containerizer/mesos/provisioner/store.hpp 
> 82a9be64264ae829773c1e2e8a4360f78641cbf6 
>   src/slave/containerizer/mesos/provisioner/store.cpp 
> 7141d63fcf2dbc3fbf00508c7f92945aab014fb2 
> 
> 
> Diff: https://reviews.apache.org/r/59012/diff/4/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 59141: Added Secret::Value to the URI fetcher interface.

[Gilbert Song]

> On May 10, 2017, 12:36 p.m., Chun-Hung Hsiao wrote:
> > include/mesos/uri/fetcher.hpp
> > Lines 78 (patched)
> > 
> >
> > I'm thinking about what is a proper interface extension for this 
> > fetcher plugin. Instead of having a `Secret::Value`, how about a more 
> > general `config` or some session info parameter for the fetcher, which may 
> > or may not come from a secret?

This new parameter in URI fetcher should be used for authentication, no matter 
which plugin it is. And it means this parameter represents sensitive 
information. By introducing an optinal field secretValue, we are expecting this 
sensitive information is always from our new secret resolver infterface 
(Secret::Value).

So I think the assumption that the URI fetcher is expecting a resolved secret 
sounds reasonable.

```
  virtual process::Future resolve(
  const Secret& secret) const = 0;
```

Thoughts?


- Gilbert


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


On May 10, 2017, 5:48 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59141/
> ---
> 
> (Updated May 10, 2017, 5:48 a.m.)
> 
> 
> Review request for mesos, Adam B, Chun-Hung Hsiao, Jie Yu, Kapil Arya, Till 
> Toenshoff, and Vinod Kone.
> 
> 
> Bugs: MESOS-7088
> https://issues.apache.org/jira/browse/MESOS-7088
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added Secret::Value to the URI fetcher interface.
> 
> 
> Diffs
> -
> 
>   include/mesos/uri/fetcher.hpp ebf86c78b794a6ef46332df788a1317bbec5d983 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
> 6db788dd0c582deadf3e91c4d21bb9c20cf94e6b 
>   src/uri/fetcher.cpp f4d1a4c69e910260dc536aa42ae03fd17403b060 
>   src/uri/fetchers/copy.hpp f4a2fb3d5156e5ebfdf7c4202f8dc9b1cd1d6ac7 
>   src/uri/fetchers/copy.cpp 5e1470503f4fa0e543680c93b2ad3e36351afc1c 
>   src/uri/fetchers/curl.hpp 083f155092d159cd83069bfdfd905d679e9ab57c 
>   src/uri/fetchers/curl.cpp 24b53c77946170cba45152c458d85b6fddfce9f8 
>   src/uri/fetchers/docker.hpp 65e01cba1d41688a8ee5da73d1d6f57515fbc7f5 
>   src/uri/fetchers/docker.cpp 44169bf5f22f0ffd9fad7bb3b8f7d2a4989c6415 
>   src/uri/fetchers/hadoop.hpp 4923dc6491d7cc6efc6ced4f5356af9f945ba5d2 
>   src/uri/fetchers/hadoop.cpp 3c5ffe607c92ea1ab66ba261bd70031f2907cea6 
> 
> 
> Diff: https://reviews.apache.org/r/59141/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 59018: Supported Image::Secret in docker URI fetcher plugin.

[Gilbert Song]

> On May 10, 2017, 11:12 a.m., Chun-Hung Hsiao wrote:
> > src/uri/fetchers/docker.cpp
> > Lines 328 (patched)
> > 
> >
> > Why are we using `secretValue` instead of `config` as in other places? 
> > Also, it seems to me that `uri` and `secretValue` are coupled, so how about 
> > putting them side by side in the parameter list? Same for the following 
> > code.
> 
> Chun-Hung Hsiao wrote:
> OK I got why we're using `secretValue`. Please refer to my comment in 
> https://reviews.apache.org/r/59141/.

replied on that patch. thanks.


- Gilbert


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


On May 10, 2017, 5:48 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59018/
> ---
> 
> (Updated May 10, 2017, 5:48 a.m.)
> 
> 
> Review request for mesos, Adam B, Chun-Hung Hsiao, Jie Yu, Kapil Arya, Till 
> Toenshoff, and Vinod Kone.
> 
> 
> Bugs: MESOS-7088
> https://issues.apache.org/jira/browse/MESOS-7088
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Supported Image::Secret in docker URI fetcher plugin.
> 
> 
> Diffs
> -
> 
>   src/uri/fetchers/docker.cpp 44169bf5f22f0ffd9fad7bb3b8f7d2a4989c6415 
> 
> 
> Diff: https://reviews.apache.org/r/59018/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> Manually tested using mesos-execute with task group. Verified that pulling 
> separate private image from different registries using different image 
> secrets works correctly.
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 58263: Inherited working dir from parent when launching a DEBUG container.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On May 10, 2017, 1:54 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58263/
> ---
> 
> (Updated May 10, 2017, 1:54 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman, Jie Yu, Kevin Klues, and Vinod Kone.
> 
> 
> Bugs: MESOS-7433
> https://issues.apache.org/jira/browse/MESOS-7433
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/containerizer.proto 
> 41f2905df690bfe88ed762f1cd1246689fa4d3ea 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 3d724945812c0359ed175ce232f70886dc4401c8 
>   src/tests/containerizer/nested_mesos_containerizer_tests.cpp 
> c163b882fb2fc463537d6906c5a47b24a9a756c4 
> 
> 
> Diff: https://reviews.apache.org/r/58263/diff/7/
> 
> 
> Testing
> ---
> 
> See https://reviews.apache.org/r/58821/
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 59145: Used a helper to create ExecutorInfo in some nested container tests.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On May 10, 2017, 2:28 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59145/
> ---
> 
> (Updated May 10, 2017, 2:28 p.m.)
> 
> 
> Review request for mesos and Gastón Kleiman.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/nested_mesos_containerizer_tests.cpp 
> c163b882fb2fc463537d6906c5a47b24a9a756c4 
> 
> 
> Diff: https://reviews.apache.org/r/59145/diff/1/
> 
> 
> Testing
> ---
> 
> make check on several Linux distros.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 58967: Set the working directory to parent task's for DEBUG containers.

[Vinod Kone]

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


Fix it, then Ship it!





include/mesos/slave/containerizer.proto
Line 194 (original), 194 (patched)


Looks like some random unicode char at the end?


- Vinod Kone


On May 10, 2017, 1:56 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58967/
> ---
> 
> (Updated May 10, 2017, 1:56 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-7433
> https://issues.apache.org/jira/browse/MESOS-7433
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> A DEBUG container's working directory should be set to the parent
> task's working directory. For the command executor case, even if
> the task itself has a root filesystem, the executor container still
> uses the host filesystem, hence
> `ContainerLaunchInfo.working_directory`, pointing to the executor's
> working directory in the host filesystem, may be different from the
> task's working directory in the root filesystem.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/containerizer.proto 
> 41f2905df690bfe88ed762f1cd1246689fa4d3ea 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 3d724945812c0359ed175ce232f70886dc4401c8 
> 
> 
> Diff: https://reviews.apache.org/r/58967/diff/3/
> 
> 
> Testing
> ---
> 
> make check on Mac OS and Fedora 24
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 58821: Added a test that verifies a task and its check share the work dir.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On May 10, 2017, 1:55 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58821/
> ---
> 
> (Updated May 10, 2017, 1:55 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-7433
> https://issues.apache.org/jira/browse/MESOS-7433
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/tests/check_tests.cpp ec0d5ee78a94991c68f38174d09b41e8f2e4be35 
> 
> 
> Diff: https://reviews.apache.org/r/58821/diff/5/
> 
> 
> Testing
> ---
> 
> `make check` on Mac OS and Fedora 24
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 59146: Added a 'UNKNOWN' field to the Update.State enumeration.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [59146]

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 10, 2017, 3:09 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59146/
> ---
> 
> (Updated May 10, 2017, 3:09 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Bannier, and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   include/mesos/resource_provider/resource_provider.proto 
> 11d01df114eafb729514dab64596c5f1ecf09f26 
>   include/mesos/v1/resource_provider/resource_provider.proto 
> 97a85ce34eeecf86d14c097712867cd24624a6ec 
> 
> 
> Diff: https://reviews.apache.org/r/59146/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 59116: Windows: Updated `support/windows-build.bat`.

[Andrew Schwartzmeyer]

> On May 10, 2017, 9:21 p.m., Mesos Reviewbot Windows wrote:
> > Patch looks great!
> > 
> > Reviews applied: [59155, 59156, 59157, 59116]
> > 
> > Passed command: support\windows-build.bat

Hey look at that! Windows Reviewbot works :D


- Andrew


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


On May 10, 2017, 8:40 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59116/
> ---
> 
> (Updated May 10, 2017, 8:40 p.m.)
> 
> 
> Review request for mesos, Jeff Coffler, John Kordich, Joseph Wu, and Li Li.
> 
> 
> Bugs: MESOS-7343
> https://issues.apache.org/jira/browse/MESOS-7343
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> It now understands the environment variable `CMAKE_GENERATOR` so that
> users can change the generator; the default is still Visual Studio 14.
> 
> Direct usage of `msbuild` was replaced with `cmake --build`.
> 
> We now build `mesos-tests` in the same pattern as `stout-tests` and
> `libprocess-tests`. If admin priveleges are missing, we skip running the
> tests, but still proceed to build the general target.
> 
> We set the CMake toolset instead of an environment variable for
> `PreferredToolArchitecture`.
> 
> 
> Diffs
> -
> 
>   support/windows-build.bat 88e177ced912b61f8169b58a50d1cf6ba0c4c0c7 
> 
> 
> Diff: https://reviews.apache.org/r/59116/diff/1/
> 
> 
> Testing
> ---
> 
> Built and tested script with clean build on Windows.
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 59116: Windows: Updated `support/windows-build.bat`.

[Andrew Schwartzmeyer]

> On May 10, 2017, 9:19 p.m., Jeff Coffler wrote:
> > support/windows-build.bat
> > Line 52 (original), 52 (patched)
> > 
> >
> > Is this correct? Did we deprecate older toolsets? I thought that, at 
> > this point, we REQUIRE the newer version of Visual Studio.

This is correct. We do not yet require, only warn that it is deprecated; and 
this commit does not change the default. If it did, I'd be forcing Joe to 
update Jenkins, but I'm not (yet anyway).


> On May 10, 2017, 9:19 p.m., Jeff Coffler wrote:
> > support/windows-build.bat
> > Line 61 (original)
> > 
> >
> > Since we're shifting from msbuild to cmake to build, does cmake assume 
> > /m (parallel builds)?

Yes.


> On May 10, 2017, 9:19 p.m., Jeff Coffler wrote:
> > support/windows-build.bat
> > Line 68 (original), 57 (patched)
> > 
> >
> > I know that we don't support NOT building Debug right now, but 
> > shouldn't this be configurable for windows-build.bat, perhaps with the 
> > default to build with debug?

We don't support it yet, so I didn't add it as a broken option.


> On May 10, 2017, 9:19 p.m., Jeff Coffler wrote:
> > support/windows-build.bat
> > Line 83 (original), 72 (patched)
> > 
> >
> > So this is actually different from Linux.
> > 
> > On Linux, if you do a `make check`, libprocess, stout, and mesos tests 
> > are all run (even if one of them has an error). But on Windows with 
> > windows-build.bat, that's not the case.
> > 
> > I think it should be. Perhaps save the error state so you can report 
> > (at the end) that libprocess and/or stout tests failed. But given that it's 
> > routine for one or more tests to fail today, it's annoying that 
> > windows-build.bat isn't as good as Linux in this regard.

Totally agree, but wasn't fixing that here. It's a separate issue (and would 
require more careful testing of the script).


- Andrew


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


On May 10, 2017, 8:40 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59116/
> ---
> 
> (Updated May 10, 2017, 8:40 p.m.)
> 
> 
> Review request for mesos, Jeff Coffler, John Kordich, Joseph Wu, and Li Li.
> 
> 
> Bugs: MESOS-7343
> https://issues.apache.org/jira/browse/MESOS-7343
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> It now understands the environment variable `CMAKE_GENERATOR` so that
> users can change the generator; the default is still Visual Studio 14.
> 
> Direct usage of `msbuild` was replaced with `cmake --build`.
> 
> We now build `mesos-tests` in the same pattern as `stout-tests` and
> `libprocess-tests`. If admin priveleges are missing, we skip running the
> tests, but still proceed to build the general target.
> 
> We set the CMake toolset instead of an environment variable for
> `PreferredToolArchitecture`.
> 
> 
> Diffs
> -
> 
>   support/windows-build.bat 88e177ced912b61f8169b58a50d1cf6ba0c4c0c7 
> 
> 
> Diff: https://reviews.apache.org/r/59116/diff/1/
> 
> 
> Testing
> ---
> 
> Built and tested script with clean build on Windows.
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 59116: Windows: Updated `support/windows-build.bat`.

[Mesos Reviewbot Windows]

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



Patch looks great!

Reviews applied: [59155, 59156, 59157, 59116]

Passed command: support\windows-build.bat

- Mesos Reviewbot Windows


On May 10, 2017, 8:40 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59116/
> ---
> 
> (Updated May 10, 2017, 8:40 p.m.)
> 
> 
> Review request for mesos, Jeff Coffler, John Kordich, Joseph Wu, and Li Li.
> 
> 
> Bugs: MESOS-7343
> https://issues.apache.org/jira/browse/MESOS-7343
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> It now understands the environment variable `CMAKE_GENERATOR` so that
> users can change the generator; the default is still Visual Studio 14.
> 
> Direct usage of `msbuild` was replaced with `cmake --build`.
> 
> We now build `mesos-tests` in the same pattern as `stout-tests` and
> `libprocess-tests`. If admin priveleges are missing, we skip running the
> tests, but still proceed to build the general target.
> 
> We set the CMake toolset instead of an environment variable for
> `PreferredToolArchitecture`.
> 
> 
> Diffs
> -
> 
>   support/windows-build.bat 88e177ced912b61f8169b58a50d1cf6ba0c4c0c7 
> 
> 
> Diff: https://reviews.apache.org/r/59116/diff/1/
> 
> 
> Testing
> ---
> 
> Built and tested script with clean build on Windows.
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 59116: Windows: Updated `support/windows-build.bat`.

[Jeff Coffler]

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




support/windows-build.bat
Line 52 (original), 52 (patched)


Is this correct? Did we deprecate older toolsets? I thought that, at this 
point, we REQUIRE the newer version of Visual Studio.



support/windows-build.bat
Line 61 (original)


Since we're shifting from msbuild to cmake to build, does cmake assume /m 
(parallel builds)?



support/windows-build.bat
Line 68 (original), 57 (patched)


I know that we don't support NOT building Debug right now, but shouldn't 
this be configurable for windows-build.bat, perhaps with the default to build 
with debug?



support/windows-build.bat
Line 83 (original), 72 (patched)


So this is actually different from Linux.

On Linux, if you do a `make check`, libprocess, stout, and mesos tests are 
all run (even if one of them has an error). But on Windows with 
windows-build.bat, that's not the case.

I think it should be. Perhaps save the error state so you can report (at 
the end) that libprocess and/or stout tests failed. But given that it's routine 
for one or more tests to fail today, it's annoying that windows-build.bat isn't 
as good as Linux in this regard.


- Jeff Coffler


On May 10, 2017, 8:40 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59116/
> ---
> 
> (Updated May 10, 2017, 8:40 p.m.)
> 
> 
> Review request for mesos, Jeff Coffler, John Kordich, Joseph Wu, and Li Li.
> 
> 
> Bugs: MESOS-7343
> https://issues.apache.org/jira/browse/MESOS-7343
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> It now understands the environment variable `CMAKE_GENERATOR` so that
> users can change the generator; the default is still Visual Studio 14.
> 
> Direct usage of `msbuild` was replaced with `cmake --build`.
> 
> We now build `mesos-tests` in the same pattern as `stout-tests` and
> `libprocess-tests`. If admin priveleges are missing, we skip running the
> tests, but still proceed to build the general target.
> 
> We set the CMake toolset instead of an environment variable for
> `PreferredToolArchitecture`.
> 
> 
> Diffs
> -
> 
>   support/windows-build.bat 88e177ced912b61f8169b58a50d1cf6ba0c4c0c7 
> 
> 
> Diff: https://reviews.apache.org/r/59116/diff/1/
> 
> 
> Testing
> ---
> 
> Built and tested script with clean build on Windows.
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 59155: CMake: Enable `/debug:fastlink` on Windows.

[Andrew Schwartzmeyer]

> On May 10, 2017, 9:07 p.m., Jeff Coffler wrote:
> > cmake/CompilationConfigure.cmake
> > Lines 240 (patched)
> > 
> >
> > I'm not thrilled that we need to do a "hand patch" to debug using VS 
> > Code. I think the vast majority of Windows DEVS will hit this.
> > 
> > But it is what it is. I'm not entirely sure how this didn't make it 
> > when I added it, I could have sworn I looked at the command lines. But I 
> > guess not. Or I did but a last-minute change broke it.

Yeah, no worries. My opinion is that the faster build is worth more than having 
to hand-patch the extension for now, especially since it's a known issue that 
will be fixed with the next extension release (I confirmed this with an author 
of the extension).


- Andrew


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


On May 10, 2017, 8:59 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59155/
> ---
> 
> (Updated May 10, 2017, 8:59 p.m.)
> 
> 
> Review request for mesos, Jeff Coffler, John Kordich, Joseph Wu, and Li Li.
> 
> 
> Bugs: MESOS-7496
> https://issues.apache.org/jira/browse/MESOS-7496
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> A previous commit attempted to enable this option, but it was not
> actually being used by the linker. This patch correctly adds the option
> to each linker flag variable for shared libraries, modules, and
> executables, and only for the debug configuration. This results in much
> faster link times.
> 
> Also, the `/zc:inline` option was removed, as it is only relevant in
> optimized builds (i.e. release configurations).
> 
> 
> Diffs
> -
> 
>   cmake/CompilationConfigure.cmake 7b2669f0c54abf9b5e0fd60f8af01e97e1f0f86a 
> 
> 
> Diff: https://reviews.apache.org/r/59155/diff/2/
> 
> 
> Testing
> ---
> 
> Built and tested on Windows with VS Code.
> 
> NOTE: With `/debug:fastlink` properly enabled, the current VS Code C++ 
> debugger does not work without a manual update. See [this 
> thread](https://github.com/Microsoft/vscode-cpptools/issues/695#issuecomment-299554289)
>  for the work-around.
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 59157: Windows: Fixed toolset handling.

[Jeff Coffler]

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


Ship it!




Ship It!

- Jeff Coffler


On May 10, 2017, 8:37 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59157/
> ---
> 
> (Updated May 10, 2017, 8:37 p.m.)
> 
> 
> Review request for mesos, Jeff Coffler, John Kordich, Joseph Wu, and Li Li.
> 
> 
> Bugs: MESOS-7498
> https://issues.apache.org/jira/browse/MESOS-7498
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Mesos must be built with the 64-bit tools. Previously, we added a
> dependency which checked if a certain environment variable was set. When
> it failed, the message would be a cryptic message:
> 
> error MSB6006: "cmd.exe" exited with code 255.
> 
> As the environment variable had to be picked up by every toolchain
> (Visual Studio Code, CMake, MSBuild, Visual Studio), it was a frequent
> cause of build frustrations.
> 
> This patch removes the `ENSURE_TOOL_ARCH` custom command, and instead
> asserts at configuration time that the toolset matches `host=x64`. With
> this toolset, CMake embeds the preferred tool architecture in the
> generated solutions, eliminating the need for an environment variable to
> be set, or for `/p:PreferredToolArchitecture=x64` to be passed to the
> build tool directly.
> 
> 
> Diffs
> -
> 
>   cmake/CompilationConfigure.cmake 7b2669f0c54abf9b5e0fd60f8af01e97e1f0f86a 
>   src/CMakeLists.txt 40d921ee7026f5ac47efbf0243e1cafab57825f9 
>   src/slave/cmake/AgentConfigure.cmake 
> 2e7ae6eaf4212b728eccf0bee957bbf88ff8f3e3 
> 
> 
> Diff: https://reviews.apache.org/r/59157/diff/1/
> 
> 
> Testing
> ---
> 
> Clean build on Windows, with environment variable explicitly unset. Tested 
> with VS Code CMake extension, and with `cmake --build .` manually, and with 
> `msbuild` manually, and with Visual Studio manually.
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 59156: CMake: Use `list(APPEND x ...)` over `set(x ${x} ...)`.

[Jeff Coffler]

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


Ship it!




Ship It!

- Jeff Coffler


On May 10, 2017, 8:34 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59156/
> ---
> 
> (Updated May 10, 2017, 8:34 p.m.)
> 
> 
> Review request for mesos, Jeff Coffler, John Kordich, Joseph Wu, and Li Li.
> 
> 
> Bugs: MESOS-7497
> https://issues.apache.org/jira/browse/MESOS-7497
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This removes a CMake anti-pattern where `set` was used to append to a
> variable, instead of the more explicit `list(APPEND)` or
> `string(APPEND)` alternatives.
> 
> 
> Diffs
> -
> 
>   cmake/CompilationConfigure.cmake 7b2669f0c54abf9b5e0fd60f8af01e97e1f0f86a 
> 
> 
> Diff: https://reviews.apache.org/r/59156/diff/1/
> 
> 
> Testing
> ---
> 
> Re-configured and built on Windows.
> 
> NOTE: This syntax should be preferred going forward. There are more instances 
> of the anti-pattern the setting of source files, but we'll take care of it in 
> a later patch to avoid too much churn.
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 59155: CMake: Enable `/debug:fastlink` on Windows.

[Jeff Coffler]

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


Ship it!




Ship It!

- Jeff Coffler


On May 10, 2017, 8:59 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59155/
> ---
> 
> (Updated May 10, 2017, 8:59 p.m.)
> 
> 
> Review request for mesos, Jeff Coffler, John Kordich, Joseph Wu, and Li Li.
> 
> 
> Bugs: MESOS-7496
> https://issues.apache.org/jira/browse/MESOS-7496
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> A previous commit attempted to enable this option, but it was not
> actually being used by the linker. This patch correctly adds the option
> to each linker flag variable for shared libraries, modules, and
> executables, and only for the debug configuration. This results in much
> faster link times.
> 
> Also, the `/zc:inline` option was removed, as it is only relevant in
> optimized builds (i.e. release configurations).
> 
> 
> Diffs
> -
> 
>   cmake/CompilationConfigure.cmake 7b2669f0c54abf9b5e0fd60f8af01e97e1f0f86a 
> 
> 
> Diff: https://reviews.apache.org/r/59155/diff/2/
> 
> 
> Testing
> ---
> 
> Built and tested on Windows with VS Code.
> 
> NOTE: With `/debug:fastlink` properly enabled, the current VS Code C++ 
> debugger does not work without a manual update. See [this 
> thread](https://github.com/Microsoft/vscode-cpptools/issues/695#issuecomment-299554289)
>  for the work-around.
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 59155: CMake: Enable `/debug:fastlink` on Windows.

[Jeff Coffler]

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




cmake/CompilationConfigure.cmake
Lines 240 (patched)


I'm not thrilled that we need to do a "hand patch" to debug using VS Code. 
I think the vast majority of Windows DEVS will hit this.

But it is what it is. I'm not entirely sure how this didn't make it when I 
added it, I could have sworn I looked at the command lines. But I guess not. Or 
I did but a last-minute change broke it.


- Jeff Coffler


On May 10, 2017, 8:59 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59155/
> ---
> 
> (Updated May 10, 2017, 8:59 p.m.)
> 
> 
> Review request for mesos, Jeff Coffler, John Kordich, Joseph Wu, and Li Li.
> 
> 
> Bugs: MESOS-7496
> https://issues.apache.org/jira/browse/MESOS-7496
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> A previous commit attempted to enable this option, but it was not
> actually being used by the linker. This patch correctly adds the option
> to each linker flag variable for shared libraries, modules, and
> executables, and only for the debug configuration. This results in much
> faster link times.
> 
> Also, the `/zc:inline` option was removed, as it is only relevant in
> optimized builds (i.e. release configurations).
> 
> 
> Diffs
> -
> 
>   cmake/CompilationConfigure.cmake 7b2669f0c54abf9b5e0fd60f8af01e97e1f0f86a 
> 
> 
> Diff: https://reviews.apache.org/r/59155/diff/2/
> 
> 
> Testing
> ---
> 
> Built and tested on Windows with VS Code.
> 
> NOTE: With `/debug:fastlink` properly enabled, the current VS Code C++ 
> debugger does not work without a manual update. See [this 
> thread](https://github.com/Microsoft/vscode-cpptools/issues/695#issuecomment-299554289)
>  for the work-around.
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 59155: CMake: Enable `/debug:fastlink` on Windows.

[Andrew Schwartzmeyer]

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

(Updated May 10, 2017, 8:59 p.m.)


Review request for mesos, Jeff Coffler, John Kordich, Joseph Wu, and Li Li.


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


Repository: mesos


Description (updated)
---

A previous commit attempted to enable this option, but it was not
actually being used by the linker. This patch correctly adds the option
to each linker flag variable for shared libraries, modules, and
executables, and only for the debug configuration. This results in much
faster link times.

Also, the `/zc:inline` option was removed, as it is only relevant in
optimized builds (i.e. release configurations).


Diffs (updated)
-

  cmake/CompilationConfigure.cmake 7b2669f0c54abf9b5e0fd60f8af01e97e1f0f86a 


Diff: https://reviews.apache.org/r/59155/diff/2/

Changes: https://reviews.apache.org/r/59155/diff/1-2/


Testing
---

Built and tested on Windows with VS Code.

NOTE: With `/debug:fastlink` properly enabled, the current VS Code C++ debugger 
does not work without a manual update. See [this 
thread](https://github.com/Microsoft/vscode-cpptools/issues/695#issuecomment-299554289)
 for the work-around.


Thanks,

Andrew Schwartzmeyer

Re: Review Request 59155: CMake: Enable `/debug:fastlink` on Windows.

[Andrew Schwartzmeyer]

> On May 10, 2017, 8:37 p.m., Joseph Wu wrote:
> > cmake/CompilationConfigure.cmake
> > Lines 239 (patched)
> > 
> >
> > What about `STATIC`?  All libraries are built with static linkage on 
> > Windows (at the moment).
> 
> Andrew Schwartzmeyer wrote:
> Oh, uh, heh. This `foreach` loop came straight from 
> [CMake](https://gitlab.kitware.com/cmake/cmake/blob/master/Modules/Platform/Windows-MSVC.cmake#L163).
>  Checking if `CMAKE_STATIC_LINKER_FLAGS_DEBUG` is the right one to add.

Yup, adding.


- Andrew


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


On May 10, 2017, 8:32 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59155/
> ---
> 
> (Updated May 10, 2017, 8:32 p.m.)
> 
> 
> Review request for mesos, Jeff Coffler, John Kordich, Joseph Wu, and Li Li.
> 
> 
> Bugs: MESOS-7496
> https://issues.apache.org/jira/browse/MESOS-7496
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> A previous commit attempted to enable this option, but it was not
> actually being used by the linker. This patch correctly adds the option
> to each linker flag variable for shared libraries, modules, and
> executables, and only for the debug configuration. This results in much
> faster link times.
> 
> Also, the `/zc:inline` option was removed, as it is only relevant in
> optimized builds (i.e. release configurations).
> 
> 
> Diffs
> -
> 
>   cmake/CompilationConfigure.cmake 7b2669f0c54abf9b5e0fd60f8af01e97e1f0f86a 
> 
> 
> Diff: https://reviews.apache.org/r/59155/diff/1/
> 
> 
> Testing
> ---
> 
> Built and tested on Windows with VS Code.
> 
> NOTE: With `/debug:fastlink` properly enabled, the current VS Code C++ 
> debugger does not work without a manual update. See [this 
> thread](https://github.com/Microsoft/vscode-cpptools/issues/695#issuecomment-299554289)
>  for the work-around.
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 59155: CMake: Enable `/debug:fastlink` on Windows.

[Andrew Schwartzmeyer]

> On May 10, 2017, 8:37 p.m., Joseph Wu wrote:
> > cmake/CompilationConfigure.cmake
> > Lines 239 (patched)
> > 
> >
> > What about `STATIC`?  All libraries are built with static linkage on 
> > Windows (at the moment).

Oh, uh, heh. This `foreach` loop came straight from 
[CMake](https://gitlab.kitware.com/cmake/cmake/blob/master/Modules/Platform/Windows-MSVC.cmake#L163).
 Checking if `CMAKE_STATIC_LINKER_FLAGS_DEBUG` is the right one to add.


- Andrew


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


On May 10, 2017, 8:32 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59155/
> ---
> 
> (Updated May 10, 2017, 8:32 p.m.)
> 
> 
> Review request for mesos, Jeff Coffler, John Kordich, Joseph Wu, and Li Li.
> 
> 
> Bugs: MESOS-7496
> https://issues.apache.org/jira/browse/MESOS-7496
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> A previous commit attempted to enable this option, but it was not
> actually being used by the linker. This patch correctly adds the option
> to each linker flag variable for shared libraries, modules, and
> executables, and only for the debug configuration. This results in much
> faster link times.
> 
> Also, the `/zc:inline` option was removed, as it is only relevant in
> optimized builds (i.e. release configurations).
> 
> 
> Diffs
> -
> 
>   cmake/CompilationConfigure.cmake 7b2669f0c54abf9b5e0fd60f8af01e97e1f0f86a 
> 
> 
> Diff: https://reviews.apache.org/r/59155/diff/1/
> 
> 
> Testing
> ---
> 
> Built and tested on Windows with VS Code.
> 
> NOTE: With `/debug:fastlink` properly enabled, the current VS Code C++ 
> debugger does not work without a manual update. See [this 
> thread](https://github.com/Microsoft/vscode-cpptools/issues/695#issuecomment-299554289)
>  for the work-around.
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Review Request 59116: Windows: Updated `support/windows-build.bat`.

[Andrew Schwartzmeyer]

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

Review request for mesos, Jeff Coffler, John Kordich, Joseph Wu, and Li Li.


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


Repository: mesos


Description (updated)
---

It now understands the environment variable `CMAKE_GENERATOR` so that
users can change the generator; the default is still Visual Studio 14.

Direct usage of `msbuild` was replaced with `cmake --build`.

We now build `mesos-tests` in the same pattern as `stout-tests` and
`libprocess-tests`. If admin priveleges are missing, we skip running the
tests, but still proceed to build the general target.

We set the CMake toolset instead of an environment variable for
`PreferredToolArchitecture`.


Diffs (updated)
-

  support/windows-build.bat 88e177ced912b61f8169b58a50d1cf6ba0c4c0c7 


Diff: https://reviews.apache.org/r/59116/diff/1/


Testing (updated)
---

Built and tested script with clean build on Windows.


Thanks,

Andrew Schwartzmeyer

Review Request 59157: Windows: Fixed toolset handling.

[Andrew Schwartzmeyer]

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

Review request for mesos, Jeff Coffler, John Kordich, Joseph Wu, and Li Li.


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


Repository: mesos


Description
---

Mesos must be built with the 64-bit tools. Previously, we added a
dependency which checked if a certain environment variable was set. When
it failed, the message would be a cryptic message:

error MSB6006: "cmd.exe" exited with code 255.

As the environment variable had to be picked up by every toolchain
(Visual Studio Code, CMake, MSBuild, Visual Studio), it was a frequent
cause of build frustrations.

This patch removes the `ENSURE_TOOL_ARCH` custom command, and instead
asserts at configuration time that the toolset matches `host=x64`. With
this toolset, CMake embeds the preferred tool architecture in the
generated solutions, eliminating the need for an environment variable to
be set, or for `/p:PreferredToolArchitecture=x64` to be passed to the
build tool directly.


Diffs
-

  cmake/CompilationConfigure.cmake 7b2669f0c54abf9b5e0fd60f8af01e97e1f0f86a 
  src/CMakeLists.txt 40d921ee7026f5ac47efbf0243e1cafab57825f9 
  src/slave/cmake/AgentConfigure.cmake 2e7ae6eaf4212b728eccf0bee957bbf88ff8f3e3 


Diff: https://reviews.apache.org/r/59157/diff/1/


Testing
---

Clean build on Windows, with environment variable explicitly unset. Tested with 
VS Code CMake extension, and with `cmake --build .` manually, and with 
`msbuild` manually, and with Visual Studio manually.


Thanks,

Andrew Schwartzmeyer

Re: Review Request 59155: CMake: Enable `/debug:fastlink` on Windows.

[Joseph Wu]

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




cmake/CompilationConfigure.cmake
Lines 239 (patched)


What about `STATIC`?  All libraries are built with static linkage on 
Windows (at the moment).


- Joseph Wu


On May 10, 2017, 1:32 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59155/
> ---
> 
> (Updated May 10, 2017, 1:32 p.m.)
> 
> 
> Review request for mesos, Jeff Coffler, John Kordich, Joseph Wu, and Li Li.
> 
> 
> Bugs: MESOS-7496
> https://issues.apache.org/jira/browse/MESOS-7496
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> A previous commit attempted to enable this option, but it was not
> actually being used by the linker. This patch correctly adds the option
> to each linker flag variable for shared libraries, modules, and
> executables, and only for the debug configuration. This results in much
> faster link times.
> 
> Also, the `/zc:inline` option was removed, as it is only relevant in
> optimized builds (i.e. release configurations).
> 
> 
> Diffs
> -
> 
>   cmake/CompilationConfigure.cmake 7b2669f0c54abf9b5e0fd60f8af01e97e1f0f86a 
> 
> 
> Diff: https://reviews.apache.org/r/59155/diff/1/
> 
> 
> Testing
> ---
> 
> Built and tested on Windows with VS Code.
> 
> NOTE: With `/debug:fastlink` properly enabled, the current VS Code C++ 
> debugger does not work without a manual update. See [this 
> thread](https://github.com/Microsoft/vscode-cpptools/issues/695#issuecomment-299554289)
>  for the work-around.
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 59156: CMake: Use `list(APPEND x ...)` over `set(x ${x} ...)`.

[Andrew Schwartzmeyer]

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

(Updated May 10, 2017, 8:34 p.m.)


Review request for mesos, Jeff Coffler, John Kordich, Joseph Wu, and Li Li.


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


Repository: mesos


Description
---

This removes a CMake anti-pattern where `set` was used to append to a
variable, instead of the more explicit `list(APPEND)` or
`string(APPEND)` alternatives.


Diffs
-

  cmake/CompilationConfigure.cmake 7b2669f0c54abf9b5e0fd60f8af01e97e1f0f86a 


Diff: https://reviews.apache.org/r/59156/diff/1/


Testing
---

Re-configured and built on Windows.

NOTE: This syntax should be preferred going forward. There are more instances 
of the anti-pattern the setting of source files, but we'll take care of it in a 
later patch to avoid too much churn.


Thanks,

Andrew Schwartzmeyer

Review Request 59156: CMake: Use `list(APPEND x ...)` over `set(x ${x} ...)`.

[Andrew Schwartzmeyer]

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

Review request for mesos, Jeff Coffler, John Kordich, Joseph Wu, and Li Li.


Repository: mesos


Description
---

This removes a CMake anti-pattern where `set` was used to append to a
variable, instead of the more explicit `list(APPEND)` or
`string(APPEND)` alternatives.


Diffs
-

  cmake/CompilationConfigure.cmake 7b2669f0c54abf9b5e0fd60f8af01e97e1f0f86a 


Diff: https://reviews.apache.org/r/59156/diff/1/


Testing
---

Re-configured and built on Windows.

NOTE: This syntax should be preferred going forward. There are more instances 
of the anti-pattern the setting of source files, but we'll take care of it in a 
later patch to avoid too much churn.


Thanks,

Andrew Schwartzmeyer

Re: Review Request 59073: Updated Docker containerizer tests to use the old naming scheme.

[Joseph Wu]

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

(Updated May 10, 2017, 1:32 p.m.)


Review request for mesos and Jie Yu.


Changes
---

Missed a line.


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


Repository: mesos


Description
---

This changes test expectations of Docker container names back to the
0.22 scheme and simultaneously updates the tests to the new
containerizer interface.


Diffs (updated)
-

  src/tests/containerizer/docker_containerizer_tests.cpp 
4eef399b05f6e35d75c7c23992f0ccda04576277 


Diff: https://reviews.apache.org/r/59073/diff/2/

Changes: https://reviews.apache.org/r/59073/diff/1-2/


Testing
---

See last patch in chain.


Thanks,

Joseph Wu

Re: Review Request 58900: Changed the default fetcher cache directory.

[Joseph Wu]

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

(Updated May 10, 2017, 1:32 p.m.)


Review request for mesos and Jie Yu.


Changes
---

Need to change the local cluster helper to create a different fetcher cache 
directory per agent.


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


Repository: mesos


Description
---

The fetcher cache directory was historically located (by default)
in `/tmp/mesos/fetch`.  The agent flag `--fetcher_cache_dir` could
be used to change this value.

The fetcher would create a subdirectory underneath `/tmp/mesos/fetch`
for each `SlaveID`.  This was done because multiple agents can run on
the same node.  If all the agents use the same default fetcher cache
directory, they will collide and cause unpredictable results.
As a result, the `SlaveID` needed to be passed into the fetcher
after the agent recovers and/or registers with the master, because
that is when the `SlaveID` is determined.

This changes the default fetcher cache directory to
`/tmp/mesos/fetch`.  The `SlaveID` subdirectory has been removed.

This change, while techically a breaking change, is safe because of
how the fetcher uses this directory.  Upon starting up, the fetcher
"recovers" by clearing this directory.  Although the subdirectory
has been removed, the fetcher still clears the fetcher cache
on startup.

This change will only cause breakages if multiple agents are run
with the same `--fetcher_cache_dir`.  In this case, each agent
will delete the fetcher caches of all the other agents.

---

With the removal of the `SlaveID` field in the fetcher's methods,
it is no longer necessary to pass in the `SlaveID` or agent Flags
at agent recovery time.  Instead, the flags can be passed in during
the fetcher's construction.

Similarly, the fetcher's "recovery" (clearing the fetcher cache)
can be done immediately upon construction, which simplifies the
code slightly.


Diffs (updated)
-

  src/local/local.cpp e47980929db2da1f31cf899a0e1fc452070e11f3 
  src/slave/containerizer/fetcher.hpp 9e3018dc087ed55c61b2824d0105bc5339b83043 
  src/slave/containerizer/fetcher.cpp a910fea5a5556afb376524c5bb2ff98d7d84e611 
  src/slave/flags.cpp bc63a6a4cb6115b4b4d592e67e34045f52b50d4c 
  src/slave/main.cpp 507d59996a90f51c7cd1e0f836a34c2e9c484791 
  src/slave/slave.cpp 209aff180db1a68ae360d7c278937fe645efdb2b 


Diff: https://reviews.apache.org/r/58900/diff/4/

Changes: https://reviews.apache.org/r/58900/diff/3-4/


Testing
---

See last patch in chain.


Thanks,

Joseph Wu

Review Request 59155: CMake: Enable `/debug:fastlink` on Windows.

[Andrew Schwartzmeyer]

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

Review request for mesos, Jeff Coffler, John Kordich, Joseph Wu, and Li Li.


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


Repository: mesos


Description
---

A previous commit attempted to enable this option, but it was not
actually being used by the linker. This patch correctly adds the option
to each linker flag variable for shared libraries, modules, and
executables, and only for the debug configuration. This results in much
faster link times.

Also, the `/zc:inline` option was removed, as it is only relevant in
optimized builds (i.e. release configurations).


Diffs
-

  cmake/CompilationConfigure.cmake 7b2669f0c54abf9b5e0fd60f8af01e97e1f0f86a 


Diff: https://reviews.apache.org/r/59155/diff/1/


Testing
---

Built and tested on Windows with VS Code.

NOTE: With `/debug:fastlink` properly enabled, the current VS Code C++ debugger 
does not work without a manual update. See [this 
thread](https://github.com/Microsoft/vscode-cpptools/issues/695#issuecomment-299554289)
 for the work-around.


Thanks,

Andrew Schwartzmeyer

Re: Review Request 56681: Use glog to log EXIT() messages.

[Benjamin Mahler]

> On March 20, 2017, 8:32 p.m., Benjamin Mahler wrote:
> > Have you seen the feedback 
> > [here](https://issues.apache.org/jira/browse/MESOS-3465?focusedCommentId=14944383=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14944383)?
> >  It would be nice to make it clear in the log that the program is exiting 
> > IMHO.
> 
> Jiang Yan Xu wrote:
> I think you meant https://issues.apache.org/jira/browse/MESOS-3465

Yes, this comment: 
https://issues.apache.org/jira/browse/MESOS-3465?focusedCommentId=14944383=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14944383


- Benjamin


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


On May 8, 2017, 11:30 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56681/
> ---
> 
> (Updated May 8, 2017, 11:30 p.m.)
> 
> 
> Review request for mesos, haosdent huang and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-7115
> https://issues.apache.org/jira/browse/MESOS-7115
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Use glog to log EXIT() messages.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/exit.hpp 
> 762864afd8f86f7e1f439ef6ea7e3965a5f147d5 
> 
> 
> Diff: https://reviews.apache.org/r/56681/diff/2/
> 
> 
> Testing
> ---
> 
> make check (Fedora 25)
> 
> **Example output after this change:**
> ```
> [jpeach@jpeach build]$ sudo ./src/mesos-agent --work_dir=/nothing/here 
> --master=phoney:5050
> WARNING: Logging before InitGoogleLogging() is written to STDERR
> I0320 11:01:10.362782   921 main.cpp:368] Build: 2017-03-20 10:43:40 by jpeach
> I0320 11:01:10.362900   921 main.cpp:369] Version: 1.3.0
> I0320 11:01:10.362912   921 main.cpp:376] Git SHA: 
> b0d82241a886e9c6239ce82c5135c4c72c44aca7
> I0320 11:01:10.381340   921 systemd.cpp:238] systemd version `231` detected
> I0320 11:01:10.381404   921 main.cpp:478] Inializing systemd state
> I0320 11:01:10.390058   921 systemd.cpp:326] Started systemd slice 
> `mesos_executors.slice`
> I0320 11:01:10.392071   921 containerizer.cpp:221] Using isolation: 
> posix/cpu,posix/mem,filesystem/posix,network/cni
> I0320 11:01:10.401108   921 linux_launcher.cpp:150] Using 
> /sys/fs/cgroup/freezer as the freezer hierarchy for the Linux launcher
> I0320 11:01:10.402873   921 provisioner.cpp:249] Using default backend 
> 'overlay'
> E0320 11:01:10.426219   921 main.cpp:507] Failed to create a master detector: 
> Failed to parse 'phoney:5050'
> ```
> 
> **Example output before this change:**
> ```
> [jpeach@jpeach build]$ sudo /opt/mesos/sbin//mesos-agent 
> --work_dir=/nothing/here --master=phoney:5050
> WARNING: Logging before InitGoogleLogging() is written to STDERR
> I0320 11:03:43.868015  1188 main.cpp:243] Build: 2017-03-10 10:49:55 by jpeach
> I0320 11:03:43.868135  1188 main.cpp:244] Version: 1.3.0
> I0320 11:03:43.868151  1188 main.cpp:251] Git SHA: 
> 487c667260ec89127e18c77b9e8c8c243cf6ec57
> I0320 11:03:43.886561  1188 systemd.cpp:238] systemd version `231` detected
> I0320 11:03:43.886631  1188 main.cpp:342] Inializing systemd state
> I0320 11:03:43.896606  1188 systemd.cpp:326] Started systemd slice 
> `mesos_executors.slice`
> I0320 11:03:43.899202  1188 containerizer.cpp:221] Using isolation: 
> posix/cpu,posix/mem,filesystem/posix,network/cni
> I0320 11:03:43.911183  1188 linux_launcher.cpp:150] Using 
> /sys/fs/cgroup/freezer as the freezer hierarchy for the Linux launcher
> I0320 11:03:43.913216  1188 provisioner.cpp:249] Using default backend 
> 'overlay'
> Failed to create a master detector: Failed to parse 'phoney:5050'
> ```
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 58900: Changed the default fetcher cache directory.

[Joseph Wu]

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

(Updated May 10, 2017, 1:24 p.m.)


Review request for mesos and Jie Yu.


Changes
---

Went with the simplest approach.  The `--fetcher_cache_dir` is no longer the 
_parent_ of the fetcher cache directory.  It is the fetcher cache directory.  
Period.


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


Repository: mesos


Description (updated)
---

The fetcher cache directory was historically located (by default)
in `/tmp/mesos/fetch`.  The agent flag `--fetcher_cache_dir` could
be used to change this value.

The fetcher would create a subdirectory underneath `/tmp/mesos/fetch`
for each `SlaveID`.  This was done because multiple agents can run on
the same node.  If all the agents use the same default fetcher cache
directory, they will collide and cause unpredictable results.
As a result, the `SlaveID` needed to be passed into the fetcher
after the agent recovers and/or registers with the master, because
that is when the `SlaveID` is determined.

This changes the default fetcher cache directory to
`/tmp/mesos/fetch`.  The `SlaveID` subdirectory has been removed.

This change, while techically a breaking change, is safe because of
how the fetcher uses this directory.  Upon starting up, the fetcher
"recovers" by clearing this directory.  Although the subdirectory
has been removed, the fetcher still clears the fetcher cache
on startup.

This change will only cause breakages if multiple agents are run
with the same `--fetcher_cache_dir`.  In this case, each agent
will delete the fetcher caches of all the other agents.

---

With the removal of the `SlaveID` field in the fetcher's methods,
it is no longer necessary to pass in the `SlaveID` or agent Flags
at agent recovery time.  Instead, the flags can be passed in during
the fetcher's construction.

Similarly, the fetcher's "recovery" (clearing the fetcher cache)
can be done immediately upon construction, which simplifies the
code slightly.


Diffs (updated)
-

  src/local/local.cpp e47980929db2da1f31cf899a0e1fc452070e11f3 
  src/slave/containerizer/fetcher.hpp 9e3018dc087ed55c61b2824d0105bc5339b83043 
  src/slave/containerizer/fetcher.cpp a910fea5a5556afb376524c5bb2ff98d7d84e611 
  src/slave/flags.cpp bc63a6a4cb6115b4b4d592e67e34045f52b50d4c 
  src/slave/main.cpp 507d59996a90f51c7cd1e0f836a34c2e9c484791 
  src/slave/slave.cpp 209aff180db1a68ae360d7c278937fe645efdb2b 


Diff: https://reviews.apache.org/r/58900/diff/3/

Changes: https://reviews.apache.org/r/58900/diff/2-3/


Testing
---

See last patch in chain.


Thanks,

Joseph Wu

Re: Review Request 58099: Added authorization for frameworks in `GetRoles` v1 API.

[Alexander Rojas]

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


Ship it!




Ship It!

- Alexander Rojas


On May 10, 2017, 3:52 p.m., Jay Guo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58099/
> ---
> 
> (Updated May 10, 2017, 3:52 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, and Benjamin Mahler.
> 
> 
> Bugs: MESOS-7260
> https://issues.apache.org/jira/browse/MESOS-7260
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added authorization for frameworks in `GetRoles` v1 API.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp e2590a17044ac019b24a24629428d4ec8adc0c31 
> 
> 
> Diff: https://reviews.apache.org/r/58099/diff/6/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jay Guo
> 
>

Re: Review Request 58096: Added authorization for frameworks in /roles endpoint.

[Alexander Rojas]

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


Ship it!




Ship It!

- Alexander Rojas


On May 10, 2017, 6:33 p.m., Jay Guo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58096/
> ---
> 
> (Updated May 10, 2017, 6:33 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, and Benjamin Mahler.
> 
> 
> Bugs: MESOS-7260
> https://issues.apache.org/jira/browse/MESOS-7260
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> While /roles displays a list of frameworksIds that register with
> a role, it did NOT filter them based on VIEW_FRAMEWORK ACL, which
> impose a security risk. This patch fixed this issue by taking a
> frameworksApprover in `Master::Http::roles()` which is used to
> filter framework IDs.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp e2590a17044ac019b24a24629428d4ec8adc0c31 
> 
> 
> Diff: https://reviews.apache.org/r/58096/diff/7/
> 
> 
> Testing
> ---
> 
> see next patch in the chain.
> 
> 
> Thanks,
> 
> Jay Guo
> 
>

Re: Review Request 58097: Added a test to check framework filtering in /roles endpoint.

[Alexander Rojas]

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


Ship it!




Ship It!

- Alexander Rojas


On May 10, 2017, 3:47 p.m., Jay Guo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58097/
> ---
> 
> (Updated May 10, 2017, 3:47 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, and Benjamin Mahler.
> 
> 
> Bugs: MESOS-7260
> https://issues.apache.org/jira/browse/MESOS-7260
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a test to check framework filtering in /roles endpoint.
> 
> 
> Diffs
> -
> 
>   src/tests/master_authorization_tests.cpp 
> e4233c19b1d9e3e2734259503d0daec4ce243667 
> 
> 
> Diff: https://reviews.apache.org/r/58097/diff/6/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jay Guo
> 
>

Re: Review Request 58095: Refactored `Master::Http::roles` to use `jsonify`.

[Alexander Rojas]

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


Ship it!




Ship It!

- Alexander Rojas


On May 10, 2017, 6:32 p.m., Jay Guo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58095/
> ---
> 
> (Updated May 10, 2017, 6:32 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, Benjamin Mahler, and 
> Michael Park.
> 
> 
> Bugs: MESOS-4732 and MESOS-7260
> https://issues.apache.org/jira/browse/MESOS-4732
> https://issues.apache.org/jira/browse/MESOS-7260
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Refactored `Master::Http::roles` to use `jsonify`.
> 
> 
> Diffs
> -
> 
>   src/common/http.hpp 93d6088e97c2384f9f6d26e010a501abf2deb43e 
>   src/common/http.cpp 167dce2b9a2d3b68a1df5b4079f701482d34db28 
>   src/master/http.cpp e2590a17044ac019b24a24629428d4ec8adc0c31 
> 
> 
> Diff: https://reviews.apache.org/r/58095/diff/7/
> 
> 
> Testing
> ---
> 
> no functional changes
> make check
> 
> 
> Thanks,
> 
> Jay Guo
> 
>

Re: Review Request 59154: Added workaround to unbreak the build with glibc <= 2.12.

[Michael Park]

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


Ship it!




Ship It!

- Michael Park


On May 10, 2017, 11:59 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59154/
> ---
> 
> (Updated May 10, 2017, 11:59 a.m.)
> 
> 
> Review request for mesos, James Peach, Michael Park, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-7378
> https://issues.apache.org/jira/browse/MESOS-7378
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In old versions of glibc, the  header must be enclosed
> in an `extern "C" { ... }` block. This prevents Mesos from linking on
> CentOS 6.9 and likely other platforms from the same timeframe.
> 
> In more recent versions of glibc, the `extern "C" { ... }` linkage
> declaration is not necessary (because the header file specifies this
> itself). In this situation, there will now be two linkage declarations,
> but that is fine: the inner-most linkage declaration takes precedence.
> 
> 
> Diffs
> -
> 
>   src/linux/fs.cpp 944509f357d2e3f52365a8359738cf170dfcc563 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/devices.cpp 
> e995ff450d710d847eab827b7021f454f475fcef 
>   src/slave/containerizer/mesos/isolators/gpu/isolator.cpp 
> 3fd14951fd93a1a6230ccd5c446fdf4ba3915878 
>   src/tests/containerizer/fs_tests.cpp 
> 0fa9451553990deadb2693494835554c5c34994f 
> 
> 
> Diff: https://reviews.apache.org/r/59154/diff/1/
> 
> 
> Testing
> ---
> 
> `make check` on OSX, CentOS 6.9 (old glibc), and recent Arch Linux (new 
> glibc).
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 59018: Supported Image::Secret in docker URI fetcher plugin.

[Chun-Hung Hsiao]

> On May 10, 2017, 6:12 p.m., Chun-Hung Hsiao wrote:
> > src/uri/fetchers/docker.cpp
> > Lines 328 (patched)
> > 
> >
> > Why are we using `secretValue` instead of `config` as in other places? 
> > Also, it seems to me that `uri` and `secretValue` are coupled, so how about 
> > putting them side by side in the parameter list? Same for the following 
> > code.

OK I got why we're using `secretValue`. Please refer to my comment in 
https://reviews.apache.org/r/59141/.


- Chun-Hung


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


On May 10, 2017, 12:48 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59018/
> ---
> 
> (Updated May 10, 2017, 12:48 p.m.)
> 
> 
> Review request for mesos, Adam B, Chun-Hung Hsiao, Jie Yu, Kapil Arya, Till 
> Toenshoff, and Vinod Kone.
> 
> 
> Bugs: MESOS-7088
> https://issues.apache.org/jira/browse/MESOS-7088
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Supported Image::Secret in docker URI fetcher plugin.
> 
> 
> Diffs
> -
> 
>   src/uri/fetchers/docker.cpp 44169bf5f22f0ffd9fad7bb3b8f7d2a4989c6415 
> 
> 
> Diff: https://reviews.apache.org/r/59018/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> Manually tested using mesos-execute with task group. Verified that pulling 
> separate private image from different registries using different image 
> secrets works correctly.
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 59015: Implemented passing docker config depended methods.

[Gilbert Song]

> On May 9, 2017, 12:19 a.m., Jie Yu wrote:
> > As we discussed offline, let's not mess the URI struct. Instead, passing 
> > the secret to the fetcher using an function parameter in the `fetch` 
> > method. This will be useful in the future also because fetcher other URIs 
> > might need secret as well.

Yep, this is fixed in patch /59141.


- Gilbert


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


On May 10, 2017, 5:47 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59015/
> ---
> 
> (Updated May 10, 2017, 5:47 a.m.)
> 
> 
> Review request for mesos, Adam B, Chun-Hung Hsiao, Jie Yu, Kapil Arya, Till 
> Toenshoff, and Vinod Kone.
> 
> 
> Bugs: MESOS-7088
> https://issues.apache.org/jira/browse/MESOS-7088
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented passing docker config depended methods.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
> 6db788dd0c582deadf3e91c4d21bb9c20cf94e6b 
> 
> 
> Diff: https://reviews.apache.org/r/59015/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 59141: Added Secret::Value to the URI fetcher interface.

[Chun-Hung Hsiao]

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




include/mesos/uri/fetcher.hpp
Lines 78 (patched)


I'm thinking about what is a proper interface extension for this fetcher 
plugin. Instead of having a `Secret::Value`, how about a more general `config` 
or some session info parameter for the fetcher, which may or may not come from 
a secret?


- Chun-Hung Hsiao


On May 10, 2017, 12:48 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59141/
> ---
> 
> (Updated May 10, 2017, 12:48 p.m.)
> 
> 
> Review request for mesos, Adam B, Chun-Hung Hsiao, Jie Yu, Kapil Arya, Till 
> Toenshoff, and Vinod Kone.
> 
> 
> Bugs: MESOS-7088
> https://issues.apache.org/jira/browse/MESOS-7088
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added Secret::Value to the URI fetcher interface.
> 
> 
> Diffs
> -
> 
>   include/mesos/uri/fetcher.hpp ebf86c78b794a6ef46332df788a1317bbec5d983 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
> 6db788dd0c582deadf3e91c4d21bb9c20cf94e6b 
>   src/uri/fetcher.cpp f4d1a4c69e910260dc536aa42ae03fd17403b060 
>   src/uri/fetchers/copy.hpp f4a2fb3d5156e5ebfdf7c4202f8dc9b1cd1d6ac7 
>   src/uri/fetchers/copy.cpp 5e1470503f4fa0e543680c93b2ad3e36351afc1c 
>   src/uri/fetchers/curl.hpp 083f155092d159cd83069bfdfd905d679e9ab57c 
>   src/uri/fetchers/curl.cpp 24b53c77946170cba45152c458d85b6fddfce9f8 
>   src/uri/fetchers/docker.hpp 65e01cba1d41688a8ee5da73d1d6f57515fbc7f5 
>   src/uri/fetchers/docker.cpp 44169bf5f22f0ffd9fad7bb3b8f7d2a4989c6415 
>   src/uri/fetchers/hadoop.hpp 4923dc6491d7cc6efc6ced4f5356af9f945ba5d2 
>   src/uri/fetchers/hadoop.cpp 3c5ffe607c92ea1ab66ba261bd70031f2907cea6 
> 
> 
> Diff: https://reviews.apache.org/r/59141/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 59013: Implemented passing Image::Secret Puller::pull().

[Chun-Hung Hsiao]

> On May 10, 2017, 6:53 p.m., Chun-Hung Hsiao wrote:
> > src/slave/containerizer/mesos/provisioner/docker/puller.hpp
> > Lines 65 (patched)
> > 
> >
> > Do we need a default value here? It seems that all subclasses need to 
> > declare this virtual function anyway.

Forget about it lol. I made a mistake.


- Chun-Hung


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


On May 10, 2017, 12:47 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59013/
> ---
> 
> (Updated May 10, 2017, 12:47 p.m.)
> 
> 
> Review request for mesos, Adam B, Chun-Hung Hsiao, Jie Yu, Kapil Arya, Till 
> Toenshoff, and Vinod Kone.
> 
> 
> Bugs: MESOS-7088
> https://issues.apache.org/jira/browse/MESOS-7088
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented passing Image::Secret Puller::pull().
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/docker/local_puller.hpp 
> 344baab5c0365c3fc2dc814887bb2b48082b050f 
>   src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp 
> 5bc68d2047a7b3e3fa30315d61073e342ba6affe 
>   src/slave/containerizer/mesos/provisioner/docker/puller.hpp 
> 6dacdb1403433a37dd31a93d4ff2e37d4685eb87 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.hpp 
> 62ddb7a332030f3116477408d8b16c19e434c159 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
> 6db788dd0c582deadf3e91c4d21bb9c20cf94e6b 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> 68ce265a5d0a61f8d9ed55dd14c630dcf893a7d2 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> 4341621767a9fa5be2c66e77ef60f0c65dae58ca 
> 
> 
> Diff: https://reviews.apache.org/r/59013/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 59154: Added workaround to unbreak the build with glibc <= 2.12.

[Jiang Yan Xu]

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


Ship it!




Ship It!

- Jiang Yan Xu


On May 10, 2017, 11:59 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59154/
> ---
> 
> (Updated May 10, 2017, 11:59 a.m.)
> 
> 
> Review request for mesos, James Peach, Michael Park, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-7378
> https://issues.apache.org/jira/browse/MESOS-7378
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In old versions of glibc, the  header must be enclosed
> in an `extern "C" { ... }` block. This prevents Mesos from linking on
> CentOS 6.9 and likely other platforms from the same timeframe.
> 
> In more recent versions of glibc, the `extern "C" { ... }` linkage
> declaration is not necessary (because the header file specifies this
> itself). In this situation, there will now be two linkage declarations,
> but that is fine: the inner-most linkage declaration takes precedence.
> 
> 
> Diffs
> -
> 
>   src/linux/fs.cpp 944509f357d2e3f52365a8359738cf170dfcc563 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/devices.cpp 
> e995ff450d710d847eab827b7021f454f475fcef 
>   src/slave/containerizer/mesos/isolators/gpu/isolator.cpp 
> 3fd14951fd93a1a6230ccd5c446fdf4ba3915878 
>   src/tests/containerizer/fs_tests.cpp 
> 0fa9451553990deadb2693494835554c5c34994f 
> 
> 
> Diff: https://reviews.apache.org/r/59154/diff/1/
> 
> 
> Testing
> ---
> 
> `make check` on OSX, CentOS 6.9 (old glibc), and recent Arch Linux (new 
> glibc).
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Review Request 59154: Added workaround to unbreak the build with glibc <= 2.12.

[Neil Conway]

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

Review request for mesos, James Peach, Michael Park, and Jiang Yan Xu.


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


Repository: mesos


Description
---

In old versions of glibc, the  header must be enclosed
in an `extern "C" { ... }` block. This prevents Mesos from linking on
CentOS 6.9 and likely other platforms from the same timeframe.

In more recent versions of glibc, the `extern "C" { ... }` linkage
declaration is not necessary (because the header file specifies this
itself). In this situation, there will now be two linkage declarations,
but that is fine: the inner-most linkage declaration takes precedence.


Diffs
-

  src/linux/fs.cpp 944509f357d2e3f52365a8359738cf170dfcc563 
  src/slave/containerizer/mesos/isolators/cgroups/subsystems/devices.cpp 
e995ff450d710d847eab827b7021f454f475fcef 
  src/slave/containerizer/mesos/isolators/gpu/isolator.cpp 
3fd14951fd93a1a6230ccd5c446fdf4ba3915878 
  src/tests/containerizer/fs_tests.cpp 0fa9451553990deadb2693494835554c5c34994f 


Diff: https://reviews.apache.org/r/59154/diff/1/


Testing
---

`make check` on OSX, CentOS 6.9 (old glibc), and recent Arch Linux (new glibc).


Thanks,

Neil Conway

Re: Review Request 59013: Implemented passing Image::Secret Puller::pull().

[Chun-Hung Hsiao]

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




src/slave/containerizer/mesos/provisioner/docker/puller.hpp
Lines 65 (patched)


Do we need a default value here? It seems that all subclasses need to 
declare this virtual function anyway.


- Chun-Hung Hsiao


On May 10, 2017, 12:47 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59013/
> ---
> 
> (Updated May 10, 2017, 12:47 p.m.)
> 
> 
> Review request for mesos, Adam B, Chun-Hung Hsiao, Jie Yu, Kapil Arya, Till 
> Toenshoff, and Vinod Kone.
> 
> 
> Bugs: MESOS-7088
> https://issues.apache.org/jira/browse/MESOS-7088
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented passing Image::Secret Puller::pull().
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/docker/local_puller.hpp 
> 344baab5c0365c3fc2dc814887bb2b48082b050f 
>   src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp 
> 5bc68d2047a7b3e3fa30315d61073e342ba6affe 
>   src/slave/containerizer/mesos/provisioner/docker/puller.hpp 
> 6dacdb1403433a37dd31a93d4ff2e37d4685eb87 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.hpp 
> 62ddb7a332030f3116477408d8b16c19e434c159 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
> 6db788dd0c582deadf3e91c4d21bb9c20cf94e6b 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> 68ce265a5d0a61f8d9ed55dd14c630dcf893a7d2 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> 4341621767a9fa5be2c66e77ef60f0c65dae58ca 
> 
> 
> Diff: https://reviews.apache.org/r/59013/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 59012: Implemented passing the secret resolver to registry puller.

[Chun-Hung Hsiao]

> On May 9, 2017, 12:17 a.m., Vinod Kone wrote:
> > src/slave/containerizer/mesos/provisioner/docker/puller.hpp
> > Lines 48 (patched)
> > 
> >
> > no need for a default?
> 
> Gilbert Song wrote:
> it does not hurt here. but we should set a default.

Actually it seems to me that no default is more preferable unless necessary 
(say for backward compatibility). More explicity makes the compiler to catch 
more bugs :)


- Chun-Hung


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


On May 10, 2017, 12:47 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59012/
> ---
> 
> (Updated May 10, 2017, 12:47 p.m.)
> 
> 
> Review request for mesos, Adam B, Chun-Hung Hsiao, Jie Yu, Kapil Arya, Till 
> Toenshoff, and Vinod Kone.
> 
> 
> Bugs: MESOS-7088
> https://issues.apache.org/jira/browse/MESOS-7088
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented passing the secret resolver to registry puller.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> b58baed64480e22f640a4852537f85922ed382ae 
>   src/slave/containerizer/mesos/provisioner/appc/store.hpp 
> 15c79e9401a821e445fbd32b34503e4fb0014d42 
>   src/slave/containerizer/mesos/provisioner/appc/store.cpp 
> 09a40a5835454bb7519d11bae4a851337a89b935 
>   src/slave/containerizer/mesos/provisioner/docker/puller.hpp 
> 6dacdb1403433a37dd31a93d4ff2e37d4685eb87 
>   src/slave/containerizer/mesos/provisioner/docker/puller.cpp 
> ac9dae8ecbb897b8ff942d11ac70281a63e06831 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.hpp 
> 62ddb7a332030f3116477408d8b16c19e434c159 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
> 6db788dd0c582deadf3e91c4d21bb9c20cf94e6b 
>   src/slave/containerizer/mesos/provisioner/docker/store.hpp 
> e1abff19c8877ad07900bb2f44deca1cdda41f58 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> 68ce265a5d0a61f8d9ed55dd14c630dcf893a7d2 
>   src/slave/containerizer/mesos/provisioner/provisioner.hpp 
> 7d6c1b93a2c0e265b9344a0fc27f1cf4ed5325f2 
>   src/slave/containerizer/mesos/provisioner/provisioner.cpp 
> be45fc59027f176b43b767e9441fd8089ceec7b4 
>   src/slave/containerizer/mesos/provisioner/store.hpp 
> 82a9be64264ae829773c1e2e8a4360f78641cbf6 
>   src/slave/containerizer/mesos/provisioner/store.cpp 
> 7141d63fcf2dbc3fbf00508c7f92945aab014fb2 
> 
> 
> Diff: https://reviews.apache.org/r/59012/diff/4/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 58907: Refactored Docker containerizer launch path per interface changes.

[Joseph Wu]

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

(Updated May 10, 2017, 11:33 a.m.)


Review request for mesos and Jie Yu.


Changes
---

Fix order of arguments in `Docker::Container::Create(...)`


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


Repository: mesos


Description
---

This continues the containerizer interface change that replaced
most "Infos" (Task, Executor, Command, Container) with a single
`ContainerConfig` and combined the nested/non-nested container
launch paths.

Notably, this commit also changes the fields stored in the Docker
containerizer to match the new interface.  The somewhat ambiguously
named `directory` field has been renamed to `containerWorkDir`.
And the copy of `Slave::flags` in each container has been removed
because it is not used.


Diffs (updated)
-

  src/slave/containerizer/docker.hpp 44efa44586455df416fc9cdc51c92bd9027f24c3 
  src/slave/containerizer/docker.cpp 2480f595bee032ea399d642653d618472db0b7a7 


Diff: https://reviews.apache.org/r/58907/diff/2/

Changes: https://reviews.apache.org/r/58907/diff/1-2/


Testing
---

See last patch in chain.


Thanks,

Joseph Wu

Re: Review Request 59017: Added support for docker spec helper 'parseAuthConfig()'.

[Chun-Hung Hsiao]

> On May 9, 2017, 11:25 p.m., Vinod Kone wrote:
> > include/mesos/docker/spec.hpp
> > Lines 83 (patched)
> > 
> >
> > no unit test for this?
> 
> Gilbert Song wrote:
> This helper was tested in unit test when the parameter is a JSON object. 
> Thought no duplicate unit test needed for the `string` parameter since they 
> are the same implementation.

Since the string version calls the JSON version, how about just testing the 
string version instead so we have both functions covered?


- Chun-Hung


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


On May 10, 2017, 12:47 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59017/
> ---
> 
> (Updated May 10, 2017, 12:47 p.m.)
> 
> 
> Review request for mesos, Adam B, Chun-Hung Hsiao, Jie Yu, Kapil Arya, Till 
> Toenshoff, and Vinod Kone.
> 
> 
> Bugs: MESOS-7088
> https://issues.apache.org/jira/browse/MESOS-7088
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added support for docker spec helper 'parseAuthConfig()'.
> 
> 
> Diffs
> -
> 
>   include/mesos/docker/spec.hpp b90f731ef07c22259715543526eaf25cb94eab03 
>   src/docker/spec.cpp 6b5588e534215451593cc20011847c3abbab9b17 
> 
> 
> Diff: https://reviews.apache.org/r/59017/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 59018: Supported Image::Secret in docker URI fetcher plugin.

[Chun-Hung Hsiao]

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



Please rebase to the latest master and resolve the conflicts.


src/uri/fetchers/docker.cpp
Lines 328 (patched)


Why are we using `secretValue` instead of `config` as in other places? 
Also, it seems to me that `uri` and `secretValue` are coupled, so how about 
putting them side by side in the parameter list? Same for the following code.



src/uri/fetchers/docker.cpp
Lines 525 (patched)


It's unfortunate that we still need `secretValue` for fetching blobs after 
we have `authHeaders` but I cannot think of a way to make it cleaner, so just 
leave a note here.


- Chun-Hung Hsiao


On May 10, 2017, 12:48 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59018/
> ---
> 
> (Updated May 10, 2017, 12:48 p.m.)
> 
> 
> Review request for mesos, Adam B, Chun-Hung Hsiao, Jie Yu, Kapil Arya, Till 
> Toenshoff, and Vinod Kone.
> 
> 
> Bugs: MESOS-7088
> https://issues.apache.org/jira/browse/MESOS-7088
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Supported Image::Secret in docker URI fetcher plugin.
> 
> 
> Diffs
> -
> 
>   src/uri/fetchers/docker.cpp 44169bf5f22f0ffd9fad7bb3b8f7d2a4989c6415 
> 
> 
> Diff: https://reviews.apache.org/r/59018/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> Manually tested using mesos-execute with task group. Verified that pulling 
> separate private image from different registries using different image 
> secrets works correctly.
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Review Request 59150: Document LIBPROCESS_REQUIRE_PEER_ADDRESS_IP_MATCH.

[James Peach]

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

Review request for mesos and Benjamin Mahler.


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


Repository: mesos


Description
---

Document LIBPROCESS_REQUIRE_PEER_ADDRESS_IP_MATCH.


Diffs
-

  docs/configuration.md c5194e7566c13514f55742aa7278e75314f69d9c 


Diff: https://reviews.apache.org/r/59150/diff/1/


Testing
---

None.


Thanks,

James Peach

Re: Review Request 58224: Optionally verify the source IP address for libprocess messages.

[James Peach]

> On May 2, 2017, 12:34 a.m., Benjamin Mahler wrote:
> > 3rdparty/libprocess/src/process.cpp
> > Lines 2878-2879 (patched)
> > 
> >
> > We could refer to the flag help for examples?

I updated the text a bit.


> On May 2, 2017, 12:34 a.m., Benjamin Mahler wrote:
> > 3rdparty/libprocess/src/process.cpp
> > Lines 2883-2884 (patched)
> > 
> >
> > How about:
> > 
> > UPID IP address validation failed: Message from X was sent from IP Y.

This is an improvement, I made this change.


- James


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


On May 10, 2017, 6:06 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58224/
> ---
> 
> (Updated May 10, 2017, 6:06 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-7401
> https://issues.apache.org/jira/browse/MESOS-7401
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In general, libprocess is unable to validate that a peer
> is a legitimate owner of the UPID it claims in a libprocess
> message. This change adds a check that the IP address in the
> UPID matches the peer address. This makes spoofing the UPID
> harder (eg. to send authenticated messages), but also breaks
> some legitimate configurations, particularly on multihomed
> hosts.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp 
> 96ce7dbc486a2f1d55d2238a8a102bf024b12b1c 
> 
> 
> Diff: https://reviews.apache.org/r/58224/diff/8/
> 
> 
> Testing
> ---
> 
> make check (Fedora 25). Light manual testing.
> 
> With LIBPROCESS_require_peer_address_ip_match=true, all Mesos tests pass 
> except ``ExamplesTest.DiskFullFramework``, however enabling this will 
> definitely break some libprocess APIs (though not in the way that Mesos uses 
> them) and legitimate multi-homed configurations. Note that setting 
> LIBPROCESS_ip=127.0.0.1 makes you multihomed for this purpose, which is why 
> ``ExamplesTest.DiskFullFramework`` breaks.
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 58224: Optionally verify the source IP address for libprocess messages.

[James Peach]

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

(Updated May 10, 2017, 6:06 p.m.)


Review request for mesos and Benjamin Mahler.


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


Repository: mesos


Description
---

In general, libprocess is unable to validate that a peer
is a legitimate owner of the UPID it claims in a libprocess
message. This change adds a check that the IP address in the
UPID matches the peer address. This makes spoofing the UPID
harder (eg. to send authenticated messages), but also breaks
some legitimate configurations, particularly on multihomed
hosts.


Diffs (updated)
-

  3rdparty/libprocess/src/process.cpp 96ce7dbc486a2f1d55d2238a8a102bf024b12b1c 


Diff: https://reviews.apache.org/r/58224/diff/8/

Changes: https://reviews.apache.org/r/58224/diff/7-8/


Testing
---

make check (Fedora 25). Light manual testing.

With LIBPROCESS_require_peer_address_ip_match=true, all Mesos tests pass except 
``ExamplesTest.DiskFullFramework``, however enabling this will definitely break 
some libprocess APIs (though not in the way that Mesos uses them) and 
legitimate multi-homed configurations. Note that setting 
LIBPROCESS_ip=127.0.0.1 makes you multihomed for this purpose, which is why 
``ExamplesTest.DiskFullFramework`` breaks.


Thanks,

James Peach

Re: Review Request 58928: Update process tests to use a non-zero UPID.

[James Peach]

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

(Updated May 10, 2017, 6:05 p.m.)


Review request for mesos and Benjamin Mahler.


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


Repository: mesos


Description
---

Some of the remote process tests were using a UPID with a zero
IP address field. In order to enable subsequent IP address
validation in the libprocess receiver, update these tests to
use a UPID containing the real IP address of the sender.


Diffs (updated)
-

  3rdparty/libprocess/src/process.cpp 96ce7dbc486a2f1d55d2238a8a102bf024b12b1c 
  3rdparty/libprocess/src/tests/process_tests.cpp 
bf90c7e78fd50ad7e16cc0a69a248ba71e2a7115 
  3rdparty/libprocess/src/tests/test_linkee.cpp 
921d67695bc0e4d601e9f74fbc625d69bf36ba50 


Diff: https://reviews.apache.org/r/58928/diff/3/

Changes: https://reviews.apache.org/r/58928/diff/2-3/


Testing
---

make check (Fedora 25)


Thanks,

James Peach

Re: Review Request 58977: Add local and peer address accessors to http::Connection.

[James Peach]

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

(Updated May 10, 2017, 6:05 p.m.)


Review request for mesos and Benjamin Mahler.


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


Repository: mesos


Description
---

Add local and peer address accessors to http::Connection.


Diffs (updated)
-

  3rdparty/libprocess/include/process/http.hpp 
650b9d8aaba5636e1a445abf9e27e018ff82e610 
  3rdparty/libprocess/src/http.cpp 9789607933745f1fc4e37f47ce1be6aecb33a6e6 


Diff: https://reviews.apache.org/r/58977/diff/2/

Changes: https://reviews.apache.org/r/58977/diff/1-2/


Testing
---

make check (Fedora 25)


Thanks,

James Peach

Re: Review Request 59014: Implemented resolving an image secret in registry puller.

[Chun-Hung Hsiao]

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


Ship it!




Ship It!

- Chun-Hung Hsiao


On May 5, 2017, 8:30 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59014/
> ---
> 
> (Updated May 5, 2017, 8:30 p.m.)
> 
> 
> Review request for mesos, Adam B, Chun-Hung Hsiao, Jie Yu, Kapil Arya, Till 
> Toenshoff, and Vinod Kone.
> 
> 
> Bugs: MESOS-7088
> https://issues.apache.org/jira/browse/MESOS-7088
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented resolving an image secret in registry puller.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
> 6db788dd0c582deadf3e91c4d21bb9c20cf94e6b 
> 
> 
> Diff: https://reviews.apache.org/r/59014/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 59013: Implemented passing Image::Secret Puller::pull().

[Chun-Hung Hsiao]

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


Ship it!




Ship It!

- Chun-Hung Hsiao


On May 10, 2017, 12:47 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59013/
> ---
> 
> (Updated May 10, 2017, 12:47 p.m.)
> 
> 
> Review request for mesos, Adam B, Chun-Hung Hsiao, Jie Yu, Kapil Arya, Till 
> Toenshoff, and Vinod Kone.
> 
> 
> Bugs: MESOS-7088
> https://issues.apache.org/jira/browse/MESOS-7088
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented passing Image::Secret Puller::pull().
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/docker/local_puller.hpp 
> 344baab5c0365c3fc2dc814887bb2b48082b050f 
>   src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp 
> 5bc68d2047a7b3e3fa30315d61073e342ba6affe 
>   src/slave/containerizer/mesos/provisioner/docker/puller.hpp 
> 6dacdb1403433a37dd31a93d4ff2e37d4685eb87 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.hpp 
> 62ddb7a332030f3116477408d8b16c19e434c159 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
> 6db788dd0c582deadf3e91c4d21bb9c20cf94e6b 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> 68ce265a5d0a61f8d9ed55dd14c630dcf893a7d2 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> 4341621767a9fa5be2c66e77ef60f0c65dae58ca 
> 
> 
> Diff: https://reviews.apache.org/r/59013/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 58096: Added authorization for frameworks in /roles endpoint.

[Jay Guo]

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

(Updated May 11, 2017, 12:33 a.m.)


Review request for mesos, Adam B, Alexander Rojas, and Benjamin Mahler.


Changes
---

rebase


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


Repository: mesos


Description
---

While /roles displays a list of frameworksIds that register with
a role, it did NOT filter them based on VIEW_FRAMEWORK ACL, which
impose a security risk. This patch fixed this issue by taking a
frameworksApprover in `Master::Http::roles()` which is used to
filter framework IDs.


Diffs (updated)
-

  src/master/http.cpp e2590a17044ac019b24a24629428d4ec8adc0c31 


Diff: https://reviews.apache.org/r/58096/diff/7/

Changes: https://reviews.apache.org/r/58096/diff/6-7/


Testing
---

see next patch in the chain.


Thanks,

Jay Guo

Re: Review Request 58095: Refactored `Master::Http::roles` to use `jsonify`.

[Jay Guo]

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

(Updated May 11, 2017, 12:32 a.m.)


Review request for mesos, Adam B, Alexander Rojas, Benjamin Mahler, and Michael 
Park.


Changes
---

rebase


Bugs: MESOS-4732 and MESOS-7260
https://issues.apache.org/jira/browse/MESOS-4732
https://issues.apache.org/jira/browse/MESOS-7260


Repository: mesos


Description
---

Refactored `Master::Http::roles` to use `jsonify`.


Diffs (updated)
-

  src/common/http.hpp 93d6088e97c2384f9f6d26e010a501abf2deb43e 
  src/common/http.cpp 167dce2b9a2d3b68a1df5b4079f701482d34db28 
  src/master/http.cpp e2590a17044ac019b24a24629428d4ec8adc0c31 


Diff: https://reviews.apache.org/r/58095/diff/7/

Changes: https://reviews.apache.org/r/58095/diff/6-7/


Testing
---

no functional changes
make check


Thanks,

Jay Guo

Re: Review Request 59018: Supported Image::Secret in docker URI fetcher plugin.

[Mesos Reviewbot]

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



Bad patch!

Reviews applied: [59018, 59017, 59016, 59141, 59015, 59014, 59013, 59012, 
59011, 59010, 59001, 59000, 58999, 58760, 58759, 58925, 58924, 58923]

Failed command: python support/apply-reviews.py -n -r 59018

Error:
2017-05-10 15:54:22 URL:https://reviews.apache.org/r/59018/diff/raw/ 
[8186/8186] -> "59018.patch" [1]
error: patch failed: src/uri/fetchers/docker.cpp:324
error: src/uri/fetchers/docker.cpp: patch does not apply

Full log: https://builds.apache.org/job/Mesos-Reviewbot/18022/console

- Mesos Reviewbot


On May 10, 2017, 12:48 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59018/
> ---
> 
> (Updated May 10, 2017, 12:48 p.m.)
> 
> 
> Review request for mesos, Adam B, Chun-Hung Hsiao, Jie Yu, Kapil Arya, Till 
> Toenshoff, and Vinod Kone.
> 
> 
> Bugs: MESOS-7088
> https://issues.apache.org/jira/browse/MESOS-7088
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Supported Image::Secret in docker URI fetcher plugin.
> 
> 
> Diffs
> -
> 
>   src/uri/fetchers/docker.cpp 44169bf5f22f0ffd9fad7bb3b8f7d2a4989c6415 
> 
> 
> Diff: https://reviews.apache.org/r/59018/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> Manually tested using mesos-execute with task group. Verified that pulling 
> separate private image from different registries using different image 
> secrets works correctly.
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 49207: Added proto message definitions to support appc runtime.

[Till Toenshoff]

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




include/mesos/appc/spec.proto
Lines 51 (patched)


This should be called `environments` as we are permitting multiple via 
`repeated`.


- Till Toenshoff


On June 30, 2016, 6 a.m., Srinivas Brahmaroutu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49207/
> ---
> 
> (Updated June 30, 2016, 6 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-4778
> https://issues.apache.org/jira/browse/MESOS-4778
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added proto message definitions to support appc runtime.
> 
> 
> Diffs
> -
> 
>   include/mesos/appc/spec.proto 4ef0473748c2f47aff1729c1d969185803fe72d4 
>   include/mesos/slave/isolator.proto f17a3a4f167eb203709d7ebac3ade220ac8641ea 
> 
> 
> Diff: https://reviews.apache.org/r/49207/diff/7/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Srinivas Brahmaroutu
> 
>

Re: Review Request 59146: Added a 'UNKNOWN' field to the Update.State enumeration.

[Jan Schlicht]

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

(Updated May 10, 2017, 5:09 p.m.)


Review request for mesos, Alexander Rukletsov, Benjamin Bannier, and Jie Yu.


Changes
---

Added v1 protobuf.


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  include/mesos/resource_provider/resource_provider.proto 
11d01df114eafb729514dab64596c5f1ecf09f26 
  include/mesos/v1/resource_provider/resource_provider.proto 
97a85ce34eeecf86d14c097712867cd24624a6ec 


Diff: https://reviews.apache.org/r/59146/diff/2/

Changes: https://reviews.apache.org/r/59146/diff/1-2/


Testing
---

make check


Thanks,

Jan Schlicht

Re: Review Request 59146: Added a 'UNKNOWN' field to the Update.State enumeration.

[Jan Schlicht]

> On May 10, 2017, 5:05 p.m., Alexander Rukletsov wrote:
> > What about V1 protos?

Dang! Sorry, will add that ASAP, always forget about them.


- Jan


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


On May 10, 2017, 5 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59146/
> ---
> 
> (Updated May 10, 2017, 5 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Bannier, and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   include/mesos/resource_provider/resource_provider.proto 
> 11d01df114eafb729514dab64596c5f1ecf09f26 
> 
> 
> Diff: https://reviews.apache.org/r/59146/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 59146: Added a 'UNKNOWN' field to the Update.State enumeration.

[Alexander Rukletsov]

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



What about V1 protos?

- Alexander Rukletsov


On May 10, 2017, 3 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59146/
> ---
> 
> (Updated May 10, 2017, 3 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Bannier, and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   include/mesos/resource_provider/resource_provider.proto 
> 11d01df114eafb729514dab64596c5f1ecf09f26 
> 
> 
> Diff: https://reviews.apache.org/r/59146/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Review Request 59147: Enabled authorization for v1 calls starting and stopping maintenance.

[Alexander Rojas]

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

Review request for mesos, Adam B and Greg Mann.


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


Repository: mesos


Description
---

Enables the use of authorization for the `START_MAINTENANCE` and
`STOP_MAINTENANCE` v1 API calls, using the ACLs `StartMaintenance` and
`StopMaintenance` respectively as well the actions of the same name as
the API calls.

It also updates the `ApiTests` to take into account the authorization
case.


Diffs
-

  src/master/http.cpp e2590a17044ac019b24a24629428d4ec8adc0c31 
  src/tests/api_tests.cpp faf3242f9c86d866c4bb5e457fcfe47c1063cc09 


Diff: https://reviews.apache.org/r/59147/diff/1/


Testing
---

make check


Thanks,

Alexander Rojas

Review Request 59146: Added a 'UNKNOWN' field to the Update.State enumeration.

[Jan Schlicht]

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

Review request for mesos, Alexander Rukletsov, Benjamin Bannier, and Jie Yu.


Repository: mesos


Description
---

See summary.


Diffs
-

  include/mesos/resource_provider/resource_provider.proto 
11d01df114eafb729514dab64596c5f1ecf09f26 


Diff: https://reviews.apache.org/r/59146/diff/1/


Testing
---

make check


Thanks,

Jan Schlicht

Review Request 59145: Used a helper to create ExecutorInfo in some nested container tests.

[Alexander Rukletsov]

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

Review request for mesos and Gastón Kleiman.


Repository: mesos


Description
---

See summary.


Diffs
-

  src/tests/containerizer/nested_mesos_containerizer_tests.cpp 
c163b882fb2fc463537d6906c5a47b24a9a756c4 


Diff: https://reviews.apache.org/r/59145/diff/1/


Testing
---

make check on several Linux distros.


Thanks,

Alexander Rukletsov

Re: Review Request 59136: Added functions to add/remove resource providers.

[Jan Schlicht]

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

(Updated May 10, 2017, 4:12 p.m.)


Review request for mesos, Benjamin Bannier and Jie Yu.


Changes
---

Remove unnecessary `link` call.


Repository: mesos


Description
---

Added functions to add/remove resource providers.


Diffs (updated)
-

  src/master/master.hpp 89d0790fd5fea59e74276f462581fe0073594732 
  src/master/master.cpp 4e7a161f431624bd78d3e9032eb8587687149cad 


Diff: https://reviews.apache.org/r/59136/diff/2/

Changes: https://reviews.apache.org/r/59136/diff/1-2/


Testing
---

make check


Thanks,

Jan Schlicht

Re: Review Request 59135: Implemented resource provider bookkeeping primitives.

[Jan Schlicht]

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

(Updated May 10, 2017, 4:11 p.m.)


Review request for mesos, Benjamin Bannier and Jie Yu.


Changes
---

Use `SlaveID` instead of agent's PID.


Repository: mesos


Description
---

Similar to the existing bookkeeping of registered agents, the master
needs to keep track of a resource provider's current resources and
what resources are offered.


Diffs (updated)
-

  src/master/master.hpp 89d0790fd5fea59e74276f462581fe0073594732 
  src/master/master.cpp 4e7a161f431624bd78d3e9032eb8587687149cad 


Diff: https://reviews.apache.org/r/59135/diff/3/

Changes: https://reviews.apache.org/r/59135/diff/2-3/


Testing
---

make check


Thanks,

Jan Schlicht

Re: Review Request 58967: Set the working directory to parent task's for DEBUG containers.

[Alexander Rukletsov]

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

(Updated May 10, 2017, 1:56 p.m.)


Review request for mesos, Gastón Kleiman, Jie Yu, and Vinod Kone.


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


Repository: mesos


Description
---

A DEBUG container's working directory should be set to the parent
task's working directory. For the command executor case, even if
the task itself has a root filesystem, the executor container still
uses the host filesystem, hence
`ContainerLaunchInfo.working_directory`, pointing to the executor's
working directory in the host filesystem, may be different from the
task's working directory in the root filesystem.


Diffs (updated)
-

  include/mesos/slave/containerizer.proto 
41f2905df690bfe88ed762f1cd1246689fa4d3ea 
  src/slave/containerizer/mesos/containerizer.cpp 
3d724945812c0359ed175ce232f70886dc4401c8 


Diff: https://reviews.apache.org/r/58967/diff/3/

Changes: https://reviews.apache.org/r/58967/diff/2-3/


Testing
---

make check on Mac OS and Fedora 24


Thanks,

Alexander Rukletsov

Re: Review Request 58821: Added a test that verifies a task and its check share the work dir.

[Alexander Rukletsov]

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

(Updated May 10, 2017, 1:55 p.m.)


Review request for mesos, Gastón Kleiman, Jie Yu, and Vinod Kone.


Changes
---

Rebased. NNTR.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/tests/check_tests.cpp ec0d5ee78a94991c68f38174d09b41e8f2e4be35 


Diff: https://reviews.apache.org/r/58821/diff/5/

Changes: https://reviews.apache.org/r/58821/diff/4-5/


Testing
---

`make check` on Mac OS and Fedora 24


Thanks,

Alexander Rukletsov

Re: Review Request 58263: Inherited working dir from parent when launching a DEBUG container.

[Alexander Rukletsov]

> On May 9, 2017, 6:08 p.m., Gastón Kleiman wrote:
> > src/tests/containerizer/nested_mesos_containerizer_tests.cpp
> > Line 579 (original), 595-598 (patched)
> > 
> >
> > Why do you prefer not to use the helper method?

Good suggestion.


- Alexander


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


On May 10, 2017, 1:54 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58263/
> ---
> 
> (Updated May 10, 2017, 1:54 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman, Jie Yu, Kevin Klues, and Vinod Kone.
> 
> 
> Bugs: MESOS-7433
> https://issues.apache.org/jira/browse/MESOS-7433
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/containerizer.proto 
> 41f2905df690bfe88ed762f1cd1246689fa4d3ea 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 3d724945812c0359ed175ce232f70886dc4401c8 
>   src/tests/containerizer/nested_mesos_containerizer_tests.cpp 
> c163b882fb2fc463537d6906c5a47b24a9a756c4 
> 
> 
> Diff: https://reviews.apache.org/r/58263/diff/7/
> 
> 
> Testing
> ---
> 
> See https://reviews.apache.org/r/58821/
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 58263: Inherited working dir from parent when launching a DEBUG container.

[Alexander Rukletsov]

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

(Updated May 10, 2017, 1:54 p.m.)


Review request for mesos, Gastón Kleiman, Jie Yu, Kevin Klues, and Vinod Kone.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  include/mesos/slave/containerizer.proto 
41f2905df690bfe88ed762f1cd1246689fa4d3ea 
  src/slave/containerizer/mesos/containerizer.cpp 
3d724945812c0359ed175ce232f70886dc4401c8 
  src/tests/containerizer/nested_mesos_containerizer_tests.cpp 
c163b882fb2fc463537d6906c5a47b24a9a756c4 


Diff: https://reviews.apache.org/r/58263/diff/7/

Changes: https://reviews.apache.org/r/58263/diff/6-7/


Testing
---

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


Thanks,

Alexander Rukletsov

Re: Review Request 58099: Added authorization for frameworks in `GetRoles` v1 API.

[Jay Guo]

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

(Updated May 10, 2017, 9:52 p.m.)


Review request for mesos, Adam B, Alexander Rojas, and Benjamin Mahler.


Changes
---

address comments.


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


Repository: mesos


Description
---

Added authorization for frameworks in `GetRoles` v1 API.


Diffs (updated)
-

  src/master/http.cpp e2590a17044ac019b24a24629428d4ec8adc0c31 


Diff: https://reviews.apache.org/r/58099/diff/6/

Changes: https://reviews.apache.org/r/58099/diff/5-6/


Testing
---


Thanks,

Jay Guo

Re: Review Request 58097: Added a test to check framework filtering in /roles endpoint.

[Jay Guo]

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

(Updated May 10, 2017, 9:47 p.m.)


Review request for mesos, Adam B, Alexander Rojas, and Benjamin Mahler.


Changes
---

rebase


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


Repository: mesos


Description
---

Added a test to check framework filtering in /roles endpoint.


Diffs (updated)
-

  src/tests/master_authorization_tests.cpp 
e4233c19b1d9e3e2734259503d0daec4ce243667 


Diff: https://reviews.apache.org/r/58097/diff/6/

Changes: https://reviews.apache.org/r/58097/diff/5-6/


Testing
---

make check


Thanks,

Jay Guo

Re: Review Request 58096: Added authorization for frameworks in /roles endpoint.

[Jay Guo]

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

(Updated May 10, 2017, 9:46 p.m.)


Review request for mesos, Adam B, Alexander Rojas, and Benjamin Mahler.


Changes
---

address comments.


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


Repository: mesos


Description
---

While /roles displays a list of frameworksIds that register with
a role, it did NOT filter them based on VIEW_FRAMEWORK ACL, which
impose a security risk. This patch fixed this issue by taking a
frameworksApprover in `Master::Http::roles()` which is used to
filter framework IDs.


Diffs (updated)
-

  src/master/http.cpp e2590a17044ac019b24a24629428d4ec8adc0c31 


Diff: https://reviews.apache.org/r/58096/diff/6/

Changes: https://reviews.apache.org/r/58096/diff/5-6/


Testing
---

see next patch in the chain.


Thanks,

Jay Guo

Re: Review Request 58095: Refactored `Master::Http::roles` to use `jsonify`.

[Jay Guo]

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

(Updated May 10, 2017, 9:42 p.m.)


Review request for mesos, Adam B, Alexander Rojas, Benjamin Mahler, and Michael 
Park.


Changes
---

address comments.


Bugs: MESOS-4732 and MESOS-7260
https://issues.apache.org/jira/browse/MESOS-4732
https://issues.apache.org/jira/browse/MESOS-7260


Repository: mesos


Description
---

Refactored `Master::Http::roles` to use `jsonify`.


Diffs (updated)
-

  src/common/http.hpp 93d6088e97c2384f9f6d26e010a501abf2deb43e 
  src/common/http.cpp 167dce2b9a2d3b68a1df5b4079f701482d34db28 
  src/master/http.cpp e2590a17044ac019b24a24629428d4ec8adc0c31 


Diff: https://reviews.apache.org/r/58095/diff/6/

Changes: https://reviews.apache.org/r/58095/diff/5-6/


Testing
---

no functional changes
make check


Thanks,

Jay Guo

Re: Review Request 59140: Fixed task_environment serializing in containerizer.

[Jie Yu]

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


Fix it, then Ship it!





src/slave/containerizer/mesos/containerizer.cpp
Lines 1371-1377 (patched)


Let's add a helper to common/protobuf_utils.cpp `createEnvironment`


- Jie Yu


On May 10, 2017, 12:34 p.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59140/
> ---
> 
> (Updated May 10, 2017, 12:34 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, and Kapil Arya.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> see summary.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 3d724945812c0359ed175ce232f70886dc4401c8 
> 
> 
> Diff: https://reviews.apache.org/r/59140/diff/1/
> 
> 
> Testing
> ---
> 
> make check and functional testing.
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 59016: Fixed the comment style issue in docker/spec.hpp.

[Gilbert Song]

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

(Updated May 10, 2017, 5:49 a.m.)


Review request for mesos, Adam B, Chun-Hung Hsiao, Jie Yu, Kapil Arya, Till 
Toenshoff, and Vinod Kone.


Repository: mesos


Description
---

Fixed the comment style issue in docker/spec.hpp.


Diffs
-

  include/mesos/docker/spec.hpp b90f731ef07c22259715543526eaf25cb94eab03 


Diff: https://reviews.apache.org/r/59016/diff/1/


Testing
---

N/A


Thanks,

Gilbert Song

Re: Review Request 59018: Supported Image::Secret in docker URI fetcher plugin.

[Gilbert Song]

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

(Updated May 10, 2017, 5:48 a.m.)


Review request for mesos, Adam B, Chun-Hung Hsiao, Jie Yu, Kapil Arya, Till 
Toenshoff, and Vinod Kone.


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


Repository: mesos


Description
---

Supported Image::Secret in docker URI fetcher plugin.


Diffs (updated)
-

  src/uri/fetchers/docker.cpp 44169bf5f22f0ffd9fad7bb3b8f7d2a4989c6415 


Diff: https://reviews.apache.org/r/59018/diff/2/

Changes: https://reviews.apache.org/r/59018/diff/1-2/


Testing
---

make check

Manually tested using mesos-execute with task group. Verified that pulling 
separate private image from different registries using different image secrets 
works correctly.


Thanks,

Gilbert Song

Review Request 59141: Added Secret::Value to the URI fetcher interface.

[Gilbert Song]

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

Review request for mesos, Adam B, Chun-Hung Hsiao, Jie Yu, Kapil Arya, Till 
Toenshoff, and Vinod Kone.


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


Repository: mesos


Description
---

Added Secret::Value to the URI fetcher interface.


Diffs
-

  include/mesos/uri/fetcher.hpp ebf86c78b794a6ef46332df788a1317bbec5d983 
  src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
6db788dd0c582deadf3e91c4d21bb9c20cf94e6b 
  src/uri/fetcher.cpp f4d1a4c69e910260dc536aa42ae03fd17403b060 
  src/uri/fetchers/copy.hpp f4a2fb3d5156e5ebfdf7c4202f8dc9b1cd1d6ac7 
  src/uri/fetchers/copy.cpp 5e1470503f4fa0e543680c93b2ad3e36351afc1c 
  src/uri/fetchers/curl.hpp 083f155092d159cd83069bfdfd905d679e9ab57c 
  src/uri/fetchers/curl.cpp 24b53c77946170cba45152c458d85b6fddfce9f8 
  src/uri/fetchers/docker.hpp 65e01cba1d41688a8ee5da73d1d6f57515fbc7f5 
  src/uri/fetchers/docker.cpp 44169bf5f22f0ffd9fad7bb3b8f7d2a4989c6415 
  src/uri/fetchers/hadoop.hpp 4923dc6491d7cc6efc6ced4f5356af9f945ba5d2 
  src/uri/fetchers/hadoop.cpp 3c5ffe607c92ea1ab66ba261bd70031f2907cea6 


Diff: https://reviews.apache.org/r/59141/diff/1/


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 59017: Added support for docker spec helper 'parseAuthConfig()'.

[Gilbert Song]

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

(Updated May 10, 2017, 5:47 a.m.)


Review request for mesos, Adam B, Chun-Hung Hsiao, Jie Yu, Kapil Arya, Till 
Toenshoff, and Vinod Kone.


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


Repository: mesos


Description
---

Added support for docker spec helper 'parseAuthConfig()'.


Diffs (updated)
-

  include/mesos/docker/spec.hpp b90f731ef07c22259715543526eaf25cb94eab03 
  src/docker/spec.cpp 6b5588e534215451593cc20011847c3abbab9b17 


Diff: https://reviews.apache.org/r/59017/diff/2/

Changes: https://reviews.apache.org/r/59017/diff/1-2/


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 59015: Implemented passing docker config depended methods.

[Gilbert Song]

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

(Updated May 10, 2017, 5:47 a.m.)


Review request for mesos, Adam B, Chun-Hung Hsiao, Jie Yu, Kapil Arya, Till 
Toenshoff, and Vinod Kone.


Summary (updated)
-

Implemented passing docker config depended methods.


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


Repository: mesos


Description (updated)
---

Implemented passing docker config depended methods.


Diffs (updated)
-

  src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
6db788dd0c582deadf3e91c4d21bb9c20cf94e6b 


Diff: https://reviews.apache.org/r/59015/diff/2/

Changes: https://reviews.apache.org/r/59015/diff/1-2/


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 59013: Implemented passing Image::Secret Puller::pull().

[Gilbert Song]

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

(Updated May 10, 2017, 5:47 a.m.)


Review request for mesos, Adam B, Chun-Hung Hsiao, Jie Yu, Kapil Arya, Till 
Toenshoff, and Vinod Kone.


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


Repository: mesos


Description
---

Implemented passing Image::Secret Puller::pull().


Diffs (updated)
-

  src/slave/containerizer/mesos/provisioner/docker/local_puller.hpp 
344baab5c0365c3fc2dc814887bb2b48082b050f 
  src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp 
5bc68d2047a7b3e3fa30315d61073e342ba6affe 
  src/slave/containerizer/mesos/provisioner/docker/puller.hpp 
6dacdb1403433a37dd31a93d4ff2e37d4685eb87 
  src/slave/containerizer/mesos/provisioner/docker/registry_puller.hpp 
62ddb7a332030f3116477408d8b16c19e434c159 
  src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
6db788dd0c582deadf3e91c4d21bb9c20cf94e6b 
  src/slave/containerizer/mesos/provisioner/docker/store.cpp 
68ce265a5d0a61f8d9ed55dd14c630dcf893a7d2 
  src/tests/containerizer/provisioner_docker_tests.cpp 
4341621767a9fa5be2c66e77ef60f0c65dae58ca 


Diff: https://reviews.apache.org/r/59013/diff/2/

Changes: https://reviews.apache.org/r/59013/diff/1-2/


Testing
---

make check


Thanks,

Gilbert Song