Re: Review Request 58761: Added --secret-fetcher flag for agent.

[Mesos Reviewbot]

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



Bad patch!

Reviews applied: [58761, 58760, 58759, 58758]

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

Error:
2017-04-27 05:55:48 URL:https://reviews.apache.org/r/58758/diff/raw/ [926/926] 
-> "58758.patch" [1]
error: patch failed: include/mesos/mesos.proto:2109
error: include/mesos/mesos.proto: patch does not apply

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

- Mesos Reviewbot


On April 26, 2017, 11:38 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58761/
> ---
> 
> (Updated April 26, 2017, 11:38 p.m.)
> 
> 
> Review request for mesos and Gilbert Song.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added --secret-fetcher flag for agent.
> 
> 
> Diffs
> -
> 
>   src/local/local.cpp e47980929db2da1f31cf899a0e1fc452070e11f3 
>   src/slave/flags.hpp 38c05af99631059958868311adcc55774fbc6015 
>   src/slave/flags.cpp f22837c8c94463c2da1ddfde9ed954631a3f38bd 
>   src/slave/main.cpp 72b141cb66f9df5bcc7b3f8cfcc2b06fcbd17e52 
> 
> 
> Diff: https://reviews.apache.org/r/58761/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 58775: Added 'Secret' field config to Image::Docker protobuf.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On April 27, 2017, 1:42 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58775/
> ---
> 
> (Updated April 27, 2017, 1:42 a.m.)
> 
> 
> Review request for mesos, Adam B, Chun-Hung Hsiao, Jie Yu, Kapil Arya, and 
> Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This field is used for Credential-per-Container, which will identify
> different docker config from secrets for image pulling.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto fbb491c0aeba4796709fd266ee16cb73a44db490 
> 
> 
> Diff: https://reviews.apache.org/r/58775/diff/1/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 58689: MESOS-7323: Made `addSlave` not activate any frameworks.

[Michael Park]

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

(Updated April 26, 2017, 7:54 p.m.)


Review request for mesos, Benjamin Bannier, Benjamin Mahler, and Neil Conway.


Changes
---

Addressed the rest of bmahler's comments regarding the test.


Summary (updated)
-

MESOS-7323: Made `addSlave` not activate any frameworks.


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


Repository: mesos


Description (updated)
---

MESOS-7323: Made `addSlave` not activate any frameworks.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.cpp 
984a0a4e2671ee7bb2a3515849342f49f2c4e3aa 
  src/tests/upgrade_tests.cpp b5a28b5161d896dff250b8ad012c8aac91c3f861 


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

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


Testing
---

`make check` and added a test.


Thanks,

Michael Park

Re: Review Request 58689: MESOS-7323: Made `addSlave` conditionally activate the framework.

[Michael Park]

> On April 25, 2017, 5:59 p.m., Benjamin Mahler wrote:
> > src/tests/upgrade_tests.cpp
> > Lines 537-539 (patched)
> > 
> >
> > This comment seems to be inconsistent with what the test is doing?
> > 
> > Also, this doesn't look like an upgrade test, should we make it a 
> > master test?

Updated the comment to more accurately reflect the test.

I was considering this to be a kind of an upgrade test, in the sense that
we're upgrading the framework from one config to another.

I'm not sure if we have some clear criteria for what qualifies as an upgrade 
test.

But I'm also fine with this living in master test.


> On April 25, 2017, 5:59 p.m., Benjamin Mahler wrote:
> > src/tests/upgrade_tests.cpp
> > Lines 612 (patched)
> > 
> >
> > Hm.. not obvious to me why this was needed, can you add a comment?

I don't think this is necessary. Removed.


> On April 25, 2017, 5:59 p.m., Benjamin Mahler wrote:
> > src/tests/upgrade_tests.cpp
> > Lines 657 (patched)
> > 
> >
> > Do you want to just pause the clock for the whole test to avoid relying 
> > on timers?

Done!


> On April 25, 2017, 5:59 p.m., Benjamin Mahler wrote:
> > src/tests/upgrade_tests.cpp
> > Lines 682-683 (patched)
> > 
> >
> > ... and the master will track this role under the framework, but the 
> > framework should not receive offers for this role?
> > 
> > Should we test for that?

Added queries to the master to test that the framework is re-tracked.


- Michael


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


On April 26, 2017, 3:47 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58689/
> ---
> 
> (Updated April 26, 2017, 3:47 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, and Neil Conway.
> 
> 
> Bugs: MESOS-7323
> https://issues.apache.org/jira/browse/MESOS-7323
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> MESOS-7323: Made `addSlave` conditionally activate the framework.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> 984a0a4e2671ee7bb2a3515849342f49f2c4e3aa 
>   src/tests/upgrade_tests.cpp b5a28b5161d896dff250b8ad012c8aac91c3f861 
> 
> 
> Diff: https://reviews.apache.org/r/58689/diff/2/
> 
> 
> Testing
> ---
> 
> `make check` and added a test.
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 58689: MESOS-7323: Made `addSlave` conditionally activate the framework.

[Mesos Reviewbot]

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



Bad review!

Reviews applied: [58689, 58688, 58687, 57254, 58112, 58110, 57564, 57528, 
57527, 57788, 57167, 57166, 56805, 57165, 57164]

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

- Mesos Reviewbot


On April 26, 2017, 10:47 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58689/
> ---
> 
> (Updated April 26, 2017, 10:47 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, and Neil Conway.
> 
> 
> Bugs: MESOS-7323
> https://issues.apache.org/jira/browse/MESOS-7323
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> MESOS-7323: Made `addSlave` conditionally activate the framework.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> 984a0a4e2671ee7bb2a3515849342f49f2c4e3aa 
>   src/tests/upgrade_tests.cpp b5a28b5161d896dff250b8ad012c8aac91c3f861 
> 
> 
> Diff: https://reviews.apache.org/r/58689/diff/2/
> 
> 
> Testing
> ---
> 
> `make check` and added a test.
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Review Request 58775: Added 'Secret' field config to Image::Docker protobuf.

[Gilbert Song]

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

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


Repository: mesos


Description
---

This field is used for Credential-per-Container, which will identify
different docker config from secrets for image pulling.


Diffs
-

  include/mesos/mesos.proto fbb491c0aeba4796709fd266ee16cb73a44db490 


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


Testing
---

make


Thanks,

Gilbert Song

Re: Review Request 58747: Fixed docker uri fetcher strict v2 schema 1 check.

[Jie Yu]

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


Ship it!




- Jie Yu


On April 26, 2017, 11:12 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58747/
> ---
> 
> (Updated April 26, 2017, 11:12 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Ilya Pronin, and Jie Yu.
> 
> 
> Bugs: MESOS-7272
> https://issues.apache.org/jira/browse/MESOS-7272
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This check was introduced from this patch:
> https://reviews.apache.org/r/53848/
> 
> The check on registry v2 schema 1 is incorrect. It does not work
> for registries that are older version < 2.3, because the ContentType
> header may be something like this "application/json; charset=utf-8".
> The check missed a note from docker registry spec that
> "application/json" will also be accepted for schema 1.
> 
> Depending on the docker registry spec doc, docker support the
> following three media type for V2 schema 1 manifest:
>   1. application/vnd.docker.distribution.manifest.v1+json
>   2. application/vnd.docker.distribution.manifest.v1+prettyjws
>   3. application/json
> For more details, see:
>   https://docs.docker.com/registry/spec/manifest-v2-1/
> 
> 
> Diffs
> -
> 
>   src/uri/fetchers/docker.cpp 1c6ab929deacfc29aa6b4f1df04c2b9782044a90 
> 
> 
> Diff: https://reviews.apache.org/r/58747/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 58748: Fixed the internet curl flaky test due to short timeout.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On April 26, 2017, 7:14 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58748/
> ---
> 
> (Updated April 26, 2017, 7:14 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed the internet curl flaky test due to short timeout.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> c26e1f9b093fda66f14e474006d8148c0b9f3245 
> 
> 
> Diff: https://reviews.apache.org/r/58748/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 58747: Fixed docker uri fetcher strict v2 schema 1 check.

[Gilbert Song]

> On April 26, 2017, 5:13 p.m., Chun-Hung Hsiao wrote:
> > src/uri/fetchers/docker.cpp
> > Lines 542 (patched)
> > 
> >
> > Should we also trigger the failure when `!contentType.isSome()`?

Thought about that. Let's keep it as the old semantic and rely on Ilya's schema 
2 support to make that change.


- Gilbert


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


On April 26, 2017, 4:12 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58747/
> ---
> 
> (Updated April 26, 2017, 4:12 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Ilya Pronin, and Jie Yu.
> 
> 
> Bugs: MESOS-7272
> https://issues.apache.org/jira/browse/MESOS-7272
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This check was introduced from this patch:
> https://reviews.apache.org/r/53848/
> 
> The check on registry v2 schema 1 is incorrect. It does not work
> for registries that are older version < 2.3, because the ContentType
> header may be something like this "application/json; charset=utf-8".
> The check missed a note from docker registry spec that
> "application/json" will also be accepted for schema 1.
> 
> Depending on the docker registry spec doc, docker support the
> following three media type for V2 schema 1 manifest:
>   1. application/vnd.docker.distribution.manifest.v1+json
>   2. application/vnd.docker.distribution.manifest.v1+prettyjws
>   3. application/json
> For more details, see:
>   https://docs.docker.com/registry/spec/manifest-v2-1/
> 
> 
> Diffs
> -
> 
>   src/uri/fetchers/docker.cpp 1c6ab929deacfc29aa6b4f1df04c2b9782044a90 
> 
> 
> Diff: https://reviews.apache.org/r/58747/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 58725: Fetching docker image manifests with 'Accept' header.

[Jie Yu]

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



Do you know if an old registry (< 2.3) will be OK with this header that it does 
not understand? Can you confirm?

- Jie Yu


On April 26, 2017, 10:27 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58725/
> ---
> 
> (Updated April 26, 2017, 10:27 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-7427
> https://issues.apache.org/jira/browse/MESOS-7427
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added 'Accept: application/vnd.docker.distribution.manifest.v1+json'
> to the headers of HTTP requests for fetching manifests from any Docker
> registry. Some registry services (e.g., Amazon ECR) check the 'Accept'
> field strictly and reject the requests if it is not specified.
> 
> 
> Diffs
> -
> 
>   src/uri/fetchers/docker.cpp 1c6ab929deacfc29aa6b4f1df04c2b9782044a90 
> 
> 
> Diff: https://reviews.apache.org/r/58725/diff/3/
> 
> 
> Testing
> ---
> 
> sudo make check
> Manually tested on a local docker private registry and an Amazon ECR 
> repository.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 58709: Prevent old Mesos agents from registering or re-registering.

[Benjamin Mahler]

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


Fix it, then Ship it!




Looks good, would be nice to pull out a MINIMUM_AGENT_VERSION constant in 
master/constants.hpp.


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


Since this is in two places I'm just a little worried about it finding it's 
way into more places and/or getting inconsistent. Could we pull out a 
MINIMUM_AGENT_VERSION into master/constants.hpp?



src/master/master.cpp
Lines 5408-5409 (patched)


Could we get the open and close quotes on the same line? Ditto below


- Benjamin Mahler


On April 25, 2017, 3:13 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58709/
> ---
> 
> (Updated April 25, 2017, 3:13 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Kapil Arya.
> 
> 
> Bugs: MESOS-6976
> https://issues.apache.org/jira/browse/MESOS-6976
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Officially, Mesos 1.0.0 (and newer) masters do not support pre-1.0.0
> Mesos agents. However, we previously allowed old agents to register,
> which resulted in several master crashes. As a short-term solution, we
> fixed those crashes by adding backward compatibility mechanisms into the
> master, but that backward compatibility code has made the master logic
> more complicated and difficult to understand.
> 
> This commit changes the master to ignore registration attempts by Mesos
> agents that precede Mesos 1.0.0. Now that this safety check is in place,
> master logic can safely assume that all agents are running at least
> Mesos 1.0.0, which will allow several simplifications to be made.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp d1cdc35a066e190ef8e0bd788e07e63b92f7fce7 
>   src/tests/master_tests.cpp d1828eb42e0aedc9330c3786bbd9bb63aa42a64e 
> 
> 
> Diff: https://reviews.apache.org/r/58709/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 58673: Fix FlagsFileTest to check for absolute path properly on Windows.

[Andrew Schwartzmeyer]

> On April 25, 2017, 11:16 p.m., Andrew Schwartzmeyer wrote:
> > 3rdparty/stout/include/stout/path.hpp
> > Lines 79-86 (patched)
> > 
> >
> > I think this got a bit out of date. We settled on `\...` being an 
> > absolute path, not just `\?...` (per below implementation).
> 
> Jeff Coffler wrote:
> I didn't have a test for this specifically. The above is accurate, 
> though. The 1-3 cases are for files on disk. Network shares are a special 
> case and, while technically an absolute path, isn't related to files on disk.
> 
> That said, the function does implement this properly. The comment isn't 
> out of date. The note was just to point out that we handled this as well.

Sorry, I mean that your third bullet should read `3. "\..."` instead of `3. 
"\?..."`.


- Andrew


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


On April 25, 2017, 6:24 p.m., Jeff Coffler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58673/
> ---
> 
> (Updated April 25, 2017, 6:24 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, John Kordich, Joseph Wu, and 
> Li Li.
> 
> 
> Bugs: MESOS-5937
> https://issues.apache.org/jira/browse/MESOS-5937
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added 3rdparty/stout/tests/path_tests.cpp tests to Windows platform,
> but disabled tests that did not pass. Enabled absolute path tests,
> adding tests that were appropriate for the Windows platform.
> 
> Note that, for Windows, absolute paths may not be valid. For example,
> a path like "\\?\abc:file.txt" is not valid. In this case, the
> path::absolute method is undefined; the expectation is that it is
> called with valid paths (absolute or relative, but valid).
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/flags/parse.hpp 
> 65edd86372596c2107e9f29cf27301e025e6620e 
>   3rdparty/stout/include/stout/path.hpp 
> 2d2088aadfa1ea82c59424242671c4fb655dede1 
>   3rdparty/stout/tests/CMakeLists.txt 
> 4bbe713f259e7858d423dcb33956d41e62a915eb 
>   3rdparty/stout/tests/flags_tests.cpp 
> e2681f8f68f6478d4c8a20c1e75ddb050d16b56d 
>   3rdparty/stout/tests/path_tests.cpp 
> 0490d93908566c46a10d91b05790e5a7f2f289bc 
> 
> 
> Diff: https://reviews.apache.org/r/58673/diff/2/
> 
> 
> Testing
> ---
> 
> Passes `make check` on the Linux platform.
> 
> On Windows, the FlagsFileTest.JSONFile now passes, along with new 
> PathTest.Absolute tests that I added due to new path::absolute handling on 
> the Windows platform.
> 
> PS C:\mesos> .\build\3rdparty\stout\tests\Debug\stout-tests.exe 
> --gtest_filter="*FlagsFileTest*"
> Note: Google Test filter = *FlagsFileTest*-
> [==] Running 2 tests from 1 test case.
> [--] Global test environment set-up.
> [--] 2 tests from FlagsFileTest
> [ RUN  ] FlagsFileTest.JSONFile
> WARNING: Logging before InitGoogleLogging() is written to STDERR
> W0425 10:05:20.959357 1060440 parse.hpp:97] Specifying an absolute filename 
> to read a command line option out of without using 'file:// is deprecated and 
> will be removed in a future release. Simply adding 'file://' to the beginning 
> of the path should eliminate this warning.
> [   OK ] FlagsFileTest.JSONFile (9 ms)
> [ RUN  ] FlagsFileTest.FilePrefix
> [   OK ] FlagsFileTest.FilePrefix (6 ms)
> [--] 2 tests from FlagsFileTest (18 ms total)
> 
> [--] Global test environment tear-down
> [==] 2 tests from 1 test case ran. (23 ms total)
> [  PASSED  ] 2 tests.
> PS C:\mesos> .\build\3rdparty\stout\tests\Debug\stout-tests.exe 
> --gtest_filter="*PathTest*"
> Note: Google Test filter = *PathTest*-
> [==] Running 2 tests from 1 test case.
> [--] Global test environment set-up.
> [--] 2 tests from PathTest
> [ RUN  ] PathTest.Absolute
> [   OK ] PathTest.Absolute (1 ms)
> [ RUN  ] PathTest.Comparison
> [   OK ] PathTest.Comparison (0 ms)
> [--] 2 tests from PathTest (2 ms total)
> 
> [--] Global test environment tear-down
> [==] 2 tests from 1 test case ran. (6 ms total)
> [  PASSED  ] 2 tests.
> 
>   YOU HAVE 4 DISABLED TESTS
> 
> PS C:\mesos>
> 
> 
> Thanks,
> 
> Jeff Coffler
> 
>

Re: Review Request 58747: Fixed docker uri fetcher strict v2 schema 1 check.

[Chun-Hung Hsiao]

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




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


Should we also trigger the failure when `!contentType.isSome()`?


- Chun-Hung Hsiao


On April 26, 2017, 11:12 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58747/
> ---
> 
> (Updated April 26, 2017, 11:12 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Ilya Pronin, and Jie Yu.
> 
> 
> Bugs: MESOS-7272
> https://issues.apache.org/jira/browse/MESOS-7272
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This check was introduced from this patch:
> https://reviews.apache.org/r/53848/
> 
> The check on registry v2 schema 1 is incorrect. It does not work
> for registries that are older version < 2.3, because the ContentType
> header may be something like this "application/json; charset=utf-8".
> The check missed a note from docker registry spec that
> "application/json" will also be accepted for schema 1.
> 
> Depending on the docker registry spec doc, docker support the
> following three media type for V2 schema 1 manifest:
>   1. application/vnd.docker.distribution.manifest.v1+json
>   2. application/vnd.docker.distribution.manifest.v1+prettyjws
>   3. application/json
> For more details, see:
>   https://docs.docker.com/registry/spec/manifest-v2-1/
> 
> 
> Diffs
> -
> 
>   src/uri/fetchers/docker.cpp 1c6ab929deacfc29aa6b4f1df04c2b9782044a90 
> 
> 
> Diff: https://reviews.apache.org/r/58747/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Review Request 58771: Added tests for usernamespace support.

[Srinivas Brahmaroutu]

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

Review request for mesos.


Repository: mesos


Description
---

WIP: Added tests for usernamespace support.


Diffs
-

  src/tests/containerizer/isolator_tests.cpp 
355e15ff69ca6bdd94821f6566fd09a280d03b47 


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


Testing
---


Thanks,

Srinivas Brahmaroutu

Review Request 58770: Enabled usernamespace support for default executor.

[Srinivas Brahmaroutu]

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

Review request for mesos.


Repository: mesos


Description
---

WIP Enabled usernamespace support for default executor.


Diffs
-

  src/Makefile.am 29da17bee13226e18757e2ad3a7a091427fd35f4 
  src/slave/containerizer/mesos/containerizer.cpp 
56db7eb1193c9812b62b9149c9c2b2dd9b66701c 
  src/slave/containerizer/mesos/isolators/user/user.hpp PRE-CREATION 
  src/slave/containerizer/mesos/launch.hpp 
49eb9128ae5a1e4ec02e2b9d9e3cb67d7a8f7663 
  src/slave/containerizer/mesos/launch.cpp 
2835beff9dbfa7f2a1cac306a58e2b1d66c14342 


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


Testing
---


Thanks,

Srinivas Brahmaroutu

Review Request 58769: Added userns flag to ContainerLaunchInfo.

[Srinivas Brahmaroutu]

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

Review request for mesos.


Repository: mesos


Description
---

WIP Added userns flag to ContainerLaunchInfo.


Diffs
-

  include/mesos/slave/containerizer.proto 
c30b1fc659ee9b3cd343899638ced6408d8b51a2 


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


Testing
---


Thanks,

Srinivas Brahmaroutu

Re: Review Request 58708: Checked validity of master and agent version numbers on startup.

[Benjamin Mahler]

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




src/master/main.cpp
Lines 257-258 (patched)


Also, we try to get the open and close quotes on the same line when 
possible, to make it clearer to the reader when we forget to close a quote. 
Ditto for the other log in the agent.


- Benjamin Mahler


On April 26, 2017, 5:35 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58708/
> ---
> 
> (Updated April 26, 2017, 5:35 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Kapil Arya.
> 
> 
> Bugs: MESOS-6976
> https://issues.apache.org/jira/browse/MESOS-6976
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> During startup, we now check that the compiled-in version number of the
> master and agent can be parsed by stout's `Version::parse` (i.e., that
> `MESOS_VERSION` is valid according to stout's extended variant of the
> Semver 2.0.0 format).
> 
> 
> Diffs
> -
> 
>   src/master/main.cpp 462ed3229d15b157a92d96860503c06368ab21b7 
>   src/slave/main.cpp 72b141cb66f9df5bcc7b3f8cfcc2b06fcbd17e52 
> 
> 
> Diff: https://reviews.apache.org/r/58708/diff/2/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 58747: Fixed docker uri fetcher strict v2 schema 1 check.

[Gilbert Song]

> On April 26, 2017, 1:01 p.m., Ilya Pronin wrote:
> > We can check if we got a schema 1 manifest by parsing manifest JSON first 
> > and checking `schemaVersion` field in it as I did here: 
> > https://reviews.apache.org/r/53850/diff/1#0 Otherwise we may get a cryptic 
> > error message saying that some fields are missing. What do you think?
> 
> Jie Yu wrote:
> Also related to https://reviews.apache.org/r/58725
> 
> Maybe we should just land Ilya's scheme 2 support? Gilbert, thouughts?
> 
> Gilbert Song wrote:
> Thank you guys.
> 
> I think they are separate issues. There is a bug in the check after 
> reading the docker spec doc:
> In https://docs.docker.com/registry/spec/manifest-v2-1/:
> ```
> Manifest Type Media Type
> manifest  “application/vnd.docker.distribution.manifest.v1+json”
> signed manifest   
> “application/vnd.docker.distribution.manifest.v1+prettyjws”
> Note that “application/json” will also be accepted for schema 1.
> ```
> 
> The check introduced in r/53848 does not capture the `Note` that 
> `“application/json” will also be accepted for schema 1`. This supposes to be 
> the backward compactible note for old registries < 2.3.
> 
> I would +1 to land Ilya's schema 2 support, but it may not catch up with 
> 1.3 release. Should we land this quick fix first then land the schema 2 
> support?
> 
> Chun-Hung Hsiao wrote:
> Do you guys prefer adding "application/json" into the integrity check, or 
> just remove this check?

@Chun, already added in the patch.


- Gilbert


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


On April 26, 2017, 4:12 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58747/
> ---
> 
> (Updated April 26, 2017, 4:12 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Ilya Pronin, and Jie Yu.
> 
> 
> Bugs: MESOS-7272
> https://issues.apache.org/jira/browse/MESOS-7272
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This check was introduced from this patch:
> https://reviews.apache.org/r/53848/
> 
> The check on registry v2 schema 1 is incorrect. It does not work
> for registries that are older version < 2.3, because the ContentType
> header may be something like this "application/json; charset=utf-8".
> The check missed a note from docker registry spec that
> "application/json" will also be accepted for schema 1.
> 
> Depending on the docker registry spec doc, docker support the
> following three media type for V2 schema 1 manifest:
>   1. application/vnd.docker.distribution.manifest.v1+json
>   2. application/vnd.docker.distribution.manifest.v1+prettyjws
>   3. application/json
> For more details, see:
>   https://docs.docker.com/registry/spec/manifest-v2-1/
> 
> 
> Diffs
> -
> 
>   src/uri/fetchers/docker.cpp 1c6ab929deacfc29aa6b4f1df04c2b9782044a90 
> 
> 
> Diff: https://reviews.apache.org/r/58747/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 58764: Fixed compilation error for libprocess.

[Michael Park]

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


Ship it!




Ship It!

- Michael Park


On April 26, 2017, 4:22 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58764/
> ---
> 
> (Updated April 26, 2017, 4:22 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed compilation error for libprocess.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp 
> 6d4c5022b5eef271cd40d131aeef13362209eb3e 
> 
> 
> Diff: https://reviews.apache.org/r/58764/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 58764: Fixed compilation error for libprocess.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On April 26, 2017, 11:22 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58764/
> ---
> 
> (Updated April 26, 2017, 11:22 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed compilation error for libprocess.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp 
> 6d4c5022b5eef271cd40d131aeef13362209eb3e 
> 
> 
> Diff: https://reviews.apache.org/r/58764/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Review Request 58761: Added --secret-fetcher flag for agent.

[Kapil Arya]

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

Review request for mesos and Gilbert Song.


Repository: mesos


Description
---

Added --secret-fetcher flag for agent.


Diffs
-

  src/local/local.cpp e47980929db2da1f31cf899a0e1fc452070e11f3 
  src/slave/flags.hpp 38c05af99631059958868311adcc55774fbc6015 
  src/slave/flags.cpp f22837c8c94463c2da1ddfde9ed954631a3f38bd 
  src/slave/main.cpp 72b141cb66f9df5bcc7b3f8cfcc2b06fcbd17e52 


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


Testing
---


Thanks,

Kapil Arya

Review Request 58760: Added default secret fetcher module.

[Kapil Arya]

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

Review request for mesos and Gilbert Song.


Repository: mesos


Description
---

Added default secret fetcher module.


Diffs
-

  src/Makefile.am 1fc453c497f278c9fc3fa5e91eb720a932915fde 
  src/secret/fetcher.hpp PRE-CREATION 
  src/secret/fetcher.cpp PRE-CREATION 


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


Testing
---


Thanks,

Kapil Arya

Review Request 58758: Added secret to Image.{docker, appc} and ContainerInfo.volume.

[Kapil Arya]

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

Review request for mesos and Gilbert Song.


Repository: mesos


Description
---

Added secret to Image.{docker,appc} and ContainerInfo.volume.


Diffs
-

  include/mesos/mesos.proto eaa2d2ac697cfc4f5aa56db0fb37363339608f43 


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


Testing
---


Thanks,

Kapil Arya

Review Request 58759: Introduced SecretFetcher module interface.

[Kapil Arya]

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

Review request for mesos and Gilbert Song.


Repository: mesos


Description
---

Introduced SecretFetcher module interface.


Diffs
-

  include/mesos/module/secretfetcher.hpp PRE-CREATION 
  include/mesos/secret/secretfetcher.hpp PRE-CREATION 
  src/Makefile.am 1fc453c497f278c9fc3fa5e91eb720a932915fde 
  src/module/manager.cpp 7d875fcb7fcec0d57274e644b0a3b67b333ac193 


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


Testing
---


Thanks,

Kapil Arya

Re: Review Request 58747: Fixed docker uri fetcher strict v2 schema 1 check.

[Chun-Hung Hsiao]

> On April 26, 2017, 8:01 p.m., Ilya Pronin wrote:
> > We can check if we got a schema 1 manifest by parsing manifest JSON first 
> > and checking `schemaVersion` field in it as I did here: 
> > https://reviews.apache.org/r/53850/diff/1#0 Otherwise we may get a cryptic 
> > error message saying that some fields are missing. What do you think?
> 
> Jie Yu wrote:
> Also related to https://reviews.apache.org/r/58725
> 
> Maybe we should just land Ilya's scheme 2 support? Gilbert, thouughts?
> 
> Gilbert Song wrote:
> Thank you guys.
> 
> I think they are separate issues. There is a bug in the check after 
> reading the docker spec doc:
> In https://docs.docker.com/registry/spec/manifest-v2-1/:
> ```
> Manifest Type Media Type
> manifest  “application/vnd.docker.distribution.manifest.v1+json”
> signed manifest   
> “application/vnd.docker.distribution.manifest.v1+prettyjws”
> Note that “application/json” will also be accepted for schema 1.
> ```
> 
> The check introduced in r/53848 does not capture the `Note` that 
> `“application/json” will also be accepted for schema 1`. This supposes to be 
> the backward compactible note for old registries < 2.3.
> 
> I would +1 to land Ilya's schema 2 support, but it may not catch up with 
> 1.3 release. Should we land this quick fix first then land the schema 2 
> support?

Do you guys prefer adding "application/json" into the integrity check, or just 
remove this check?


- Chun-Hung


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


On April 26, 2017, 11:12 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58747/
> ---
> 
> (Updated April 26, 2017, 11:12 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Ilya Pronin, and Jie Yu.
> 
> 
> Bugs: MESOS-7272
> https://issues.apache.org/jira/browse/MESOS-7272
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This check was introduced from this patch:
> https://reviews.apache.org/r/53848/
> 
> The check on registry v2 schema 1 is incorrect. It does not work
> for registries that are older version < 2.3, because the ContentType
> header may be something like this "application/json; charset=utf-8".
> The check missed a note from docker registry spec that
> "application/json" will also be accepted for schema 1.
> 
> Depending on the docker registry spec doc, docker support the
> following three media type for V2 schema 1 manifest:
>   1. application/vnd.docker.distribution.manifest.v1+json
>   2. application/vnd.docker.distribution.manifest.v1+prettyjws
>   3. application/json
> For more details, see:
>   https://docs.docker.com/registry/spec/manifest-v2-1/
> 
> 
> Diffs
> -
> 
>   src/uri/fetchers/docker.cpp 1c6ab929deacfc29aa6b4f1df04c2b9782044a90 
> 
> 
> Diff: https://reviews.apache.org/r/58747/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 58640: Updated the Overwrite docker provisioner test for MESOS-7280.

[Chun-Hung Hsiao]

> On April 26, 2017, 7:16 p.m., Neil Conway wrote:
> > src/tests/containerizer/provisioner_docker_tests.cpp
> > Lines 811 (patched)
> > 
> >
> > Wouldn't `path::join` be preferrable here?
> 
> Chun-Hung Hsiao wrote:
> Let me open a new PR to fix this.

Fixed in https://reviews.apache.org/r/58766/.


- Chun-Hung


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


On April 22, 2017, 1:56 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58640/
> ---
> 
> (Updated April 22, 2017, 1:56 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-7280
> https://issues.apache.org/jira/browse/MESOS-7280
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Created a non-empty file 'abc' under the agent's work directory, which
> is linked by the symlink '/xyz' in the 2nd layer of the testing image.
> This file should not be overwritten by the empty file '/xyz' in the 3rd
> layer. Please see the following link for more details:
>   https://hub.docker.com/r/chhsiao/overwrite/
> The testing image is updated in a way that it is compatible to the
> previous Overwrite test.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> c26e1f9b093fda66f14e474006d8148c0b9f3245 
> 
> 
> Diff: https://reviews.apache.org/r/58640/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make test
> Manually tested on commit bc12a58 to make sure that the 'abc' becomes empty 
> in the unpatched copy backend.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 58765: Fix build failure introduced in 8990ebb.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On April 26, 2017, 11:32 p.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58765/
> ---
> 
> (Updated April 26, 2017, 11:32 p.m.)
> 
> 
> Review request for mesos, Vinod Kone and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix build failure introduced in 8990ebb.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp 
> 6d4c5022b5eef271cd40d131aeef13362209eb3e 
> 
> 
> Diff: https://reviews.apache.org/r/58765/diff/1/
> 
> 
> Testing
> ---
> 
> Tests passed.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>

Review Request 58766: Fixed a path join issue in ProvisionerDockerBackendTest.

[Chun-Hung Hsiao]

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

Review request for mesos, Gilbert Song, Jie Yu, and Neil Conway.


Repository: mesos


Description
---

Replaced a string concatenation with path::join in
ProvisionerDockerBackendTest.ROOT_INTERNET_CURL_Overwrite.


Diffs
-

  src/tests/containerizer/provisioner_docker_tests.cpp 
52e57cef81f79ebae4d5305708929396c5d75680 


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


Testing
---

sudo make check


Thanks,

Chun-Hung Hsiao

Review Request 58765: Fix build failure introduced in 8990ebb.

[Anindya Sinha]

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

Review request for mesos, Vinod Kone and Jiang Yan Xu.


Repository: mesos


Description
---

Fix build failure introduced in 8990ebb.


Diffs
-

  3rdparty/libprocess/src/process.cpp 6d4c5022b5eef271cd40d131aeef13362209eb3e 


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


Testing
---

Tests passed.


Thanks,

Anindya Sinha

Review Request 58764: Fixed compilation error for libprocess.

[Chun-Hung Hsiao]

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

Review request for mesos and Michael Park.


Repository: mesos


Description
---

Fixed compilation error for libprocess.


Diffs
-

  3rdparty/libprocess/src/process.cpp 6d4c5022b5eef271cd40d131aeef13362209eb3e 


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


Testing
---

make check


Thanks,

Chun-Hung Hsiao

Re: Review Request 58708: Checked validity of master and agent version numbers on startup.

[Benjamin Mahler]

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


Fix it, then Ship it!




Looks good, just seems to be in an inconsistent location across the master and 
agent files?


src/master/main.cpp
Lines 252-260 (patched)


It seems like this is in a different spot across the agent and master code, 
seems to me like we should do this before flag loading and build info logging 
in both cases.

Since this will be an error regardless of getting the flags right.


- Benjamin Mahler


On April 26, 2017, 5:35 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58708/
> ---
> 
> (Updated April 26, 2017, 5:35 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Kapil Arya.
> 
> 
> Bugs: MESOS-6976
> https://issues.apache.org/jira/browse/MESOS-6976
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> During startup, we now check that the compiled-in version number of the
> master and agent can be parsed by stout's `Version::parse` (i.e., that
> `MESOS_VERSION` is valid according to stout's extended variant of the
> Semver 2.0.0 format).
> 
> 
> Diffs
> -
> 
>   src/master/main.cpp 462ed3229d15b157a92d96860503c06368ab21b7 
>   src/slave/main.cpp 72b141cb66f9df5bcc7b3f8cfcc2b06fcbd17e52 
> 
> 
> Diff: https://reviews.apache.org/r/58708/diff/2/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 58707: Enhanced stout's Version to support prerelease and build labels.

[Benjamin Mahler]

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



Really happy to see these patches!

At a high level the main thing was whether we can simplify the parsing code, by 
using a right-to-left parse instead of left-to-right, which seems better suited 
to the trailing optional prerelease version / build metadata.


3rdparty/stout/include/stout/version.hpp
Lines 39-40 (original), 41-42 (patched)


See below about documenting that this is not strictly semver, in that we 
allow minor and patch to be optional.



3rdparty/stout/include/stout/version.hpp
Lines 39-41 (original), 41-44 (patched)


See below about documenting that the missing minor and patch versions is 
technically invalid semver, maybe add a TODO to allow that only in a distinct 
"tolerant" parse or something.



3rdparty/stout/include/stout/version.hpp
Line 50 (original), 59 (patched)


Hm.. separator sounds like the separator itself? E.g. ".", "+", "-", etc.

Perhaps something like `end_of_numeric_component`? Or consistently using 
"offset" as done below?



3rdparty/stout/include/stout/version.hpp
Lines 51-54 (original), 60-63 (patched)


I wasn't able to see how "raw" describes what this is storing, you could 
call it `numericComponent` or `numericVersion`, but we could also avoid it:

```
std::vector numeric_components =
  strings::split(s.substr(0, end_of_numeric_component), ".");
```



3rdparty/stout/include/stout/version.hpp
Lines 61-65 (original), 73-80 (patched)


Per (2) in the spec, we have to consider any leading zeros to be invalid, 
and we need to reject negatives.

For the latter, I think `numify(...)` would fail for 
negatives?



3rdparty/stout/include/stout/version.hpp
Lines 85-130 (patched)


Hm.. this was rather tricky to read, specifically handling the 
optionalities of prerelease version and build metadata. I wonder if it would be 
more readable if we were to parse from right-to-left rather than left-to-right:

In pseudo-code:

```
// Parse build metadata.
remaining, optional build_metadata = split(input, "+");

// Parse pre-release version (note that '-' is allowed within it).
remaining, optional prerelease_version = split(input, "-", 2);

// Parse numeric version.
major, optional minor, optional patch = split(remaining, ".");
```

With a right-to-left approach we could parse by splitting things out one by 
one, without needing to deal with the complication of dealing with whether we 
find the prerelease version or build metadata first / whether build metadata 
follows the prerelease version.

It seems to have less cognitive overhead for the reader, compared to 
figuring out whether the find / substr logic has any off by one errors.

Thoughts?



3rdparty/stout/include/stout/version.hpp
Lines 86 (patched)


s/=/-/



3rdparty/stout/include/stout/version.hpp
Lines 66-71 (original), 139-182 (patched)


These are made public but I can't see how these would be useful to any 
callers, could we just inline lambdas them in the parse function for now until 
these prove needed by some callers?



3rdparty/stout/include/stout/version.hpp
Lines 142 (patched)


One extra complication here is: "Numeric identifiers MUST NOT include 
leading zeroes" only for prerelease version. It seems ambiguous to me, for 
example:

1.1.1-00  // Invalid
1.1.1-00a // Valid or Invalid? Not sure if 001a is defined as numeric or 
not.

I think it's valid, given what is said about precedence: "identifiers 
consisting of only digits are compared numerically and identifiers with letters 
or hyphens are compared lexically in ASCII sort order". Seems to imply mixing 
means it's not treated as numeric validation-wise.



3rdparty/stout/include/stout/version.hpp
Lines 156-157 (patched)


In general we try to open and close the quote on the same line if possible, 
as it tends to reduce the amount of times we forget to close the quote, ditto 
elsewhere



3rdparty/stout/include/stout/version.hpp
Lines 187-188 (patched)


Ditto w.r.t. use of vector instead of string here, it seems a little 
confusing for the caller to have to pass vectors here instead of 

Re: Review Request 58725: Fetching docker image manifests with 'Accept' header.

[Gilbert Song]

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


Ship it!




Ship It!

- Gilbert Song


On April 26, 2017, 3:27 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58725/
> ---
> 
> (Updated April 26, 2017, 3:27 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-7427
> https://issues.apache.org/jira/browse/MESOS-7427
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added 'Accept: application/vnd.docker.distribution.manifest.v1+json'
> to the headers of HTTP requests for fetching manifests from any Docker
> registry. Some registry services (e.g., Amazon ECR) check the 'Accept'
> field strictly and reject the requests if it is not specified.
> 
> 
> Diffs
> -
> 
>   src/uri/fetchers/docker.cpp 1c6ab929deacfc29aa6b4f1df04c2b9782044a90 
> 
> 
> Diff: https://reviews.apache.org/r/58725/diff/3/
> 
> 
> Testing
> ---
> 
> sudo make check
> Manually tested on a local docker private registry and an Amazon ECR 
> repository.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 58747: Fixed docker uri fetcher strict v2 schema 1 check.

[Gilbert Song]

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

(Updated April 26, 2017, 4:12 p.m.)


Review request for mesos, Chun-Hung Hsiao, Ilya Pronin, and Jie Yu.


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


Repository: mesos


Description (updated)
---

This check was introduced from this patch:
https://reviews.apache.org/r/53848/

The check on registry v2 schema 1 is incorrect. It does not work
for registries that are older version < 2.3, because the ContentType
header may be something like this "application/json; charset=utf-8".
The check missed a note from docker registry spec that
"application/json" will also be accepted for schema 1.

Depending on the docker registry spec doc, docker support the
following three media type for V2 schema 1 manifest:
  1. application/vnd.docker.distribution.manifest.v1+json
  2. application/vnd.docker.distribution.manifest.v1+prettyjws
  3. application/json
For more details, see:
  https://docs.docker.com/registry/spec/manifest-v2-1/


Diffs (updated)
-

  src/uri/fetchers/docker.cpp 1c6ab929deacfc29aa6b4f1df04c2b9782044a90 


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

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


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 58689: MESOS-7323: Made `addSlave` conditionally activate the framework.

[Michael Park]

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

(Updated April 26, 2017, 3:47 p.m.)


Review request for mesos, Benjamin Bannier, Benjamin Mahler, and Neil Conway.


Changes
---

Addressed a part of bmahler's comments.


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


Repository: mesos


Description (updated)
---

MESOS-7323: Made `addSlave` conditionally activate the framework.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.cpp 
984a0a4e2671ee7bb2a3515849342f49f2c4e3aa 
  src/tests/upgrade_tests.cpp b5a28b5161d896dff250b8ad012c8aac91c3f861 


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

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


Testing
---

`make check` and added a test.


Thanks,

Michael Park

Re: Review Request 58689: MESOS-7323: Made `addSlave` conditionally activate the framework.

[Michael Park]

> On April 25, 2017, 5:59 p.m., Benjamin Mahler wrote:
> > src/master/allocator/mesos/hierarchical.cpp
> > Lines 379 (patched)
> > 
> >
> > Hm.. I was looking for a `framework.active = true` in activateFramework 
> > but seems this was missed?

As we discussed, turns out I didn't need the `active` member. Removed.


> On April 25, 2017, 5:59 p.m., Benjamin Mahler wrote:
> > src/master/allocator/mesos/hierarchical.cpp
> > Lines 483-485 (original), 487-493 (patched)
> > 
> >
> > Per our conversation, this doesn't seem necessary (intuitively addSlave 
> > shouldn't induce an activation of the framework within a role).
> > 
> > I guess we can just remove the activation code here and that makes it a 
> > 2 line fix? No need to store the 'active' state anymore either?

Yep! Thanks for this.


- Michael


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


On April 24, 2017, 4:18 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58689/
> ---
> 
> (Updated April 24, 2017, 4:18 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, and Neil Conway.
> 
> 
> Bugs: MESOS-7323
> https://issues.apache.org/jira/browse/MESOS-7323
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> f84b0574ce9a392c9528c87b04b01dbb2053cff7 
>   src/master/allocator/mesos/hierarchical.cpp 
> 051f749dd5921a322ca930a042c31814616d38f9 
>   src/tests/upgrade_tests.cpp b5a28b5161d896dff250b8ad012c8aac91c3f861 
> 
> 
> Diff: https://reviews.apache.org/r/58689/diff/1/
> 
> 
> Testing
> ---
> 
> `make check` and added a test.
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 58688: Removed an incorrect CHECK in the master.

[Michael Park]

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

(Updated April 26, 2017, 3:47 p.m.)


Review request for mesos, Benjamin Bannier, Benjamin Mahler, and Neil Conway.


Changes
---

Rebased.


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


Repository: mesos


Description (updated)
---

Removed an incorrect CHECK in the master.


Diffs (updated)
-

  src/master/master.cpp a0cb505a1300637f42fa8062c57deb1590779dd4 


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

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


Testing
---

`make check`, and added a test in https://reviews.apache.org/r/58689


Thanks,

Michael Park

Re: Review Request 58687: Made the activation of a client in `Sorter::add` explicit.

[Michael Park]

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

(Updated April 26, 2017, 3:47 p.m.)


Review request for mesos, Benjamin Bannier, Benjamin Mahler, and Neil Conway.


Changes
---

Addressed bmahler's comments.


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


Repository: mesos


Description (updated)
---

Made the activation of a client in `Sorter::add` explicit.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.cpp 
984a0a4e2671ee7bb2a3515849342f49f2c4e3aa 
  src/master/allocator/sorter/drf/sorter.cpp 
73b8e8ccdee9d1a88bf6a1726b8ff67008ef339f 
  src/master/allocator/sorter/sorter.hpp 
0c9f17255e9ed9dea56322bf7633505faf2a1a33 
  src/tests/sorter_tests.cpp 3d183bf3602ed632cbe8242c90a499d2e7848499 


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

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


Testing
---

`make check`


Thanks,

Michael Park

Re: Review Request 58687: Made the activation of a client in `Sorter::add` explicit.

[Michael Park]

> On April 25, 2017, 4:49 p.m., Benjamin Mahler wrote:
> > src/master/allocator/sorter/drf/sorter.cpp
> > Lines 151-155 (original)
> > 
> >
> > Do we still want the CHECKs or some variant of the comment?

Kept the `CHECK`s here.


> On April 25, 2017, 4:49 p.m., Benjamin Mahler wrote:
> > src/master/allocator/sorter/drf/sorter.cpp
> > Lines 151-156 (original)
> > 
> >
> > Could we document these new semantics in the sorter interface?

Added a comment on the sorter interface.


- Michael


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


On April 24, 2017, 4:18 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58687/
> ---
> 
> (Updated April 24, 2017, 4:18 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, and Neil Conway.
> 
> 
> Bugs: MESOS-7323
> https://issues.apache.org/jira/browse/MESOS-7323
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> 051f749dd5921a322ca930a042c31814616d38f9 
>   src/master/allocator/sorter/drf/sorter.cpp 
> ed54680cecb637931fc344fbcf8fd3b14cc24295 
>   src/tests/sorter_tests.cpp 43bd85798aef0c89751b725ebf35308a5e9e997a 
> 
> 
> Diff: https://reviews.apache.org/r/58687/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 51258: Add documentation for the ExternalContainerLogger module.

[Srdjan Grubor]

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



This didn't build on master since the new objects use ContainerIO instead of 
SubprocessInfo - I have a fixup patch though that you can apply at 
https://github.com/endlessm/mesos/commit/46370660a2ef6fea2caf926d822e0e76aaac2917.patch

- Srdjan Grubor


On Feb. 13, 2017, 10:42 p.m., Will Rouesnel wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51258/
> ---
> 
> (Updated Feb. 13, 2017, 10:42 p.m.)
> 
> 
> Review request for mesos and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add documentation for the ExternalContainerLogger module.
> 
> 
> Diffs
> -
> 
>   docs/logging.md 75c438570694c974c077296d925059469b4d344c 
> 
> 
> Diff: https://reviews.apache.org/r/51258/diff/6/
> 
> 
> Testing
> ---
> 
> Markdown is valid and parses correctly.
> 
> 
> Thanks,
> 
> Will Rouesnel
> 
>

Re: Review Request 58749: Fixed a typo in container-image.md doc.

[Chun-Hung Hsiao]

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


Ship it!




Ship It!

- Chun-Hung Hsiao


On April 26, 2017, 7:20 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58749/
> ---
> 
> (Updated April 26, 2017, 7:20 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed a typo in container-image.md doc.
> 
> 
> Diffs
> -
> 
>   docs/container-image.md fad4fd22533de960c2c5cfc52ae7d13a9cffdc67 
> 
> 
> Diff: https://reviews.apache.org/r/58749/diff/1/
> 
> 
> Testing
> ---
> 
> N/A.
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 58747: Fixed docker uri fetcher strict v2 schema 1 check.

[Gilbert Song]

> On April 26, 2017, 1:01 p.m., Ilya Pronin wrote:
> > We can check if we got a schema 1 manifest by parsing manifest JSON first 
> > and checking `schemaVersion` field in it as I did here: 
> > https://reviews.apache.org/r/53850/diff/1#0 Otherwise we may get a cryptic 
> > error message saying that some fields are missing. What do you think?
> 
> Jie Yu wrote:
> Also related to https://reviews.apache.org/r/58725
> 
> Maybe we should just land Ilya's scheme 2 support? Gilbert, thouughts?

Thank you guys.

I think they are separate issues. There is a bug in the check after reading the 
docker spec doc:
In https://docs.docker.com/registry/spec/manifest-v2-1/:
```
Manifest Type   Media Type
manifest“application/vnd.docker.distribution.manifest.v1+json”
signed manifest “application/vnd.docker.distribution.manifest.v1+prettyjws”
Note that “application/json” will also be accepted for schema 1.
```

The check introduced in r/53848 does not capture the `Note` that 
`“application/json” will also be accepted for schema 1`. This supposes to be 
the backward compactible note for old registries < 2.3.

I would +1 to land Ilya's schema 2 support, but it may not catch up with 1.3 
release. Should we land this quick fix first then land the schema 2 support?


- Gilbert


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


On April 26, 2017, 12:14 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58747/
> ---
> 
> (Updated April 26, 2017, 12:14 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Ilya Pronin, and Jie Yu.
> 
> 
> Bugs: MESOS-7272
> https://issues.apache.org/jira/browse/MESOS-7272
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This check was introduced from this patch:
> https://reviews.apache.org/r/53848/
> 
> The check on registry v2 schema 1 is too strict. It does not work
> for registries that are older version < 2.3, because the ContentType
> header may be something like this "application/json; charset=utf-8".
> No schema information is included in the manifest header for old
> registries.
> 
> 
> Diffs
> -
> 
>   src/uri/fetchers/docker.cpp 1c6ab929deacfc29aa6b4f1df04c2b9782044a90 
> 
> 
> Diff: https://reviews.apache.org/r/58747/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 58725: Fetching docker image manifests with 'Accept' header.

[Chun-Hung Hsiao]

> On April 26, 2017, 7:28 p.m., Jie Yu wrote:
> > src/uri/fetchers/docker.cpp
> > Lines 504-505 (patched)
> > 
> >
> > I'd introduce:
> > ```
> > class Headers {
> > public:
> >   http::Headers operator+(const http::Headers& that) const;
> > }
> > ```
> > 
> > so that you can do:
> > ```
> > curl(manifestUri, authHeaders + manifestHeaders)
> > ```

Fixed in https://reviews.apache.org/r/58753/.


- Chun-Hung


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


On April 26, 2017, 10:27 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58725/
> ---
> 
> (Updated April 26, 2017, 10:27 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-7427
> https://issues.apache.org/jira/browse/MESOS-7427
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added 'Accept: application/vnd.docker.distribution.manifest.v1+json'
> to the headers of HTTP requests for fetching manifests from any Docker
> registry. Some registry services (e.g., Amazon ECR) check the 'Accept'
> field strictly and reject the requests if it is not specified.
> 
> 
> Diffs
> -
> 
>   src/uri/fetchers/docker.cpp 1c6ab929deacfc29aa6b4f1df04c2b9782044a90 
> 
> 
> Diff: https://reviews.apache.org/r/58725/diff/3/
> 
> 
> Testing
> ---
> 
> sudo make check
> Manually tested on a local docker private registry and an Amazon ECR 
> repository.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 58725: Fetching docker image manifests with 'Accept' header.

[Chun-Hung Hsiao]

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

(Updated April 26, 2017, 10:27 p.m.)


Review request for mesos, Gilbert Song and Jie Yu.


Changes
---

Addressed Jie's comments.


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


Repository: mesos


Description
---

Added 'Accept: application/vnd.docker.distribution.manifest.v1+json'
to the headers of HTTP requests for fetching manifests from any Docker
registry. Some registry services (e.g., Amazon ECR) check the 'Accept'
field strictly and reject the requests if it is not specified.


Diffs (updated)
-

  src/uri/fetchers/docker.cpp 1c6ab929deacfc29aa6b4f1df04c2b9782044a90 


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

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


Testing
---

sudo make check
Manually tested on a local docker private registry and an Amazon ECR repository.


Thanks,

Chun-Hung Hsiao

Re: Review Request 58287: Print corresponding address when socket shutdown.

[Vinod Kone]

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


Ship it!




Made some minor adjustments before committing.

- Vinod Kone


On April 20, 2017, 11:34 a.m., Andy Pang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58287/
> ---
> 
> (Updated April 20, 2017, 11:34 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Print corresponding address when socket shutdown.
> Default just print socket 'fd',it's not convenient
> to find corresponding address.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp 92efa91 
> 
> 
> Diff: https://reviews.apache.org/r/58287/diff/5/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> File Attachments
> 
> 
> process.patch
>   
> https://reviews.apache.org/media/uploaded/files/2017/04/13/d5736720-1262-484a-8efd-6e1eeada944d__process.patch
> 
> 
> Thanks,
> 
> Andy Pang
> 
>

Re: Review Request 58262: Inherited environment from parent when launching a DEBUG container.

[Vinod Kone]

> On April 26, 2017, 3:58 a.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/containerizer.cpp
> > Lines 1396 (patched)
> > 
> >
> > I'd suggest we list the priority for environments for debug container 
> > here in the comments as well.
> > ```
> > 1) user specified env
> > 2) returned by isolators
> > 3) passed from containerizer
> > 4) inheirited from the parent
> > ```

Are either 2) or 3) specified by the operator? If yes, should operator be able 
to override user's env settings?


- Vinod


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


On April 25, 2017, 9:06 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58262/
> ---
> 
> (Updated April 25, 2017, 9:06 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman, Jie Yu, Kevin Klues, and Vinod Kone.
> 
> 
> Bugs: MESOS-6782
> https://issues.apache.org/jira/browse/MESOS-6782
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 29a99f33e646593127b9dc126f398f7bca88e21d 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 56db7eb1193c9812b62b9149c9c2b2dd9b66701c 
>   src/slave/containerizer/mesos/paths.hpp 
> d85fd34660faacd9c73de2ba7b87b3bbd4b6007b 
>   src/slave/containerizer/mesos/paths.cpp 
> ed4bbd2491e71ad1e4a41e0575b514377d02da9b 
>   src/tests/containerizer/nested_mesos_containerizer_tests.cpp 
> 29007898ec04e922266068a8519731b13d351a82 
> 
> 
> Diff: https://reviews.apache.org/r/58262/diff/4/
> 
> 
> Testing
> ---
> 
> See https://reviews.apache.org/r/58718/
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 58262: Inherited environment from parent when launching a DEBUG container.

[Vinod Kone]

> On April 13, 2017, 12:45 a.m., Vinod Kone wrote:
> > src/slave/containerizer/mesos/containerizer.cpp
> > Lines 1386-1389 (original), 1401-1404 (patched)
> > 
> >
> > Previously debug containers had this set in the env as well? But not 
> > anymore?
> 
> Alexander Rukletsov wrote:
> Correct. `environment` represents "basic" environment, e.g. agent 
> environment. I'd argue that we should not set it for debug containers, but 
> rather inherit parent's basic environment as part of inheritance process. 
> Moreover, currently, debug container launch path does not set `environment` 
> at all: 
> https://github.com/apache/mesos/blob/aca6796b703a8925319087d33d7dc5e5539f50d3/src/slave/containerizer/mesos/containerizer.cpp#L1830

I see. Ok.


- Vinod


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


On April 25, 2017, 9:06 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58262/
> ---
> 
> (Updated April 25, 2017, 9:06 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman, Jie Yu, Kevin Klues, and Vinod Kone.
> 
> 
> Bugs: MESOS-6782
> https://issues.apache.org/jira/browse/MESOS-6782
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 29a99f33e646593127b9dc126f398f7bca88e21d 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 56db7eb1193c9812b62b9149c9c2b2dd9b66701c 
>   src/slave/containerizer/mesos/paths.hpp 
> d85fd34660faacd9c73de2ba7b87b3bbd4b6007b 
>   src/slave/containerizer/mesos/paths.cpp 
> ed4bbd2491e71ad1e4a41e0575b514377d02da9b 
>   src/tests/containerizer/nested_mesos_containerizer_tests.cpp 
> 29007898ec04e922266068a8519731b13d351a82 
> 
> 
> Diff: https://reviews.apache.org/r/58262/diff/4/
> 
> 
> Testing
> ---
> 
> See https://reviews.apache.org/r/58718/
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 58718: Added a test that verifies a task's env var is seen by its check.

[Vinod Kone]

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


Fix it, then Ship it!




Nice test!


src/tests/check_tests.cpp
Lines 521 (patched)


Move this to #548?



src/tests/check_tests.cpp
Lines 556-557 (patched)


Why does one take a ref and other a copy?


- Vinod Kone


On April 25, 2017, 9:06 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58718/
> ---
> 
> (Updated April 25, 2017, 9:06 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman, Jie Yu, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/tests/check_tests.cpp ec0d5ee78a94991c68f38174d09b41e8f2e4be35 
> 
> 
> Diff: https://reviews.apache.org/r/58718/diff/1/
> 
> 
> Testing
> ---
> 
> `make check` on various Linux distributions.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 58753: Concatenation for HTTP Headers.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On April 26, 2017, 9:14 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58753/
> ---
> 
> (Updated April 26, 2017, 9:14 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, and Neil Conway.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added http::Headers::operator+().
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> 5c013ca4311acc22dcb1096aadee984a27be8d95 
> 
> 
> Diff: https://reviews.apache.org/r/58753/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Review Request 58753: Concatenation for HTTP Headers.

[Chun-Hung Hsiao]

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

Review request for mesos, Gilbert Song, Jie Yu, and Neil Conway.


Repository: mesos


Description
---

Added http::Headers::operator+().


Diffs
-

  3rdparty/libprocess/include/process/http.hpp 
5c013ca4311acc22dcb1096aadee984a27be8d95 


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


Testing
---

make check


Thanks,

Chun-Hung Hsiao

Re: Review Request 58449: CMake: Bump minimum version to 3.7.0 on Windows.

[Joseph Wu]

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


Ship it!




Confirmed that this review chain works with a clean build (after re-installing 
MSVC to 2017 and CMake to 3.8)

- Joseph Wu


On April 13, 2017, 7:10 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58449/
> ---
> 
> (Updated April 13, 2017, 7:10 p.m.)
> 
> 
> Review request for mesos, Jeff Coffler, John Kordich, Joseph Wu, and Li Li.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The `SOURCE_SUBDIR` command to `ExternalProject_Add` was added in CMake
> 3.7.0, and is necessary to most cleanly build an external CMake built
> project where the `CMakeLists.txt` is in a subfolder of the project.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt ea529ec2d5c2b9be4f19c67c2033c3f4b9073c1f 
> 
> 
> Diff: https://reviews.apache.org/r/58449/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 58448: Windows: Updated ZooKeeper to use CMake.

[Joseph Wu]

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


Ship it!




The trailing whitespace in the patch breaks our linter, but since all those 
whitespaces are in the ZK source, there's nothing we can do about it :(

- Joseph Wu


On April 19, 2017, 11:20 a.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58448/
> ---
> 
> (Updated April 19, 2017, 11:20 a.m.)
> 
> 
> Review request for mesos, Jeff Coffler, John Kordich, Joseph Wu, and Li Li.
> 
> 
> Bugs: ZOOKEEPER-2756
> https://issues.apache.org/jira/browse/ZOOKEEPER-2756
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This unblocks us from building exclusively with VS 2017. The previous
> patch to ZooKeeper only added VS 2015 support. This patch replaces it
> with a CMake build system that will generate whichever solution we need
> for Windows (and can replace the Autotools system on Linux).
> 
> We're updating to 3.5.2-alpha as the existing 3.5.1 rebundle was a
> source tarball, and so missing the necessary generated files. The most
> currently used version was based off a random commit. 3.5.2-alpha is the
> latest 3.5.x release of ZooKeeper (3.5.x itself is alpha, 3.5.2 is
> semi-stable, in comparison to 3.5.3 which is in RC).
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt bb61ef0514fb164f35b34bb6be1bbebb4d1a1861 
>   3rdparty/cmake/Mesos3rdpartyConfigure.cmake 
> c60652688a23f8628f133b7890ff39e38fc8ae94 
>   3rdparty/cmake/Versions.cmake 912726351ff744dd839b8d1c8d64dcc373d879be 
>   3rdparty/zookeeper-3.5.2-alpha.patch PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/58448/diff/1/
> 
> 
> Testing
> ---
> 
> cmake and make check on Linux
> stout-tests, libprocess-tests, and mesos-tests on Windows: all tests passed. 
> (Note: my machine updated to the Creators Update today, disabling long path 
> support; the previous way to enable it doesn't seem to work, so I re-ran the 
> failed tests on a machine that hadn't updated and still had long path 
> support, they passed).
> 
> Also tested the build on a clean machine with nothing but the VS 2017 build 
> tools (no IDE, no prior VS installations); mesos-tests builds no problem.
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 58747: Fixed docker uri fetcher strict v2 schema 1 check.

[Jie Yu]

> On April 26, 2017, 8:01 p.m., Ilya Pronin wrote:
> > We can check if we got a schema 1 manifest by parsing manifest JSON first 
> > and checking `schemaVersion` field in it as I did here: 
> > https://reviews.apache.org/r/53850/diff/1#0 Otherwise we may get a cryptic 
> > error message saying that some fields are missing. What do you think?

Also related to https://reviews.apache.org/r/58725

Maybe we should just land Ilya's scheme 2 support? Gilbert, thouughts?


- Jie


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


On April 26, 2017, 7:14 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58747/
> ---
> 
> (Updated April 26, 2017, 7:14 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Ilya Pronin, and Jie Yu.
> 
> 
> Bugs: MESOS-7272
> https://issues.apache.org/jira/browse/MESOS-7272
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This check was introduced from this patch:
> https://reviews.apache.org/r/53848/
> 
> The check on registry v2 schema 1 is too strict. It does not work
> for registries that are older version < 2.3, because the ContentType
> header may be something like this "application/json; charset=utf-8".
> No schema information is included in the manifest header for old
> registries.
> 
> 
> Diffs
> -
> 
>   src/uri/fetchers/docker.cpp 1c6ab929deacfc29aa6b4f1df04c2b9782044a90 
> 
> 
> Diff: https://reviews.apache.org/r/58747/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 58447: CMake: Cleaned up 3rdparty dependencies.

[Joseph Wu]

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


Ship it!




I'm going to make a couple minor whitespace additions (to make things 
consistent within the files).  But otherwise LGTM.


3rdparty/CMakeLists.txt
Line 46 (original), 26-27 (patched)


Whitespace things like: 2 spaces before each of these sections.



3rdparty/CMakeLists.txt
Lines 54-55 (original), 51-52 (patched)


And one newline before comments like this.


- Joseph Wu


On April 13, 2017, 7:03 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58447/
> ---
> 
> (Updated April 13, 2017, 7:03 p.m.)
> 
> 
> Review request for mesos, Jeff Coffler, John Kordich, Joseph Wu, and Li Li.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This commit removes duplicate code from `3rdparty/CMakeLists.txt`,
> and consolidates platform-specific versions into `Versions.cmake`.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt bb61ef0514fb164f35b34bb6be1bbebb4d1a1861 
>   3rdparty/cmake/Mesos3rdpartyConfigure.cmake 
> c60652688a23f8628f133b7890ff39e38fc8ae94 
>   3rdparty/cmake/Versions.cmake 912726351ff744dd839b8d1c8d64dcc373d879be 
>   cmake/CompilationConfigure.cmake 1c5466960f5ac73b8fc81edf7950cc68ed744301 
> 
> 
> Diff: https://reviews.apache.org/r/58447/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 58640: Updated the Overwrite docker provisioner test for MESOS-7280.

[Neil Conway]

> On April 26, 2017, 7:05 p.m., Jie Yu wrote:
> > src/tests/containerizer/provisioner_docker_tests.cpp
> > Lines 827 (patched)
> > 
> >
> > EXPECT_SOME
> 
> Chun-Hung Hsiao wrote:
> Should the `ASSERT_SOME` in Line 813 be changed to `EXPECT_SOME` as well? 
> If it should be then I'll open a new PR for address it and Niel's suggestion.

You'd want `EXPECT_SOME` if you want the test to continue even if the check 
fails. Offhand I'd think `ASSERT_SOME` is better here.


- Neil


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


On April 22, 2017, 1:56 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58640/
> ---
> 
> (Updated April 22, 2017, 1:56 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-7280
> https://issues.apache.org/jira/browse/MESOS-7280
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Created a non-empty file 'abc' under the agent's work directory, which
> is linked by the symlink '/xyz' in the 2nd layer of the testing image.
> This file should not be overwritten by the empty file '/xyz' in the 3rd
> layer. Please see the following link for more details:
>   https://hub.docker.com/r/chhsiao/overwrite/
> The testing image is updated in a way that it is compatible to the
> previous Overwrite test.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> c26e1f9b093fda66f14e474006d8148c0b9f3245 
> 
> 
> Diff: https://reviews.apache.org/r/58640/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make test
> Manually tested on commit bc12a58 to make sure that the 'abc' becomes empty 
> in the unpatched copy backend.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 58640: Updated the Overwrite docker provisioner test for MESOS-7280.

[Chun-Hung Hsiao]

> On April 26, 2017, 7:05 p.m., Jie Yu wrote:
> > src/tests/containerizer/provisioner_docker_tests.cpp
> > Lines 827 (patched)
> > 
> >
> > EXPECT_SOME

Should the `ASSERT_SOME` in Line 813 be changed to `EXPECT_SOME` as well? If it 
should be then I'll open a new PR for address it and Niel's suggestion.


- Chun-Hung


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


On April 22, 2017, 1:56 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58640/
> ---
> 
> (Updated April 22, 2017, 1:56 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-7280
> https://issues.apache.org/jira/browse/MESOS-7280
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Created a non-empty file 'abc' under the agent's work directory, which
> is linked by the symlink '/xyz' in the 2nd layer of the testing image.
> This file should not be overwritten by the empty file '/xyz' in the 3rd
> layer. Please see the following link for more details:
>   https://hub.docker.com/r/chhsiao/overwrite/
> The testing image is updated in a way that it is compatible to the
> previous Overwrite test.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> c26e1f9b093fda66f14e474006d8148c0b9f3245 
> 
> 
> Diff: https://reviews.apache.org/r/58640/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make test
> Manually tested on commit bc12a58 to make sure that the 'abc' becomes empty 
> in the unpatched copy backend.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 58725: Fetching docker image manifests with 'Accept' header.

[Chun-Hung Hsiao]

> On April 26, 2017, 7:28 p.m., Jie Yu wrote:
> > Have you tested with all registries that we support?

I've run the unit tests, which should cover all supported public registries.
Also manually tested on a local private registry and Amazen ECR.


- Chun-Hung


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


On April 26, 2017, 6:20 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58725/
> ---
> 
> (Updated April 26, 2017, 6:20 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-7427
> https://issues.apache.org/jira/browse/MESOS-7427
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added 'Accept: application/vnd.docker.distribution.manifest.v1+json'
> to the headers of HTTP requests for fetching manifests from any Docker
> registry. Some registry services (e.g., Amazon ECR) check the 'Accept'
> field strictly and reject the requests if it is not specified.
> 
> 
> Diffs
> -
> 
>   src/uri/fetchers/docker.cpp 1c6ab929deacfc29aa6b4f1df04c2b9782044a90 
> 
> 
> Diff: https://reviews.apache.org/r/58725/diff/2/
> 
> 
> Testing
> ---
> 
> sudo make check
> Manually tested on a local docker private registry and an Amazon ECR 
> repository.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 58747: Fixed docker uri fetcher strict v2 schema 1 check.

[Ilya Pronin]

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



We can check if we got a schema 1 manifest by parsing manifest JSON first and 
checking `schemaVersion` field in it as I did here: 
https://reviews.apache.org/r/53850/diff/1#0 Otherwise we may get a cryptic 
error message saying that some fields are missing. What do you think?

- Ilya Pronin


On April 26, 2017, 8:14 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58747/
> ---
> 
> (Updated April 26, 2017, 8:14 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Ilya Pronin, and Jie Yu.
> 
> 
> Bugs: MESOS-7272
> https://issues.apache.org/jira/browse/MESOS-7272
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This check was introduced from this patch:
> https://reviews.apache.org/r/53848/
> 
> The check on registry v2 schema 1 is too strict. It does not work
> for registries that are older version < 2.3, because the ContentType
> header may be something like this "application/json; charset=utf-8".
> No schema information is included in the manifest header for old
> registries.
> 
> 
> Diffs
> -
> 
>   src/uri/fetchers/docker.cpp 1c6ab929deacfc29aa6b4f1df04c2b9782044a90 
> 
> 
> Diff: https://reviews.apache.org/r/58747/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 58725: Fetching docker image manifests with 'Accept' header.

[Jie Yu]

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


Fix it, then Ship it!




Have you tested with all registries that we support?


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


one line above.



src/uri/fetchers/docker.cpp
Lines 504-505 (patched)


I'd introduce:
```
class Headers {
public:
  http::Headers operator+(const http::Headers& that) const;
}
```

so that you can do:
```
curl(manifestUri, authHeaders + manifestHeaders)
```


- Jie Yu


On April 26, 2017, 6:20 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58725/
> ---
> 
> (Updated April 26, 2017, 6:20 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-7427
> https://issues.apache.org/jira/browse/MESOS-7427
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added 'Accept: application/vnd.docker.distribution.manifest.v1+json'
> to the headers of HTTP requests for fetching manifests from any Docker
> registry. Some registry services (e.g., Amazon ECR) check the 'Accept'
> field strictly and reject the requests if it is not specified.
> 
> 
> Diffs
> -
> 
>   src/uri/fetchers/docker.cpp 1c6ab929deacfc29aa6b4f1df04c2b9782044a90 
> 
> 
> Diff: https://reviews.apache.org/r/58725/diff/2/
> 
> 
> Testing
> ---
> 
> sudo make check
> Manually tested on a local docker private registry and an Amazon ECR 
> repository.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 58640: Updated the Overwrite docker provisioner test for MESOS-7280.

[Chun-Hung Hsiao]

> On April 26, 2017, 7:16 p.m., Neil Conway wrote:
> > src/tests/containerizer/provisioner_docker_tests.cpp
> > Lines 811 (patched)
> > 
> >
> > Wouldn't `path::join` be preferrable here?

Let me open a new PR to fix this.


- Chun-Hung


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


On April 22, 2017, 1:56 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58640/
> ---
> 
> (Updated April 22, 2017, 1:56 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-7280
> https://issues.apache.org/jira/browse/MESOS-7280
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Created a non-empty file 'abc' under the agent's work directory, which
> is linked by the symlink '/xyz' in the 2nd layer of the testing image.
> This file should not be overwritten by the empty file '/xyz' in the 3rd
> layer. Please see the following link for more details:
>   https://hub.docker.com/r/chhsiao/overwrite/
> The testing image is updated in a way that it is compatible to the
> previous Overwrite test.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> c26e1f9b093fda66f14e474006d8148c0b9f3245 
> 
> 
> Diff: https://reviews.apache.org/r/58640/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make test
> Manually tested on commit bc12a58 to make sure that the 'abc' becomes empty 
> in the unpatched copy backend.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 57935: Added a check to ensure scalar quantities match in `DRFSorter::update`.

[Neil Conway]

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



Hey Anindya -- can you rebase this RR for the recent sorter changes? I should 
be able to review and commit it shortly after that.

- Neil Conway


On March 26, 2017, 1:16 a.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57935/
> ---
> 
> (Updated March 26, 2017, 1:16 a.m.)
> 
> 
> Review request for mesos and Neil Conway.
> 
> 
> Bugs: MESOS-7138
> https://issues.apache.org/jira/browse/MESOS-7138
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The old and new allocations only varies in roles (i.e. reservations), or
> `DiskInfo` (for persistent volumes) or shared counts. In these cases, we
> do not re-calculate the shares since the total allocation quantity is
> unchanged.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/sorter/drf/sorter.cpp 
> ed54680cecb637931fc344fbcf8fd3b14cc24295 
> 
> 
> Diff: https://reviews.apache.org/r/57935/diff/1/
> 
> 
> Testing
> ---
> 
> All tests passed.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>

Review Request 58749: Fixed a typo in container-image.md doc.

[Gilbert Song]

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

Review request for mesos, Chun-Hung Hsiao and Jie Yu.


Repository: mesos


Description
---

Fixed a typo in container-image.md doc.


Diffs
-

  docs/container-image.md fad4fd22533de960c2c5cfc52ae7d13a9cffdc67 


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


Testing
---

N/A.


Thanks,

Gilbert Song

Re: Review Request 58533: Added a benchmark test for hierarchical sorter.

[Neil Conway]

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



Note that a made a few cosmetic fixes; see 
https://github.com/apache/mesos/commit/f5d77f05b008c063ed6330ed80d08d625827b31a

- Neil Conway


On April 20, 2017, 3:54 a.m., Jay Guo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58533/
> ---
> 
> (Updated April 20, 2017, 3:54 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Neil Conway.
> 
> 
> Bugs: MESOS-7395
> https://issues.apache.org/jira/browse/MESOS-7395
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This test is very similar to Sorter_BENCHMARK_Test.FullSort, except
> that hierarchical role is built instead of flat one.
> 
> 
> Diffs
> -
> 
>   src/tests/sorter_tests.cpp 43bd85798aef0c89751b725ebf35308a5e9e997a 
> 
> 
> Diff: https://reviews.apache.org/r/58533/diff/2/
> 
> 
> Testing
> ---
> 
> `./bin/mesos-tests.sh --benchmark 
> --gtest_filter="AgentAndClientCount/HSorter_BENCHMARK_Test.FullSort*"`
> 
> 
> Thanks,
> 
> Jay Guo
> 
>

Re: Review Request 58640: Updated the Overwrite docker provisioner test for MESOS-7280.

[Neil Conway]

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




src/tests/containerizer/provisioner_docker_tests.cpp
Lines 811 (patched)


Wouldn't `path::join` be preferrable here?


- Neil Conway


On April 22, 2017, 1:56 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58640/
> ---
> 
> (Updated April 22, 2017, 1:56 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-7280
> https://issues.apache.org/jira/browse/MESOS-7280
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Created a non-empty file 'abc' under the agent's work directory, which
> is linked by the symlink '/xyz' in the 2nd layer of the testing image.
> This file should not be overwritten by the empty file '/xyz' in the 3rd
> layer. Please see the following link for more details:
>   https://hub.docker.com/r/chhsiao/overwrite/
> The testing image is updated in a way that it is compatible to the
> previous Overwrite test.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> c26e1f9b093fda66f14e474006d8148c0b9f3245 
> 
> 
> Diff: https://reviews.apache.org/r/58640/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make test
> Manually tested on commit bc12a58 to make sure that the 'abc' becomes empty 
> in the unpatched copy backend.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Review Request 58747: Fixed docker uri fetcher strict v2 schema 1 check.

[Gilbert Song]

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

Review request for mesos, Chun-Hung Hsiao, Ilya Pronin, and Jie Yu.


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


Repository: mesos


Description
---

This check was introduced from this patch:
https://reviews.apache.org/r/53848/

The check on registry v2 schema 1 is too strict. It does not work
for registries that are older version < 2.3, because the ContentType
header may be something like this "application/json; charset=utf-8".
No schema information is included in the manifest header for old
registries.


Diffs
-

  src/uri/fetchers/docker.cpp 1c6ab929deacfc29aa6b4f1df04c2b9782044a90 


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


Testing
---

make check


Thanks,

Gilbert Song

Review Request 58748: Fixed the internet curl flaky test due to short timeout.

[Gilbert Song]

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

Review request for mesos and Jie Yu.


Repository: mesos


Description
---

Fixed the internet curl flaky test due to short timeout.


Diffs
-

  src/tests/containerizer/provisioner_docker_tests.cpp 
c26e1f9b093fda66f14e474006d8148c0b9f3245 


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


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 58640: Updated the Overwrite docker provisioner test for MESOS-7280.

[Jie Yu]

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


Fix it, then Ship it!





src/tests/containerizer/provisioner_docker_tests.cpp
Lines 827 (patched)


EXPECT_SOME


- Jie Yu


On April 22, 2017, 1:56 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58640/
> ---
> 
> (Updated April 22, 2017, 1:56 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-7280
> https://issues.apache.org/jira/browse/MESOS-7280
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Created a non-empty file 'abc' under the agent's work directory, which
> is linked by the symlink '/xyz' in the 2nd layer of the testing image.
> This file should not be overwritten by the empty file '/xyz' in the 3rd
> layer. Please see the following link for more details:
>   https://hub.docker.com/r/chhsiao/overwrite/
> The testing image is updated in a way that it is compatible to the
> previous Overwrite test.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> c26e1f9b093fda66f14e474006d8148c0b9f3245 
> 
> 
> Diff: https://reviews.apache.org/r/58640/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make test
> Manually tested on commit bc12a58 to make sure that the 'abc' becomes empty 
> in the unpatched copy backend.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 58533: Added a benchmark test for hierarchical sorter.

[Neil Conway]

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


Ship it!




Ship It!

- Neil Conway


On April 20, 2017, 3:54 a.m., Jay Guo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58533/
> ---
> 
> (Updated April 20, 2017, 3:54 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Neil Conway.
> 
> 
> Bugs: MESOS-7395
> https://issues.apache.org/jira/browse/MESOS-7395
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This test is very similar to Sorter_BENCHMARK_Test.FullSort, except
> that hierarchical role is built instead of flat one.
> 
> 
> Diffs
> -
> 
>   src/tests/sorter_tests.cpp 43bd85798aef0c89751b725ebf35308a5e9e997a 
> 
> 
> Diff: https://reviews.apache.org/r/58533/diff/2/
> 
> 
> Testing
> ---
> 
> `./bin/mesos-tests.sh --benchmark 
> --gtest_filter="AgentAndClientCount/HSorter_BENCHMARK_Test.FullSort*"`
> 
> 
> Thanks,
> 
> Jay Guo
> 
>

Re: Review Request 57190: Updated agent for hierarchical roles.

[Neil Conway]

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


Ship it!




Ship It!

- Neil Conway


On March 31, 2017, 12:54 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57190/
> ---
> 
> (Updated March 31, 2017, 12:54 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Jie Yu, Michael Park, and Neil 
> Conway.
> 
> 
> Bugs: MESOS-7047
> https://issues.apache.org/jira/browse/MESOS-7047
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This commit adjusts the way persistent volumes are stored on the
> agent. Instead of interpreting the role of the volume as a literal
> path, we replace `/` with ` ` when creating the path. This prevents
> that subdirectories are created for volumes with hierarchical roles.
> Directly interpreting the role as a path is undesirable as it can lead
> to volumes overlapping (e.g., a volume with role `a/b/c/d` and id `id`
> would be visible as `id` in a volume with role `a/b/c` and id `d`).
> 
> 
> Diffs
> -
> 
>   src/slave/paths.cpp ef22ee167f16030f02d28c8e6bab6c2ca4812d8f 
>   src/tests/role_tests.cpp 0433c0599eac5f4648bc0dfe3a0fa8d5f7a836ca 
> 
> 
> Diff: https://reviews.apache.org/r/57190/diff/5/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 58725: Fetching docker image manifests with 'Accept' header.

[Chun-Hung Hsiao]

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

(Updated April 26, 2017, 6:20 p.m.)


Review request for mesos, Gilbert Song and Jie Yu.


Changes
---

Addressed Gilbert's comments.


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


Repository: mesos


Description
---

Added 'Accept: application/vnd.docker.distribution.manifest.v1+json'
to the headers of HTTP requests for fetching manifests from any Docker
registry. Some registry services (e.g., Amazon ECR) check the 'Accept'
field strictly and reject the requests if it is not specified.


Diffs (updated)
-

  src/uri/fetchers/docker.cpp 1c6ab929deacfc29aa6b4f1df04c2b9782044a90 


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

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


Testing
---

sudo make check
Manually tested on a local docker private registry and an Amazon ECR repository.


Thanks,

Chun-Hung Hsiao

Re: Review Request 57516: Updated CHANGELOG for hierarchical roles.

[Neil Conway]

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

(Updated April 26, 2017, 5:58 p.m.)


Review request for mesos, Benjamin Bannier, Benjamin Mahler, and Michael Park.


Changes
---

Update CHANGELOG to discuss hierarchical reservations.


Repository: mesos


Description
---

Updated CHANGELOG for hierarchical roles.


Diffs (updated)
-

  CHANGELOG 08f10da1c46071a45f2cf26e28874536ba34ec54 


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

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


Testing
---

No functional change.


Thanks,

Neil Conway

Re: Review Request 57516: Updated CHANGELOG for hierarchical roles.

[Neil Conway]

> On April 24, 2017, 9:11 p.m., Benjamin Mahler wrote:
> > CHANGELOG
> > Lines 16-17 (patched)
> > 
> >
> > It would also be nice to mention lack of the full plan here for 
> > reservations as well. E.g. we want reservations to "eng" to go to the "eng" 
> > tree instead of just the "eng" role.
> > 
> > Hm.. come to think of it, won't we be breaking things if we have people 
> > assuming "eng" reservations don't go to the tree and we change this?

Updated the CHANGELOG text to note the current and planned behavior of 
reservations for hierarchies.

Note that there's a good chance we'll be able to land proper support for 
hierarchical reservations in 1.3.0


- Neil


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


On April 25, 2017, 8:07 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57516/
> ---
> 
> (Updated April 25, 2017, 8:07 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated CHANGELOG for hierarchical roles.
> 
> 
> Diffs
> -
> 
>   CHANGELOG f98ce8c01041535967b07e8d9360cfbf90ac7814 
> 
> 
> Diff: https://reviews.apache.org/r/57516/diff/5/
> 
> 
> Testing
> ---
> 
> No functional change.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 57534: Added and implemented RegisterAgent ACL.

[Jiang Yan Xu]

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

(Updated April 26, 2017, 10:53 a.m.)


Review request for mesos, Adam B, Anindya Sinha, Alexander Rojas, and Greg Mann.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Added and implemented RegisterAgent ACL.


Diffs (updated)
-

  include/mesos/authorizer/acls.proto ae10027eb716d4dcdeddf924223bcd4faed36de9 
  include/mesos/authorizer/authorizer.proto 
3ae5df5c276fc48b21e131ca9e0944315fe632e3 
  src/authorizer/local/authorizer.cpp 3eb59d3cc1c4c7f3ab3768bc42e2bd27c9300b8f 
  src/tests/authorization_tests.cpp 937a726e3ef4e44e07d3c8e30f72f6900889a6e3 


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

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


Testing
---

make check.


Thanks,

Jiang Yan Xu

Re: Review Request 58708: Checked validity of master and agent version numbers on startup.

[Neil Conway]

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

(Updated April 26, 2017, 5:35 p.m.)


Review request for mesos, Benjamin Mahler and Kapil Arya.


Changes
---

Tweak commit description.


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


Repository: mesos


Description (updated)
---

During startup, we now check that the compiled-in version number of the
master and agent can be parsed by stout's `Version::parse` (i.e., that
`MESOS_VERSION` is valid according to stout's extended variant of the
Semver 2.0.0 format).


Diffs (updated)
-

  src/master/main.cpp 462ed3229d15b157a92d96860503c06368ab21b7 
  src/slave/main.cpp 72b141cb66f9df5bcc7b3f8cfcc2b06fcbd17e52 


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

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


Testing
---

`make check`


Thanks,

Neil Conway

Re: Review Request 58707: Enhanced stout's Version to support prerelease and build labels.

[Neil Conway]

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

(Updated April 26, 2017, 5:35 p.m.)


Review request for mesos, Benjamin Mahler and Kapil Arya.


Changes
---

Tweaked comments, rebase.


Summary (updated)
-

Enhanced stout's Version to support prerelease and build labels.


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


Repository: mesos


Description
---

Previously, Stout's `Version` abstraction only supported a subset of
Semver: version numbers with three numeric components (an optional
trailing "label" with a leading hyphen was supported but ignored).

This commit adds support for SemVer 2.0.0, which defines two additional
optional fields: a "prerelease label" and a "build metadata label",
e.g., "1.2.3-alpha.1+foo". Both labels consist of a series of
dot-separated identifiers.


Diffs (updated)
-

  3rdparty/stout/include/stout/version.hpp 
7717c85b95d29cefe8f19f3cada4b7402d4d446f 
  3rdparty/stout/tests/version_tests.cpp 
724ed2292fdd3c5f4c98facf82260078b66a0e97 


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

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


Testing
---

`make check`


Thanks,

Neil Conway

Re: Review Request 58706: Improved unit tests for Version in stout.

[Neil Conway]

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

(Updated April 26, 2017, 5:35 p.m.)


Review request for mesos, Benjamin Mahler and Kapil Arya.


Changes
---

Addressed review comments, fix compile error with old G++.


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


Repository: mesos


Description
---

Switch to table-based test cases, validate that version parsing produces
the expected results more precisely.


Diffs (updated)
-

  3rdparty/stout/tests/version_tests.cpp 
724ed2292fdd3c5f4c98facf82260078b66a0e97 


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

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


Testing
---

`make check`


Thanks,

Neil Conway

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

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [58262, 58718, 58263]

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 April 25, 2017, 9:12 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58263/
> ---
> 
> (Updated April 25, 2017, 9:12 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman, Jie Yu, Kevin Klues, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 29a99f33e646593127b9dc126f398f7bca88e21d 
>   src/slave/containerizer/mesos/containerizer.cpp 
> bc611a5e085de10e9953b5f942d98f2b5747fce6 
> 
> 
> Diff: https://reviews.apache.org/r/58263/diff/2/
> 
> 
> Testing
> ---
> 
> make check on Mac OS.
> Linux pending.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 58725: Fetching docker image manifests with 'Accept' header.

[Gilbert Song]

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




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


Please fix the indentation.



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


Could you paste the link to the docker doc for `Accept` header.


- Gilbert Song


On April 25, 2017, 5:30 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58725/
> ---
> 
> (Updated April 25, 2017, 5:30 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-7427
> https://issues.apache.org/jira/browse/MESOS-7427
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added 'Accept: application/vnd.docker.distribution.manifest.v1+json'
> to the headers of HTTP requests for fetching manifests from any Docker
> registry. Some registry services (e.g., Amazon ECR) check the 'Accept'
> field strictly and reject the requests if it is not specified.
> 
> 
> Diffs
> -
> 
>   src/uri/fetchers/docker.cpp 1c6ab929deacfc29aa6b4f1df04c2b9782044a90 
> 
> 
> Diff: https://reviews.apache.org/r/58725/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> Manually tested on a local docker private registry and an Amazon ECR 
> repository.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 58676: Logged when an agent (re-)registration request is received.

[Mesos Reviewbot]

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



Bad patch!

Reviews applied: [58676, 57731, 57730, 57710, 57535, 57534, 57520]

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

Error:
2017-04-26 10:11:50 URL:https://reviews.apache.org/r/57534/diff/raw/ 
[5343/5343] -> "57534.patch" [1]
error: patch failed: include/mesos/authorizer/authorizer.proto:189
error: include/mesos/authorizer/authorizer.proto: patch does not apply

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

- Mesos Reviewbot


On April 25, 2017, 5:33 p.m., Jiang Yan Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58676/
> ---
> 
> (Updated April 25, 2017, 5:33 p.m.)
> 
> 
> Review request for mesos, Anindya Sinha and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> - This log happens after the master has done initial validation but
>before authorization, which is consistent with the (re-)register
>framework code paths.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp d1cdc35a066e190ef8e0bd788e07e63b92f7fce7 
> 
> 
> Diff: https://reviews.apache.org/r/58676/diff/1/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>