Re: Review Request 39398: Sync TaskStatus::Reason enumerations in v1 API.

[Guangya Liu]

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

Ship it!


Ship It!

- Guangya Liu


On 十月 16, 2015, 4:43 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39398/
> ---
> 
> (Updated 十月 16, 2015, 4:43 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, and Joris Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The changes originate from: https://reviews.apache.org/r/38746
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/mesos.proto 0ac8aa6988908fb8fbf274eddaf82f32d2eeaff4 
> 
> Diff: https://reviews.apache.org/r/39398/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 39018: Added JSON parsing for Resources.

[Guangya Liu]

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



src/common/resources.cpp (line 274)


static?



src/common/resources.cpp (line 316)


static?



src/common/resources.cpp (line 348)


static



src/common/resources.cpp (line 363)


s/Value_Type/Value::Type

ditto as following



src/common/resources.cpp (line 488)


Shall we add the text into the log message to improve debug?



src/v1/resources.cpp (line 275)


static?


- Guangya Liu


On 十月 14, 2015, 3:50 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39018/
> ---
> 
> (Updated 十月 14, 2015, 3:50 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Jie Yu, and Michael 
> Park.
> 
> 
> Bugs: MESOS-2467
> https://issues.apache.org/jira/browse/MESOS-2467
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This includes code changes necessary for JSON parsing of Resources. 
> Documentation changes will be posted soon in another review 
> (https://reviews.apache.org/r/39102/).
> 
> 
> Diffs
> -
> 
>   include/mesos/resources.hpp 6c3a065945eb56dc88df9c977e5ca11d4cbcbf61 
>   include/mesos/v1/resources.hpp fe8925ac851b74d1b37919f00afc7ed816f47ea5 
>   src/common/resources.cpp 601388c35a1bff37c58e753d1870d53b8d0af2d1 
>   src/tests/resources_tests.cpp 6584fc6c39e6ffe9f8085576677dcc669f127697 
>   src/v1/resources.cpp dc868903472f8f3a1ddc56092e3f8f81d953ce39 
> 
> Diff: https://reviews.apache.org/r/39018/diff/
> 
> 
> Testing
> ---
> 
> New code was added to `ResourcesTest.ParsingFromJSON`, and two new tests were 
> added: `ResourcesTest.ParsingFromJSONWithRoles` and 
> `ResourcesTest.ParsingFromJSONError`. These attempt to test common error 
> scenarios that might show up in user-supplied JSON.
> 
> `make check` was used to confirm that all tests pass, with one notable 
> failure (ResourcesTest.ParsingFromJSONError) related to a picojson issue 
> tracked here: https://issues.apache.org/jira/browse/MESOS-3698
> 
> The original resources parsing format is used throughout many other tests 
> (check `src/tests/sorter_tests.cpp`, for example), so it's clear that the 
> original parsing continues to work correctly.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 39102: Added documentation for JSON resources.

[Guangya Liu]

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


Looks good. But we need first make sure the backend code merged first. 
https://reviews.apache.org/r/39018/

- Guangya Liu


On 十月 9, 2015, 4:13 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39102/
> ---
> 
> (Updated 十月 9, 2015, 4:13 p.m.)
> 
> 
> Review request for mesos, Adam B and Neil Conway.
> 
> 
> Bugs: MESOS-2467
> https://issues.apache.org/jira/browse/MESOS-2467
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added documentation for JSON resources.
> 
> 
> Diffs
> -
> 
>   docs/attributes-resources.md f712d094f14426515dabde45f98d6c1ae36c3447 
>   docs/configuration.md 2ab476a2d2c43e309b570d73ecac80e27b296e7e 
> 
> Diff: https://reviews.apache.org/r/39102/diff/
> 
> 
> Testing
> ---
> 
> Viewed the relevant documentation sections ('Attributes and Resources' & 
> 'Configuration') using the mesos-website-container: 
> https://github.com/mesosphere/mesos-website-container
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 39353: Fixed and added tests for docker image name parsing.

[Timothy Chen]

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


Thanks for adding this! Are you looking to also work on docker store stuff?

- Timothy Chen


On Oct. 15, 2015, 7:17 p.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39353/
> ---
> 
> (Updated Oct. 15, 2015, 7:17 p.m.)
> 
> 
> Review request for mesos, Jojy Varghese and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The existing docker image parsing did not work in many cases: if a digest is 
> present, a registry port is included. And it could not resolve the ambiguity 
> that exists between the registry and the repository.
> 
> This follows docker's approach to parsing, which is a bit hacky due to the 
> way the registry and repository are disambiguated, see the comments.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto f2ea4fc62507ce30b7a7981fd6a8c4749eb97190 
>   src/slave/containerizer/provisioner/docker/message.hpp 
> 6368bf4caec6f8c3ac97282f41c55381f920bce9 
>   src/slave/containerizer/provisioner/docker/message.proto 
> bbac2e6c1f40a7ca3f9227baca56a44cd43f58c6 
>   src/slave/containerizer/provisioner/docker/store.cpp 
> 637c97c0973bda492826803a962c99635647692d 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> 9c3c45a81be6398722a37911788e347a4e91cce8 
> 
> Diff: https://reviews.apache.org/r/39353/diff/
> 
> 
> Testing
> ---
> 
> Added a test.
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Re: Review Request 39380: Windows: Added support for `stout/flags/flags.hpp`.

[Alex Clemmer]

> On Oct. 16, 2015, 8:15 p.m., Joseph Wu wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/flags/parse.hpp, line 20
> > 
> >
> > Is this addition related?
> > 
> > (It's good to have nevertheless.)

Yep, there's a direct reference in this file to `Bytes`, which will cause 
compile errors when we pull the inclusion of `os.hpp` elsewhere.


- Alex


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


On Oct. 16, 2015, 7:50 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39380/
> ---
> 
> (Updated Oct. 16, 2015, 7:50 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Added support for `stout/flags/flags.hpp`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp 
> b91318cfd5748b3387bb6b32b910f327853274dd 
>   3rdparty/libprocess/3rdparty/stout/include/stout/flags/parse.hpp 
> e34ed78386130a1e757be1b1f1a2dbf794b7cd37 
> 
> Diff: https://reviews.apache.org/r/39380/diff/
> 
> 
> Testing
> ---
> 
> CMake `make check` on Ubuntu 15, OS X 10.10, and ran `check` target in VS2015 
> on Windows 10.
> 
> Autotools `make check` on Ubuntu 15.
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 39382: Windows: Moved `os::rm` to its own file, `stout/os/rm.hpp`.

[Alex Clemmer]

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

(Updated Oct. 17, 2015, 6:54 p.m.)


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


Repository: mesos


Description
---

Windows: Moved `os::rm` to its own file, `stout/os/rm.hpp`.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
9e9c3119ad18f4cbc70c70095c71dc4fd19553df 
  3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 
5141c1369af60afd6cd5c70c6295d575ea960a83 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/rm.hpp PRE-CREATION 
  3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 
a042d061159c83e1652e7288b90809981d2818c9 
  3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp 
2b0966889af73238a08e29f1136d0ce286a0ffda 

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


Testing
---

CMake `make check` on Ubuntu 15, OS X 10.10, and ran `check` target in VS2015 
on Windows 10.

Autotools `make check` on Ubuntu 15.


Thanks,

Alex Clemmer

Re: Review Request 39382: Windows: Moved `os::rm` to its own file, `stout/os/rm.hpp`.

[Alex Clemmer]

> On Oct. 16, 2015, 9:19 p.m., Mesos ReviewBot wrote:
> > Bad patch!
> > 
> > Reviews applied: [39076, 39091, 39092, 39093, 39096, 39097, 39180]
> > 
> > Failed command: ./support/apply-review.sh -n -r 39180
> > 
> > Error:
> >  2015-10-16 21:19:05 URL:https://reviews.apache.org/r/39180/diff/raw/ 
> > [16738/16738] -> "39180.patch" [1]
> > error: patch failed: src/tests/scheduler_http_api_tests.cpp:295
> > error: src/tests/scheduler_http_api_tests.cpp: patch does not apply
> > Failed to apply patch

Looks like someone pushed to master after I rebased. Sorry!


- Alex


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


On Oct. 16, 2015, 7:50 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39382/
> ---
> 
> (Updated Oct. 16, 2015, 7:50 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Moved `os::rm` to its own file, `stout/os/rm.hpp`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
> 9e9c3119ad18f4cbc70c70095c71dc4fd19553df 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 
> 5141c1369af60afd6cd5c70c6295d575ea960a83 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/rm.hpp PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 
> a042d061159c83e1652e7288b90809981d2818c9 
>   3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp 
> 2b0966889af73238a08e29f1136d0ce286a0ffda 
> 
> Diff: https://reviews.apache.org/r/39382/diff/
> 
> 
> Testing
> ---
> 
> CMake `make check` on Ubuntu 15, OS X 10.10, and ran `check` target in VS2015 
> on Windows 10.
> 
> Autotools `make check` on Ubuntu 15.
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 39382: Windows: Moved `os::rm` to its own file, `stout/os/rm.hpp`.

[Mesos ReviewBot]

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


Bad patch!

Reviews applied: [39076, 39091, 39092, 39093, 39096, 39097, 39180]

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

Error:
 2015-10-17 19:10:49 URL:https://reviews.apache.org/r/39180/diff/raw/ 
[16738/16738] -> "39180.patch" [1]
error: patch failed: src/tests/scheduler_http_api_tests.cpp:295
error: src/tests/scheduler_http_api_tests.cpp: patch does not apply
Failed to apply patch

- Mesos ReviewBot


On Oct. 17, 2015, 6:54 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39382/
> ---
> 
> (Updated Oct. 17, 2015, 6:54 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Moved `os::rm` to its own file, `stout/os/rm.hpp`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
> 9e9c3119ad18f4cbc70c70095c71dc4fd19553df 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 
> 5141c1369af60afd6cd5c70c6295d575ea960a83 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/rm.hpp PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 
> a042d061159c83e1652e7288b90809981d2818c9 
>   3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp 
> 2b0966889af73238a08e29f1136d0ce286a0ffda 
> 
> Diff: https://reviews.apache.org/r/39382/diff/
> 
> 
> Testing
> ---
> 
> CMake `make check` on Ubuntu 15, OS X 10.10, and ran `check` target in VS2015 
> on Windows 10.
> 
> Autotools `make check` on Ubuntu 15.
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 29550: Libprocess: Added "tests" target for make.

[Joris Van Remoortere]

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

Ship it!


Ship It!

- Joris Van Remoortere


On Oct. 14, 2015, 6:31 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29550/
> ---
> 
> (Updated Oct. 14, 2015, 6:31 p.m.)
> 
> 
> Review request for mesos, Cody Maloney, Joris Van Remoortere, Timothy Chen, 
> and Timothy St. Clair.
> 
> 
> Bugs: MESOS-2273
> https://issues.apache.org/jira/browse/MESOS-2273
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> 'make check' allows one to build and run the test suite. However,
> 'make tests' allows one to build the test suite without running it.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/Makefile.am 
> e64e3d9e958fab3c7c25fad17dcc2b279d255500 
>   3rdparty/libprocess/Makefile.am eb7393904988afc0bc5e9f7dadd545e0d6c04e5f 
> 
> Diff: https://reviews.apache.org/r/29550/diff/
> 
> 
> Testing
> ---
> 
> Ran make tests.
> 
> For added test, ran "make clean; make; make tests" followed by "make check" 
> and verified that no additional target are build in the "make check" step.
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 29551: Added "tests" target for make.

[Joris Van Remoortere]

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

Ship it!


Ship It!

- Joris Van Remoortere


On Oct. 14, 2015, 6:31 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29551/
> ---
> 
> (Updated Oct. 14, 2015, 6:31 p.m.)
> 
> 
> Review request for mesos, Cody Maloney, Joris Van Remoortere, Timothy Chen, 
> and Timothy St. Clair.
> 
> 
> Bugs: MESOS-2273
> https://issues.apache.org/jira/browse/MESOS-2273
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> 'make check' allows one to build and run the test suite. However,
> 'make tests' allows one to build the test suite without running it.
> 
> 
> Diffs
> -
> 
>   3rdparty/Makefile.am 7c244a053044ae347f9505cb78a71a6636e26586 
>   Makefile.am cb289b4f21516c3027429d8f8ba72d799f975bd4 
>   docs/submitting-a-patch.md cbd99fe3fd16270316596700e3fc292243a9 
>   src/Makefile.am d855cb83277c3e0e2ee3feacaf6ad0962223ef6e 
> 
> Diff: https://reviews.apache.org/r/29551/diff/
> 
> 
> Testing
> ---
> 
> Ran 'make tests'.
> 
> For added test, ran "make clean; make; make tests" followed by "make check" 
> and verified that no additional target are build in the "make check" step.
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 39330: libprocess: Replaced usage of "volatile" with std::atomic.

[Joris Van Remoortere]

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


Thanks for follow up with this Neil.
Let's simplify the patch with the 2 comments below and then cycle quickly to 
get it committed.


3rdparty/libprocess/include/process/owned.hpp (line 35)


Let's fix the typos in a separate patch as per offline discussion.



3rdparty/libprocess/include/process/owned.hpp (line 82)


Wish we could use `nullptr`, but we currently only allow `NULL` for now.
Here and below.


- Joris Van Remoortere


On Oct. 14, 2015, 10:29 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39330/
> ---
> 
> (Updated Oct. 14, 2015, 10:29 p.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-3326
> https://issues.apache.org/jira/browse/MESOS-3326
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See MESOS-3326. We adopted std::atomic in most of the code base earlier 
> (commits
> 87de003c6e8a, 4b938052b6af, and 4a01850c5540), but a few places were omitted;
> those locations are fixed by this commit.
> 
> There's one remaining place to improve: we use the GCC intrinsic
> __sync_synchronize() in 3rdparty/libprocess/include/process/logging.h. Because
> that is used to protect modifications to the FLAGS_v variable defined by glog,
> we can't easily adapt it to use std::atomic.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/owned.hpp 
> bc5b527152c8864544ad58070c0bfc81639056da 
>   3rdparty/libprocess/include/process/shared.hpp 
> 021807b961bb55f11c9e04327135bd83f4d86c21 
>   3rdparty/libprocess/src/tests/process_tests.cpp 
> e5277de5b5bdea4a44606cda7fbf69a559aeebbe 
> 
> Diff: https://reviews.apache.org/r/39330/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 39398: Sync TaskStatus::Reason enumerations in v1 API.

[Joris Van Remoortere]

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

Ship it!


Ship It!

- Joris Van Remoortere


On Oct. 16, 2015, 4:43 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39398/
> ---
> 
> (Updated Oct. 16, 2015, 4:43 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Jie Yu, and Joris Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The changes originate from: https://reviews.apache.org/r/38746
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/mesos.proto 0ac8aa6988908fb8fbf274eddaf82f32d2eeaff4 
> 
> Diff: https://reviews.apache.org/r/39398/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 39201: Included `stout/check.hpp` in `future.hpp`.

[Joris Van Remoortere]

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

Ship it!


- Joris Van Remoortere


On Oct. 13, 2015, 6:28 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39201/
> ---
> 
> (Updated Oct. 13, 2015, 6:28 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The Windows integration work will require us to support `stout/net.hpp`,
> but this file includes `stout/os.hpp`, which would be onerous to port.
> 
> To avoid having to port that entire file, we simply roped in what we
> needed from `stout/net.hpp`. A consequence of unincluding os.hpp is that
> `process/future.hpp` now requires a direct inclusion of
> `stout/check.hpp`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/future.hpp 
> 9006b8a83d03eab6e67de12a954110029b7d150e 
> 
> Diff: https://reviews.apache.org/r/39201/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 39102: Added documentation for JSON resources.

[Greg Mann]

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

(Updated Oct. 17, 2015, 11:05 p.m.)


Review request for mesos, Adam B and Neil Conway.


Changes
---

Added dependency.


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


Repository: mesos


Description
---

Added documentation for JSON resources.


Diffs
-

  docs/attributes-resources.md f712d094f14426515dabde45f98d6c1ae36c3447 
  docs/configuration.md 2ab476a2d2c43e309b570d73ecac80e27b296e7e 

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


Testing
---

Viewed the relevant documentation sections ('Attributes and Resources' & 
'Configuration') using the mesos-website-container: 
https://github.com/mesosphere/mesos-website-container


Thanks,

Greg Mann

Re: Review Request 39102: Added documentation for JSON resources.

[Greg Mann]

> On Oct. 17, 2015, 8:44 a.m., Guangya Liu wrote:
> > Looks good. But we need first make sure the backend code merged first. 
> > https://reviews.apache.org/r/39018/

Thanks; altered this review to depend on 39018.


- Greg


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


On Oct. 17, 2015, 11:05 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39102/
> ---
> 
> (Updated Oct. 17, 2015, 11:05 p.m.)
> 
> 
> Review request for mesos, Adam B and Neil Conway.
> 
> 
> Bugs: MESOS-2467
> https://issues.apache.org/jira/browse/MESOS-2467
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added documentation for JSON resources.
> 
> 
> Diffs
> -
> 
>   docs/attributes-resources.md f712d094f14426515dabde45f98d6c1ae36c3447 
>   docs/configuration.md 2ab476a2d2c43e309b570d73ecac80e27b296e7e 
> 
> Diff: https://reviews.apache.org/r/39102/diff/
> 
> 
> Testing
> ---
> 
> Viewed the relevant documentation sections ('Attributes and Resources' & 
> 'Configuration') using the mesos-website-container: 
> https://github.com/mesosphere/mesos-website-container
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 39388: Explicitly set the `LIBPROCESS_IP` env variable for docker containers.

[Michael Park]

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

(Updated Oct. 17, 2015, 11:18 p.m.)


Review request for mesos and Niklas Nielsen.


Changes
---

Added a test, expanded upon the comment


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/docker/docker.cpp 56d63dc75637c9f89a239af371f476a85a570696 
  src/tests/containerizer/docker_containerizer_tests.cpp 
4bb65afd0ee61cafef68e064a697fdce65d60058 

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


Testing (updated)
---

Added `DockerContainerizerTest.ROOT_DOCKER_LaunchWithLibprocessIP` test which 
fails without the changes made to `src/docker/docker.cpp`.


Thanks,

Michael Park

Re: Review Request 39102: Added documentation for JSON resources.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [39211, 39018, 39102]

All tests passed.

- Mesos ReviewBot


On Oct. 17, 2015, 11:05 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39102/
> ---
> 
> (Updated Oct. 17, 2015, 11:05 p.m.)
> 
> 
> Review request for mesos, Adam B and Neil Conway.
> 
> 
> Bugs: MESOS-2467
> https://issues.apache.org/jira/browse/MESOS-2467
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added documentation for JSON resources.
> 
> 
> Diffs
> -
> 
>   docs/attributes-resources.md f712d094f14426515dabde45f98d6c1ae36c3447 
>   docs/configuration.md 2ab476a2d2c43e309b570d73ecac80e27b296e7e 
> 
> Diff: https://reviews.apache.org/r/39102/diff/
> 
> 
> Testing
> ---
> 
> Viewed the relevant documentation sections ('Attributes and Resources' & 
> 'Configuration') using the mesos-website-container: 
> https://github.com/mesosphere/mesos-website-container
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 39208: Windows: Add windows support to `stout/protobuf.hpp`.

[Joris Van Remoortere]

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

Ship it!



3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp (lines 42 - 44)


let's pull the subdir out.
let's include close as per Joseph's comment.


- Joris Van Remoortere


On Oct. 13, 2015, 6:31 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39208/
> ---
> 
> (Updated Oct. 13, 2015, 6:31 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Bugs: MESOS-3634
> https://issues.apache.org/jira/browse/MESOS-3634
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Add windows support to `stout/protobuf.hpp`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp 
> 2285ce9eaba668d5215c108849055fe92163da4d 
> 
> Diff: https://reviews.apache.org/r/39208/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 39210: Windows: Moved `realpath` to its own file, `stout/os/realpath.hpp`.

[Joris Van Remoortere]

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

Ship it!



3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/exists.hpp (line 19)


let's split out this subdir.


- Joris Van Remoortere


On Oct. 15, 2015, 8:31 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39210/
> ---
> 
> (Updated Oct. 15, 2015, 8:31 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Bugs: MESOS-3632
> https://issues.apache.org/jira/browse/MESOS-3632
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Moved `realpath` to its own file, `stout/os/realpath.hpp`.
> 
> (NOTE: This does not resolve MESOS-3632, it's only a partial solution.)
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
> 9e9c3119ad18f4cbc70c70095c71dc4fd19553df 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 
> 5141c1369af60afd6cd5c70c6295d575ea960a83 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/realpath.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/exists.hpp 
> ddcda7bdb294272a7a64a7a46e0576e8c4430eec 
> 
> Diff: https://reviews.apache.org/r/39210/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 39209: Windows: Move ::UUID to stout::UUID to avoid namespace collision.

[Joris Van Remoortere]

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

Ship it!


Changing the summary to reflect the code change.

- Joris Van Remoortere


On Oct. 13, 2015, 6:33 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39209/
> ---
> 
> (Updated Oct. 13, 2015, 6:33 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The `Windows.h` head includes a header that defines the UUID struct for
> the DCE RPC API. This is dumped into the global namespace, just like our
> UUID implementation found in `stout/uuid.hpp`.
> 
> Since this causes a compilation error on Windows, we simply move our
> UUID into `stout::`. We further add a `using stout::UUID;` in the file
> so that we don't have to change every callsite in the Mesos codebase
> that uses it.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/uuid.hpp 
> bc167f1fa9e64f89138f131e726e7733c66da29c 
> 
> Diff: https://reviews.apache.org/r/39209/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 39207: Windows: Move `write` to its own file, `stout/os/write.hpp`.

[Joris Van Remoortere]

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

Ship it!



3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp (lines 88 - 91)


I think in other places we alphabetize these even with conditional 
compilation blocks right?



3rdparty/libprocess/3rdparty/stout/include/stout/os/write.hpp (lines 20 - 21)


let's split out the sub-directory.


- Joris Van Remoortere


On Oct. 13, 2015, 6:30 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39207/
> ---
> 
> (Updated Oct. 13, 2015, 6:30 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Bugs: MESOS-3632
> https://issues.apache.org/jira/browse/MESOS-3632
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Move `write` to its own file, `stout/os/write.hpp`.
> 
> (This part isn't going in the commit message, but just FYI, we're doing this 
> (1) because we need it to support `slave/state.hpp`, (2) because we don't 
> want to port all of `os.hpp` to get this one function, and (3) because 
> `stout/os/read.hpp` already exists, so it makes sense to have one for `write` 
> as well.)
> 
> (Also, please do not close MESOS-3632, this is just a partial solution.)
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
> 9e9c3119ad18f4cbc70c70095c71dc4fd19553df 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 
> 5141c1369af60afd6cd5c70c6295d575ea960a83 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/write.hpp PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 
> a042d061159c83e1652e7288b90809981d2818c9 
>   3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp 
> 2b0966889af73238a08e29f1136d0ce286a0ffda 
> 
> Diff: https://reviews.apache.org/r/39207/diff/
> 
> 
> Testing
> ---
> 
> Ran `make check` on Windows 10, OS X 10.10, Ubuntu 15. On Ubuntu 15, we also 
> checked that the autotools solution builds and the tests run as well.
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 39182: Windows: Enable ip_tests.

[Joris Van Remoortere]

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

Ship it!



3rdparty/libprocess/3rdparty/stout/include/stout/windows.hpp (line 18)


Comments end with periods.
e.g. `// For `_mkdir`.`
Same below.



3rdparty/libprocess/3rdparty/stout/include/stout/windows.hpp (lines 22 - 23)


I remember from a long time ago that we include in this order to avoid 
symbol re-definitions.
We can't expect everyone to know this, let's add a comment here.


- Joris Van Remoortere


On Oct. 13, 2015, 6:27 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39182/
> ---
> 
> (Updated Oct. 13, 2015, 6:27 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Bugs: MESOS-3439 and MESOS-3629
> https://issues.apache.org/jira/browse/MESOS-3439
> https://issues.apache.org/jira/browse/MESOS-3629
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Enable ip_tests.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/duration.hpp 
> 90551541f59889e96b21dbe1b65d3904850464c2 
>   3rdparty/libprocess/3rdparty/stout/include/stout/ip.hpp 
> d1e2df6151149e03ffb4a76e2c24ff78b891e016 
>   3rdparty/libprocess/3rdparty/stout/include/stout/windows.hpp 
> 50e35f43d87c69a83a9e7d039d1881404ea8be38 
>   3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt 
> 94292f8a46ec31bbaf6e52f48109322bbe123f70 
> 
> Diff: https://reviews.apache.org/r/39182/diff/
> 
> 
> Testing
> ---
> 
> Ran stout_tests from VS.
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 39203: CMake: fixed typo in agent include directory configuration.

[Joris Van Remoortere]

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

Ship it!


- Joris Van Remoortere


On Oct. 13, 2015, 6:29 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39203/
> ---
> 
> (Updated Oct. 13, 2015, 6:29 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> CMake: fixed typo in agent include directory configuration.
> 
> 
> Diffs
> -
> 
>   src/slave/cmake/SlaveConfigure.cmake 
> 5817547526ac6851e51baa05f1a72e225fc65791 
> 
> Diff: https://reviews.apache.org/r/39203/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 39204: Windows: Added support for `stout/os/read.hpp`.

[Joris Van Remoortere]

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

Ship it!


- Joris Van Remoortere


On Oct. 13, 2015, 6:29 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39204/
> ---
> 
> (Updated Oct. 13, 2015, 6:29 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Bugs: MESOS-3641
> https://issues.apache.org/jira/browse/MESOS-3641
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Added support for `stout/os/read.hpp`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
> 9e9c3119ad18f4cbc70c70095c71dc4fd19553df 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/read.hpp 
> ffacce164980157ea225a4e64e33b8bf8ec38ab7 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/read.hpp 
> d5698a5b44fd6083ac3119d6825d31f46efb2f38 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/read.hpp 
> 09d63329f16f13d408742f9fc8f596d76c4d70c9 
>   3rdparty/libprocess/3rdparty/stout/include/stout/windows.hpp 
> 50e35f43d87c69a83a9e7d039d1881404ea8be38 
> 
> Diff: https://reviews.apache.org/r/39204/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 39181: Windows: Added support for `stout/net.hpp`.

[Joris Van Remoortere]

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

Ship it!


Verified that the code was just moved.

- Joris Van Remoortere


On Oct. 13, 2015, 6:27 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39181/
> ---
> 
> (Updated Oct. 13, 2015, 6:27 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Bugs: MESOS-3630 and MESOS-3631
> https://issues.apache.org/jira/browse/MESOS-3630
> https://issues.apache.org/jira/browse/MESOS-3631
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Added support for `stout/net.hpp`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
> 9e9c3119ad18f4cbc70c70095c71dc4fd19553df 
>   3rdparty/libprocess/3rdparty/stout/include/stout/net.hpp 
> 3f829bafe96526bc2263c9f228f85de38c435f60 
>   3rdparty/libprocess/3rdparty/stout/include/stout/posix/net.hpp 
> 11e3895ee46e36faca0d2e1b436b61576826e472 
>   3rdparty/libprocess/3rdparty/stout/include/stout/windows/net.hpp 
> 4f82796c2b9ef6a9198be145d969c5fce933be49 
> 
> Diff: https://reviews.apache.org/r/39181/diff/
> 
> 
> Testing
> ---
> 
> Ran stout tests from VS.
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 39202: CMake: Moved libevent, gmock, http-parser to CMake on Windows.

[Joris Van Remoortere]

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

Ship it!



3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake (lines 78 - 82)


Any reason not to keep these alphabetized?


- Joris Van Remoortere


On Oct. 15, 2015, 8:31 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39202/
> ---
> 
> (Updated Oct. 15, 2015, 8:31 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> CMake: Moved libevent, gmock, http-parser to CMake on Windows.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/CMakeLists.txt 
> f0de224b0d7924344fb1945b387b728a7241df87 
>   3rdparty/libprocess/3rdparty/http-parser/CMakeLists.txt.template 
> PRE-CREATION 
>   3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake 
> 577aac82f527f3cce9f5b0cd24cc51091c311789 
>   3rdparty/libprocess/cmake/ProcessConfigure.cmake 
> 60108ff598fb075584196aa3d8e8e66e726c9f2a 
>   3rdparty/libprocess/cmake/ProcessTestsConfigure.cmake 
> d60bce237ec73f462fd373590a3589d554e76fc3 
> 
> Diff: https://reviews.apache.org/r/39202/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 39180: Windows: Added support for `stout/os/open.hpp`.

[Joris Van Remoortere]

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

Ship it!


Please don't include other rebased patches in to your diffs.

- Joris Van Remoortere


On Oct. 15, 2015, 8:31 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39180/
> ---
> 
> (Updated Oct. 15, 2015, 8:31 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Bugs: MESOS-3693
> https://issues.apache.org/jira/browse/MESOS-3693
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Added support for `stout/os/open.hpp`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
> 9e9c3119ad18f4cbc70c70095c71dc4fd19553df 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/open.hpp 
> 43f261fd7a60b534f642f826ebf6ab18d72180c2 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/open.hpp 
> 023993d859e3a101ca387c1a514cd75de0d2beb1 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/open.hpp 
> 14fa11765c222cb4e80f5e45360d0f05facb578e 
>   3rdparty/libprocess/3rdparty/stout/include/stout/windows.hpp 
> 50e35f43d87c69a83a9e7d039d1881404ea8be38 
>   src/Makefile.am 96ce73b301c55d23bf4a5292e3d028148426a878 
>   src/sched/sched.cpp 9c5e3b8e42605f4647d897ba02ea3a17e494f355 
>   src/tests/scheduler_http_api_tests.cpp 
> ae1f6ffda966a2eca1eda578261a0a0c4432e6b3 
> 
> Diff: https://reviews.apache.org/r/39180/diff/
> 
> 
> Testing
> ---
> 
> Ran both stout_tests from VS.
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Review Request 39415: Error out when root qdisc already exists

[Cong Wang]

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

Review request for mesos, Ian Downes and Jie Yu.


Repository: mesos


Description
---

When we enable --egress_unique_flow_per_container, we need to install an 
fq_codel qdisc on root, but if a qdisc already exists on root, we should not 
continue since it could be not fq_codel at all, so just exit with error.


Diffs
-

  src/slave/containerizer/isolators/network/port_mapping.cpp 
e6bb75e6f5ba48a0c4cf6dd8f353e5f5843d0eef 

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


Testing
---

Manual tests, with a pre-installed HTB qdisc and without.


Thanks,

Cong Wang

Review Request 39416: Document --egress_unique_flow_per_container in docs/configuration.md

[Cong Wang]

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

Review request for mesos, Ian Downes and Jie Yu.


Repository: mesos


Description
---

Flag --egress_unique_flow_per_container is documented in help message, but not 
in docs/configuration.md. We should document it there too, with more details.


Diffs
-

  docs/configuration.md 9443d5fc2de0f46f9e117b08e29b09ff3a4579c6 

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


Testing
---

None.


Thanks,

Cong Wang

Review Request 39417: Add --egress_flow_classifier_parent flag

[Cong Wang]

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

Review request for mesos, Ian Downes and Jie Yu.


Repository: mesos


Description
---

When --egress_unique_flow_per_container is enabled, we need to install a flow 
classifier (fq_codel) qdisc on egress side. This flag specifies where to 
install it in the hierarchy. By default, we install it at root. But, for 
example, if you have already installed HTB qdisc at root, you may want this to 
be installed in other place than root, specify an HTB class ID as its parent 
here.


Diffs
-

  docs/configuration.md 9443d5fc2de0f46f9e117b08e29b09ff3a4579c6 
  src/slave/containerizer/isolators/network/port_mapping.cpp 
e6bb75e6f5ba48a0c4cf6dd8f353e5f5843d0eef 
  src/slave/flags.hpp 3e93b52a5874f52361d5a9c685499a7032014a73 
  src/slave/flags.cpp 1bf394ea62fde29caa6705cd5d156eae452adbf2 

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


Testing
---

Manual tests, with and without a pre-installed HTB qdisc and classes.


Thanks,

Cong Wang

Re: Review Request 39345: Enable build on FreeBSD, start porting components.

[David Forsythe]

> On Oct. 15, 2015, 5:21 p.m., Mesos ReviewBot wrote:
> > Bad patch!
> > 
> > Reviews applied: [39345]
> > 
> > Failed command: ./support/apply-review.sh -n -r 39345
> > 
> > Error:
> >  2015-10-15 17:21:02 URL:https://reviews.apache.org/r/39345/diff/raw/ 
> > [22252/22252] -> "39345.patch" [1]
> > Successfully applied: Enable build on FreeBSD, start porting components.
> > 
> > Enable build on FreeBSD, start porting components.
> > 
> > My build steps are:
> > 
> > - Install dependencies from http://mesos.apache.org/gettingstarted/
> > - Install libexecinfo
> > - Install clang36 (system clang is 3.4)
> > - boostrap
> > - ../configure --with-curl=/usr/local --with-apr=/usr/local 
> > --with-svn=/usr/local CC=clang36 CXX=clang++36
> > - gmake CC=clang36 CXX=clang++36
> > - gmake CC=clang36 CXX=clang++36 check
> > 
> > I disabled one test because I haven't had a chance to debug it and I wanted 
> > to get a bit further in the test suite. A check run is attached.
> > 
> > 
> > Review: https://reviews.apache.org/r/39345
> > Checking 14 files using filter 
> > --filter=-,+build/class,+build/deprecated,+build/endif_comment,+readability/todo,+readability/namespace,+runtime/vlog,+whitespace/blank_line,+whitespace/comma,+whitespace/end_of_line,+whitespace/ending_newline,+whitespace/forcolon,+whitespace/indent,+whitespace/line_length,+whitespace/operators,+whitespace/semicolon,+whitespace/tab,+whitespace/todo
> > Total errors found: 0
> > ERROR: Commit spanning multiple projects.
> > 
> > Please use separate commits for mesos, libprocess and stout.
> > 
> > Paths grouped by project:
> > mesos:
> >   configure.ac
> >   src/Makefile.am
> > stout:
> >   3rdparty/libprocess/3rdparty/stout/include/stout/ip.hpp
> >   3rdparty/libprocess/3rdparty/stout/include/stout/mac.hpp
> >   3rdparty/libprocess/3rdparty/stout/include/stout/net.hpp
> >   3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp
> >   3rdparty/libprocess/3rdparty/stout/include/stout/os/freebsd.hpp
> >   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/fork.hpp
> >   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/sendfile.hpp
> >   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/signals.hpp
> >   3rdparty/libprocess/3rdparty/stout/include/stout/os/sysctl.hpp
> >   3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp
> >   3rdparty/libprocess/3rdparty/stout/tests/dynamiclibrary_tests.cpp
> >   3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp
> > libprocess:
> >   3rdparty/libprocess/3rdparty/Makefile.am
> >   3rdparty/libprocess/configure.ac
> >   3rdparty/libprocess/src/poll_socket.cpp
> >   3rdparty/libprocess/src/tests/http_tests.cpp
> > Failed to commit patch

Is there any chance of this landing as is, or will I need to submit seperate 
reviews?


- David


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


On Oct. 15, 2015, 5:16 p.m., David Forsythe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39345/
> ---
> 
> (Updated Oct. 15, 2015, 5:16 p.m.)
> 
> 
> Review request for mesos and Ian Downes.
> 
> 
> Bugs: https://issues.apache.org/jira/browse/MESOS-1563
> 
> https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/MESOS-1563
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enable build on FreeBSD, start porting components.
> 
> My build steps are:
> 
> - Install dependencies from http://mesos.apache.org/gettingstarted/
> - Install libexecinfo
> - Install clang36 (system clang is 3.4)
> - boostrap
> - ../configure --with-curl=/usr/local --with-apr=/usr/local 
> --with-svn=/usr/local CC=clang36 CXX=clang++36
> - gmake CC=clang36 CXX=clang++36
> - gmake CC=clang36 CXX=clang++36 check
> 
> I disabled one test because I haven't had a chance to debug it and I wanted 
> to get a bit further in the test suite. A check run is attached.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/Makefile.am 
> e64e3d9e958fab3c7c25fad17dcc2b279d255500 
>   3rdparty/libprocess/3rdparty/stout/include/stout/ip.hpp 
> d1e2df6151149e03ffb4a76e2c24ff78b891e016 
>   3rdparty/libprocess/3rdparty/stout/include/stout/mac.hpp 
> 9428717fac4655898d7768957f02937592e1a398 
>   3rdparty/libprocess/3rdparty/stout/include/stout/net.hpp 
> 3f829bafe96526bc2263c9f228f85de38c435f60 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 
> 5141c1369af60afd6cd5c70c6295d575ea960a83 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/freebsd.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/fork.hpp 
> 7eb51e8771e95f57548fc35753e75c6d56cd97cd 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/sendfile.h

Re: Review Request 39018: Added JSON parsing for Resources.

[Michael Park]

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



src/common/resources.cpp (line 274)


`s/validateParsedResource/validateCommandLineResource/`?



src/common/resources.cpp (line 277)


Remove newline



src/common/resources.cpp (lines 280 - 281)


Might be better to print out the `stringify(resource)` rather than just the 
`resource.name()`?



src/common/resources.cpp (line 316)


The `nameTypes` parameter of this function is a in/out parameter. This 
pattern is generally discouraged, since reasoning about the state injected into 
this function as well as the non-local side-effects made within this function 
is difficult to reason about.

While it may be slightly less performant, I think we can make 
`validateParsedResource` be `validateParsedResources` and encapsulate this test 
within that function. What do you think?



src/common/resources.cpp (line 377)


I think you need to check for `resource.role() == "*"` instead, since 
`role` has `[default = "*"]`. Please verify.



src/common/resources.cpp (line 483)


Remove newline.


- Michael Park


On Oct. 14, 2015, 3:50 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39018/
> ---
> 
> (Updated Oct. 14, 2015, 3:50 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Jie Yu, and Michael 
> Park.
> 
> 
> Bugs: MESOS-2467
> https://issues.apache.org/jira/browse/MESOS-2467
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This includes code changes necessary for JSON parsing of Resources. 
> Documentation changes will be posted soon in another review 
> (https://reviews.apache.org/r/39102/).
> 
> 
> Diffs
> -
> 
>   include/mesos/resources.hpp 6c3a065945eb56dc88df9c977e5ca11d4cbcbf61 
>   include/mesos/v1/resources.hpp fe8925ac851b74d1b37919f00afc7ed816f47ea5 
>   src/common/resources.cpp 601388c35a1bff37c58e753d1870d53b8d0af2d1 
>   src/tests/resources_tests.cpp 6584fc6c39e6ffe9f8085576677dcc669f127697 
>   src/v1/resources.cpp dc868903472f8f3a1ddc56092e3f8f81d953ce39 
> 
> Diff: https://reviews.apache.org/r/39018/diff/
> 
> 
> Testing
> ---
> 
> New code was added to `ResourcesTest.ParsingFromJSON`, and two new tests were 
> added: `ResourcesTest.ParsingFromJSONWithRoles` and 
> `ResourcesTest.ParsingFromJSONError`. These attempt to test common error 
> scenarios that might show up in user-supplied JSON.
> 
> `make check` was used to confirm that all tests pass, with one notable 
> failure (ResourcesTest.ParsingFromJSONError) related to a picojson issue 
> tracked here: https://issues.apache.org/jira/browse/MESOS-3698
> 
> The original resources parsing format is used throughout many other tests 
> (check `src/tests/sorter_tests.cpp`, for example), so it's clear that the 
> original parsing continues to work correctly.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 39388: Explicitly set the `LIBPROCESS_IP` env variable for docker containers.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [39388]

All tests passed.

- Mesos ReviewBot


On Oct. 17, 2015, 11:18 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39388/
> ---
> 
> (Updated Oct. 17, 2015, 11:18 p.m.)
> 
> 
> Review request for mesos and Niklas Nielsen.
> 
> 
> Bugs: MESOS-3740
> https://issues.apache.org/jira/browse/MESOS-3740
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp 56d63dc75637c9f89a239af371f476a85a570696 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 4bb65afd0ee61cafef68e064a697fdce65d60058 
> 
> Diff: https://reviews.apache.org/r/39388/diff/
> 
> 
> Testing
> ---
> 
> Added `DockerContainerizerTest.ROOT_DOCKER_LaunchWithLibprocessIP` test which 
> fails without the changes made to `src/docker/docker.cpp`.
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 38705: Added support for applying a review chain (apply-reviews.py).

[Artem Harutyunyan]

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

(Updated Oct. 17, 2015, 7:37 p.m.)


Review request for mesos, Benjamin Hindman, Joris Van Remoortere, Joseph Wu, 
and Vinod Kone.


Changes
---

Bugfix and cleanup.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  support/apply-reviews.py PRE-CREATION 

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


Testing
---

Tested the script with python 2.7.


Thanks,

Artem Harutyunyan

Re: Review Request 38883: Removed calls to apply-review.sh script. Added support for amending commit messages.

[Artem Harutyunyan]

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

(Updated Oct. 17, 2015, 7:39 p.m.)


Review request for mesos, Benjamin Hindman, Joris Van Remoortere, Joseph Wu, 
and Vinod Kone.


Changes
---

Cleanup.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  support/apply-reviews.py PRE-CREATION 

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


Testing
---

Tested the script with python 2.7.


Thanks,

Artem Harutyunyan

Re: Review Request 39410: Added support for github to apply-reviews.py.

[Artem Harutyunyan]

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

(Updated Oct. 17, 2015, 7:40 p.m.)


Review request for mesos, Adam B, Joris Van Remoortere, Joseph Wu, Marco 
Massenzio, and Vinod Kone.


Changes
---

Bugfix and Cleanup.


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


Repository: mesos


Description
---

Added support for github to apply-reviews.py.


Diffs (updated)
-

  support/apply-reviews.py PRE-CREATION 

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


Testing
---

Tested with python 2.7


Thanks,

Artem Harutyunyan

Review Request 39420: Added '--parent' option and made apply-review.sh call apply-reviews.py.

[Artem Harutyunyan]

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

Review request for mesos, Joris Van Remoortere, Joseph Wu, Marco Massenzio, and 
Vinod Kone.


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


Repository: mesos


Description
---

Added '--parent' option and made apply-review.sh call apply-reviews.py.


Diffs
-

  support/apply-review.sh 6391451542e9e8847ec38e2ad9d9acf552afead3 
  support/apply-reviews.py PRE-CREATION 

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


Testing
---

Tested with python 2.7.

- with and without '-p'.
- Tested reviews with and without parents.


Thanks,

Artem Harutyunyan

Re: Review Request 39417: Add --egress_flow_classifier_parent flag

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [39415, 39416, 39417]

All tests passed.

- Mesos ReviewBot


On Oct. 18, 2015, 12:29 a.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39417/
> ---
> 
> (Updated Oct. 18, 2015, 12:29 a.m.)
> 
> 
> Review request for mesos, Ian Downes and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When --egress_unique_flow_per_container is enabled, we need to install a flow 
> classifier (fq_codel) qdisc on egress side. This flag specifies where to 
> install it in the hierarchy. By default, we install it at root. But, for 
> example, if you have already installed HTB qdisc at root, you may want this 
> to be installed in other place than root, specify an HTB class ID as its 
> parent here.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md 9443d5fc2de0f46f9e117b08e29b09ff3a4579c6 
>   src/slave/containerizer/isolators/network/port_mapping.cpp 
> e6bb75e6f5ba48a0c4cf6dd8f353e5f5843d0eef 
>   src/slave/flags.hpp 3e93b52a5874f52361d5a9c685499a7032014a73 
>   src/slave/flags.cpp 1bf394ea62fde29caa6705cd5d156eae452adbf2 
> 
> Diff: https://reviews.apache.org/r/39417/diff/
> 
> 
> Testing
> ---
> 
> Manual tests, with and without a pre-installed HTB qdisc and classes.
> 
> 
> Thanks,
> 
> Cong Wang
> 
>

Re: Review Request 39420: Added '--parent' option and made apply-review.sh call apply-reviews.py.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [38705, 38883, 39410, 39420]

All tests passed.

- Mesos ReviewBot


On Oct. 18, 2015, 2:43 a.m., Artem Harutyunyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39420/
> ---
> 
> (Updated Oct. 18, 2015, 2:43 a.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere, Joseph Wu, Marco Massenzio, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-3468
> https://issues.apache.org/jira/browse/MESOS-3468
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added '--parent' option and made apply-review.sh call apply-reviews.py.
> 
> 
> Diffs
> -
> 
>   support/apply-review.sh 6391451542e9e8847ec38e2ad9d9acf552afead3 
>   support/apply-reviews.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/39420/diff/
> 
> 
> Testing
> ---
> 
> Tested with python 2.7.
> 
> - with and without '-p'.
> - Tested reviews with and without parents.
> 
> 
> Thanks,
> 
> Artem Harutyunyan
> 
>

Re: Review Request 39388: Explicitly set the `LIBPROCESS_IP` env variable for docker containers.

[haosdent huang]

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



src/docker/docker.cpp (line 430)


Seems we already have this env in 
https://github.com/apache/mesos/blob/master/src/slave/containerizer/containerizer.cpp#L254


- haosdent huang


On Oct. 17, 2015, 11:18 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39388/
> ---
> 
> (Updated Oct. 17, 2015, 11:18 p.m.)
> 
> 
> Review request for mesos and Niklas Nielsen.
> 
> 
> Bugs: MESOS-3740
> https://issues.apache.org/jira/browse/MESOS-3740
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp 56d63dc75637c9f89a239af371f476a85a570696 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 4bb65afd0ee61cafef68e064a697fdce65d60058 
> 
> Diff: https://reviews.apache.org/r/39388/diff/
> 
> 
> Testing
> ---
> 
> Added `DockerContainerizerTest.ROOT_DOCKER_LaunchWithLibprocessIP` test which 
> fails without the changes made to `src/docker/docker.cpp`.
> 
> 
> Thanks,
> 
> Michael Park
> 
>