Re: Review Request 53715: Define docker `--entrypoint` for `Windows`.

[Daniel Pravat]

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

(Updated Nov. 23, 2016, 6:59 a.m.)


Review request for mesos, Alex Naparu, Artem Harutyunyan, Alex Clemmer, Joseph 
Wu, and Michael Park.


Repository: mesos


Description
---

Define docker `--entrypoint` for `Windows`.


Diffs
-

  3rdparty/stout/include/stout/os/posix/shell.hpp 
70a9184e85ba322f1cd4ce5d12b963cd4b3ad500 
  3rdparty/stout/include/stout/os/windows/shell.hpp 
17e3d564564abebf1d558b7a7a277aef3c87e5ae 

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


Testing
---


Thanks,

Daniel Pravat

Re: Review Request 52190: Removed deprecated compiler warnings.

[Daniel Pravat]

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

(Updated Nov. 23, 2016, 6:56 a.m.)


Review request for mesos, Alex Naparu, Artem Harutyunyan, Alex Clemmer, Joseph 
Wu, and Michael Park.


Repository: mesos


Description
---

Removed deprecated compiler warnings.


Diffs
-

  cmake/CompilationConfigure.cmake 11a8507eb773391073a7b945e2aac503262f86b7 

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


Testing
---


Thanks,

Daniel Pravat

Re: Review Request 53949: Added test helper to obtain unused port.

[Benjamin Bannier]

> On Nov. 23, 2016, 3:20 a.m., haosdent huang wrote:
> > src/tests/utils.cpp, line 82
> > 
> >
> > Should we move the comment
> > 
> > ```
> > // Bind to port 0 to obtain a random unused port.
> > ```
> > at here?

Was thinking about this as well, but went with the current form as the comment 
really applies to the whole approach this helper is taking internally.


- Benjamin


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


On Nov. 22, 2016, 4:49 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53949/
> ---
> 
> (Updated Nov. 22, 2016, 4:49 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and haosdent huang.
> 
> 
> Bugs: MESOS-6618
> https://issues.apache.org/jira/browse/MESOS-6618
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test helper to obtain unused port.
> 
> 
> Diffs
> -
> 
>   src/tests/utils.hpp 140ebaaae43b03568ec49891635f0660cdfb4c85 
>   src/tests/utils.cpp eb36616f68d81d33d4bd04a7f23295e8c7558fc8 
> 
> Diff: https://reviews.apache.org/r/53949/diff/
> 
> 
> Testing
> ---
> 
> Tested as part of https://reviews.apache.org/r/53950/.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 54021: Updated a Docker containerizer test.

[Adam B]

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


Fix it, then Ship it!




Looks good, just fix the comment.


src/tests/containerizer/docker_containerizer_tests.cpp (line 552)


Not quite a sentence. Perhaps "Verify the ContainerStatus fields in the 
TaskStatus."


- Adam B


On Nov. 22, 2016, 10:06 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54021/
> ---
> 
> (Updated Nov. 22, 2016, 10:06 p.m.)
> 
> 
> Review request for mesos, Adam B, Kevin Klues, and Vinod Kone.
> 
> 
> Bugs: MESOS-6625
> https://issues.apache.org/jira/browse/MESOS-6625
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The test is updated to check for ContainerID in ContainerStatus.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> c478d56ffd734b26fd2dbd7bb1ca02ce929f3f16 
> 
> Diff: https://reviews.apache.org/r/54021/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 52972: Replaced POSIX `int` with `int_fd` abstraction in `stout` folder.

[Daniel Pravat]

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

(Updated Nov. 23, 2016, 6:31 a.m.)


Review request for mesos, Alex Naparu, Artem Harutyunyan, Alex Clemmer, Joseph 
Wu, and Michael Park.


Repository: mesos


Description
---

Replaced POSIX `int` with `int_fd` abstraction in `stout` folder.


Diffs (updated)
-

  3rdparty/stout/include/stout/net.hpp 083b8d1959dcd305d0aa8892082b39cb03786ebf 
  3rdparty/stout/include/stout/os/mktemp.hpp 
2dfd35605eb4202a5475fe0e0d2f1fd27690a2de 
  3rdparty/stout/include/stout/os/open.hpp 
2a357926860b1523c51f12c7edee2babe6afbfa3 
  3rdparty/stout/include/stout/os/read.hpp 
d0b657db5b7dc0c1e11edc13fd6830a9f6273867 
  3rdparty/stout/include/stout/os/socket.hpp 
e9d9306e63aff2be083b4254fbf6f31c37edc424 
  3rdparty/stout/include/stout/os/touch.hpp 
84d49bb8adc2612e380f037fd42c47166d55f593 
  3rdparty/stout/include/stout/os/windows/close.hpp 
9f1566bff19ee872418bce8a43a119c4f0a70a7c 
  3rdparty/stout/include/stout/os/windows/fcntl.hpp 
2bc794a405e59d80c1e8308c0049d2306347adfa 
  3rdparty/stout/include/stout/os/windows/ftruncate.hpp 
a7f53ad2e8735b515590af84c0efce3edcc1bebf 
  3rdparty/stout/include/stout/os/windows/read.hpp 
09190f97f33db5dfa023f937f8f2a4a62ed1ec15 
  3rdparty/stout/include/stout/os/windows/sendfile.hpp 
d6358cc02c1eea9298907da1f74eb7eeaeec7d21 
  3rdparty/stout/include/stout/os/windows/shell.hpp 
17e3d564564abebf1d558b7a7a277aef3c87e5ae 
  3rdparty/stout/include/stout/os/windows/socket.hpp 
1f6551bc30cf31842513df0fed43ee134c620ebd 
  3rdparty/stout/include/stout/os/windows/write.hpp 
23488708ae366b8571bb8b4805f67d2054223fff 
  3rdparty/stout/include/stout/os/write.hpp 
24a69d8f60efd3c2888d464d75164c758b3701a2 
  3rdparty/stout/include/stout/posix/os.hpp 
c37e64db662ba3cee83d2f55de0f9d71ad72c038 
  3rdparty/stout/include/stout/protobuf.hpp 
80cb20f40a7ddd4309d27973eef9fca9e4052b64 
  3rdparty/stout/include/stout/windows/os.hpp 
de9b04ad82443038a0f4408bc72cae1540a1beaf 
  3rdparty/stout/tests/os/filesystem_tests.cpp 
22460842c0db5dc5b6effbc2bdfce043ed47db6d 
  3rdparty/stout/tests/os_tests.cpp ad23ec00de7770a5024c084627b0ff1e4cc2a439 

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


Testing
---


Thanks,

Daniel Pravat

Re: Review Request 52625: Replaced POSIX `int` with `int_fd` abstraction in `libprocess` folder.

[Daniel Pravat]

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

(Updated Nov. 23, 2016, 6:31 a.m.)


Review request for mesos, Alex Naparu, Alex Clemmer, Joseph Wu, and Michael 
Park.


Repository: mesos


Description
---

On POSIX this should have no effect.


Diffs (updated)
-

  3rdparty/libprocess/include/process/io.hpp 
eec5efd7e6b71a783f2bb40826054d0488cee71f 
  3rdparty/libprocess/include/process/network.hpp 
52110667185370a4c92e2fa524819ab1f34bdec9 
  3rdparty/libprocess/include/process/socket.hpp 
f798af7879546d71e8ef4a295c9cf489a70cb61f 
  3rdparty/libprocess/include/process/subprocess_base.hpp 
1d02454d5541f96cb4928bf027fcae3764989d67 
  3rdparty/libprocess/include/process/windows/subprocess.hpp 
f452f6743d01f0b99010fa5e5bcbaae1358c8241 
  3rdparty/libprocess/src/io.cpp d930ceebc90468e082b984e41385549f906dc6ae 
  3rdparty/libprocess/src/libevent_poll.cpp 
0803ba33622c86df38b3efd4f1e3197edf93a0af 
  3rdparty/libprocess/src/poll_socket.hpp 
d04f3f2d1bcf70464ac659b29f96574bbd233414 
  3rdparty/libprocess/src/poll_socket.cpp 
eb7b48713edd30b545d7be95b5d51b0f71bd422a 
  3rdparty/libprocess/src/process.cpp b5ee0b3861cbb035198796c9a7a68d43922652c1 
  3rdparty/libprocess/src/socket.cpp 7f93168e1572f8669f67a4c5e6e5467259b7a407 
  3rdparty/libprocess/src/subprocess_windows.cpp 
20cad52d4a4d7fc51487e150a849972eb19ed08e 
  3rdparty/libprocess/src/tests/subprocess_tests.cpp 
59c17692012ddfb540ecdd48560c73c42a15f061 

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


Testing
---


Thanks,

Daniel Pravat

Re: Review Request 53706: Implemented `os::user' on Windows.

[Daniel Pravat]

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

(Updated Nov. 23, 2016, 6:30 a.m.)


Review request for mesos, Alex Naparu, Artem Harutyunyan, Alex Clemmer, Joseph 
Wu, and Michael Park.


Repository: mesos


Description
---

Implemented `os::user' on Windows.


Diffs (updated)
-

  3rdparty/stout/include/stout/os/windows/su.hpp 
1bb70964adbb80aa6502fbfe69de2c34dc74e655 

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


Testing
---


Thanks,

Daniel Pravat

Review Request 54021: Updated a Docker containerizer test.

[Jie Yu]

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

Review request for mesos, Adam B, Kevin Klues, and Vinod Kone.


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


Repository: mesos


Description
---

The test is updated to check for ContainerID in ContainerStatus.


Diffs
-

  src/tests/containerizer/docker_containerizer_tests.cpp 
c478d56ffd734b26fd2dbd7bb1ca02ce929f3f16 

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


Testing
---

sudo make check


Thanks,

Jie Yu

Re: Review Request 54001: Addes status method to DockerContainerizer.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [54001]

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 Nov. 22, 2016, 9:51 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54001/
> ---
> 
> (Updated Nov. 22, 2016, 9:51 p.m.)
> 
> 
> Review request for mesos, Kevin Klues and Vinod Kone.
> 
> 
> Bugs: MESOS-6625
> https://issues.apache.org/jira/browse/MESOS-6625
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch added the implementation of the 'status' method to
> DockerContainerizer. Previously, this method is missing. Currently,
> the only status the DockerContainerizer reports is the ContainerID.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.hpp 006f929eca0e0a6b1de941821ac72869ba393d2d 
>   src/slave/containerizer/docker.cpp c2ed5240aab9ea9d1a386c44c94e5ae7e98d313c 
> 
> Diff: https://reviews.apache.org/r/54001/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 54014: Fixed missing protobuf java package definition.

[Vijay Srinivasaraghavan]

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

(Updated Nov. 23, 2016, 5:46 a.m.)


Review request for mesos and Anand Mazumdar.


Changes
---

Updated Depends On to make it a linear review chain.


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


Repository: mesos


Description
---

Fixed missing protobuf java package definition.


Diffs
-

  include/mesos/v1/allocator/allocator.proto 
73d45b37a7afc47366a4a01a36912f30b47c30b1 

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


Testing
---

Ran "make check"


Thanks,

Vijay Srinivasaraghavan

Re: Review Request 53825: MESOS-6597 Enabled java protos generation for all V1 proto files.

[Vijay Srinivasaraghavan]

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

(Updated Nov. 23, 2016, 5:46 a.m.)


Review request for mesos, Anand Mazumdar and Zameer Manji.


Changes
---

Updated Depends On to make it a linear review chain.


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


Repository: mesos


Description
---

MESOS-6597 Enabled java protos generation for all V1 proto files.


Diffs
-

  src/Makefile.am 5e0b8406f7f624bd8b03ff76b887f20e22fc66e0 

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


Testing
---

Built mesos and confirmed jar file containing new java PB wrappers for V1 API. 
Ran standard unit test (make tests)


Thanks,

Vijay Srinivasaraghavan

Review Request 54019: Modernized code to use `foreachpair` with LinkedHashMap.

[Neil Conway]

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

Review request for mesos and Michael Park.


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


Repository: mesos


Description
---

Various places were doing `foreach` over the output of
`LinkedHashMap::keys()` or `LinkedHashMap::values()`; this can be
replaced with `foreachkey` and `foreachvalue`, respectively.


Diffs
-

  src/examples/long_lived_executor.cpp 5c43ee780997c105e4f3dd6081d4c5418bef7ab6 
  src/examples/test_http_executor.cpp b9b6bfdc55dead5b70d40b4389e65a2194babc0e 
  src/exec/exec.cpp 1dc20390907253a466b7272b7f8b33ea14afb236 
  src/hook/manager.cpp 24885226a788a7abd851e12b527f74fa972ec935 
  src/launcher/default_executor.cpp f4e1ea4816fee9a2f6d59bb534933b66013f11cf 
  src/launcher/executor.cpp ce0b199551447504bb95743df4ce9ec4a0443cd4 
  src/master/master.cpp e03a2e8025943825a2902102c43dc0eb66bacb6a 
  src/slave/http.cpp aa9f492fc74283b5a6f24a8b7189f1ecbe41488f 
  src/slave/slave.cpp 521f08d59cd78f9089d58cd3294f0ee4a099cd7f 

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


Testing
---

`make check`


Thanks,

Neil Conway

Review Request 54018: Enhanced LinkedHashMap to support `foreachpair` and friends.

[Neil Conway]

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

Review request for mesos and Michael Park.


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


Repository: mesos


Description
---

This requires providing `begin` and `end` member functions that return
iterators in insertion order. With the new implementation of
LinkedHashMap, this is easy to do.


Diffs
-

  3rdparty/stout/include/stout/linkedhashmap.hpp 
f48cc5933f9a786be325f71c83151376d1ffe844 
  3rdparty/stout/tests/linkedhashmap_tests.cpp 
376462d1b472586f1547512e6e0a1646c4f76a44 

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


Testing
---

`make check`


Thanks,

Neil Conway

Review Request 54017: Changed implementation of LinkedHashMap.

[Neil Conway]

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

Review request for mesos and Michael Park.


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


Repository: mesos


Description
---

LinkedHashMap used two containers: a std::list and a
hashmap>. That approach
worked fine, but it made it difficult to support iterating over the
key-value pairs in the map without jumping through hoops or copying.

Instead, this commit uses a std::list> and a
hashmap>::iterator>. This makes it
easy to support iterating over the entries in the map in insertion
order: we can just iterate over the list.

This commit just changes the implementation of LinkedHashMap; support
for iteration will be added in a subsequent commit.


Diffs
-

  3rdparty/stout/include/stout/linkedhashmap.hpp 
f48cc5933f9a786be325f71c83151376d1ffe844 
  3rdparty/stout/tests/linkedhashmap_tests.cpp 
376462d1b472586f1547512e6e0a1646c4f76a44 

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


Testing
---

`make check`


Thanks,

Neil Conway

Review Request 54016: Fixed a few places to use `foreachkey` / `foreachvalue`.

[Neil Conway]

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

Review request for mesos and Michael Park.


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


Repository: mesos


Description
---

This is preferrable to doing `foreach` over `hashmap::keys()` or
`hashmap::values()`, respectively.


Diffs
-

  src/launcher/default_executor.cpp f4e1ea4816fee9a2f6d59bb534933b66013f11cf 
  src/slave/containerizer/fetcher.cpp d200c117579bc1c2d9d24f14bf4da8f650d3f562 

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


Testing
---

`make check`


Thanks,

Neil Conway

Re: Review Request 53825: MESOS-6597 Enabled java protos generation for all V1 proto files.

[Vijay Srinivasaraghavan]

> On Nov. 20, 2016, 9:04 p.m., Anand Mazumdar wrote:
> > Nice first patch and welcome to the community!
> > 
> > - Can you update the Testing Done section with details on testing?
> > - We also support Python bindings. Do you mind adding these protos to our 
> > python build too in a follow up review? (See my comments later on how you 
> > can use review dependencies if you are new to ReviewBoard)
> 
> Vijay Srinivasaraghavan wrote:
> Thanks Anand. I have made the python binding changes locally on my 
> machine. Do you want me to create a new JIRA bug for this issue and create a 
> new review or add the patch to this JIRA/review itself? I am little confused 
> as to how the dependencies are handled with RB.
> 
> Anand Mazumdar wrote:
> No need to create a separate issue. It would be fine to create a new 
> review with it only containing the python related changes. This would be 
> another commit based on top of this one. You can then use `post-reviews.py` 
> and it should handle setting the dependencies for you. Feel free to ping me 
> on IRC/Slack if you run into any issues (anandm)

Thanks for the clarification. I have attached a new patch with "53825" as 
dependendency


> On Nov. 20, 2016, 9:04 p.m., Anand Mazumdar wrote:
> > include/mesos/v1/allocator/allocator.proto, lines 21-22
> > 
> >
> > These changes seem unrelated to this change i.e., java protos 
> > generation. We prefer single logical atomic commits in Mesos. For more info 
> > see: http://mesos.apache.org/documentation/latest/submitting-a-patch/
> > 
> > Can you create a separate patch for this and make this review dependent 
> > on it? The `post-reviews.py` script would do it automatically for you.
> 
> Vijay Srinivasaraghavan wrote:
> Actually, this fix is also part of the same issue. The proto file was 
> missing specific java package definition and hence the java file gets 
> generated in a package location different from the other java files.
> 
> Anand Mazumdar wrote:
> hmm, are they the same? They are two separate issues:
> 
> 1. The proto file was missing specific java package definition. This 
> would impact any user trying to use the proto file and is not related to 
> including the generated files in the Mesos JAR.
> 2. Include the generated files in the Mesos JAR.
> 
> This review (from its summary/description) caters to _only_ build changes 
> in the MakeFile itself and not protobuf related changes.

I see your point. I have attached new patch/review marking "53825" as dependent.


- Vijay


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


On Nov. 23, 2016, 4:31 a.m., Vijay Srinivasaraghavan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53825/
> ---
> 
> (Updated Nov. 23, 2016, 4:31 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Zameer Manji.
> 
> 
> Bugs: MESOS-6597
> https://issues.apache.org/jira/browse/MESOS-6597
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> MESOS-6597 Enabled java protos generation for all V1 proto files.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 5e0b8406f7f624bd8b03ff76b887f20e22fc66e0 
> 
> Diff: https://reviews.apache.org/r/53825/diff/
> 
> 
> Testing
> ---
> 
> Built mesos and confirmed jar file containing new java PB wrappers for V1 
> API. Ran standard unit test (make tests)
> 
> 
> Thanks,
> 
> Vijay Srinivasaraghavan
> 
>

Review Request 54014: Fixed missing protobuf java package definition.

[Vijay Srinivasaraghavan]

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

Review request for mesos and Anand Mazumdar.


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


Repository: mesos


Description
---

Fixed missing protobuf java package definition.


Diffs
-

  include/mesos/v1/allocator/allocator.proto 
73d45b37a7afc47366a4a01a36912f30b47c30b1 

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


Testing
---

Ran "make check"


Thanks,

Vijay Srinivasaraghavan

Review Request 54015: Added V1 API protos to python bindings.

[Vijay Srinivasaraghavan]

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

Review request for mesos and Anand Mazumdar.


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


Repository: mesos


Description
---

Added V1 API protos to python bindings.


Diffs
-

  src/Makefile.am 5e0b8406f7f624bd8b03ff76b887f20e22fc66e0 

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


Testing
---

Ran "make check"


Thanks,

Vijay Srinivasaraghavan

Re: Review Request 53610: Added health checks documentation.

[haosdent huang]

> On Nov. 9, 2016, 5:32 p.m., haosdent huang wrote:
> > docs/health-checks.md, line 29
> > 
> >
> > s/functionality/functionalities/g
> 
> Alexander Rukletsov wrote:
> I believe it is fine to use singular here. That's what my dictionary says:
> "the range of operations that can be run on a computer or other 
> electronic system: new software with additional functionality."

Got it! Thanks you :)


> On Nov. 9, 2016, 5:32 p.m., haosdent huang wrote:
> > docs/health-checks.md, line 53
> > 
> >
> > Should remove `---` in this line?
> 
> Alexander Rukletsov wrote:
> It is supposed to be rendered as em dash, for example, look at failover 
> section in Quota doc: https://mesos.apache.org/documentation/latest/quota/

Cool


- haosdent


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


On Nov. 20, 2016, 6:52 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53610/
> ---
> 
> (Updated Nov. 20, 2016, 6:52 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman, haosdent huang, Neil Conway, and 
> Till Toenshoff.
> 
> 
> Bugs: MESOS-5597
> https://issues.apache.org/jira/browse/MESOS-5597
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   docs/health-checks.md PRE-CREATION 
>   docs/home.md a5811480de050352dca6c0f7e4e64d3d2351c2d5 
> 
> Diff: https://reviews.apache.org/r/53610/diff/
> 
> 
> Testing
> ---
> 
> https://gist.github.com/rukletsov/7200c36b2fd1e81f78f2583e68b31fd1
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 53825: MESOS-6597 Enabled java protos generation for all V1 proto files.

[Vijay Srinivasaraghavan]

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

(Updated Nov. 23, 2016, 4:31 a.m.)


Review request for mesos, Anand Mazumdar and Zameer Manji.


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


Repository: mesos


Description
---

MESOS-6597 Enabled java protos generation for all V1 proto files.


Diffs (updated)
-

  src/Makefile.am 5e0b8406f7f624bd8b03ff76b887f20e22fc66e0 

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


Testing
---

Built mesos and confirmed jar file containing new java PB wrappers for V1 API. 
Ran standard unit test (make tests)


Thanks,

Vijay Srinivasaraghavan

Review Request 54013: Added user doc for nested container and task group.

[Gilbert Song]

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

Review request for mesos, Benjamin Mahler, Artem Harutyunyan, Jie Yu, and Vinod 
Kone.


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


Repository: mesos


Description
---

Added user doc for nested container and task group.


Diffs
-

  docs/nested-container-and-task-group.md PRE-CREATION 

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


Testing
---

Tested by gist view. Here is the link:

https://github.com/Gilbert88/mesos/blob/doc_pod/docs/nested-container-and-task-group.md


Thanks,

Gilbert Song

Re: Review Request 54007: Printed complete health check configuration on task launch.

[haosdent huang]

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




src/health-check/health_checker.cpp (line 213)


No sure if `check.SerializeAsString()` would be better or not. Or we could 
add a `stringify()` for protobuf message?


- haosdent huang


On Nov. 23, 2016, 1:11 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54007/
> ---
> 
> (Updated Nov. 23, 2016, 1:11 a.m.)
> 
> 
> Review request for mesos, Gastón Kleiman and haosdent huang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/health-check/health_checker.cpp 
> af5500be249c74a4a5e64bf38dea607173e2f998 
> 
> Diff: https://reviews.apache.org/r/54007/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 53974: Added support to handle ATTACH_CONTAINER_OUPUT in the io switchbaord.

[Mesos ReviewBot]

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



Bad patch!

Reviews applied: [53974, 53939, 53837, 53938, 53936, 53704]

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

Error:
2016-11-23 04:00:23 URL:https://reviews.apache.org/r/53704/diff/raw/ 
[25354/25354] -> "53704.patch" [1]
error: patch failed: src/tests/containerizer/docker_volume_isolator_tests.cpp:29
error: src/tests/containerizer/docker_volume_isolator_tests.cpp: patch does not 
apply
error: patch failed: src/tests/containerizer/mesos_containerizer_tests.cpp:43
error: src/tests/containerizer/mesos_containerizer_tests.cpp: patch does not 
apply

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

- Mesos ReviewBot


On Nov. 22, 2016, 11:07 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53974/
> ---
> 
> (Updated Nov. 22, 2016, 11:07 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-6467
> https://issues.apache.org/jira/browse/MESOS-6467
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added support to handle ATTACH_CONTAINER_OUPUT in the io switchbaord.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/io_switchboard/main.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/53974/diff/
> 
> 
> Testing
> ---
> 
> GTEST_FILTER="" make -j check
> sudo src/mesos-tests
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 54004: Renamed functions in HealthChecker for clarity.

[haosdent huang]

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


Ship it!




Ship It!

- haosdent huang


On Nov. 23, 2016, 12:29 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54004/
> ---
> 
> (Updated Nov. 23, 2016, 12:29 a.m.)
> 
> 
> Review request for mesos, Gastón Kleiman and haosdent huang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Use descriptive function names instead of underscore
> prefixes for functions in HealthChecker.
> 
> 
> Diffs
> -
> 
>   src/health-check/health_checker.hpp 
> 837d1358e418d21536da488e4a23cbfa41db6060 
>   src/health-check/health_checker.cpp 
> af5500be249c74a4a5e64bf38dea607173e2f998 
> 
> Diff: https://reviews.apache.org/r/54004/diff/
> 
> 
> Testing
> ---
> 
> See https://reviews.apache.org/r/52873/.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 53877: Added temporary file environment passing towards docker.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [53877]

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 Nov. 22, 2016, 10:01 p.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53877/
> ---
> 
> (Updated Nov. 22, 2016, 10:01 p.m.)
> 
> 
> Review request for mesos, Adam B, Gastón Kleiman, Jie Yu, Kapil Arya, and 
> Joseph Wu.
> 
> 
> Bugs: MESOS-6566
> https://issues.apache.org/jira/browse/MESOS-6566
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> see summary.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp f03ea7fa55e976e44282503261ac50e1502592a2 
> 
> Diff: https://reviews.apache.org/r/53877/diff/
> 
> 
> Testing
> ---
> 
> ```
> $ make check
> ```
> 
> ```
> $ ./src/mesos-execute --command="sleep 200" --containerizer=docker 
> --docker_image=alpine --env='{"foo": "bar"}' --master=149.202.202.185:5050
> ```
> 
> ```
> $ ps aux
> root 27645  0.2  0.0 372852 30180 ?Sl   15:50   0:00 docker -H 
> unix:///var/run/docker.sock run --cpu-shares 1024 --memory 134217728 
> --env-file /tmp/l53ILz/ktpuDS -v 
> /tmp/mesos/slaves/91b671fd-3c83-425e-96c2-26ecdc410028-S0/frameworks/91b671fd-3c83-425e-96c2-26ecdc410028-0001/executors/test/runs/a75fc411-3d18-44f1-a562-9f759c281da0:/mnt/mesos/sandbox
>  --net host --entrypoint /bin/sh --name 
> mesos-91b671fd-3c83-425e-96c2-26ecdc410028-S0.a75fc411-3d18-44f1-a562-9f759c281da0
>  alpine -c sleep 200
> ```
> 
> ```
> $ more /tmp/l53ILz/ktpuDS
> foo=bar
> MESOS_SANDBOX=/mnt/mesos/sandbox
> MESOS_CONTAINER_NAME=mesos-91b671fd-3c83-425e-96c2-26ecdc410028-S0.a75fc411-3d18-44f1-a562-9f759c281da0
> ```
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 53938: Added helper to get the io switchboard server address.

[Kevin Klues]

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

(Updated Nov. 23, 2016, 2:45 a.m.)


Review request for mesos, Benjamin Hindman, Jie Yu, and Vinod Kone.


Changes
---

Rebased with changes to previous commits.


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


Repository: mesos


Description
---

For now, the io switchboard server will always hosted on a unix domain
socket. The path limit for a unix domain socket is 100 bytes, so we
can't just use a file in our runtime directory as the socket file we
connect to. Instead, we store the path to the socket file in our
runtime directory and read it from there.


Diffs (updated)
-

  src/slave/containerizer/mesos/utils.hpp 
a54106dc4893bb222f42ede936ac9029e817faf9 
  src/slave/containerizer/mesos/utils.cpp 
4e2a01495909c07f320af416ff4dc59f7328c710 

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


Testing
---

make


Thanks,

Kevin Klues

Re: Review Request 53837: Added a per container io switchboard server process.

[Kevin Klues]

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

(Updated Nov. 23, 2016, 2:45 a.m.)


Review request for mesos, Benjamin Hindman, Jie Yu, and Vinod Kone.


Changes
---

Rebased with changes to previous commits.


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


Repository: mesos


Description
---

Currently, this process simply intercepts the stdout/stderr of a
container and writes it to the stdout/stderr FDs set up by the
container logger. We also send the stdout/stderr data to a simple HTTP
server that we launch on a unix domain socket set up by the agent.
Right now this server is just a stub and doesn't do anything useful.

In future commits, we will expand this HTTP server to handle
'ATTACH_CONTAINER_INPUT' and 'ATTACH_CONTAINER_OUTPUT' calls on behalf
of a container. It will use the stdout/stderr messages passed to it to
and send that data over the response stream to any clients connected
with an 'ATTACH_CONTAINER_OUTPUT' call. Likewise, it will take any
input streamed in over a 'ATTACH_CONTAINER_INPUT' request and write it
to a container's stdin.

We don't currently handle recovering access to the io switchboard
server process after agent restarts. We will add that in a subsequent
commit.


Diffs (updated)
-

  src/Makefile.am 5e0b8406f7f624bd8b03ff76b887f20e22fc66e0 
  src/slave/containerizer/mesos/containerizer.cpp 
e47a120bfbb607cc0cdbdaed934ae15f15666ae3 
  src/slave/containerizer/mesos/io/switchboard.hpp PRE-CREATION 
  src/slave/containerizer/mesos/io/switchboard.cpp PRE-CREATION 
  src/slave/containerizer/mesos/io/switchboard_main.hpp PRE-CREATION 
  src/slave/containerizer/mesos/io/switchboard_main.cpp PRE-CREATION 

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


Testing
---

GTEST_FILTER="" make -j check
sudo src/mesos-tests


Thanks,

Kevin Klues

Re: Review Request 53704: Added a level of indirection for logger through an IO Switchboard.

[Kevin Klues]

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

(Updated Nov. 23, 2016, 2:40 a.m.)


Review request for mesos, Benjamin Hindman, Jie Yu, and Vinod Kone.


Changes
---

Addressed all of Jie's comments.


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


Repository: mesos


Description
---

The purpose of this component is to feed stdin to a container from an
external source, as well as redirect the stdin/stdout of a container
to multiple targets.

In this commit, we simply add the IOSwitchboard as a component that
interposes on the fds set up to communicate between the logger and a
container.

In the future, we will expand this component to (optionaly) launch a
sidecar HTTP server process which will be responsible for handling
'ATTACH_CONTAINER_INPUT' and 'ATTACH_CONTAINER_OUTPUT' calls on behalf
of a container to redirect the stdin/stdout/sderr of a container to
external clients.


Diffs (updated)
-

  src/CMakeLists.txt aef9ae6d2872dc15725c01ce85b657965485605f 
  src/Makefile.am 5e0b8406f7f624bd8b03ff76b887f20e22fc66e0 
  src/slave/containerizer/mesos/containerizer.hpp 
c8d43f918981d06a73662cf1be61081915816ca5 
  src/slave/containerizer/mesos/containerizer.cpp 
e47a120bfbb607cc0cdbdaed934ae15f15666ae3 
  src/slave/containerizer/mesos/io/switchboard.hpp PRE-CREATION 
  src/slave/containerizer/mesos/io/switchboard.cpp PRE-CREATION 
  src/tests/containerizer/docker_volume_isolator_tests.cpp 
2f21b49535856186e153cd299dd1eda11495fa17 
  src/tests/containerizer/mesos_containerizer_tests.cpp 
5aae23b1b470d5323ecc21fb5df7ad8ae2498dfa 

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


Testing
---

GTEST_FILTER="" make -j check
sudo src/mesos-tests


Thanks,

Kevin Klues

Re: Review Request 53950: Avoided hardcoding ports in some tests.

[haosdent huang]

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


Ship it!




Ship It!

- haosdent huang


On Nov. 22, 2016, 7:49 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53950/
> ---
> 
> (Updated Nov. 22, 2016, 7:49 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and haosdent huang.
> 
> 
> Bugs: MESOS-6618
> https://issues.apache.org/jira/browse/MESOS-6618
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Avoided hardcoding ports in some tests.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> c478d56ffd734b26fd2dbd7bb1ca02ce929f3f16 
>   src/tests/health_check_tests.cpp 2c7022a1210ade6d7eb594c4f8934f2bdb2aee35 
> 
> Diff: https://reviews.apache.org/r/53950/diff/
> 
> 
> Testing
> ---
> 
> make check (SSL build OS X clang trunk w/ optimizations; default build 
> ubuntu-14.04 gcc-4.8.4 w/o optimizations)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 53929: Improved performance for `getQuotaRoleAllocatedResources` in allocator.

[Guangya Liu]

> On 十一月 21, 2016, 7:27 p.m., Benjamin Mahler wrote:
> > I pushed the change since this is cleaner and it looks like you ran the 
> > benchmark. Any reason you didn't include the results?
> > 
> > Do we have benchmark coverage of the quota code paths? If not, did you 
> > write a small custom benchmark just to confirm this is indeed faster?

Had an offline discussion with Ben, the conclusion for this is:

1) File a new JIRA ticket for quota allocation benchmark test: 
https://issues.apache.org/jira/browse/MESOS-6630
2) No need to add benchmark for this patch as I have a small internal benchmark 
test for https://reviews.apache.org/r/51683/ which can cover this code change.


- Guangya


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


On 十一月 19, 2016, 2:44 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53929/
> ---
> 
> (Updated 十一月 19, 2016, 2:44 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In allocator `getQuotaRoleAllocatedResources`, it is trying to omits
> dynamic reservation, persistent volume info and additionally strip
> `role` in it. But it is traversing all `Resource` object from the
> `Resources` object, strip the `role` and then using `+=` to add each
> `Resource` object back.
> 
> The `+=` for `Resource` object will invoke `validate` which will impact
> the performance. This fix is calling `flatten()` directly to strip the
> `role` for the all of the `Resource` objects and this will not invoke
> `validate`.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> c8f9492ee1b69e125a1e841116d22a578a9b524e 
> 
> Diff: https://reviews.apache.org/r/53929/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> ```
> ./bin/mesos-tests.sh  --gtest_filter="HierarchicalAllocatorTest.*"
> ./bin/mesos-tests.sh  
> --gtest_filter="*HierarchicalAllocator_BENCHMARK_Test.*/0" --benchmark
> ```
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 53949: Added test helper to obtain unused port.

[haosdent huang]

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


Fix it, then Ship it!




Ship It!


src/tests/utils.cpp (line 82)


Should we move the comment

```
// Bind to port 0 to obtain a random unused port.
```
at here?


- haosdent huang


On Nov. 22, 2016, 3:49 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53949/
> ---
> 
> (Updated Nov. 22, 2016, 3:49 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and haosdent huang.
> 
> 
> Bugs: MESOS-6618
> https://issues.apache.org/jira/browse/MESOS-6618
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test helper to obtain unused port.
> 
> 
> Diffs
> -
> 
>   src/tests/utils.hpp 140ebaaae43b03568ec49891635f0660cdfb4c85 
>   src/tests/utils.cpp eb36616f68d81d33d4bd04a7f23295e8c7558fc8 
> 
> Diff: https://reviews.apache.org/r/53949/diff/
> 
> 
> Testing
> ---
> 
> Tested as part of https://reviews.apache.org/r/53950/.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 54001: Addes status method to DockerContainerizer.

[Adam B]

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


Ship it!




Ship It!

- Adam B


On Nov. 22, 2016, 1:51 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54001/
> ---
> 
> (Updated Nov. 22, 2016, 1:51 p.m.)
> 
> 
> Review request for mesos, Kevin Klues and Vinod Kone.
> 
> 
> Bugs: MESOS-6625
> https://issues.apache.org/jira/browse/MESOS-6625
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch added the implementation of the 'status' method to
> DockerContainerizer. Previously, this method is missing. Currently,
> the only status the DockerContainerizer reports is the ContainerID.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.hpp 006f929eca0e0a6b1de941821ac72869ba393d2d 
>   src/slave/containerizer/docker.cpp c2ed5240aab9ea9d1a386c44c94e5ae7e98d313c 
> 
> Diff: https://reviews.apache.org/r/54001/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 53803: Added a new libprocess HTTP test.

[Benjamin Mahler]

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



Thanks for taking on this issue and testing it!


3rdparty/libprocess/src/tests/http_tests.cpp (lines 203 - 205)


This should be a SocketTest.ReceiveEOF within socket_tests.cpp? There 
doesn't appear to be any HTTP involved here and we're just interested in the 
socket behavior.



3rdparty/libprocess/src/tests/http_tests.cpp (lines 203 - 204)


A lot of these comments seem to have a really long line followed by a 
really short line. I would suggest trying to wrap these comments to be a little 
less "jagged", as I find it's easier on the eyes, but up to you:

```
// This test verifies that an EOF sent on a socket will be correctly 
received by
// the other end.
```

vs.

```
// This test verifies that an EOF sent on a socket
// will be correctly received by the other end.
```

Ditto below.



3rdparty/libprocess/src/tests/http_tests.cpp (lines 210 - 212)


Is it possible to just use the try directly?

```
Try client = Socket::create();
ASSERT_SOME(client);

...

client->recv();
```

Ditto below for the server socket:

```
Try server = Socket::create();
ASSERT_SOME(server);

server->bind(Address());
```

Ditto for the Address, can you use it via the Try instead of pulling out 
another variable?



3rdparty/libprocess/src/tests/http_tests.cpp (lines 218 - 219)


Can you use explicit shutdowns instead of relying on the scoping?



3rdparty/libprocess/src/tests/http_tests.cpp (line 225)


Using the default constructor seems a little less clear than:

```
server.bind(Address::LOCALHOST_ANY())
```

Also can you check the result? And could you just use the result to avoid 
the need to call server.address()?

```
Try server_address = server.bind(Address::LOCALHOST_ANY());

ASSERT_SOME(server_address);

...
// use it: server_address.get();
```



3rdparty/libprocess/src/tests/http_tests.cpp (lines 237 - 240)


Can this just be:

```
AWAIT_READY(server_socket.send(data));
AWAIT_EXPECT_EQ(data, client_socket.recv(data.size()));
```

Or is there some reason you wanted to call recv first? If so, a comment 
would be great. Note that you don't need the '`receive`' variable to be in the 
outer scope.



3rdparty/libprocess/src/tests/http_tests.cpp (lines 243 - 248)


Oh, can you just put your shutdown code before this patch?



3rdparty/libprocess/src/tests/http_tests.cpp (lines 250 - 252)


Can this just be:

```
AWAIT_EXPECT_EQ("", client_socket.recv());
```

i.e. why do you need to pass 1? and doesn't look like you need to store the 
future in a variable?


- Benjamin Mahler


On Nov. 16, 2016, 7:20 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53803/
> ---
> 
> (Updated Nov. 16, 2016, 7:20 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Joseph Wu.
> 
> 
> Bugs: MESOS-5966
> https://issues.apache.org/jira/browse/MESOS-5966
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds HTTPTest.SocketEOF to verify that EOFs are
> reliably received whether or not there is a pending recv()
> request at the time the EOF is received.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> 533104c93dd1eaf67bf3752163d2e0cad090078d 
> 
> Diff: https://reviews.apache.org/r/53803/diff/
> 
> 
> Testing
> ---
> 
> Testing details are at the end of this review chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 53546: Added stub classes for rest cgroups subsystems.

[Jason Lai]

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



Hi Haosdent! I think the subsystem name for PIDs is `pids` instead of `pid` 
(look at the example below). We should rename it also for the purpose of 
avoiding confusion with the PID namespace isolator.

```
$ cat /proc/cgroups
#subsys_namehierarchy   num_cgroups enabled
cpuset  3   21  1
cpu 2   150 1
cpuacct 2   150 1
blkio   5   150 1
memory  10  208 1
devices 7   150 1
freezer 9   21  1
net_cls 6   21  1
perf_event  8   21  1
net_prio6   21  1
pids4   152 1
```

- Jason Lai


On Nov. 15, 2016, 5:18 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53546/
> ---
> 
> (Updated Nov. 15, 2016, 5:18 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jason Lai, Jie Yu, Qian Zhang, and 
> Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6558
> https://issues.apache.org/jira/browse/MESOS-6558
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added stub classes for rest cgroups subsystems.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt aef9ae6d2872dc15725c01ce85b657965485605f 
>   src/Makefile.am 5a47c93388234a68c3c486a021ccdbe3213c5bac 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 67cc595278f124cdf518d2f4fcfb257439f067e2 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
> 2f6723c64261fb3295626d6479fe844fb23b0650 
>   src/slave/containerizer/mesos/isolators/cgroups/constants.hpp 
> 8c73efdfcfd3cf48a12cf81fcd075166dc51d7e3 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
> bf7dc9e7efb183879bfb525cc7ecfac4cc53dbef 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/blkio.hpp 
> PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/blkio.cpp 
> PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/cpuset.hpp 
> PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/cpuset.cpp 
> PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/hugetlb.hpp 
> PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/hugetlb.cpp 
> PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/net_prio.hpp 
> PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/net_prio.cpp 
> PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/pid.hpp 
> PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/pid.cpp 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/53546/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 53614: Updated the markdown style guide with headings capitalization.

[Mesos ReviewBot]

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



Bad patch!

Reviews applied: [53614, 53613, 53612, 53611, 53610]

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

Error:
2016-11-23 01:19:59 URL:https://reviews.apache.org/r/53610/diff/raw/ 
[13442/13442] -> "53610.patch" [1]
error: patch failed: docs/home.md:27
error: docs/home.md: patch does not apply

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

- Mesos ReviewBot


On Nov. 9, 2016, 5:08 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53614/
> ---
> 
> (Updated Nov. 9, 2016, 5:08 p.m.)
> 
> 
> Review request for mesos, Joerg Schad and Neil Conway.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   docs/markdown-style-guide.md 667d6dfbcf6c0864085cdfb19f4e53fb49a49c44 
> 
> Diff: https://reviews.apache.org/r/53614/diff/
> 
> 
> Testing
> ---
> 
> None: not a functional change.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 53546: Added stub classes for rest cgroups subsystems.

[Jason Lai]

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




src/CMakeLists.txt (line 173)


Should be `pids.cpp` instead of `pid.cpp`?



src/Makefile.am (line 1058)


`pids.cpp`?



src/Makefile.am (line 1100)


`pids.hpp`?



src/slave/containerizer/mesos/containerizer.cpp (line 309)


`{"cgroups/pids", ::create}`,?



src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp (line 90)


`pids` instead of `pid`?



src/slave/containerizer/mesos/isolators/cgroups/constants.hpp (line 52)


`const std::string CGROUP_SUBSYSTEM_PIDS_NAME = "pids";`



src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp (line 33)


See above



src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp (line 63)


`{CGROUP_SUBSYSTEM_PIDS_NAME, ::create},`



src/slave/containerizer/mesos/isolators/cgroups/subsystems/cpuset.hpp (line 38)


`CpuSetSubsystem`?



src/slave/containerizer/mesos/isolators/cgroups/subsystems/cpuset.cpp (line 37)


`CpuSetSubsystem`?



src/slave/containerizer/mesos/isolators/cgroups/subsystems/pid.hpp (lines 38 - 
54)


`PidsSubsystem`?



src/slave/containerizer/mesos/isolators/cgroups/subsystems/pid.cpp (line 19)


`pids.hpp`?



src/slave/containerizer/mesos/isolators/cgroups/subsystems/pid.cpp (lines 29 - 
41)


`PidsSubsystem` and `"cgroups-pids-subsystem"`?


- Jason Lai


On Nov. 15, 2016, 5:18 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53546/
> ---
> 
> (Updated Nov. 15, 2016, 5:18 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jason Lai, Jie Yu, Qian Zhang, and 
> Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6558
> https://issues.apache.org/jira/browse/MESOS-6558
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added stub classes for rest cgroups subsystems.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt aef9ae6d2872dc15725c01ce85b657965485605f 
>   src/Makefile.am 5a47c93388234a68c3c486a021ccdbe3213c5bac 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 67cc595278f124cdf518d2f4fcfb257439f067e2 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
> 2f6723c64261fb3295626d6479fe844fb23b0650 
>   src/slave/containerizer/mesos/isolators/cgroups/constants.hpp 
> 8c73efdfcfd3cf48a12cf81fcd075166dc51d7e3 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
> bf7dc9e7efb183879bfb525cc7ecfac4cc53dbef 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/blkio.hpp 
> PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/blkio.cpp 
> PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/cpuset.hpp 
> PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/cpuset.cpp 
> PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/hugetlb.hpp 
> PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/hugetlb.cpp 
> PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/net_prio.hpp 
> PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/net_prio.cpp 
> PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/pid.hpp 
> PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/pid.cpp 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/53546/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 52866: Refactored HealthChecker::reschedule to take duration as an argument.

[Alexander Rukletsov]

> On Oct. 21, 2016, 7:35 p.m., Benjamin Mahler wrote:
> > src/health-check/health_checker.cpp, line 188
> > 
> >
> > Isn't this going to lead to some slightly confusing logging where we 
> > say "Rescheduling" for the first health check?
> 
> Alexander Rukletsov wrote:
> ```
> I 15:07:43.066488 1601536 health_checker.cpp:193] Health check 
> starting in 0ns, grace period 0ns
> I 15:07:43.066573 1601536 health_checker.cpp:618] Rescheduling health 
> check in 0ns
> W 15:07:43.158612 3747840 health_checker.cpp:215] Health check failed 
> 1 times consecutively: COMMAND health check failed: Command returned exited 
> with status 1
> I 15:07:43.158704 3747840 health_checker.cpp:618] Rescheduling health 
> check in 0ns
> ```
> I don't think it is very confusing, but if you have a strong opinion, 
> I'll change that.
> 
> Benjamin Mahler wrote:
> Could we ensure (separately from this change) that we log the entire 
> health check config when using vlog level 1?
> 
> Seems like it would be an easy improvement to go from 
> s/Rescheduling/Scheduling/ for every instance of this log line?

Posted https://reviews.apache.org/r/54007/


- Alexander


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


On Nov. 23, 2016, 12:30 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52866/
> ---
> 
> (Updated Nov. 23, 2016, 12:30 a.m.)
> 
> 
> Review request for mesos, Gastón Kleiman and haosdent huang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> To facilitate code reuse, HealthChecker::reschedule() is generalized.
> This will become even more valuable when we add pause/resume functions.
> 
> 
> Diffs
> -
> 
>   src/health-check/health_checker.hpp 
> 837d1358e418d21536da488e4a23cbfa41db6060 
>   src/health-check/health_checker.cpp 
> af5500be249c74a4a5e64bf38dea607173e2f998 
> 
> Diff: https://reviews.apache.org/r/52866/diff/
> 
> 
> Testing
> ---
> 
> See https://reviews.apache.org/r/52873/.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 54007: Printed complete health check configuration on task launch.

[Alexander Rukletsov]

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

(Updated Nov. 23, 2016, 1:11 a.m.)


Review request for mesos, Gastón Kleiman and haosdent huang.


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/health-check/health_checker.cpp af5500be249c74a4a5e64bf38dea607173e2f998 

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


Testing
---

make check


Thanks,

Alexander Rukletsov

Review Request 54007: Printed complete health check configuration on task launch.

[Alexander Rukletsov]

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

Review request for mesos, Gastón Kleiman and haosdent huang.


Repository: mesos


Description
---

See summary.


Diffs
-

  src/health-check/health_checker.cpp af5500be249c74a4a5e64bf38dea607173e2f998 

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


Testing
---

make check


Thanks,

Alexander Rukletsov

Re: Review Request 52871: Ensured default executor ignores health updates for terminated tasks.

[Alexander Rukletsov]

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

(Updated Nov. 23, 2016, 1:06 a.m.)


Review request for mesos, Anand Mazumdar, Benjamin Mahler, Gastón Kleiman, and 
haosdent huang.


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


Repository: mesos


Description
---

After the task has been terminated, its health updates become
irrelevant and should be ignored. Also if the default executor
shuts down, we can safely stop all health checkers.

Technically health checking should be stopped right before
TASK_KILLING update is sent to avoid subsequent TASK_RUNNING
updates, but the default executor currently does not support
TASK_KILLING.


Diffs (updated)
-

  src/launcher/default_executor.cpp f4e1ea4816fee9a2f6d59bb534933b66013f11cf 

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


Testing
---

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


Thanks,

Alexander Rukletsov

Re: Review Request 53308: Added new hook for modifying the executor environment.

[Adam B]

> On Nov. 10, 2016, 7:19 a.m., Kapil Arya wrote:
> > Before doing full review, I am wondering if we can create a new protobuf 
> > called `DockerExecutorPrepareInfo` or something similar with the idea that 
> > we can pass on not only environment variables, but also volumes (and any 
> > other options in future). The volumes are interesting because a module 
> > might want to make available certain file paths within Docker executor. Is 
> > this something reasonable?
> 
> Till Toenshoff wrote:
> I like that idea a lot. Let me reiterate with that on this patch.

Are we dropping this patch in favor of https://reviews.apache.org/r/53998/ ?


- Adam


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


On Oct. 31, 2016, 8:49 a.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53308/
> ---
> 
> (Updated Oct. 31, 2016, 8:49 a.m.)
> 
> 
> Review request for mesos, Adam B, Jie Yu, Kapil Arya, and Joseph Wu.
> 
> 
> Bugs: MESOS-6396
> https://issues.apache.org/jira/browse/MESOS-6396
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allows for modifying the environment effective for the executor itself.
> Additionally allows the hook to receive the location of the sandbox folder.
> 
> 
> Diffs
> -
> 
>   include/mesos/hook.hpp c8a58641e0768628a9028a70d8c28f7fb412 
>   src/hook/manager.hpp 5ecfcab48da808c84d36f9bcfcb5a8e0ad2167e5 
>   src/hook/manager.cpp 24885226a788a7abd851e12b527f74fa972ec935 
>   src/slave/containerizer/docker.cpp f720320e112687e8ae972f04159b3b4cb7a58476 
> 
> Diff: https://reviews.apache.org/r/53308/diff/
> 
> 
> Testing
> ---
> 
> make check and functional test in a hook-module
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 53998: Fixed hook to allow for executor environment modifications.

[Adam B]

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


Fix it, then Ship it!




This change looks fine to me too.


src/slave/containerizer/docker.cpp (lines 1148 - 1154)


Do we need to worry about this part for 
https://issues.apache.org/jira/browse/MESOS-6566 ?


- Adam B


On Nov. 22, 2016, 12:26 p.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53998/
> ---
> 
> (Updated Nov. 22, 2016, 12:26 p.m.)
> 
> 
> Review request for mesos, Adam B, Jie Yu, Kapil Arya, and Joseph Wu.
> 
> 
> Bugs: MESOS-6396
> https://issues.apache.org/jira/browse/MESOS-6396
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When being used on a task run by a command executor, the hook
> `slavePreLaunchDockerEnvironmentDecorator` did not allow for extending
> the executor's own environment but only the environment of the task.
> By using the hook returned environment for the executor in any case
> and supplying a task specific environment for the command executor,
> that limitation is fixed.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.cpp c2ed5240aab9ea9d1a386c44c94e5ae7e98d313c 
> 
> Diff: https://reviews.apache.org/r/53998/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> * WIP - functional testing pending - WIP *
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 53802: Eliminated an EOF race condition in libprocess SSL socket.

[Greg Mann]

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

(Updated Nov. 23, 2016, 12:58 a.m.)


Review request for mesos, Benjamin Mahler and Joseph Wu.


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


Repository: mesos


Description (updated)
---

Previously, it was possible for an SSL socket to either:
1) Fail to receive an EOF if the EOF event was received when
   there was no pending recv() request.
2) Fail to receive all data sent on the sending side if an
   EOF event was received before all sent data was read.

This patch eliminates these race conditions to ensure reliable
receipt of both sent data and EOFs.


Diffs (updated)
-

  3rdparty/libprocess/src/libevent_ssl_socket.hpp 
acb00d41c637a318b2f16fff9e97998b9c79b809 
  3rdparty/libprocess/src/libevent_ssl_socket.cpp 
21f878ee81db32ad35878ec053c3f2de3637196c 

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


Testing
---

Testing details are found at the end of this chain.


Thanks,

Greg Mann

Re: Review Request 53802: Eliminated an EOF race condition in libprocess SSL socket.

[Greg Mann]

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

(Updated Nov. 23, 2016, 12:54 a.m.)


Review request for mesos, Benjamin Mahler and Joseph Wu.


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


Repository: mesos


Description (updated)
---

Previously, it was possible for an SSL socket to either:
1) Fail to receive an EOF if the EOF event was received when
   there was no pending recv() request.
2) Fail to receive all data sent on the sending side if an
   EOF event was received before all sent data was read.


This patch eliminates these race conditions to ensure reliable
receipt of both sent data and EOFs.


Diffs
-

  3rdparty/libprocess/src/libevent_ssl_socket.hpp 
acb00d41c637a318b2f16fff9e97998b9c79b809 
  3rdparty/libprocess/src/libevent_ssl_socket.cpp 
21f878ee81db32ad35878ec053c3f2de3637196c 

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


Testing
---

Testing details are found at the end of this chain.


Thanks,

Greg Mann

Re: Review Request 52872: Used callback instead of `send()` for health status updates.

[Alexander Rukletsov]

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

(Updated Nov. 23, 2016, 12:52 a.m.)


Review request for mesos, Anand Mazumdar, Benjamin Mahler, Gastón Kleiman, and 
haosdent huang.


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


Repository: mesos


Description
---

Since HealthChecker is now used as a library only and does not live
in a separate OS process, there is no need to use libprocess message
sending for health status updates; a callback will do.


Diffs (updated)
-

  src/docker/executor.cpp eefbc0c2936eb93044c2d8a1b50155171f398562 
  src/health-check/health_checker.hpp 837d1358e418d21536da488e4a23cbfa41db6060 
  src/health-check/health_checker.cpp af5500be249c74a4a5e64bf38dea607173e2f998 
  src/launcher/default_executor.cpp f4e1ea4816fee9a2f6d59bb534933b66013f11cf 
  src/launcher/executor.cpp ce0b199551447504bb95743df4ce9ec4a0443cd4 

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


Testing
---

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


Thanks,

Alexander Rukletsov

Re: Review Request 52873: Cleaned up private members in HealthChecker class.

[Alexander Rukletsov]

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

(Updated Nov. 23, 2016, 12:52 a.m.)


Review request for mesos, Gastón Kleiman and haosdent huang.


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/health-check/health_checker.hpp 837d1358e418d21536da488e4a23cbfa41db6060 
  src/health-check/health_checker.cpp af5500be249c74a4a5e64bf38dea607173e2f998 

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


Testing
---

make check


Thanks,

Alexander Rukletsov

Re: Review Request 53802: Eliminated an EOF race condition in libprocess SSL socket.

[Greg Mann]

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

(Updated Nov. 23, 2016, 12:52 a.m.)


Review request for mesos, Benjamin Mahler and Joseph Wu.


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


Repository: mesos


Description (updated)
---

Eliminated an EOF race condition in libprocess SSL socket.


Diffs (updated)
-

  3rdparty/libprocess/src/libevent_ssl_socket.hpp 
acb00d41c637a318b2f16fff9e97998b9c79b809 
  3rdparty/libprocess/src/libevent_ssl_socket.cpp 
21f878ee81db32ad35878ec053c3f2de3637196c 

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


Testing
---

Testing details are found at the end of this chain.


Thanks,

Greg Mann

Re: Review Request 52868: Health checks may be stopped on demand.

[Alexander Rukletsov]

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

(Updated Nov. 23, 2016, 12:51 a.m.)


Review request for mesos, Benjamin Mahler, Gastón Kleiman, and haosdent huang.


Summary (updated)
-

Health checks may be stopped on demand.


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


Repository: mesos


Description
---

See summary.


Diffs
-

  src/health-check/health_checker.hpp 837d1358e418d21536da488e4a23cbfa41db6060 
  src/health-check/health_checker.cpp af5500be249c74a4a5e64bf38dea607173e2f998 

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


Testing
---

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


Thanks,

Alexander Rukletsov

Re: Review Request 52865: Refactored HealthChecker to never stop health checking.

[Alexander Rukletsov]

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

(Updated Nov. 23, 2016, 12:50 a.m.)


Review request for mesos, Anand Mazumdar, Benjamin Mahler, Gastón Kleiman, and 
haosdent huang.


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


Repository: mesos


Description (updated)
---

Prior to this patch, HealthChecker would stop performing health
checks after it marks the task for kill. Since tasks' lifecycle
is managed by scheduler-executor, HealthChecker should never stop
health checking on its own.

Allowing health checks to run forever enables the scheduler
make the decision about how to deal with unhealthy tasks.


Diffs
-

  src/docker/executor.cpp eefbc0c2936eb93044c2d8a1b50155171f398562 
  src/health-check/health_checker.hpp 837d1358e418d21536da488e4a23cbfa41db6060 
  src/health-check/health_checker.cpp af5500be249c74a4a5e64bf38dea607173e2f998 
  src/launcher/default_executor.cpp f4e1ea4816fee9a2f6d59bb534933b66013f11cf 
  src/launcher/executor.cpp ce0b199551447504bb95743df4ce9ec4a0443cd4 

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


Testing
---

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


Thanks,

Alexander Rukletsov

Re: Review Request 52871: Ensured default executor ignores health updates for terminated tasks.

[Alexander Rukletsov]

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

(Updated Nov. 23, 2016, 12:50 a.m.)


Review request for mesos, Anand Mazumdar, Benjamin Mahler, Gastón Kleiman, and 
haosdent huang.


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


Repository: mesos


Description (updated)
---

After the task has been terminated, its health updates become
irrelevant and should be ignored. Also if the default executor
shuts down, we can safely stop all health checkers.

Technically health checking should be stopped right before
TASK_KILLING update is sent to avoid subsequent TASK_RUNNING
updates, but the default executor currently does not support
TASK_KILLING.


Diffs (updated)
-

  src/launcher/default_executor.cpp f4e1ea4816fee9a2f6d59bb534933b66013f11cf 

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


Testing
---

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


Thanks,

Alexander Rukletsov

Re: Review Request 52869: Ensured command executor stops health checking terminated tasks.

[Alexander Rukletsov]

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

(Updated Nov. 23, 2016, 12:48 a.m.)


Review request for mesos, Benjamin Mahler, Gastón Kleiman, and haosdent huang.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/launcher/executor.cpp ce0b199551447504bb95743df4ce9ec4a0443cd4 

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


Testing
---

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


Thanks,

Alexander Rukletsov

Re: Review Request 52870: Ensured docker executor stops health checking terminated tasks.

[Alexander Rukletsov]

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

(Updated Nov. 23, 2016, 12:48 a.m.)


Review request for mesos, Benjamin Mahler, Gastón Kleiman, and haosdent huang.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/docker/executor.cpp eefbc0c2936eb93044c2d8a1b50155171f398562 

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


Testing
---

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


Thanks,

Alexander Rukletsov

Re: Review Request 52868: Added pause/resume functionality to HealthChecker.

[Alexander Rukletsov]

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

(Updated Nov. 23, 2016, 12:45 a.m.)


Review request for mesos, Benjamin Mahler, Gastón Kleiman, and haosdent huang.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/health-check/health_checker.hpp 837d1358e418d21536da488e4a23cbfa41db6060 
  src/health-check/health_checker.cpp af5500be249c74a4a5e64bf38dea607173e2f998 

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


Testing
---

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


Thanks,

Alexander Rukletsov

Re: Review Request 52867: Used `Duration::create()` for double -> Duration conversion.

[Alexander Rukletsov]

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

(Updated Nov. 23, 2016, 12:44 a.m.)


Review request for mesos, Daniel Pravat, Gastón Kleiman, haosdent huang, and 
Joseph Wu.


Repository: mesos


Description
---

Additionally persist health check parameters from the `HealthCheck`
protobuf as class members to avoid code duplication.


Diffs (updated)
-

  src/health-check/health_checker.hpp 837d1358e418d21536da488e4a23cbfa41db6060 
  src/health-check/health_checker.cpp af5500be249c74a4a5e64bf38dea607173e2f998 

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


Testing
---

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


Thanks,

Alexander Rukletsov

Re: Review Request 53313: Windows: Disable persistent state for Windows master.

[Alex Clemmer]

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

(Updated Nov. 22, 2016, 4:30 p.m.)


Review request for mesos, Daniel Pravat and Joseph Wu.


Changes
---

Added a JIRA.


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


Repository: mesos


Description
---

Many agent tests currently use the master in some form. Of those,
relatively few use the leveldb-based persistence. Since we cannot
currently compile leveldb on Windows, and since running these tests on
Windows involves compiling and running the master, this commit will
remove the direct dependency on the replicated log for windows builds of
the master.


Diffs
-

  src/local/local.cpp 257179443827ffdfc946ed655a6840fcea70d454 
  src/master/main.cpp 2d2dfb7d632f3c7be1796efd8f0a1f4d18760261 

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


Testing
---


Thanks,

Alex Clemmer

Re: Review Request 52866: Refactored HealthChecker::reschedule to take duration as an argument.

[Alexander Rukletsov]

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

(Updated Nov. 23, 2016, 12:30 a.m.)


Review request for mesos, Gastón Kleiman and haosdent huang.


Repository: mesos


Description
---

To facilitate code reuse, HealthChecker::reschedule() is generalized.
This will become even more valuable when we add pause/resume functions.


Diffs (updated)
-

  src/health-check/health_checker.hpp 837d1358e418d21536da488e4a23cbfa41db6060 
  src/health-check/health_checker.cpp af5500be249c74a4a5e64bf38dea607173e2f998 

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


Testing
---

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


Thanks,

Alexander Rukletsov

Review Request 54004: Renamed functions in HealthChecker for clarity.

[Alexander Rukletsov]

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

Review request for mesos, Gastón Kleiman and haosdent huang.


Repository: mesos


Description
---

Use descriptive function names instead of underscore
prefixes for functions in HealthChecker.


Diffs
-

  src/health-check/health_checker.hpp 837d1358e418d21536da488e4a23cbfa41db6060 
  src/health-check/health_checker.cpp af5500be249c74a4a5e64bf38dea607173e2f998 

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


Testing
---

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


Thanks,

Alexander Rukletsov

Re: Review Request 52865: Refactored HealthChecker to never stop health checking.

[Alexander Rukletsov]

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

(Updated Nov. 23, 2016, 12:28 a.m.)


Review request for mesos, Anand Mazumdar, Benjamin Mahler, Gastón Kleiman, and 
haosdent huang.


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


Repository: mesos


Description
---

Prior to this patch, HealthChecker would stop performing health
checks after it marks the task for kill. Since tasks' lifecycle
is managed by scheduler-executor, HealthChecker should never stop
health checking on its own.


Diffs (updated)
-

  src/docker/executor.cpp eefbc0c2936eb93044c2d8a1b50155171f398562 
  src/health-check/health_checker.hpp 837d1358e418d21536da488e4a23cbfa41db6060 
  src/health-check/health_checker.cpp af5500be249c74a4a5e64bf38dea607173e2f998 
  src/launcher/default_executor.cpp f4e1ea4816fee9a2f6d59bb534933b66013f11cf 
  src/launcher/executor.cpp ce0b199551447504bb95743df4ce9ec4a0443cd4 

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


Testing
---

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


Thanks,

Alexander Rukletsov

Re: Review Request 53802: Eliminated an EOF race condition in libprocess SSL socket.

[Joseph Wu]

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




3rdparty/libprocess/src/libevent_ssl_socket.cpp (lines 246 - 248)


We should probably amend this comment block with another section to explain 
the EOF bool.
Same with the last line of this diff.



3rdparty/libprocess/src/libevent_ssl_socket.cpp (lines 406 - 410)


I'm wondering if separating out this critical section into several chunks 
will introduce any bugs.

It may be worthwhile to move each of the three swaps into their associated 
`if-statement`.


- Joseph Wu


On Nov. 22, 2016, 11:55 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53802/
> ---
> 
> (Updated Nov. 22, 2016, 11:55 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Joseph Wu.
> 
> 
> Bugs: MESOS-5966
> https://issues.apache.org/jira/browse/MESOS-5966
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, it was possible for an SSL socket to either:
> 1) Fail to receive an EOF if the EOF event was received when
>there was no pending recv() request.
> 2) Fail to receive all data sent on the sending side if an
>EOF event was received before all sent data was read.
>   
> This patch eliminates these race conditions to ensure reliable
> receipt of both sent data and EOFs.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/libevent_ssl_socket.hpp 
> acb00d41c637a318b2f16fff9e97998b9c79b809 
>   3rdparty/libprocess/src/libevent_ssl_socket.cpp 
> 21f878ee81db32ad35878ec053c3f2de3637196c 
> 
> Diff: https://reviews.apache.org/r/53802/diff/
> 
> 
> Testing
> ---
> 
> Testing details are found at the end of this chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 53974: Added support to handle ATTACH_CONTAINER_OUPUT in the io switchbaord.

[Kevin Klues]

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

(Updated Nov. 22, 2016, 11:07 p.m.)


Review request for mesos, Benjamin Hindman, Jie Yu, and Vinod Kone.


Changes
---

Updatd based on offline suggestions from Anand about how to better manage the 
`HttpConnections` we are storing.


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


Repository: mesos


Description
---

Added support to handle ATTACH_CONTAINER_OUPUT in the io switchbaord.


Diffs (updated)
-

  src/slave/containerizer/mesos/io_switchboard/main.cpp PRE-CREATION 

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


Testing
---

GTEST_FILTER="" make -j check
sudo src/mesos-tests


Thanks,

Kevin Klues

Re: Review Request 53877: Added temporary file environment passing towards docker.

[Adam B]

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


Fix it, then Ship it!




Looks great!


src/docker/docker.cpp (lines 574 - 582)


Looks like you're doing the close in both the `if(write.isError())` above 
and here below. You could pull it out of the if and just do it immediately 
after the write, regardless of error.


- Adam B


On Nov. 22, 2016, 2:01 p.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53877/
> ---
> 
> (Updated Nov. 22, 2016, 2:01 p.m.)
> 
> 
> Review request for mesos, Adam B, Gastón Kleiman, Jie Yu, Kapil Arya, and 
> Joseph Wu.
> 
> 
> Bugs: MESOS-6566
> https://issues.apache.org/jira/browse/MESOS-6566
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> see summary.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp f03ea7fa55e976e44282503261ac50e1502592a2 
> 
> Diff: https://reviews.apache.org/r/53877/diff/
> 
> 
> Testing
> ---
> 
> ```
> $ make check
> ```
> 
> ```
> $ ./src/mesos-execute --command="sleep 200" --containerizer=docker 
> --docker_image=alpine --env='{"foo": "bar"}' --master=149.202.202.185:5050
> ```
> 
> ```
> $ ps aux
> root 27645  0.2  0.0 372852 30180 ?Sl   15:50   0:00 docker -H 
> unix:///var/run/docker.sock run --cpu-shares 1024 --memory 134217728 
> --env-file /tmp/l53ILz/ktpuDS -v 
> /tmp/mesos/slaves/91b671fd-3c83-425e-96c2-26ecdc410028-S0/frameworks/91b671fd-3c83-425e-96c2-26ecdc410028-0001/executors/test/runs/a75fc411-3d18-44f1-a562-9f759c281da0:/mnt/mesos/sandbox
>  --net host --entrypoint /bin/sh --name 
> mesos-91b671fd-3c83-425e-96c2-26ecdc410028-S0.a75fc411-3d18-44f1-a562-9f759c281da0
>  alpine -c sleep 200
> ```
> 
> ```
> $ more /tmp/l53ILz/ktpuDS
> foo=bar
> MESOS_SANDBOX=/mnt/mesos/sandbox
> MESOS_CONTAINER_NAME=mesos-91b671fd-3c83-425e-96c2-26ecdc410028-S0.a75fc411-3d18-44f1-a562-9f759c281da0
> ```
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 53995: Added API handler for ATTACH_CONTAINER_OUTPUT.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [53994, 53995]

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 Nov. 22, 2016, 6:45 p.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53995/
> ---
> 
> (Updated Nov. 22, 2016, 6:45 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Benjamin Mahler.
> 
> 
> Bugs: MESOS-6473
> https://issues.apache.org/jira/browse/MESOS-6473
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added API handler for ATTACH_CONTAINER_OUTPUT.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp aa9f492fc74283b5a6f24a8b7189f1ecbe41488f 
>   src/slave/slave.hpp 0cc105418bf4e35b604ebe412048237814e495fb 
>   src/slave/validation.cpp b3d8bcd0f0855d2a0a96821900d996ba86e11578 
>   src/tests/api_tests.cpp 8889e7807ecead736eaac3910332e86d594e8cec 
> 
> Diff: https://reviews.apache.org/r/53995/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>

Re: Review Request 53704: Added a level of indirection for logger through an IO Switchboard.

[Jie Yu]

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


Fix it, then Ship it!





src/CMakeLists.txt (line 329)


I'd suggest to use "mesos/io/switchboard.cpp" and 
"mesos/io/switchboard_main.cpp" in case we have other io related classes or 
functions to move to the same folder.

In the future, maybe we want to move logger to 'io/' subfolder as well.



src/slave/containerizer/mesos/containerizer.hpp (line 293)


const?



src/slave/containerizer/mesos/io_switchboard/io_switchboard.hpp (lines 51 - 58)


I'd suggest we initialize those in a constructor and move the impl to the 
cpp file.



src/slave/containerizer/mesos/io_switchboard/io_switchboard.hpp (line 61)


Should we return `Try` here?



src/slave/containerizer/mesos/io_switchboard/io_switchboard.cpp (line 28)


kill this line



src/slave/containerizer/mesos/io_switchboard/io_switchboard.cpp (line 68)


insert a new line above.


- Jie Yu


On Nov. 20, 2016, 8:33 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53704/
> ---
> 
> (Updated Nov. 20, 2016, 8:33 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-6467
> https://issues.apache.org/jira/browse/MESOS-6467
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The purpose of this component is to feed stdin to a container from an
> external source, as well as redirect the stdin/stdout of a container
> to multiple targets.
> 
> In this commit, we simply add the IOSwitchboard as a component that
> interposes on the fds set up to communicate between the logger and a
> container.
> 
> In the future, we will expand this component to (optionaly) launch a
> sidecar HTTP server process which will be responsible for handling
> 'ATTACH_CONTAINER_INPUT' and 'ATTACH_CONTAINER_OUTPUT' calls on behalf
> of a container to redirect the stdin/stdout/sderr of a container to
> external clients.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt aef9ae6d2872dc15725c01ce85b657965485605f 
>   src/Makefile.am 5a47c93388234a68c3c486a021ccdbe3213c5bac 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 272052ddf85b50f817a110a9a83566b011598985 
>   src/slave/containerizer/mesos/containerizer.cpp 
> ec4ae32485a7ab6c9f73c512004d1220482a188e 
>   src/slave/containerizer/mesos/io_switchboard/io_switchboard.hpp 
> PRE-CREATION 
>   src/slave/containerizer/mesos/io_switchboard/io_switchboard.cpp 
> PRE-CREATION 
>   src/tests/containerizer/docker_volume_isolator_tests.cpp 
> 2f21b49535856186e153cd299dd1eda11495fa17 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> 5aae23b1b470d5323ecc21fb5df7ad8ae2498dfa 
> 
> Diff: https://reviews.apache.org/r/53704/diff/
> 
> 
> Testing
> ---
> 
> GTEST_FILTER="" make -j check
> sudo src/mesos-tests
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 53997: Fix SSL downgrade pathway for temporary/persistent sockets.

[Joseph Wu]

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

(Updated Nov. 22, 2016, 2:01 p.m.)


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


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


Repository: mesos


Description
---

This fixes some potential CHECK failures when a libprocess process
has (1) SSL downgrade enabled and (2) temporary and persistent
connections open with the same remote address.  The second point is
only possible if messages are to a remote address without a persistent
connection and then a persistent connection is created.

The SSL downgrade path was only checking if the address of a socket
matched when performing the downgrade.  The code must also check to
see if the socket itself matches.


Diffs
-

  3rdparty/libprocess/src/process.cpp 84971fa5151991c51e78abdbc736c719e30588f1 

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


Testing
---

See related ticket for the clunky unit test.  Ran that test in repetition.

make check


Thanks,

Joseph Wu

Re: Review Request 53877: Added temporary file environment passing towards docker.

[Till Toenshoff]

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

(Updated Nov. 22, 2016, 10:01 p.m.)


Review request for mesos, Adam B, Gastón Kleiman, Jie Yu, Kapil Arya, and 
Joseph Wu.


Changes
---

Addressed comments - thanks for those reviews!


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


Repository: mesos


Description
---

see summary.


Diffs (updated)
-

  src/docker/docker.cpp f03ea7fa55e976e44282503261ac50e1502592a2 

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


Testing
---

```
$ make check
```

```
$ ./src/mesos-execute --command="sleep 200" --containerizer=docker 
--docker_image=alpine --env='{"foo": "bar"}' --master=149.202.202.185:5050
```

```
$ ps aux
root 27645  0.2  0.0 372852 30180 ?Sl   15:50   0:00 docker -H 
unix:///var/run/docker.sock run --cpu-shares 1024 --memory 134217728 --env-file 
/tmp/l53ILz/ktpuDS -v 
/tmp/mesos/slaves/91b671fd-3c83-425e-96c2-26ecdc410028-S0/frameworks/91b671fd-3c83-425e-96c2-26ecdc410028-0001/executors/test/runs/a75fc411-3d18-44f1-a562-9f759c281da0:/mnt/mesos/sandbox
 --net host --entrypoint /bin/sh --name 
mesos-91b671fd-3c83-425e-96c2-26ecdc410028-S0.a75fc411-3d18-44f1-a562-9f759c281da0
 alpine -c sleep 200
```

```
$ more /tmp/l53ILz/ktpuDS
foo=bar
MESOS_SANDBOX=/mnt/mesos/sandbox
MESOS_CONTAINER_NAME=mesos-91b671fd-3c83-425e-96c2-26ecdc410028-S0.a75fc411-3d18-44f1-a562-9f759c281da0
```


Thanks,

Till Toenshoff

Re: Review Request 53997: Fix SSL downgrade pathway for temporary/persistent sockets.

[Joseph Wu]

> On Nov. 22, 2016, 1:19 p.m., Benjamin Mahler wrote:
> > 3rdparty/libprocess/src/process.cpp, lines 2476-2477
> > 
> >
> > Do you want an else if here to match the close logic? Otherwise, maybe 
> > add back the newline here?

Matching the `close` logic sounds like a good idea.  To completely match the 
logic (and hence the assumption that the set of temporary vs persistent sockets 
do not overlap), I'd also want to flip the if-statements around: swap 
persistent ones first, else temporary ones.


- Joseph


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


On Nov. 22, 2016, 2:01 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53997/
> ---
> 
> (Updated Nov. 22, 2016, 2:01 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-6621
> https://issues.apache.org/jira/browse/MESOS-6621
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This fixes some potential CHECK failures when a libprocess process
> has (1) SSL downgrade enabled and (2) temporary and persistent
> connections open with the same remote address.  The second point is
> only possible if messages are to a remote address without a persistent
> connection and then a persistent connection is created.
> 
> The SSL downgrade path was only checking if the address of a socket
> matched when performing the downgrade.  The code must also check to
> see if the socket itself matches.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp 
> 84971fa5151991c51e78abdbc736c719e30588f1 
> 
> Diff: https://reviews.apache.org/r/53997/diff/
> 
> 
> Testing
> ---
> 
> See related ticket for the clunky unit test.  Ran that test in repetition.
> 
> make check
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 53997: Fix SSL downgrade pathway for temporary/persistent sockets.

[Joseph Wu]

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

(Updated Nov. 22, 2016, 2:01 p.m.)


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


Changes
---

Flip the order of swapping the sockets and add an `if-else`.


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


Repository: mesos


Description
---

This fixes some potential CHECK failures when a libprocess process
has (1) SSL downgrade enabled and (2) temporary and persistent
connections open with the same remote address.  The second point is
only possible if messages are to a remote address without a persistent
connection and then a persistent connection is created.

The SSL downgrade path was only checking if the address of a socket
matched when performing the downgrade.  The code must also check to
see if the socket itself matches.


Diffs (updated)
-

  3rdparty/libprocess/src/process.cpp 84971fa5151991c51e78abdbc736c719e30588f1 

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


Testing
---

See related ticket for the clunky unit test.  Ran that test in repetition.

make check


Thanks,

Joseph Wu

Review Request 54001: Addes status method to DockerContainerizer.

[Jie Yu]

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

Review request for mesos, Kevin Klues and Vinod Kone.


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


Repository: mesos


Description
---

This patch added the implementation of the 'status' method to
DockerContainerizer. Previously, this method is missing. Currently,
the only status the DockerContainerizer reports is the ContainerID.


Diffs
-

  src/slave/containerizer/docker.hpp 006f929eca0e0a6b1de941821ac72869ba393d2d 
  src/slave/containerizer/docker.cpp c2ed5240aab9ea9d1a386c44c94e5ae7e98d313c 

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


Testing
---

sudo make check


Thanks,

Jie Yu

Re: Review Request 53975: Moved server socket deletion in 'process::finalize()'.

[Joseph Wu]

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


Ship it!




Ship It!

- Joseph Wu


On Nov. 21, 2016, 10:53 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53975/
> ---
> 
> (Updated Nov. 21, 2016, 10:53 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Joseph Wu.
> 
> 
> Bugs: MESOS-5966
> https://issues.apache.org/jira/browse/MESOS-5966
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We need to destroy the server socket and discard its
> future before we finalize the process manager
> because the socket's `onDiscard` callback may need to
> run in the event loop, but process manager
> finalization stops the event loop.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp 
> ab2b5a9d38a3001d6a5daa1807fecb630c4b154d 
> 
> Diff: https://reviews.apache.org/r/53975/diff/
> 
> 
> Testing
> ---
> 
> Testing details at the end of this review chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 53998: Fixed hook to allow for executor environment modifications.

[Joseph Wu]

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



LGTM.

A reasonable change (to have the hook modify both the executor and task 
environments).

- Joseph Wu


On Nov. 22, 2016, 12:26 p.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53998/
> ---
> 
> (Updated Nov. 22, 2016, 12:26 p.m.)
> 
> 
> Review request for mesos, Adam B, Jie Yu, Kapil Arya, and Joseph Wu.
> 
> 
> Bugs: MESOS-6396
> https://issues.apache.org/jira/browse/MESOS-6396
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When being used on a task run by a command executor, the hook
> `slavePreLaunchDockerEnvironmentDecorator` did not allow for extending
> the executor's own environment but only the environment of the task.
> By using the hook returned environment for the executor in any case
> and supplying a task specific environment for the command executor,
> that limitation is fixed.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.cpp c2ed5240aab9ea9d1a386c44c94e5ae7e98d313c 
> 
> Diff: https://reviews.apache.org/r/53998/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> * WIP - functional testing pending - WIP *
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 53825: MESOS-6597 Enabled java protos generation for all V1 proto files.

[Anand Mazumdar]

> On Nov. 20, 2016, 9:04 p.m., Anand Mazumdar wrote:
> > Nice first patch and welcome to the community!
> > 
> > - Can you update the Testing Done section with details on testing?
> > - We also support Python bindings. Do you mind adding these protos to our 
> > python build too in a follow up review? (See my comments later on how you 
> > can use review dependencies if you are new to ReviewBoard)
> 
> Vijay Srinivasaraghavan wrote:
> Thanks Anand. I have made the python binding changes locally on my 
> machine. Do you want me to create a new JIRA bug for this issue and create a 
> new review or add the patch to this JIRA/review itself? I am little confused 
> as to how the dependencies are handled with RB.

No need to create a separate issue. It would be fine to create a new review 
with it only containing the python related changes. This would be another 
commit based on top of this one. You can then use `post-reviews.py` and it 
should handle setting the dependencies for you. Feel free to ping me on 
IRC/Slack if you run into any issues (anandm)


> On Nov. 20, 2016, 9:04 p.m., Anand Mazumdar wrote:
> > include/mesos/v1/allocator/allocator.proto, lines 21-22
> > 
> >
> > These changes seem unrelated to this change i.e., java protos 
> > generation. We prefer single logical atomic commits in Mesos. For more info 
> > see: http://mesos.apache.org/documentation/latest/submitting-a-patch/
> > 
> > Can you create a separate patch for this and make this review dependent 
> > on it? The `post-reviews.py` script would do it automatically for you.
> 
> Vijay Srinivasaraghavan wrote:
> Actually, this fix is also part of the same issue. The proto file was 
> missing specific java package definition and hence the java file gets 
> generated in a package location different from the other java files.

hmm, are they the same? They are two separate issues:

1. The proto file was missing specific java package definition. This would 
impact any user trying to use the proto file and is not related to including 
the generated files in the Mesos JAR.
2. Include the generated files in the Mesos JAR.

This review (from its summary/description) caters to _only_ build changes in 
the MakeFile itself and not protobuf related changes.


- Anand


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


On Nov. 21, 2016, 4:46 a.m., Vijay Srinivasaraghavan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53825/
> ---
> 
> (Updated Nov. 21, 2016, 4:46 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Zameer Manji.
> 
> 
> Bugs: MESOS-6597
> https://issues.apache.org/jira/browse/MESOS-6597
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> MESOS-6597 Enabled java protos generation for all V1 proto files.
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/allocator/allocator.proto 
> 73d45b37a7afc47366a4a01a36912f30b47c30b1 
>   src/Makefile.am 5a47c93388234a68c3c486a021ccdbe3213c5bac 
> 
> Diff: https://reviews.apache.org/r/53825/diff/
> 
> 
> Testing
> ---
> 
> Built mesos and confirmed jar file containing new java PB wrappers for V1 
> API. Ran standard unit test (make tests)
> 
> 
> Thanks,
> 
> Vijay Srinivasaraghavan
> 
>

Re: Review Request 53990: Added POSIX socket shutdown types to Windows header.

[Benjamin Mahler]

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


Ship it!




Ship It!

- Benjamin Mahler


On Nov. 22, 2016, 6:20 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53990/
> ---
> 
> (Updated Nov. 22, 2016, 6:20 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Joseph Wu.
> 
> 
> Bugs: MESOS-5658 and MESOS-5966
> https://issues.apache.org/jira/browse/MESOS-5658
> https://issues.apache.org/jira/browse/MESOS-5966
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds two missing BSD socket shutdown types
> to a Windows header which maps these POSIX constants
> to their Windows counterparts.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/windows.hpp 
> 3782aa0f5c8636fef3cd47e78be0b9860b735a02 
> 
> Diff: https://reviews.apache.org/r/53990/diff/
> 
> 
> Testing
> ---
> 
> Testing details are found at the end of this review chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 53611: Improved comments around health checks in mesos.proto.

[Neil Conway]

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


Fix it, then Ship it!





include/mesos/mesos.proto (line 414)


See below.



include/mesos/v1/mesos.proto (line 414)


"// Amount of time after the task is launched during which health check 
failures are ignored. Note that once the health check passes for the first 
time, the grace period does not apply anymore."


Would also be good to cover whether `grace_period_seconds` includes 
`delay_seconds` or not.

- Neil Conway


On Nov. 9, 2016, 5:05 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53611/
> ---
> 
> (Updated Nov. 9, 2016, 5:05 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman, haosdent huang, Neil Conway, and 
> Till Toenshoff.
> 
> 
> Bugs: MESOS-5597
> https://issues.apache.org/jira/browse/MESOS-5597
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 380575904a5a29ee53bdc87ae8791ed14e3cafca 
>   include/mesos/v1/mesos.proto 5b542ff9cd0b5f301c088d3f4b2fe17ae760d368 
> 
> Diff: https://reviews.apache.org/r/53611/diff/
> 
> 
> Testing
> ---
> 
> None: not a functional change.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 53610: Added health checks documentation.

[Neil Conway]

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




docs/health-checks.md (line 155)


Timeouts are treated as failures, right?


- Neil Conway


On Nov. 20, 2016, 6:52 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53610/
> ---
> 
> (Updated Nov. 20, 2016, 6:52 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman, haosdent huang, Neil Conway, and 
> Till Toenshoff.
> 
> 
> Bugs: MESOS-5597
> https://issues.apache.org/jira/browse/MESOS-5597
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   docs/health-checks.md PRE-CREATION 
>   docs/home.md a5811480de050352dca6c0f7e4e64d3d2351c2d5 
> 
> Diff: https://reviews.apache.org/r/53610/diff/
> 
> 
> Testing
> ---
> 
> https://gist.github.com/rukletsov/7200c36b2fd1e81f78f2583e68b31fd1
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 53491: Disabled tests relying on filtering HTTP events.

[Benjamin Mahler]

> On Nov. 22, 2016, 8:35 p.m., Benjamin Mahler wrote:
> > This didn't have a ship it, I was expecting the test filter changes to be 
> > in place before we committed the changes. Are you working on these now?
> 
> Anand Mazumdar wrote:
> Looks like there is some disconnect. We had an offline discussion at 
> MesosCon China on this and I remember you agreeing that we can commit these 
> patches with the tests disabled and I can work on the filtering changes in 
> the interim. I have already filed an issue (blocker) yesterday and would 
> start working on it soon: https://issues.apache.org/jira/browse/MESOS-6623
> 
> I am happy to revert the patches if this is not what you had in mind.

Sorry about that! It sounds ok to me, as long as we follow up on MESOS-6623 
quickly.


- Benjamin


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


On Nov. 22, 2016, 12:27 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53491/
> ---
> 
> (Updated Nov. 22, 2016, 12:27 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-6466
> https://issues.apache.org/jira/browse/MESOS-6466
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Some tests that rely on filtering HTTP events based on type won't
> work now since the request body is not yet known when `visit()`
> is invoked. These would be fixed as part of a separate JIRA
> issue.
> 
> 
> Diffs
> -
> 
>   src/tests/master_tests.cpp c8cd89228eb4e55c9a9655f9de39cb070e14520c 
>   src/tests/slave_recovery_tests.cpp 99c27399297e3b04af167029d200d8ac418af2a6 
> 
> Diff: https://reviews.apache.org/r/53491/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 53610: Added health checks documentation.

[Neil Conway]

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




docs/health-checks.md (line 159)


"`grace_period_seconds` is the number of seconds after the task is launched 
during which health check failures are ignored. Note that once a health check 
succeeds for the first time, the grace period does not apply anymore."


- Neil Conway


On Nov. 20, 2016, 6:52 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53610/
> ---
> 
> (Updated Nov. 20, 2016, 6:52 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman, haosdent huang, Neil Conway, and 
> Till Toenshoff.
> 
> 
> Bugs: MESOS-5597
> https://issues.apache.org/jira/browse/MESOS-5597
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   docs/health-checks.md PRE-CREATION 
>   docs/home.md a5811480de050352dca6c0f7e4e64d3d2351c2d5 
> 
> Diff: https://reviews.apache.org/r/53610/diff/
> 
> 
> Testing
> ---
> 
> https://gist.github.com/rukletsov/7200c36b2fd1e81f78f2583e68b31fd1
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 53614: Updated the markdown style guide with headings capitalization.

[Neil Conway]

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


Ship it!




Ship It!

- Neil Conway


On Nov. 9, 2016, 5:08 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53614/
> ---
> 
> (Updated Nov. 9, 2016, 5:08 p.m.)
> 
> 
> Review request for mesos, Joerg Schad and Neil Conway.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   docs/markdown-style-guide.md 667d6dfbcf6c0864085cdfb19f4e53fb49a49c44 
> 
> Diff: https://reviews.apache.org/r/53614/diff/
> 
> 
> Testing
> ---
> 
> None: not a functional change.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 53613: Fixed style issues in quota.md.

[Neil Conway]

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


Fix it, then Ship it!





docs/quota.md (line 357)


I think the way this was before ("(see ...)") is actually better style.


- Neil Conway


On Nov. 9, 2016, 5:09 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53613/
> ---
> 
> (Updated Nov. 9, 2016, 5:09 p.m.)
> 
> 
> Review request for mesos, Joerg Schad and Neil Conway.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Correct caption hierarchy, update headings to conform to
> title case, and ensure two blank lines between sections.
> 
> 
> Diffs
> -
> 
>   docs/quota.md d7e8d58eeeac9d9ffc1a6f7614c7ddce6d7a9b89 
> 
> Diff: https://reviews.apache.org/r/53613/diff/
> 
> 
> Testing
> ---
> 
> None: not a functional change.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 53491: Disabled tests relying on filtering HTTP events.

[Anand Mazumdar]

> On Nov. 22, 2016, 8:35 p.m., Benjamin Mahler wrote:
> > This didn't have a ship it, I was expecting the test filter changes to be 
> > in place before we committed the changes. Are you working on these now?

Looks like there is some disconnect. We had an offline discussion at MesosCon 
China on this and I remember you agreeing that we can commit these patches with 
the tests disabled and I can work on the filtering changes in the interim. I 
have already filed an issue (blocker) yesterday and would start working on it 
soon: https://issues.apache.org/jira/browse/MESOS-6623

I am happy to revert the patches if this is not what you had in mind.


- Anand


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


On Nov. 22, 2016, 12:27 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53491/
> ---
> 
> (Updated Nov. 22, 2016, 12:27 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-6466
> https://issues.apache.org/jira/browse/MESOS-6466
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Some tests that rely on filtering HTTP events based on type won't
> work now since the request body is not yet known when `visit()`
> is invoked. These would be fixed as part of a separate JIRA
> issue.
> 
> 
> Diffs
> -
> 
>   src/tests/master_tests.cpp c8cd89228eb4e55c9a9655f9de39cb070e14520c 
>   src/tests/slave_recovery_tests.cpp 99c27399297e3b04af167029d200d8ac418af2a6 
> 
> Diff: https://reviews.apache.org/r/53491/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 53612: Fixed minor style issues in docs.

[Neil Conway]

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


Ship it!




Ship It!

- Neil Conway


On Nov. 9, 2016, 5:05 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53612/
> ---
> 
> (Updated Nov. 9, 2016, 5:05 p.m.)
> 
> 
> Review request for mesos, Joerg Schad and Neil Conway.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> According to our markdown style guide, Notes should be decorated as
> **NOTE:** and not __NOTE:__ and formatted without extra indentation
> or extra symbols.
> 
> 
> Diffs
> -
> 
>   docs/quota.md d7e8d58eeeac9d9ffc1a6f7614c7ddce6d7a9b89 
>   docs/reservation.md 20db7c97d5c9d651c6d79864e5093c2fd4bdfc04 
>   docs/sandbox.md af0036df05b31bc0bc028d283b7c5a0c60674327 
>   docs/ssl.md 2f7d928ef94b9fb611e071b01755c002554bad40 
> 
> Diff: https://reviews.apache.org/r/53612/diff/
> 
> 
> Testing
> ---
> 
> None: not a functional change.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 53997: Fix SSL downgrade pathway for temporary/persistent sockets.

[Benjamin Mahler]

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


Ship it!





3rdparty/libprocess/src/process.cpp (lines 2476 - 2477)


Do you want an else if here to match the close logic? Otherwise, maybe add 
back the newline here?


- Benjamin Mahler


On Nov. 22, 2016, 8:23 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53997/
> ---
> 
> (Updated Nov. 22, 2016, 8:23 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-6621
> https://issues.apache.org/jira/browse/MESOS-6621
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This fixes some potential CHECK failures when a libprocess process
> has (1) SSL downgrade enabled and (2) temporary and persistent
> connections open with the same remote address.  The second point is
> only possible if messages are to a remote address without a persistent
> connection and then a persistent connection is created.
> 
> The SSL downgrade path was only checking if the address of a socket
> matched when performing the downgrade.  The code must also check to
> see if the socket itself matches.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp 
> 6dc10dfcbcb712218fa42d9c7865e00f4a6df3b9 
> 
> Diff: https://reviews.apache.org/r/53997/diff/
> 
> 
> Testing
> ---
> 
> See related ticket for the clunky unit test.  Ran that test in repetition.
> 
> make check
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 53991: Added master API call for `UPDATE_QUOTA`.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [52284, 53679, 53691, 52103, 53991]

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 Nov. 22, 2016, 6:31 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53991/
> ---
> 
> (Updated Nov. 22, 2016, 6:31 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Xiaojian Huang.
> 
> 
> Bugs: MESOS-4941
> https://issues.apache.org/jira/browse/MESOS-4941
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added master API call for `UPDATE_QUOTA`.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/master.proto 3553c683c17004ac1831ec90271aa8584c950e53 
>   include/mesos/v1/master/master.proto 
> 022b491b7d5c49c5aeddf4ffc97c148f55629c95 
>   src/master/http.cpp 90cbed1ba411e18906fe9c26bc14576a26d1b7b9 
>   src/master/master.hpp b6c610b79aab146ece6b9be59be86f283d1c9ee8 
>   src/master/quota_handler.cpp 5578663f26d9b737499dc4f5a286c34c37645442 
>   src/master/validation.cpp 42d9b4a8784c2a161b74d7b46619cc22272e14e3 
>   src/tests/api_tests.cpp 8889e7807ecead736eaac3910332e86d594e8cec 
> 
> Diff: https://reviews.apache.org/r/53991/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 53877: Added temporary file environment passing towards docker.

[Till Toenshoff]

> On Nov. 22, 2016, 7:57 p.m., Kapil Arya wrote:
> > src/docker/docker.cpp, line 535
> > 
> >
> > Minor nit: Should we change `file` to something like 
> > `environmentVariables` or `environment` because it's a string, not a file.

Yeah, I struggled with that naming -- `environment` is taken, 
`environmentVariables` it is.


- Till


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


On Nov. 22, 2016, 7:36 p.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53877/
> ---
> 
> (Updated Nov. 22, 2016, 7:36 p.m.)
> 
> 
> Review request for mesos, Adam B, Gastón Kleiman, Jie Yu, Kapil Arya, and 
> Joseph Wu.
> 
> 
> Bugs: MESOS-6566
> https://issues.apache.org/jira/browse/MESOS-6566
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> see summary.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp f03ea7fa55e976e44282503261ac50e1502592a2 
> 
> Diff: https://reviews.apache.org/r/53877/diff/
> 
> 
> Testing
> ---
> 
> ```
> $ make check
> ```
> 
> ```
> $ ./src/mesos-execute --command="sleep 200" --containerizer=docker 
> --docker_image=alpine --env='{"foo": "bar"}' --master=149.202.202.185:5050
> ```
> 
> ```
> $ ps aux
> root 27645  0.2  0.0 372852 30180 ?Sl   15:50   0:00 docker -H 
> unix:///var/run/docker.sock run --cpu-shares 1024 --memory 134217728 
> --env-file /tmp/l53ILz/ktpuDS -v 
> /tmp/mesos/slaves/91b671fd-3c83-425e-96c2-26ecdc410028-S0/frameworks/91b671fd-3c83-425e-96c2-26ecdc410028-0001/executors/test/runs/a75fc411-3d18-44f1-a562-9f759c281da0:/mnt/mesos/sandbox
>  --net host --entrypoint /bin/sh --name 
> mesos-91b671fd-3c83-425e-96c2-26ecdc410028-S0.a75fc411-3d18-44f1-a562-9f759c281da0
>  alpine -c sleep 200
> ```
> 
> ```
> $ more /tmp/l53ILz/ktpuDS
> foo=bar
> MESOS_SANDBOX=/mnt/mesos/sandbox
> MESOS_CONTAINER_NAME=mesos-91b671fd-3c83-425e-96c2-26ecdc410028-S0.a75fc411-3d18-44f1-a562-9f759c281da0
> ```
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 53491: Disabled tests relying on filtering HTTP events.

[Benjamin Mahler]

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



This didn't have a ship it, I was expecting the test filter changes to be in 
place before we committed the changes. Are you working on these now?

- Benjamin Mahler


On Nov. 22, 2016, 12:27 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53491/
> ---
> 
> (Updated Nov. 22, 2016, 12:27 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-6466
> https://issues.apache.org/jira/browse/MESOS-6466
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Some tests that rely on filtering HTTP events based on type won't
> work now since the request body is not yet known when `visit()`
> is invoked. These would be fixed as part of a separate JIRA
> issue.
> 
> 
> Diffs
> -
> 
>   src/tests/master_tests.cpp c8cd89228eb4e55c9a9655f9de39cb070e14520c 
>   src/tests/slave_recovery_tests.cpp 99c27399297e3b04af167029d200d8ac418af2a6 
> 
> Diff: https://reviews.apache.org/r/53491/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 53610: Added health checks documentation.

[Neil Conway]

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




docs/health-checks.md (line 9)


"e.g.," not "e.g."



docs/health-checks.md (line 12)


Rather than saying "health check their tasks out-of-band", I'd say:

"some frameworks implement their own logic for checking the health of their 
tasks. This is typically done by having the framework scheduler send a "ping" 
request (e.g., via HTTP) to the host where the task is running and arranging 
for the task or executor to respond to the ping."

"though" => "Although"



docs/health-checks.md (line 19)


The phrase "incorporating network failures in health check information is 
not always desirable" is vague. What is the specific concern here?



docs/health-checks.md (line 21)


Isn't a major advantage of Mesos-native health checks is that you avoid the 
scalability problems of having a single scheduler handle the health checks for 
a potentially large number of tasks?



docs/health-checks.md (line 23)


I think this would benefit from some more discussion of the high-level 
architecture of Mesos-native health checks. For example:

* the traditional "scheduler health check" pattern involves a single 
scheduler node and a collection of agents; Mesos-native health checks run on 
the agent. This improves scalability but means that detecting network faults is 
a separate concern.
* when a task fails Mesos-native health checks, what happens to it? how 
does the framework scheduler learn about this?
* what happens if a task is running on a partitioned agent -- will it still 
be health-checked? If those health-checks fail, will the task be terminated?

Some of this is discussed below, but I think it would be better to briefly 
discuss it at the beginning of the document to set context for what follows.



docs/health-checks.md (line 26)


s/, as well as provides/. Mesos 1.2.0 also provides/



docs/health-checks.md (line 27)


"implementations for"



docs/health-checks.md (line 33)


"This technique allows detecting and reporting process crashes, but ..."



docs/health-checks.md (line 46)


s/nor/or/



docs/health-checks.md (line 56)


"to honor the `HealthCheck` field in `TaskInfo`"

I'd also strike "and to implement health checks" as redundant.



docs/health-checks.md (line 58)


"the reference implementation for"



docs/health-checks.md (line 65)


"The command is" -> "A command health check specifies an arbitrary command 
that is used to validate the health of the task. The executor launches the 
command and inspects its exit status: `0` is treated as success (the task is 
healthy), while any other exit status interpreted to mean the task is 
unhealthy."



docs/health-checks.md (line 98)


"e.g.,"



docs/health-checks.md (line 202)


Can we elaborate here -- that means a task that has failed health checks 
will typically be `RUNNING` with `healthy == false`? Is it possible to see 
other task states where the `health` field is set to false?



docs/health-checks.md (line 206)


"all unhealthy status updates"

"as well as the first healthy update"

"i.e., when the task has started, or after one or more unhealthy updates 
have occurred"



docs/health-checks.md (line 208)


/opt for/use/



docs/health-checks.md (line 254)


I wouldn't use an exclamation point here.



docs/health-checks.md (line 263)


"large value"



docs/health-checks.md (line 264)


'introduce a "global" policy'



docs/health-checks.md (line 267)


Why do they have to listen on all interfaces? i.e., listening on 127.0.0.1 
as well as whatever service interface/address they require should be 
sufficient, no?


- Neil Conway


On Nov. 20, 2016, 6:52 p.m., Alexander 

Re: Review Request 53950: Avoided hardcoding ports in some tests.

[Gastón Kleiman]

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


Ship it!




Ship It!

- Gastón Kleiman


On Nov. 22, 2016, 7:49 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53950/
> ---
> 
> (Updated Nov. 22, 2016, 7:49 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and haosdent huang.
> 
> 
> Bugs: MESOS-6618
> https://issues.apache.org/jira/browse/MESOS-6618
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Avoided hardcoding ports in some tests.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> c478d56ffd734b26fd2dbd7bb1ca02ce929f3f16 
>   src/tests/health_check_tests.cpp 2c7022a1210ade6d7eb594c4f8934f2bdb2aee35 
> 
> Diff: https://reviews.apache.org/r/53950/diff/
> 
> 
> Testing
> ---
> 
> make check (SSL build OS X clang trunk w/ optimizations; default build 
> ubuntu-14.04 gcc-4.8.4 w/o optimizations)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 53998: Fixed hook to allow for executor environment modifications.

[Till Toenshoff]

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

(Updated Nov. 22, 2016, 8:26 p.m.)


Review request for mesos, Adam B, Jie Yu, Kapil Arya, and Joseph Wu.


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


Repository: mesos


Description (updated)
---

When being used on a task run by a command executor, the hook
`slavePreLaunchDockerEnvironmentDecorator` did not allow for extending
the executor's own environment but only the environment of the task.
By using the hook returned environment for the executor in any case
and supplying a task specific environment for the command executor,
that limitation is fixed.


Diffs
-

  src/slave/containerizer/docker.cpp c2ed5240aab9ea9d1a386c44c94e5ae7e98d313c 

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


Testing
---

make check

* WIP - functional testing pending - WIP *


Thanks,

Till Toenshoff

Re: Review Request 53877: Added temporary file environment passing towards docker.

[Gastón Kleiman]

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




src/docker/docker.cpp (line 535)


+1



src/docker/docker.cpp (line 562)


`const string&`


- Gastón Kleiman


On Nov. 22, 2016, 7:36 p.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53877/
> ---
> 
> (Updated Nov. 22, 2016, 7:36 p.m.)
> 
> 
> Review request for mesos, Adam B, Gastón Kleiman, Jie Yu, Kapil Arya, and 
> Joseph Wu.
> 
> 
> Bugs: MESOS-6566
> https://issues.apache.org/jira/browse/MESOS-6566
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> see summary.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp f03ea7fa55e976e44282503261ac50e1502592a2 
> 
> Diff: https://reviews.apache.org/r/53877/diff/
> 
> 
> Testing
> ---
> 
> ```
> $ make check
> ```
> 
> ```
> $ ./src/mesos-execute --command="sleep 200" --containerizer=docker 
> --docker_image=alpine --env='{"foo": "bar"}' --master=149.202.202.185:5050
> ```
> 
> ```
> $ ps aux
> root 27645  0.2  0.0 372852 30180 ?Sl   15:50   0:00 docker -H 
> unix:///var/run/docker.sock run --cpu-shares 1024 --memory 134217728 
> --env-file /tmp/l53ILz/ktpuDS -v 
> /tmp/mesos/slaves/91b671fd-3c83-425e-96c2-26ecdc410028-S0/frameworks/91b671fd-3c83-425e-96c2-26ecdc410028-0001/executors/test/runs/a75fc411-3d18-44f1-a562-9f759c281da0:/mnt/mesos/sandbox
>  --net host --entrypoint /bin/sh --name 
> mesos-91b671fd-3c83-425e-96c2-26ecdc410028-S0.a75fc411-3d18-44f1-a562-9f759c281da0
>  alpine -c sleep 200
> ```
> 
> ```
> $ more /tmp/l53ILz/ktpuDS
> foo=bar
> MESOS_SANDBOX=/mnt/mesos/sandbox
> MESOS_CONTAINER_NAME=mesos-91b671fd-3c83-425e-96c2-26ecdc410028-S0.a75fc411-3d18-44f1-a562-9f759c281da0
> ```
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Review Request 53997: Fix SSL downgrade pathway for temporary/persistent sockets.

[Joseph Wu]

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

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


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


Repository: mesos


Description
---

This fixes some potential CHECK failures when a libprocess process
has (1) SSL downgrade enabled and (2) temporary and persistent
connections open with the same remote address.  The second point is
only possible if messages are to a remote address without a persistent
connection and then a persistent connection is created.

The SSL downgrade path was only checking if the address of a socket
matched when performing the downgrade.  The code must also check to
see if the socket itself matches.


Diffs
-

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

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


Testing
---

See related ticket for the clunky unit test.  Ran that test in repetition.

make check


Thanks,

Joseph Wu

Review Request 53998: Fixed hook to allow for executor environment modifications.

[Till Toenshoff]

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

Review request for mesos, Adam B, Jie Yu, Kapil Arya, and Joseph Wu.


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


Repository: mesos


Description
---

When being used on a task run by a command executor, the hook
`slavePreLaunchDockerEnvironmentDecorator` did now allow for extending
the executor's own environment but only the environment of the task.
By using the hook returned environment for the executor in any case
and supplying a task specific environment for the command executor,
that limitation is fixed.


Diffs
-

  src/slave/containerizer/docker.cpp c2ed5240aab9ea9d1a386c44c94e5ae7e98d313c 

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


Testing
---

make check

* WIP - functional testing pending - WIP *


Thanks,

Till Toenshoff

Re: Review Request 53758: CMake: Added test sources to the build.

[Joseph Wu]

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

(Updated Nov. 22, 2016, noon)


Review request for mesos, Alex Clemmer and Joris Van Remoortere.


Changes
---

Add kill policy tests helper to the test util sources.


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


Repository: mesos


Description
---

This adds most existing tests to the CMake build.  This excludes
a few special cases, just as Java, Python, and network isolator
tests.

This review replaces: https://reviews.apache.org/r/49921/


Diffs (updated)
-

  CMakeLists.txt 1362d65afc23816c34173866a3fdbce45936921d 
  src/CMakeLists.txt aef9ae6d2872dc15725c01ce85b657965485605f 
  src/tests/CMakeLists.txt 90b92251b7e133124f6d9a896190ceea402cbff1 
  src/tests/cmake/MesosTestsConfigure.cmake 
3f543c010ae87ff04e6b45745bc49ef65b6590ff 

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


Testing
---

cmake ..
make


Thanks,

Joseph Wu

Re: Review Request 53805: Updated libprocess test to use new 'Socket::shutdown' parameter.

[Greg Mann]

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

(Updated Nov. 22, 2016, 7:57 p.m.)


Review request for mesos, Benjamin Mahler and Joseph Wu.


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


Repository: mesos


Description
---

This patch updates `HTTPTest.SocketEOF` to remove a sleep
and make use of `Socket::shutdown` with the new `how`
parameter.


Diffs
-

  3rdparty/libprocess/src/tests/http_tests.cpp 
533104c93dd1eaf67bf3752163d2e0cad090078d 

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


Testing (updated)
---

All testing was done on CentOS 7:

`make check` ran and all tests passed.

`3rdparty/libprocess/libprocess-tests --gtest_repeat=-1 
--gtest_break_on_failure` ran for several hundred repetitions with no failures.

`3rdparty/libprocess/libprocess-tests --gtest_filter="*HTTPTest*" 
--gtest_repeat=-1 --gtest_break_on_failure` ran for about 1,000 repetitions 
without failing.

`3rdparty/libprocess/libprocess-tests --gtest_filter="*HTTPTest.Endpoints/*" 
--gtest_repeat=-1 --gtest_break_on_failure` ran for about 10,000 repetitions 
without failing.

`bin/mesos-tests.sh --gtest_repeat=-1 --gtest_break_on_failure` ran for several 
hundred repetitions with no failures.

`bin/mesos-tests.sh --gtest_filter="*SchedulerSSLTest*" --gtest_repeat=-1 
--gtest_break_on_failure` ran for about 2,000 repetitions without failing.


Thanks,

Greg Mann

Re: Review Request 53877: Added temporary file environment passing towards docker.

[Kapil Arya]

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


Ship it!





src/docker/docker.cpp (line 535)


Minor nit: Should we change `file` to something like `environmentVariables` 
or `environment` because it's a string, not a file.


- Kapil Arya


On Nov. 22, 2016, 2:36 p.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53877/
> ---
> 
> (Updated Nov. 22, 2016, 2:36 p.m.)
> 
> 
> Review request for mesos, Adam B, Gastón Kleiman, Jie Yu, Kapil Arya, and 
> Joseph Wu.
> 
> 
> Bugs: MESOS-6566
> https://issues.apache.org/jira/browse/MESOS-6566
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> see summary.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp f03ea7fa55e976e44282503261ac50e1502592a2 
> 
> Diff: https://reviews.apache.org/r/53877/diff/
> 
> 
> Testing
> ---
> 
> ```
> $ make check
> ```
> 
> ```
> $ ./src/mesos-execute --command="sleep 200" --containerizer=docker 
> --docker_image=alpine --env='{"foo": "bar"}' --master=149.202.202.185:5050
> ```
> 
> ```
> $ ps aux
> root 27645  0.2  0.0 372852 30180 ?Sl   15:50   0:00 docker -H 
> unix:///var/run/docker.sock run --cpu-shares 1024 --memory 134217728 
> --env-file /tmp/l53ILz/ktpuDS -v 
> /tmp/mesos/slaves/91b671fd-3c83-425e-96c2-26ecdc410028-S0/frameworks/91b671fd-3c83-425e-96c2-26ecdc410028-0001/executors/test/runs/a75fc411-3d18-44f1-a562-9f759c281da0:/mnt/mesos/sandbox
>  --net host --entrypoint /bin/sh --name 
> mesos-91b671fd-3c83-425e-96c2-26ecdc410028-S0.a75fc411-3d18-44f1-a562-9f759c281da0
>  alpine -c sleep 200
> ```
> 
> ```
> $ more /tmp/l53ILz/ktpuDS
> foo=bar
> MESOS_SANDBOX=/mnt/mesos/sandbox
> MESOS_CONTAINER_NAME=mesos-91b671fd-3c83-425e-96c2-26ecdc410028-S0.a75fc411-3d18-44f1-a562-9f759c281da0
> ```
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 50857: Modified a scheduler test to run with SSL enabled.

[Greg Mann]

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

(Updated Nov. 22, 2016, 7:56 p.m.)


Review request for mesos, Joseph Wu and Vinod Kone.


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


Repository: mesos


Description
---

This patch modifies the test SchedulerTest.Teardown to
be parametrized by both ContentType and SSL configuration,
and renames it to SchedulerSSLTest.RunTaskAndTeardown.
This allows the test to verify the scheduler's behavior with
SSL both enabled and disabled.


Diffs
-

  src/tests/scheduler_tests.cpp c031823843a8ae5ecbccf60b9edf83ec1afcc5e6 

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


Testing (updated)
---

The test was run in repetition as follows:

`GTEST_REPEAT=-1 GTEST_BREAK_ON_FAILURE=1 
GTEST_FILTER="ContentTypeAndSSLConfig/SchedulerSSLTest.RunTaskAndTeardown*" 
bin/mesos-tests.sh`

More testing details are included at the end of this chain.


Thanks,

Greg Mann

Re: Review Request 50969: Made use of SSL flags to determine scheduler/executor scheme.

[Greg Mann]

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

(Updated Nov. 22, 2016, 7:56 p.m.)


Review request for mesos, Joseph Wu and Vinod Kone.


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


Repository: mesos


Description
---

This patch updates the HTTP scheduler and executor
libraries to use process::network::openssl::flags()
to determine whether or not SSL is enabled.


Diffs
-

  src/executor/executor.cpp 1d47b52d89eedee59d160badd6313d34e3bdb6d2 
  src/scheduler/scheduler.cpp 4b6fae14fb88df8e541f9e32fa4cfe93496a3d36 

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


Testing (updated)
---

Testing details are included at the end of this chain.


Thanks,

Greg Mann

Re: Review Request 50737: Parametrized libprocess HTTPTests by SSL configuration.

[Greg Mann]

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

(Updated Nov. 22, 2016, 7:56 p.m.)


Review request for mesos, Anand Mazumdar, Joseph Wu, and Vinod Kone.


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


Repository: mesos


Description
---

This patch parametrizes the HTTP tests in libprocess so
that they run with SSL both enabled and disabled when
the library was compiled with SSL support.


Diffs
-

  3rdparty/libprocess/src/tests/http_tests.cpp 
533104c93dd1eaf67bf3752163d2e0cad090078d 

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


Testing (updated)
---

Testing details are found at the end of this chain.


Thanks,

Greg Mann

Re: Review Request 51065: Changed hostname used for SSL cert creation in tests.

[Greg Mann]

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

(Updated Nov. 22, 2016, 7:55 p.m.)


Review request for mesos, Joseph Wu and Vinod Kone.


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


Repository: mesos


Description
---

This changes the SSL helpers in libprocess to generate
certs using a hostname determined using the same address
that is advertised by libprocess. This assures that
validation of the certificates will succeed. The test
`SSLTest.BasicSameProcess` is also updated to accommodate
this change.


Diffs
-

  3rdparty/libprocess/include/process/ssl/gtest.hpp 
21a0fc45b55a368a21b3e616c751ab43eebd4902 
  3rdparty/libprocess/src/tests/ssl_tests.cpp 
55c8c309571b1892b0acc4d766eda9bb98085a6f 

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


Testing (updated)
---

Testing details are found at the end of this chain.


Thanks,

Greg Mann

Re: Review Request 53802: Eliminated an EOF race condition in libprocess SSL socket.

[Greg Mann]

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

(Updated Nov. 22, 2016, 7:55 p.m.)


Review request for mesos, Benjamin Mahler and Joseph Wu.


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


Repository: mesos


Description (updated)
---

Previously, it was possible for an SSL socket to either:
1) Fail to receive an EOF if the EOF event was received when
   there was no pending recv() request.
2) Fail to receive all data sent on the sending side if an
   EOF event was received before all sent data was read.

This patch eliminates these race conditions to ensure reliable
receipt of both sent data and EOFs.


Diffs
-

  3rdparty/libprocess/src/libevent_ssl_socket.hpp 
acb00d41c637a318b2f16fff9e97998b9c79b809 
  3rdparty/libprocess/src/libevent_ssl_socket.cpp 
21f878ee81db32ad35878ec053c3f2de3637196c 

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


Testing (updated)
---

Testing details are found at the end of this chain.


Thanks,

Greg Mann

Re: Review Request 53817: Added missing cleanup to libprocess 'finalize()'.

[Greg Mann]

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

(Updated Nov. 22, 2016, 7:54 p.m.)


Review request for mesos, Benjamin Mahler, Joseph Wu, and Vinod Kone.


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


Repository: mesos


Description (updated)
---

The `finalize()` function in libprocess previously did
not delete the thread-local `Executor` which is used to
defer execution to an unspecified context. This object
must be deleted during finalization so that the
`__executor__` macro creates a new process if libprocess
is subsequently reinitialized.


Diffs
-

  3rdparty/libprocess/src/process.cpp ab2b5a9d38a3001d6a5daa1807fecb630c4b154d 

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


Testing (updated)
---

Testing details are found at the end of this chain.


Thanks,

Greg Mann

Re: Review Request 53610: Added health checks documentation.

[Neil Conway]

> On Nov. 9, 2016, 5:32 p.m., haosdent huang wrote:
> > docs/health-checks.md, line 8
> > 
> >
> > `misbehave, or become unresponsive` should be `misbehave or become 
> > unresponsive`?

I think this is better as-is ("Oxford comma" style). Or at any rate we should 
just pick a style and be consistent.


> On Nov. 9, 2016, 5:32 p.m., haosdent huang wrote:
> > docs/health-checks.md, line 6
> > 
> >
> > Is `Health Checking For Tasks` a better title?

Agreed.


- Neil


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


On Nov. 20, 2016, 6:52 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53610/
> ---
> 
> (Updated Nov. 20, 2016, 6:52 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman, haosdent huang, Neil Conway, and 
> Till Toenshoff.
> 
> 
> Bugs: MESOS-5597
> https://issues.apache.org/jira/browse/MESOS-5597
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   docs/health-checks.md PRE-CREATION 
>   docs/home.md a5811480de050352dca6c0f7e4e64d3d2351c2d5 
> 
> Diff: https://reviews.apache.org/r/53610/diff/
> 
> 
> Testing
> ---
> 
> https://gist.github.com/rukletsov/7200c36b2fd1e81f78f2583e68b31fd1
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>