Re: Review Request 61290: Linked `mesos` target to `stout` interface library.

2017-08-07 Thread Joseph Wu

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


Ship it!




[Tentative "Ship It!" as I review this entire chain.]

Going to mention the motivation for this change being the fact that stout has a 
CMakeLists file now.

- Joseph Wu


On Aug. 1, 2017, 5:53 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61290/
> ---
> 
> (Updated Aug. 1, 2017, 5:53 p.m.)
> 
> 
> Review request for mesos and Joseph Wu.
> 
> 
> Bugs: MESOS-7811
> https://issues.apache.org/jira/browse/MESOS-7811
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Linked `mesos` target to `stout` interface library.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt f4feaf90b4750338cfa2127524a5e6556a86d063 
> 
> 
> Diff: https://reviews.apache.org/r/61290/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>



Re: Review Request 61463: Fixed a bug in the test `NamespacesIsolatorTest.ROOT_PidNamespace`.

2017-08-07 Thread Qian Zhang


> On Aug. 8, 2017, 8:05 a.m., Gastón Kleiman wrote:
> > src/tests/containerizer/isolator_tests.cpp
> > Line 176 (original), 176 (patched)
> > 
> >
> > `/proc/self/ns/ipc` is also a symlink, should we change this line too?

Good catch!


- Qian


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


On Aug. 7, 2017, 11:12 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61463/
> ---
> 
> (Updated Aug. 7, 2017, 11:12 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman, Gilbert Song, Jie Yu, Kevin Klues, 
> and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In this test, we used the command `stat -c %i /proc/self/ns/pid` to
> get the current process's pid namespace inode, that is not correct
> since `/proc/self/ns/pid` is a symbol link, we should get the inode
> that the link references rather than the link itself. So we need to
> add a `-L` option in the `stat` command.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/isolator_tests.cpp 
> f3c541c92f8ecc5c12356363bbbf3561d3b75230 
> 
> 
> Diff: https://reviews.apache.org/r/61463/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>



Re: Review Request 61289: Removed unnecessary includes of `StoutConfigure`.

2017-08-07 Thread Joseph Wu

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


Ship it!




[Tentative "Ship It!" as I review this entire chain.]

Going to mention stout being an interface library as a reason why these lines 
are unnecessary.

- Joseph Wu


On Aug. 1, 2017, 5:53 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61289/
> ---
> 
> (Updated Aug. 1, 2017, 5:53 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed unnecessary includes of `StoutConfigure`.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt f4feaf90b4750338cfa2127524a5e6556a86d063 
>   src/master/cmake/MasterConfigure.cmake 
> 173dc36b5a1b7ff23cd0542ea4313ae6e5e0ae3f 
>   src/slave/cmake/AgentConfigure.cmake 
> af2f74f82b68920427f36c0c863b97c0191b8f06 
> 
> 
> Diff: https://reviews.apache.org/r/61289/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>



Re: Review Request 61288: Linked `libprocess` to `stout` interface library.

2017-08-07 Thread Joseph Wu

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




3rdparty/libprocess/src/CMakeLists.txt
Line 115 (original)


Note to self: If there is a review later in the chain that removes the 
other source grouping from this file, squash this line in with that review.


- Joseph Wu


On Aug. 1, 2017, 5:53 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61288/
> ---
> 
> (Updated Aug. 1, 2017, 5:53 p.m.)
> 
> 
> Review request for mesos and Joseph Wu.
> 
> 
> Bugs: MESOS-7811
> https://issues.apache.org/jira/browse/MESOS-7811
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Linked `libprocess` to `stout` interface library.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake 
> 3e621f97b683012beca5705fbf3e4000d686f3ab 
>   3rdparty/libprocess/cmake/ProcessConfigure.cmake 
> 75af774e1460319cef1efdf801eb97f34eada0e7 
>   3rdparty/libprocess/cmake/ProcessTestsConfigure.cmake 
> 5a23990825de04f27bd5f74c82e10b702974f875 
>   3rdparty/libprocess/src/CMakeLists.txt 
> fbad7a0dd0347a308ba504ff7daeed187dd1f94d 
> 
> 
> Diff: https://reviews.apache.org/r/61288/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>



Re: Review Request 61288: Linked `libprocess` to `stout` interface library.

2017-08-07 Thread Joseph Wu

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


Ship it!




[Tentative "Ship It!" as I review this entire chain.]

I'm going to mention https://reviews.apache.org/r/61286/ in the commit 
description.  (And I might tweak your chain's dependencies a little later, so 
that they are logically grouped.)

- Joseph Wu


On Aug. 1, 2017, 5:53 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61288/
> ---
> 
> (Updated Aug. 1, 2017, 5:53 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Linked `libprocess` to `stout` interface library.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake 
> 3e621f97b683012beca5705fbf3e4000d686f3ab 
>   3rdparty/libprocess/cmake/ProcessConfigure.cmake 
> 75af774e1460319cef1efdf801eb97f34eada0e7 
>   3rdparty/libprocess/cmake/ProcessTestsConfigure.cmake 
> 5a23990825de04f27bd5f74c82e10b702974f875 
>   3rdparty/libprocess/src/CMakeLists.txt 
> fbad7a0dd0347a308ba504ff7daeed187dd1f94d 
> 
> 
> Diff: https://reviews.apache.org/r/61288/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>



Re: Review Request 61287: Removed `GroupSource` from `stout`.

2017-08-07 Thread Joseph Wu

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


Ship it!




[Tentative "Ship It!" as I review this entire chain.]

I'm going to mention that the source grouping (as it is currently) is still 
somewhat brittle and isn't necessary for the CMake MVP.  That's why we're 
removing it for now.

- Joseph Wu


On Aug. 1, 2017, 5:53 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61287/
> ---
> 
> (Updated Aug. 1, 2017, 5:53 p.m.)
> 
> 
> Review request for mesos and Joseph Wu.
> 
> 
> Bugs: MESOS-6721
> https://issues.apache.org/jira/browse/MESOS-6721
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed `GroupSource` from `stout`.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/cmake/StoutConfigure.cmake 
> 69474a830e3cbd2e3e2718e4a7569137fc6df6b8 
>   3rdparty/stout/tests/CMakeLists.txt 
> 953be64f1fb675fd1166fa77d0b3a56a5763d243 
> 
> 
> Diff: https://reviews.apache.org/r/61287/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>



Re: Review Request 61286: Added `stout` interface library.

2017-08-07 Thread Joseph Wu

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


Ship it!




[Tentative "Ship It!" as I review this entire chain.]

- Joseph Wu


On Aug. 1, 2017, 5:54 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61286/
> ---
> 
> (Updated Aug. 1, 2017, 5:54 p.m.)
> 
> 
> Review request for mesos and Joseph Wu.
> 
> 
> Bugs: MESOS-7811
> https://issues.apache.org/jira/browse/MESOS-7811
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> CMake provides a mechanism for dealing with header only libraries such
> as `stout`: an interface library. Thus CMake will treat `stout` as a
> "library" and automatically add the necessary compiler and link
> information to targets which depend on `stout`.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/CMakeLists.txt PRE-CREATION 
>   3rdparty/stout/cmake/StoutConfigure.cmake 
> 69474a830e3cbd2e3e2718e4a7569137fc6df6b8 
>   3rdparty/stout/cmake/StoutTestsConfigure.cmake 
> 43baaca784063773fdd700365cadb3532e5c6b3f 
>   3rdparty/stout/tests/CMakeLists.txt 
> 953be64f1fb675fd1166fa77d0b3a56a5763d243 
> 
> 
> Diff: https://reviews.apache.org/r/61286/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>



Re: Review Request 61433: Checked openssl and zlib as required libraries for Mesos.

2017-08-07 Thread Chun-Hung Hsiao

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

(Updated Aug. 8, 2017, 12:44 a.m.)


Review request for mesos, Benjamin Bannier and Jie Yu.


Summary (updated)
-

Checked openssl and zlib as required libraries for Mesos.


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


Repository: mesos


Description (updated)
---

This patch is for grpc support in Mesos. Since grpc depends on protobuf,
ssl and zlib, we make ssl and zlib required packages for Mesos to build.
Despite of the requirement changes, the `--enable-ssl` and
`--enable-zlib` flags are still kept, since these flags control whether
we want to compress and encrypt the communications.


Diffs
-

  configure.ac b2eeedab65d70718451cb8bbe556794b6d8e8db6 


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


Testing
---

make


Thanks,

Chun-Hung Hsiao



Re: Review Request 61433: Checked openssl and zlib as required libraries.

2017-08-07 Thread Chun-Hung Hsiao

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

(Updated Aug. 8, 2017, 12:37 a.m.)


Review request for mesos, Benjamin Bannier and Jie Yu.


Changes
---

Avoided setting `-rpath`.


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


Repository: mesos


Description (updated)
---

This patch is for grpc support in Mesos. Since grpc depends on protobuf,
ssl and zlib, we make ssl and zlib required packages for Mesos to build.
Despite of the requirement changes, The the `--enable-ssl` and
`--enable-zlib` flags are still kept, since these flags control whether
we want to compress and encrypt the communications.


Diffs (updated)
-

  configure.ac b2eeedab65d70718451cb8bbe556794b6d8e8db6 


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

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


Testing
---

make


Thanks,

Chun-Hung Hsiao



Re: Review Request 61098: Added unit tests for gRPC support in libprocess.

2017-08-07 Thread Chun-Hung Hsiao

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

(Updated Aug. 8, 2017, 12:36 a.m.)


Review request for mesos, Benjamin Mahler, Jie Yu, Joseph Wu, and Zhitao Li.


Changes
---

Addressed Jie's comments.


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


Repository: mesos


Description
---

We tested the following 6 scenarios when making gRPC calls:

1. The server responds with an OK.
2. The server responds with a failure.
3. The client discards the call before the server starts.
4. The client discards the call when the server is processing it.
5. The client makes mulitple concurrent calls.
6. The client shuts down during a call.
7. The server is unreachable.
8. The server dose not respond in time.


Diffs (updated)
-

  3rdparty/libprocess/src/tests/grpc_tests.cpp PRE-CREATION 
  3rdparty/libprocess/src/tests/grpc_tests.proto PRE-CREATION 


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

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


Testing
---

N/A


Thanks,

Chun-Hung Hsiao



Re: Review Request 61463: Fixed a bug in the test `NamespacesIsolatorTest.ROOT_PidNamespace`.

2017-08-07 Thread Gastón Kleiman

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


Fix it, then Ship it!





src/tests/containerizer/isolator_tests.cpp
Line 176 (original), 176 (patched)


`/proc/self/ns/ipc` is also a symlink, should we change this line too?


- Gastón Kleiman


On Aug. 7, 2017, 3:12 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61463/
> ---
> 
> (Updated Aug. 7, 2017, 3:12 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman, Gilbert Song, Jie Yu, Kevin Klues, 
> and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In this test, we used the command `stat -c %i /proc/self/ns/pid` to
> get the current process's pid namespace inode, that is not correct
> since `/proc/self/ns/pid` is a symbol link, we should get the inode
> that the link references rather than the link itself. So we need to
> add a `-L` option in the `stat` command.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/isolator_tests.cpp 
> f3c541c92f8ecc5c12356363bbbf3561d3b75230 
> 
> 
> Diff: https://reviews.apache.org/r/61463/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>



Re: Review Request 61463: Fixed a bug in the test `NamespacesIsolatorTest.ROOT_PidNamespace`.

2017-08-07 Thread Gilbert Song

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


Ship it!




Ship It!

- Gilbert Song


On Aug. 7, 2017, 8:12 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61463/
> ---
> 
> (Updated Aug. 7, 2017, 8:12 a.m.)
> 
> 
> Review request for mesos, Gastón Kleiman, Gilbert Song, Jie Yu, Kevin Klues, 
> and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In this test, we used the command `stat -c %i /proc/self/ns/pid` to
> get the current process's pid namespace inode, that is not correct
> since `/proc/self/ns/pid` is a symbol link, we should get the inode
> that the link references rather than the link itself. So we need to
> add a `-L` option in the `stat` command.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/isolator_tests.cpp 
> f3c541c92f8ecc5c12356363bbbf3561d3b75230 
> 
> 
> Diff: https://reviews.apache.org/r/61463/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>



Re: Review Request 61284: Made `libprocess` use default linking.

2017-08-07 Thread Joseph Wu

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


Ship it!




[Tentative "Ship It!" as I review this entire chain.]

Because I like to add context to commit messages, I'm going to extend your 
commit message to mention why these things are redundant:
(: the `BUILD_SHARED_LIBS` and `CMAKE_POSITION_INDEPENDENT_CODE` global 
variables :)

- Joseph Wu


On Aug. 1, 2017, 5:52 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61284/
> ---
> 
> (Updated Aug. 1, 2017, 5:52 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The static/shared specifier and the PIC property were redundant.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/CMakeLists.txt 
> fbad7a0dd0347a308ba504ff7daeed187dd1f94d 
> 
> 
> Diff: https://reviews.apache.org/r/61284/diff/1/
> 
> 
> Testing
> ---
> 
> Actually, this whole file was rewritten later, with this change included. 
> Could be squashed.
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>



Re: Review Request 61428: Added pid ns sharing based on agent flag and protobuf message field.

2017-08-07 Thread Gilbert Song


> On Aug. 4, 2017, 5:33 p.m., Gilbert Song wrote:
> > src/slave/containerizer/mesos/isolators/namespaces/pid.cpp
> > Lines 130 (patched)
> > 
> >
> > Could we reverse two logics above? so that we can avoid the size check 
> > here. E.g.,
> > ```
> > if (sharePidNamespace) {
> >   return launchInfo;
> > }
> > ```
> > 
> > similar to the short circuit logic for DEBUG container.
> 
> Qian Zhang wrote:
> Could you elaborate a bit more? Which two logics are you talking about?

Do you think this logic looks clearer (please help verify its correctness 
first)?
```
  ContainerLaunchInfo launchInfo;

  bool sharePidNamespace =
containerConfig.container_info().linux_info().share_pid_namespace();

  if (containerId.has_parent()) {
launchInfo.add_enter_namespaces(CLONE_NEWPID);

if (containerConfig.has_container_class() &&
containerConfig.container_class() == ContainerClass::DEBUG) {
  return launchInfo;
}
  } else {
if (flags.disallow_sharing_agent_pid_namespace && sharePidNamespace) {
  return Failure(
  "Sharing agent pid namespace with "
  "top-level container is not allowed");
}
  }

  if (sharePidNamespace) {
return launchInfo;
  }

  launchInfo.add_clone_namespaces(CLONE_NEWPID);
  launchInfo.add_pre_exec_commands()->set_value(
  "mount -n -t proc proc /proc -o nosuid,noexec,nodev");

  return launchInfo;
```


- Gilbert


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


On Aug. 6, 2017, 7:55 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61428/
> ---
> 
> (Updated Aug. 6, 2017, 7:55 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman, Gilbert Song, Jie Yu, Kevin Klues, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-7853
> https://issues.apache.org/jira/browse/MESOS-7853
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added pid ns sharing based on agent flag and protobuf message field.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/namespaces/pid.hpp 
> 2b316dbdf4a3735771af5bed80c6251d0d1cbd50 
>   src/slave/containerizer/mesos/isolators/namespaces/pid.cpp 
> f1dfc9f7398ffc029d7180d7f014a515338cb3f4 
> 
> 
> Diff: https://reviews.apache.org/r/61428/diff/4/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>



Review Request 61484: Added a benchmark for ns::clone.

2017-08-07 Thread Jie Yu

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

Review request for mesos, Benjamin Mahler, Greg Mann, and Joseph Wu.


Repository: mesos


Description
---

This can be used to reproduce the bug described in MESOS-7858.


Diffs
-

  src/tests/containerizer/ns_tests.cpp 0c748960488e1ef8356273a0f7ff6e0aaa21ad47 


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


Testing
---

Manually run the benchmark


Thanks,

Jie Yu



Re: Review Request 61434: Updated health-checks.md to include description of generalized checks.

2017-08-07 Thread Mesos Reviewbot Windows

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



Patch looks great!

Reviews applied: [61434]

Passed command: support\windows-build.bat

- Mesos Reviewbot Windows


On Aug. 4, 2017, 6:14 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61434/
> ---
> 
> (Updated Aug. 4, 2017, 6:14 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman, Greg Mann, and Vinod Kone.
> 
> 
> Bugs: MESOS-7349
> https://issues.apache.org/jira/browse/MESOS-7349
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   docs/health-checks.md d0749347d1f9398004e56b03503d655b5fba75f4 
> 
> 
> Diff: https://reviews.apache.org/r/61434/diff/1/
> 
> 
> Testing
> ---
> 
> https://gist.github.com/rukletsov/517a136cb6a0e8b909db30b36b13eded
> 
> Additionally rendered in MacDown.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>



Review Request 61483: Added a test using CMD health checks + DefaultExecutor w/ Docker image.

2017-08-07 Thread Gastón Kleiman

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

Review request for mesos, Alexander Rukletsov and Vinod Kone.


Repository: mesos


Description
---

Added a test using CMD health checks + DefaultExecutor w/ Docker image.


Diffs
-

  src/tests/health_check_tests.cpp e824d1b2c2b17423394da0b089ae8940450378b7 


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


Testing
---

`bin/mesos-tests.sh 
--gtest_filter="HealthCheckTest.DefaultExecutorWithDockerImageCommandHealthCheck"`


Thanks,

Gastón Kleiman



Re: Review Request 61477: Used async signal safe assert in ns::clone.

2017-08-07 Thread Joseph Wu

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


Ship it!




Ship It!

- Joseph Wu


On Aug. 7, 2017, 12:07 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61477/
> ---
> 
> (Updated Aug. 7, 2017, 12:07 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Joseph Wu.
> 
> 
> Bugs: MESOS-7858
> https://issues.apache.org/jira/browse/MESOS-7858
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Used async signal safe assert in ns::clone.
> 
> 
> Diffs
> -
> 
>   src/linux/ns.hpp 941c7ee243e0e486ee263a9e6c4f6d0a7398427d 
> 
> 
> Diff: https://reviews.apache.org/r/61477/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>



Re: Review Request 60003: Reduced copying in defer, dispatch and Future.

2017-08-07 Thread Mesos Reviewbot Windows

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



Patch looks great!

Reviews applied: [60002, 60003]

Passed command: support\windows-build.bat

- Mesos Reviewbot Windows


On Aug. 4, 2017, 10:40 a.m., Dmitry Zhuk wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60003/
> ---
> 
> (Updated Aug. 4, 2017, 10:40 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, haosdent huang, James Peach, and 
> Michael Park.
> 
> 
> Bugs: MESOS-7713
> https://issues.apache.org/jira/browse/MESOS-7713
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This reduces number of copies made for each parameter in
> a piece of code like this:
> 
> ```
> future.then(defer(pid, &SomeProcess::someMethod, param1, param2));
> ```
> 
> For the objects that do not support move semantics
> (e.g., protobuf messages), number of copies is reduced from 8-10 to 6.
> If move semantics is supported, then number of copies is reduced from
> 6-7 to 1 if parameter is passed with `std::move`, or 2 otherwise.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/defer.hpp 
> 7f3369e723cb244e97930621cbba89cf7873567d 
>   3rdparty/libprocess/include/process/deferred.hpp 
> e446621be11ac51f5f91c417cc8975e363c2f715 
>   3rdparty/libprocess/include/process/dispatch.hpp 
> 3a0793888dc0df5e3ec31b06f47cd920c71e0db9 
>   3rdparty/libprocess/include/process/future.hpp 
> cce950509f58022e79bb51a6e72ea1a005b9cb50 
>   3rdparty/libprocess/include/process/http.hpp 
> f637999174d92a98208b5fc49a65f9929efb11a0 
>   3rdparty/libprocess/src/tests/benchmarks.cpp 
> 245ff7742b1e2c01c42a66d35d8e4f686603244f 
> 
> 
> Diff: https://reviews.apache.org/r/60003/diff/4/
> 
> 
> Testing
> ---
> 
> make check
> 
> Number of copies was checked by using defer to subscribe process for Future 
> callbacks, and passing parameters that count number of copies made.
> 
> 
> Thanks,
> 
> Dmitry Zhuk
> 
>



Re: Review Request 61476: Used clone instead of fork in ns::clone.

2017-08-07 Thread Jie Yu


> On Aug. 7, 2017, 9:34 p.m., Greg Mann wrote:
> > src/linux/ns.hpp
> > Lines 538-539 (original), 585-586 (patched)
> > 
> >
> > Should this comment be removed?
> 
> Greg Mann wrote:
> Sorry, I mean maybe it should be updated to mention 'clone' instead of 
> 'fork'?

I'd prefer we still keep the name `fork` here. The reason we use a clone to 
mimic a fork is due to a glibc bug.


- Jie


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


On Aug. 7, 2017, 8:02 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61476/
> ---
> 
> (Updated Aug. 7, 2017, 8:02 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Benjamin Mahler, and Joseph Wu.
> 
> 
> Bugs: MESOS-7858
> https://issues.apache.org/jira/browse/MESOS-7858
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Used clone instead of fork in ns::clone.
> 
> 
> Diffs
> -
> 
>   src/linux/ns.hpp 941c7ee243e0e486ee263a9e6c4f6d0a7398427d 
> 
> 
> Diff: https://reviews.apache.org/r/61476/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>



Re: Review Request 61476: Used clone instead of fork in ns::clone.

2017-08-07 Thread Joseph Wu

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


Ship it!




LGTM.  Just needs some tweaks to comments (+ the ones the Greg pointed out).


src/linux/ns.hpp
Lines 258-267 (original), 258-267 (patched)


This comment block will also need an update since this is a now a 
fork-and-double-clone (instead of double-fork-and-clone).


- Joseph Wu


On Aug. 7, 2017, 1:02 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61476/
> ---
> 
> (Updated Aug. 7, 2017, 1:02 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Benjamin Mahler, and Joseph Wu.
> 
> 
> Bugs: MESOS-7858
> https://issues.apache.org/jira/browse/MESOS-7858
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Used clone instead of fork in ns::clone.
> 
> 
> Diffs
> -
> 
>   src/linux/ns.hpp 941c7ee243e0e486ee263a9e6c4f6d0a7398427d 
> 
> 
> Diff: https://reviews.apache.org/r/61476/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>



Re: Review Request 60511: Added MockMetadataManager and test concurrent prune and pull.

2017-08-07 Thread Zhitao Li

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

(Updated Aug. 7, 2017, 9:49 p.m.)


Review request for mesos, Gilbert Song and Jason Lai.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Added MockMetadataManager and test concurrent prune and pull.


Diffs (updated)
-

  src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp 
954da1681778878c8aff6150002e52ecb648d1bb 
  src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp 
d86afd2a6ff0bf87e624db1c99255c85068bf6ab 
  src/slave/containerizer/mesos/provisioner/docker/store.hpp 
1cf68665d33bd40a7605d26c96fb7b618407fdd0 
  src/slave/containerizer/mesos/provisioner/docker/store.cpp 
8058dcb31b9de59fa8d3638b553632235542 
  src/tests/containerizer/provisioner_docker_tests.cpp 
920be77b16178a4458d72145020c015130799ec4 


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

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


Testing
---


Thanks,

Zhitao Li



Re: Review Request 56721: Implemented pruneImages with a mark and sweep in docker store.

2017-08-07 Thread Zhitao Li

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

(Updated Aug. 7, 2017, 9:48 p.m.)


Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.


Changes
---

Rebase to take out layer size tracking.


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


Repository: mesos


Description
---

This includes the following changes:
- add a `pruneImages()` function on the chain of relevant classes;
- implement prune in docker store;
- fix mock interface to keep existing tests pass.


Diffs (updated)
-

  src/slave/containerizer/composing.hpp 
bef6d880ca1d815fc21d9d017f6de2d26ad5218f 
  src/slave/containerizer/composing.cpp 
a003e1b80dc9b4dec5b3fbbadb2daecf855c90c7 
  src/slave/containerizer/containerizer.hpp 
0954ed6175e4c8f963bf371e54e0f9ffe7bc9c1c 
  src/slave/containerizer/docker.hpp b602a5698cae12686f51c4b9370a06042cda6270 
  src/slave/containerizer/docker.cpp 2fe92272d7ac6d916371c55affe24598255f10eb 
  src/slave/containerizer/mesos/containerizer.hpp 
fd586306f7a6d3d2cdb58481f0ef4906de8d0f88 
  src/slave/containerizer/mesos/containerizer.cpp 
6f100b516f2750732acaed693f7023b1e44b39d9 
  src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp 
954da1681778878c8aff6150002e52ecb648d1bb 
  src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp 
d86afd2a6ff0bf87e624db1c99255c85068bf6ab 
  src/slave/containerizer/mesos/provisioner/docker/paths.hpp 
232c027f8f96da0cb30b957bce4607d3695050d2 
  src/slave/containerizer/mesos/provisioner/docker/paths.cpp 
cd684b33eb308ce1eeb4539a5b2d51985d835db7 
  src/slave/containerizer/mesos/provisioner/docker/store.hpp 
1cf68665d33bd40a7605d26c96fb7b618407fdd0 
  src/slave/containerizer/mesos/provisioner/docker/store.cpp 
8058dcb31b9de59fa8d3638b553632235542 
  src/slave/containerizer/mesos/provisioner/provisioner.hpp 
7cba54ce490d1e6e17081cd7e04fd6759ceddb8e 
  src/slave/containerizer/mesos/provisioner/provisioner.cpp 
3d4da9069c8bc8b71243f5ebd0d6f68b7d9c0cac 
  src/slave/containerizer/mesos/provisioner/store.hpp 
01ab83dca79e51b8c96d18ee65705912b0ac8324 
  src/slave/containerizer/mesos/provisioner/store.cpp 
cc5cc81e05f29bb0e11ffa13cdb8d63d4397114f 
  src/tests/containerizer.hpp 4bd40c32625bc1f7998d523c6ee81bf78ac74538 
  src/tests/containerizer.cpp 1d2b6391cf7a7545fa44206c59d05764f3e8cdfb 
  src/tests/containerizer/mock_containerizer.hpp 
0adcb01e6c12d6cc4abed1f14fa2df833ffc6569 


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

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


Testing
---


Thanks,

Zhitao Li



Re: Review Request 61476: Used clone instead of fork in ns::clone.

2017-08-07 Thread Greg Mann


> On Aug. 7, 2017, 9:34 p.m., Greg Mann wrote:
> > src/linux/ns.hpp
> > Lines 538-539 (original), 585-586 (patched)
> > 
> >
> > Should this comment be removed?

Sorry, I mean maybe it should be updated to mention 'clone' instead of 'fork'?


- Greg


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


On Aug. 7, 2017, 8:02 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61476/
> ---
> 
> (Updated Aug. 7, 2017, 8:02 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Benjamin Mahler, and Joseph Wu.
> 
> 
> Bugs: MESOS-7858
> https://issues.apache.org/jira/browse/MESOS-7858
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Used clone instead of fork in ns::clone.
> 
> 
> Diffs
> -
> 
>   src/linux/ns.hpp 941c7ee243e0e486ee263a9e6c4f6d0a7398427d 
> 
> 
> Diff: https://reviews.apache.org/r/61476/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>



Re: Review Request 61476: Used clone instead of fork in ns::clone.

2017-08-07 Thread Greg Mann

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




src/linux/ns.hpp
Lines 577 (patched)


s/long/longer/



src/linux/ns.hpp
Lines 538-539 (original), 585-586 (patched)


Should this comment be removed?


- Greg Mann


On Aug. 7, 2017, 8:02 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61476/
> ---
> 
> (Updated Aug. 7, 2017, 8:02 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Benjamin Mahler, and Joseph Wu.
> 
> 
> Bugs: MESOS-7858
> https://issues.apache.org/jira/browse/MESOS-7858
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Used clone instead of fork in ns::clone.
> 
> 
> Diffs
> -
> 
>   src/linux/ns.hpp 941c7ee243e0e486ee263a9e6c4f6d0a7398427d 
> 
> 
> Diff: https://reviews.apache.org/r/61476/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>



Re: Review Request 53993: Updated quota doc to support quota update.

2017-08-07 Thread Mesos Reviewbot Windows

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



Patch looks great!

Reviews applied: [52284, 53679, 53691, 52103, 54135, 53993]

Passed command: support\windows-build.bat

- Mesos Reviewbot Windows


On Aug. 7, 2017, 4:35 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53993/
> ---
> 
> (Updated Aug. 7, 2017, 4:35 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Xiaojian Huang, and Neil 
> Conway.
> 
> 
> Bugs: MESOS-4941
> https://issues.apache.org/jira/browse/MESOS-4941
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated quota doc to support quota update.
> 
> 
> Diffs
> -
> 
>   docs/quota.md f36ab1a22a7858c8b28a13f94a37728d6f56c5e1 
> 
> 
> Diff: https://reviews.apache.org/r/53993/diff/6/
> 
> 
> Testing
> ---
> 
> https://github.com/zhitaoli/mesos/blob/public/quota_update/docs/quota.md
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>



Re: Review Request 61361: Replaced `MESOS_3RDPARTY_*` with `CMAKE_CURRENT_*_DIR`.

2017-08-07 Thread Andrew Schwartzmeyer

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

(Updated Aug. 7, 2017, 1:58 p.m.)


Review request for mesos.


Repository: mesos


Description
---

This variable was superfluous, and set as a side effect of
`Mesos3rdpartyConfigure.cmake`.

Review: https://reviews.apache.org/r/61361


Diffs (updated)
-

  3rdparty/CMakeLists.txt f4feaf90b4750338cfa2127524a5e6556a86d063 
  3rdparty/cmake/Mesos3rdpartyConfigure.cmake 
fa1202a5a6a7d37d94b52e2142528febd1b23a10 


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

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


Testing
---


Thanks,

Andrew Schwartzmeyer



Re: Review Request 61474: Removed an unused function in src/linux/ns.hpp.

2017-08-07 Thread Joseph Wu

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


Ship it!




Looks like the last reference to this function was removed as part of this 
cleanup: https://reviews.apache.org/r/51963/
(Want to mention that in your commit description?)

- Joseph Wu


On Aug. 7, 2017, 12:05 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61474/
> ---
> 
> (Updated Aug. 7, 2017, 12:05 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Joseph Wu.
> 
> 
> Bugs: MESOS-7858
> https://issues.apache.org/jira/browse/MESOS-7858
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed an unused function in src/linux/ns.hpp.
> 
> 
> Diffs
> -
> 
>   src/linux/ns.hpp 941c7ee243e0e486ee263a9e6c4f6d0a7398427d 
>   src/tests/containerizer/ns_tests.cpp 
> 0c748960488e1ef8356273a0f7ff6e0aaa21ad47 
> 
> 
> Diff: https://reviews.apache.org/r/61474/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>



Re: Review Request 61470: Agent log available with the `/agent/path`.

2017-08-07 Thread Mesos Reviewbot Windows

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



Patch looks great!

Reviews applied: [61470]

Passed command: support\windows-build.bat

- Mesos Reviewbot Windows


On Aug. 7, 2017, 8:55 a.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61470/
> ---
> 
> (Updated Aug. 7, 2017, 8:55 a.m.)
> 
> 
> Review request for mesos and Adam B.
> 
> 
> Bugs: MESOS-7864
> https://issues.apache.org/jira/browse/MESOS-7864
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In the effort to get rid of the slave word, this patch aliases the log
> path  in the  agent and  makes it  available through  the `/agent/log`
> path.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp 7381530515f86faf4c3e8f82bcd9483f6cf0498b 
> 
> 
> Diff: https://reviews.apache.org/r/61470/diff/1/
> 
> 
> Testing
> ---
> 
> ```sh
> make check
> ```
> 
> and
> 
> ```sh
> ./bin/mesos-master.sh \
>   --work_dir=/tmp/alexander/mesos/master \
>   --log_dir=/tmp/alexander/mesos/master/log &
>   
> sudo ./bin/mesos-agent.sh \
>   --work_dir=/tmp/mesos/agent \
>   --log_dir=/tmp/mesos/agent/log &
>   
> http 127.0.0.1:5051/files/read path==/agent/log offset==0 length==50
> 
> # Expected response:
> #
> # HTTP/1.1 200 OK
> # Content-Length: 75
> # Content-Type: application/json
> # Date: Mon, 07 Aug 2017 15:51:08 GMT
> # 
> # {
> # "data": "Log file created at: 2017/08/07 11:50:21\nRunning o",
> # "offset": 0
> # }
> ```
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>



Re: Review Request 61466: Added the test `ROOT_CGROUPS_LaunchNestedSharePidNamespace`.

2017-08-07 Thread Mesos Reviewbot Windows

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



Patch looks great!

Reviews applied: [61270, 61406, 61428, 61463, 61464, 61465, 61466]

Passed command: support\windows-build.bat

- Mesos Reviewbot Windows


On Aug. 7, 2017, 3:14 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61466/
> ---
> 
> (Updated Aug. 7, 2017, 3:14 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman, Gilbert Song, Jie Yu, Kevin Klues, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-7853
> https://issues.apache.org/jira/browse/MESOS-7853
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added the test `ROOT_CGROUPS_LaunchNestedSharePidNamespace`.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/nested_mesos_containerizer_tests.cpp 
> e46643434dc85d766bd549a037f36a89a6738678 
> 
> 
> Diff: https://reviews.apache.org/r/61466/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>



Re: Review Request 61172: Added mesos.http and mesos.exceptions for CLI.

2017-08-07 Thread Eric Chung

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



ping

- Eric Chung


On Aug. 3, 2017, 9:41 p.m., Eric Chung wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61172/
> ---
> 
> (Updated Aug. 3, 2017, 9:41 p.m.)
> 
> 
> Review request for mesos, Armand Grillet, Jason Lai, and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added mesos.http and mesos.exceptions for CLI.
> 
> Part of MESOS-7310, this patch adds the mesos.http and mesos.exceptions
> modules, which provides a Resource class and its descendants for
> abstracting away common operations over http connectioins with JSON
> serialization.
> 
> 
> Diffs
> -
> 
>   src/python/cli_new/bootstrap c84e549e59bc7a07b5b70c24c63bc0c16881e442 
>   src/python/cli_new/pylint.config 63fbb94fbf47cccd1053b4c1eafac18b88dbc818 
>   src/python/lib/mesos/exceptions.py PRE-CREATION 
>   src/python/lib/mesos/http.py PRE-CREATION 
>   src/python/lib/requirements.in e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 
>   src/python/lib/tests/conftest.py PRE-CREATION 
>   src/python/lib/tests/test_exceptions.py PRE-CREATION 
>   src/python/lib/tests/test_http.py PRE-CREATION 
>   src/python/lib/tox.ini 8ad030d2dbf1fb7a04f1eaadb587462b4ef3f054 
> 
> 
> Diff: https://reviews.apache.org/r/61172/diff/3/
> 
> 
> Testing
> ---
> 
> under src/python/lib, call `tox` for running unit tests. The test should pass 
> and test coverage should be at 100%.
> 
> 
> Thanks,
> 
> Eric Chung
> 
>



Re: Review Request 61361: Replaced `MESOS_3RDPARTY_*` with `CMAKE_CURRENT_*_DIR`.

2017-08-07 Thread Andrew Schwartzmeyer

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

(Updated Aug. 7, 2017, 1:05 p.m.)


Review request for mesos.


Changes
---

Missed libev.


Repository: mesos


Description (updated)
---

This variable was superfluous, and set as a side effect of
`Mesos3rdpartyConfigure.cmake`.

Review: https://reviews.apache.org/r/61361


Diffs (updated)
-

  3rdparty/CMakeLists.txt f4feaf90b4750338cfa2127524a5e6556a86d063 


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

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


Testing
---


Thanks,

Andrew Schwartzmeyer



Re: Review Request 61137: Cleaned up style in example frameworks.

2017-08-07 Thread Mesos Reviewbot Windows

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



Patch looks great!

Reviews applied: [61112, 61110, 6, 61137]

Passed command: support\windows-build.bat

- Mesos Reviewbot Windows


On Aug. 7, 2017, 1:53 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61137/
> ---
> 
> (Updated Aug. 7, 2017, 1:53 p.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-7814
> https://issues.apache.org/jira/browse/MESOS-7814
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/examples/balloon_framework.cpp dfd049b860345adc33c3e774dcdff0320da107f6 
>   src/examples/disk_full_framework.cpp 
> 215b6d29cf5f52973f2b211e9fc72ca6ec2afa7a 
>   src/examples/docker_no_executor_framework.cpp 
> 4a58f11fc8892f23ade1d8c872ab9b4fc580d478 
>   src/examples/dynamic_reservation_framework.cpp 
> f3b1c8c4d2684e827fc10776fef4f2287e315b85 
>   src/examples/load_generator_framework.cpp 
> abb70f42a3a755afaa1d56b1491058b90958f030 
>   src/examples/long_lived_framework.cpp 
> 2a79dfd3f81e5254922895af45c2b72d80ce5f49 
>   src/examples/no_executor_framework.cpp 
> 7d841c6f364e1b671ec829aa9bff3b1f8ecf55ef 
>   src/examples/persistent_volume_framework.cpp 
> 17140294a1eaeeeb92e9e14fc8638182b3a0682e 
>   src/examples/test_framework.cpp 9dbc18b039ee13cc1ec9454bd41cb3cfe30d63b4 
>   src/examples/test_http_framework.cpp 
> a0cd84ab6dfdd8d6ed3403c6e06fabae6d5b46c9 
> 
> 
> Diff: https://reviews.apache.org/r/61137/diff/4/
> 
> 
> Testing
> ---
> 
> `$ make check`
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>



Re: Review Request 61473: Do not kill non partition aware tasks.

2017-08-07 Thread Megha Sharma

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



We talked about 2 approaches and approach 2 seemed like a cleaner way to 
address the issue.

Approach 1: ShutdownFrameworkMessage

1. Upon agent re-registration, master will add tasks even for non-PA frameworks 
on this agent. This is needed by the master to do correct resource accounting 
and not offer resources already in use on this agent. We need to mutate the 
TaskState on the Task before adding them to the master's data structures since 
the TaskState might be non-terminal when the agent sends these tasks with 
ReregisterSlaveMessage. And the master has already sent TASK_LOST for these 
tasks to the frameworks so we need to set the TaskState to TASK_LOST so that 
any future reconciliations with the framework doesn't have this task 
transitioning from TASK_LOST to TASK_RUNNNG/TASK_STARTING. This is to avoid 
unnecessary confusion about task state as observed by the framework but indeed 
this could have happened with non-strict registry as well where the framework 
can actually receive a non terminal task state update after receiving a 
TASK_LOST for the same task in the past.

2. When the agent re-registers, the master will continue to send a 
ShutdownFrameworkMessage to the agent to kill the tasks pertaining to non-PA 
frameworks on the agent as it does today. An additional optional field will be 
added to the ShutdownFrameworkMessage to indicate whether or not the shutdown 
was initiated internally.

3. During framework shutdown the state of the framework is set to 
Framework::TERMINATING which prevents it from launching new tasks. Here, since 
the framework is not really terminating so in order to allow it to launch new 
tasks, the agent will not set the state to terminating if the 
ShutdownFrameworkMessage is generated internally.

4. The framework shutdown today doesn't generate any status updates which needs 
to change. The status updates will be sent if the framework shutdown is 
triggered internally, this is needed to remove the tasks of non-PA frameworks 
that got added when the agent re-registered (1).

Approach 2: Do not shutdown non-PA framework when agent re-registers and let 
the frameworks make the decision on what needs to be done when they receive 
non-terminal status updates for tasks for which they have already received a 
TASK_LOST. This hopefully won't break any frameworks since it could have 
happened in the past with non-strict registry as well and frameworks should be 
resilient enough to handle this scenario.

Let me know if I have missed anything

- Megha Sharma


On Aug. 7, 2017, 6:23 p.m., Megha Sharma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61473/
> ---
> 
> (Updated Aug. 7, 2017, 6:23 p.m.)
> 
> 
> Review request for mesos and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-7215
> https://issues.apache.org/jira/browse/MESOS-7215
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Master will not kill the tasks for non-Partition aware frameworks
> when an unreachable agent re-registers with the master.
> Master used to send a ShutdownFrameworkMessages to the agent
> to kill the tasks from non partition aware frameworks including the
> ones that are still registered which was problematic because the offer
> from this agent could still go to the same framework which could then
> launch new tasks. The agent would then receive tasks of the same
> framework and ignore them because it thinks the framework is shutting
> down. The framework is not shutting down of course, so from the master
> and the scheduler’s perspective the task is pending in STAGING forever
> until the next agent reregistration, which could happen much later.
> This commit fixes the problem by not shutting down the non-partition
> aware frameworks on such an agent.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 959091c8ec03b6ac7bcb5d21b04d2f7d5aff7d54 
>   src/master/master.hpp b802fd153a10f6012cea381f153c28cc78cae995 
>   src/master/master.cpp 7f38a5e21884546d4b4c866ca5918db779af8f99 
>   src/tests/partition_tests.cpp 62a84f797201ccd18b71490949e3130d2b9c3668 
> 
> 
> Diff: https://reviews.apache.org/r/61473/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Megha Sharma
> 
>



Review Request 61477: Used async signal safe assert in ns::clone.

2017-08-07 Thread Jie Yu

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

Review request for mesos, Benjamin Mahler and Joseph Wu.


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


Repository: mesos


Description
---

Used async signal safe assert in ns::clone.


Diffs
-

  src/linux/ns.hpp 941c7ee243e0e486ee263a9e6c4f6d0a7398427d 


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


Testing
---

make check


Thanks,

Jie Yu



Review Request 61476: Used clone instead of fork in ns::clone.

2017-08-07 Thread Jie Yu

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

Review request for mesos, Benjamin Mahler and Joseph Wu.


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


Repository: mesos


Description
---

Used clone instead of fork in ns::clone.


Diffs
-

  src/linux/ns.hpp 941c7ee243e0e486ee263a9e6c4f6d0a7398427d 


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


Testing
---

make check


Thanks,

Jie Yu



Review Request 61474: Removed an unused function in src/linux/ns.hpp.

2017-08-07 Thread Jie Yu

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

Review request for mesos, Benjamin Mahler and Joseph Wu.


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


Repository: mesos


Description
---

Removed an unused function in src/linux/ns.hpp.


Diffs
-

  src/linux/ns.hpp 941c7ee243e0e486ee263a9e6c4f6d0a7398427d 
  src/tests/containerizer/ns_tests.cpp 0c748960488e1ef8356273a0f7ff6e0aaa21ad47 


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


Testing
---

make check


Thanks,

Jie Yu



Re: Review Request 61434: Updated health-checks.md to include description of generalized checks.

2017-08-07 Thread Alexander Rukletsov


> On Aug. 4, 2017, 7:18 p.m., Vinod Kone wrote:
> > docs/health-checks.md
> > Lines 110 (patched)
> > 
> >
> > s/1./2./ ? or is this markdown style?

Yeah, actual numbers have no effect on the rendered HTML: 
https://daringfireball.net/projects/markdown/syntax#list
This approach is less error-prone to future additions, but I don't have a 
strong opinion.


- Alexander


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


On Aug. 4, 2017, 6:14 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61434/
> ---
> 
> (Updated Aug. 4, 2017, 6:14 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman, Greg Mann, and Vinod Kone.
> 
> 
> Bugs: MESOS-7349
> https://issues.apache.org/jira/browse/MESOS-7349
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   docs/health-checks.md d0749347d1f9398004e56b03503d655b5fba75f4 
> 
> 
> Diff: https://reviews.apache.org/r/61434/diff/1/
> 
> 
> Testing
> ---
> 
> https://gist.github.com/rukletsov/517a136cb6a0e8b909db30b36b13eded
> 
> Additionally rendered in MacDown.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>



Re: Review Request 61458: Added documentation of parallel test execution config flag.

2017-08-07 Thread Mesos Reviewbot Windows

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



Patch looks great!

Reviews applied: [6, 60646, 61458]

Passed command: support\windows-build.bat

- Mesos Reviewbot Windows


On Aug. 7, 2017, 11:52 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61458/
> ---
> 
> (Updated Aug. 7, 2017, 11:52 a.m.)
> 
> 
> Review request for mesos and Till Toenshoff.
> 
> 
> Bugs: MESOS-4809
> https://issues.apache.org/jira/browse/MESOS-4809
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The '--enable-parallel-test-execution' configure flag was added some
> time ago without explicit documentation. This patch adds accompanying
> configure flag markdown documentation.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md 041c3dfb9c0c1718770f74dfb33a9f5d6fbe9b61 
> 
> 
> Diff: https://reviews.apache.org/r/61458/diff/1/
> 
> 
> Testing
> ---
> 
> Viewed generated site with `dev` target.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>



Re: Review Request 58898: Do not kill non partition aware tasks.

2017-08-07 Thread Megha Sharma


> On May 10, 2017, 11:24 p.m., Neil Conway wrote:
> > My apologies for the delay in reviewing this.
> > 
> > High-level comments:
> > 
> > (a) Can we improve the description of the problem in the commit summary? It 
> > took me quite a while to figure out what is actually going on here. My 
> > understanding is:
> > 
> > (1) Agent re-registers
> > (2) Master sends `ShutdownFrameworkMessage` for non-PA frameworks on the 
> > agent
> > (3) Master offers agent resources to framework
> > (4) Framework launches new task on agent _before the agent has finished 
> > shutting down the framework_
> > (5) Agent ignores launch task because it believes the framework is still 
> > terminating.
> > 
> > This is a race condition, because if the agent finished shutting down the 
> > framework (in #4) before the launch task was received, there would not be a 
> > problem. Is my understanding correct?
> > 
> > (2) I'd prefer a new unit test that constructs exactly this scenario, 
> > rather than changing existing unit tests.
> > 
> > (3) The new behavior is that the framework will receive `TASK_KILLED` for 
> > any non-PA tasks running on a partitioned agent that re-registers. This 
> > does not seem ideal, because `TASK_KILLED` _normally_ corresponds to a 
> > framework-initiated `killTask` operation.
> > 
> > (4) Thinking out loud, what if a custom executor ignores the `killTask` 
> > request? When shutting down a framework, it seems we forcibly destroy the 
> > container (via `containerizer->destroy()`), if the executor doesn't exit 
> > promptly upon receiving the framework shutdown request. AFAIK we don't have 
> > similar logic for `killTask` requests.
> > 
> > 3 + 4 suggests that maybe we want a different way to terminate the task on 
> > the agent -- let's discuss.
> 
> Megha Sharma wrote:
> Summarizing the 2 approaches we talked about.
> 
> Approach 1: ShutdownFrameworkMessage
> 
> 1. Upon agent re-registration, master will add tasks even for non-PA 
> frameworks on this agent. This is needed by the master to do correct resource 
> accounting and not offer resources already in use on this agent. We need to 
> mutate the TaskState on the Task before adding them to the master's data 
> structures since the TaskState might be non-terminal when the agent sends 
> these tasks with ReregisterSlaveMessage. And the master has already sent 
> TASK_LOST for these tasks to the frameworks so we need to set the TaskState 
> to TASK_LOST so that any future reconciliations with the framework doesn't 
> have this task transitioning from TASK_LOST to TASK_RUNNNG/TASK_STARTING. 
> This is to avoid unnecessary confusion about task state as observed by the 
> framework but indeed this could have happened with non-strict registry as 
> well where the framework can actually receive a non terminal task state 
> update after receiving a TASK_LOST for the same task in the past.
> 
> 2. When the agent re-registers, the master will continue to send a 
> ShutdownFrameworkMessage to the agent to kill the tasks pertaining to non-PA 
> frameworks on the agent as it does today. An additional optional field will 
> be added to the ShutdownFrameworkMessage to indicate whether or not the 
> shutdown was initiated internally.
> 
> 3. During framework shutdown the state of the framework is set to 
> Framework::TERMINATING which prevents it from launching new tasks. Here, 
> since the framework is not really terminating so in order to allow it to 
> launch new tasks, the agent will not set the state to terminating if the 
> ShutdownFrameworkMessage is generated internally.
> 
> 4. The framework shutdown today doesn't generate any status updates which 
> needs to change. The status updates will be sent if the framework shutdown is 
> triggered internally, this is needed to remove the tasks of non-PA frameworks 
> that got added when the agent re-registered (1).
> 
> Approach 2: Do not shutdown non-PA framework when agent re-registers and 
> let the frameworks make the decision on what needs to be done when they 
> receive non-terminal status updates for tasks for which they have already 
> received a TASK_LOST. This hopefully won't break any frameworks since it 
> could have happened in the past with non-strict registry as well and 
> frameworks should be resilient enough to handle this scenario.
> 
> Let me know if I have missed anything. Personally, I am in favor of 
> approach 1 as it seems less catastrophic for the frameworks. What do you 
> think?
> 
> Megha Sharma wrote:
> The proposed solution to fix the race between new task launches and 
> shutdown framework on the agent, was to not kill the non-partition aware 
> tasks when an unreachable agent re-registers with the master. So now as far 
> as Mesos internals are concerned, the tasks from non-partition aware 
> frameworks are to be treated the same way as partition aware tasks and one 
> way to do it cleanly in Mesos was to tran

Re: Review Request 61473: Do not kill non partition aware tasks.

2017-08-07 Thread Megha Sharma

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

(Updated Aug. 7, 2017, 6:23 p.m.)


Review request for mesos and Jiang Yan Xu.


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


Repository: mesos


Description
---

Master will not kill the tasks for non-Partition aware frameworks
when an unreachable agent re-registers with the master.
Master used to send a ShutdownFrameworkMessages to the agent
to kill the tasks from non partition aware frameworks including the
ones that are still registered which was problematic because the offer
from this agent could still go to the same framework which could then
launch new tasks. The agent would then receive tasks of the same
framework and ignore them because it thinks the framework is shutting
down. The framework is not shutting down of course, so from the master
and the scheduler’s perspective the task is pending in STAGING forever
until the next agent reregistration, which could happen much later.
This commit fixes the problem by not shutting down the non-partition
aware frameworks on such an agent.


Diffs
-

  src/master/http.cpp 959091c8ec03b6ac7bcb5d21b04d2f7d5aff7d54 
  src/master/master.hpp b802fd153a10f6012cea381f153c28cc78cae995 
  src/master/master.cpp 7f38a5e21884546d4b4c866ca5918db779af8f99 
  src/tests/partition_tests.cpp 62a84f797201ccd18b71490949e3130d2b9c3668 


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


Testing (updated)
---

make check


Thanks,

Megha Sharma



Review Request 61473: Do not kill non partition aware tasks.

2017-08-07 Thread Megha Sharma

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

Review request for mesos and Jiang Yan Xu.


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


Repository: mesos


Description
---

Master will not kill the tasks for non-Partition aware frameworks
when an unreachable agent re-registers with the master.
Master used to send a ShutdownFrameworkMessages to the agent
to kill the tasks from non partition aware frameworks including the
ones that are still registered which was problematic because the offer
from this agent could still go to the same framework which could then
launch new tasks. The agent would then receive tasks of the same
framework and ignore them because it thinks the framework is shutting
down. The framework is not shutting down of course, so from the master
and the scheduler’s perspective the task is pending in STAGING forever
until the next agent reregistration, which could happen much later.
This commit fixes the problem by not shutting down the non-partition
aware frameworks on such an agent.


Diffs
-

  src/master/http.cpp 959091c8ec03b6ac7bcb5d21b04d2f7d5aff7d54 
  src/master/master.hpp b802fd153a10f6012cea381f153c28cc78cae995 
  src/master/master.cpp 7f38a5e21884546d4b4c866ca5918db779af8f99 
  src/tests/partition_tests.cpp 62a84f797201ccd18b71490949e3130d2b9c3668 


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


Testing
---


Thanks,

Megha Sharma



Re: Review Request 61272: Added a MockResourceProvider.

2017-08-07 Thread Jan Schlicht

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

(Updated Aug. 7, 2017, 8:15 p.m.)


Review request for mesos, Benjamin Bannier and Jie Yu.


Changes
---

Rebased to support `EndpointDetector`.


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


Repository: mesos


Description
---

Added a MockResourceProvider.


Diffs (updated)
-

  src/tests/mesos.hpp 6f06261d81870b923b7053daf8205327c4ac6a45 


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

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


Testing
---

make check


Thanks,

Jan Schlicht



Re: Review Request 58898: Do not kill non partition aware tasks.

2017-08-07 Thread Mesos Reviewbot Windows

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



Bad patch!

Reviews applied: [58898]

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

Error:
Traceback (most recent call last):
  File "support/apply-reviews.py", line 417, in 
main()
  File "support/apply-reviews.py", line 412, in main
reviewboard(options)
  File "support/apply-reviews.py", line 402, in reviewboard
apply_review(options)
  File "support/apply-reviews.py", line 160, in apply_review
commit_patch(options)
  File "support/apply-reviews.py", line 261, in commit_patch
message.write(data['message'])
UnicodeEncodeError: 'ascii' codec can't encode character u'\u2019' in position 
655: ordinal not in range(128)

Full log: http://mesos-winbot.westus.cloudapp.azure.com/logs/212/console

- Mesos Reviewbot Windows


On Aug. 6, 2017, 4:35 a.m., Megha Sharma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58898/
> ---
> 
> (Updated Aug. 6, 2017, 4:35 a.m.)
> 
> 
> Review request for mesos, Neil Conway and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-7215
> https://issues.apache.org/jira/browse/MESOS-7215
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Master will not kill the tasks for non-Partition aware frameworks
> when an unreachable agent re-registers with the master.
> Master used to send a ShutdownFrameworkMessages to the agent
> to kill the tasks from non partition aware frameworks including the
> ones that are still registered which was problematic because the offer
> from this agent could still go to the same framework which could then
> launch new tasks. The agent would then receive tasks of the same
> framework and ignore them because it thinks the framework is shutting
> down. The framework is not shutting down of course, so from the master
> and the scheduler’s perspective the task is pending in STAGING forever
> until the next agent reregistration, which could happen much later.
> This commit fixes the problem by not shutting down the non-partition
> aware frameworks on such an agent.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 959091c8ec03b6ac7bcb5d21b04d2f7d5aff7d54 
>   src/master/master.hpp b802fd153a10f6012cea381f153c28cc78cae995 
>   src/master/master.cpp 7f38a5e21884546d4b4c866ca5918db779af8f99 
>   src/tests/partition_tests.cpp 62a84f797201ccd18b71490949e3130d2b9c3668 
> 
> 
> Diff: https://reviews.apache.org/r/58898/diff/3/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Megha Sharma
> 
>



Re: Review Request 61446: Added a test to verify a fix for MESOS-7863.

2017-08-07 Thread Mesos Reviewbot Windows

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



Patch looks great!

Reviews applied: [61440, 61441, 61442, 61443, 61444, 61445, 61446]

Passed command: support\windows-build.bat

- Mesos Reviewbot Windows


On Aug. 5, 2017, 2:55 a.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61446/
> ---
> 
> (Updated Aug. 5, 2017, 2:55 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-7783 and MESOS-7863
> https://issues.apache.org/jira/browse/MESOS-7783
> https://issues.apache.org/jira/browse/MESOS-7863
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This test ensures that the agent does not consider a framework
> with killed pending tasks as an idle framework. Previously the
> agent would remove them and incorrectly consider the framework
> idle during '__run', which resulted in dropped TASK_KILLED
> status updates, see MESOS-7863.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_tests.cpp 1d9d142ed9e801b79535a2c28f5a94ffbf1bf160 
> 
> 
> Diff: https://reviews.apache.org/r/61446/diff/1/
> 
> 
> Testing
> ---
> 
> Ran in repetition
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>



Re: Review Request 61363: Rewrote `libprocess` CMake build.

2017-08-07 Thread Andrew Schwartzmeyer

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

(Updated Aug. 7, 2017, 10:43 a.m.)


Review request for mesos.


Changes
---

Linked publicly to SSL and crypto libraries.


Repository: mesos


Description (updated)
---

Review: https://reviews.apache.org/r/61363


Diffs (updated)
-

  3rdparty/libprocess/CMakeLists.txt 6312e20ec0102edbef086bf61ce4256109ae8fa0 
  3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake 
3e621f97b683012beca5705fbf3e4000d686f3ab 
  3rdparty/libprocess/cmake/ProcessConfigure.cmake 
75af774e1460319cef1efdf801eb97f34eada0e7 
  3rdparty/libprocess/cmake/ProcessTestsConfigure.cmake 
5a23990825de04f27bd5f74c82e10b702974f875 
  3rdparty/libprocess/src/CMakeLists.txt 
fbad7a0dd0347a308ba504ff7daeed187dd1f94d 
  3rdparty/libprocess/src/tests/CMakeLists.txt 
27451c275b1f5a2eb7ada78f8cbe4b7c2c63ca92 


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

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


Testing
---


Thanks,

Andrew Schwartzmeyer



Re: Review Request 61357: Imported `libev` and `libevent` libraries.

2017-08-07 Thread Andrew Schwartzmeyer

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

(Updated Aug. 7, 2017, 10:42 a.m.)


Review request for mesos.


Changes
---

Added OPENSSL_ROOT_DIR forwarding.


Repository: mesos


Description (updated)
---

The complicated install steps are not necessary when the library is
imported properly, as CMake is aware of its exact location.

Review: https://reviews.apache.org/r/61357


Diffs (updated)
-

  3rdparty/CMakeLists.txt f4feaf90b4750338cfa2127524a5e6556a86d063 


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

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


Testing
---


Thanks,

Andrew Schwartzmeyer



Re: Review Request 60931: Added test cases for framework streaming events.

2017-08-07 Thread Mesos Reviewbot Windows

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



Patch looks great!

Reviews applied: [60928, 60929, 60930, 60931]

Passed command: support\windows-build.bat

- Mesos Reviewbot Windows


On July 27, 2017, 6:24 p.m., Quinn Leng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60931/
> ---
> 
> (Updated July 27, 2017, 6:24 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Greg Mann, and Zhitao Li.
> 
> 
> Bugs: MESOS-6101
> https://issues.apache.org/jira/browse/MESOS-6101
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test cases for 'FRAMEWORK_ADDED', 'FRAMEWORK_UPDATED' and
> 'FRAMEWORK_REMOVED' events in v1 operator API.
> 
> 
> Diffs
> -
> 
>   src/tests/api_tests.cpp f22ca28c819712d8797e0c0c69dc1ebf1fe5ac1f 
> 
> 
> Diff: https://reviews.apache.org/r/60931/diff/5/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Quinn Leng
> 
>



Re: Review Request 53993: Updated quota doc to support quota update.

2017-08-07 Thread Zhitao Li

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

(Updated Aug. 7, 2017, 4:35 p.m.)


Review request for mesos, Alexander Rukletsov, Xiaojian Huang, and Neil Conway.


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


Repository: mesos


Description
---

Updated quota doc to support quota update.


Diffs
-

  docs/quota.md f36ab1a22a7858c8b28a13f94a37728d6f56c5e1 


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


Testing (updated)
---

https://github.com/zhitaoli/mesos/blob/public/quota_update/docs/quota.md


Thanks,

Zhitao Li



Re: Review Request 53993: Updated quota doc to support quota update.

2017-08-07 Thread Zhitao Li

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

(Updated Aug. 7, 2017, 4:35 p.m.)


Review request for mesos, Alexander Rukletsov, Xiaojian Huang, and Neil Conway.


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


Repository: mesos


Description
---

Updated quota doc to support quota update.


Diffs (updated)
-

  docs/quota.md f36ab1a22a7858c8b28a13f94a37728d6f56c5e1 


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

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


Testing (updated)
---

https://github.com/zhitaoli/mesos/tree/public/quota_update


Thanks,

Zhitao Li



Re: Review Request 61455: Tweaked checker logs to avoid repeated information.

2017-08-07 Thread Gastón Kleiman

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


Ship it!




Ship It!

- Gastón Kleiman


On Aug. 7, 2017, 10:47 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61455/
> ---
> 
> (Updated Aug. 7, 2017, 10:47 a.m.)
> 
> 
> Review request for mesos and Gastón Kleiman.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Macro `WSTRINGIFY` already prints some test before printing the exit
> code of the command or termination signal. Hence adding "returned"
> word prior to invoking this macro is not necessary and might even be
> misleading.
> 
> 
> Diffs
> -
> 
>   src/checks/checker_process.cpp d92588e9e7235cf403b9b18cd7cd58e652a9f2a2 
>   src/checks/health_checker.cpp 41fd94906407b59b1f27033324279d6a0ee2cf4b 
> 
> 
> Diff: https://reviews.apache.org/r/61455/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>



Review Request 61470: Agent log available with the `/agent/path`.

2017-08-07 Thread Alexander Rojas

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

Review request for mesos and Adam B.


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


Repository: mesos


Description
---

In the effort to get rid of the slave word, this patch aliases the log
path  in the  agent and  makes it  available through  the `/agent/log`
path.


Diffs
-

  src/slave/slave.cpp 7381530515f86faf4c3e8f82bcd9483f6cf0498b 


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


Testing
---

```sh
make check
```

and

```sh
./bin/mesos-master.sh \
  --work_dir=/tmp/alexander/mesos/master \
  --log_dir=/tmp/alexander/mesos/master/log &
  
sudo ./bin/mesos-agent.sh \
  --work_dir=/tmp/mesos/agent \
  --log_dir=/tmp/mesos/agent/log &
  
http 127.0.0.1:5051/files/read path==/agent/log offset==0 length==50

# Expected response:
#
# HTTP/1.1 200 OK
# Content-Length: 75
# Content-Type: application/json
# Date: Mon, 07 Aug 2017 15:51:08 GMT
# 
# {
# "data": "Log file created at: 2017/08/07 11:50:21\nRunning o",
# "offset": 0
# }
```


Thanks,

Alexander Rojas



Review Request 61466: Added the test `ROOT_CGROUPS_LaunchNestedSharePidNamespace`.

2017-08-07 Thread Qian Zhang

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

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


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


Repository: mesos


Description
---

Added the test `ROOT_CGROUPS_LaunchNestedSharePidNamespace`.


Diffs
-

  src/tests/containerizer/nested_mesos_containerizer_tests.cpp 
e46643434dc85d766bd549a037f36a89a6738678 


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


Testing
---


Thanks,

Qian Zhang



Review Request 61465: Added test `NamespacesIsolatorTest.ROOT_SharePidNamespaceWhenDisallow`.

2017-08-07 Thread Qian Zhang

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

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


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


Repository: mesos


Description
---

Added test `NamespacesIsolatorTest.ROOT_SharePidNamespaceWhenDisallow`.


Diffs
-

  src/tests/containerizer/isolator_tests.cpp 
f3c541c92f8ecc5c12356363bbbf3561d3b75230 


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


Testing
---


Thanks,

Qian Zhang



Review Request 61464: Added a test `NamespacesIsolatorTest.ROOT_SharePidNamespace`.

2017-08-07 Thread Qian Zhang

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

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


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


Repository: mesos


Description
---

Added a test `NamespacesIsolatorTest.ROOT_SharePidNamespace`.


Diffs
-

  src/tests/containerizer/isolator_tests.cpp 
f3c541c92f8ecc5c12356363bbbf3561d3b75230 


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


Testing
---


Thanks,

Qian Zhang



Review Request 61463: Fixed a bug in the test `NamespacesIsolatorTest.ROOT_PidNamespace`.

2017-08-07 Thread Qian Zhang

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

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


Repository: mesos


Description
---

In this test, we used the command `stat -c %i /proc/self/ns/pid` to
get the current process's pid namespace inode, that is not correct
since `/proc/self/ns/pid` is a symbol link, we should get the inode
that the link references rather than the link itself. So we need to
add a `-L` option in the `stat` command.


Diffs
-

  src/tests/containerizer/isolator_tests.cpp 
f3c541c92f8ecc5c12356363bbbf3561d3b75230 


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


Testing
---


Thanks,

Qian Zhang



Re: Review Request 61137: Cleaned up style in example frameworks.

2017-08-07 Thread Armand Grillet

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

(Updated Aug. 7, 2017, 1:53 p.m.)


Review request for mesos and Alexander Rukletsov.


Changes
---

Applied changes on every example frameworks.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/examples/balloon_framework.cpp dfd049b860345adc33c3e774dcdff0320da107f6 
  src/examples/disk_full_framework.cpp 215b6d29cf5f52973f2b211e9fc72ca6ec2afa7a 
  src/examples/docker_no_executor_framework.cpp 
4a58f11fc8892f23ade1d8c872ab9b4fc580d478 
  src/examples/dynamic_reservation_framework.cpp 
f3b1c8c4d2684e827fc10776fef4f2287e315b85 
  src/examples/load_generator_framework.cpp 
abb70f42a3a755afaa1d56b1491058b90958f030 
  src/examples/long_lived_framework.cpp 
2a79dfd3f81e5254922895af45c2b72d80ce5f49 
  src/examples/no_executor_framework.cpp 
7d841c6f364e1b671ec829aa9bff3b1f8ecf55ef 
  src/examples/persistent_volume_framework.cpp 
17140294a1eaeeeb92e9e14fc8638182b3a0682e 
  src/examples/test_framework.cpp 9dbc18b039ee13cc1ec9454bd41cb3cfe30d63b4 
  src/examples/test_http_framework.cpp a0cd84ab6dfdd8d6ed3403c6e06fabae6d5b46c9 


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

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


Testing
---

`$ make check`


Thanks,

Armand Grillet



Re: Review Request 61111: Extracted strings into constants in example frameworks.

2017-08-07 Thread Armand Grillet

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

(Updated Aug. 7, 2017, 12:50 p.m.)


Review request for mesos, Alexander Rukletsov and Till Toenshoff.


Changes
---

Applied changes on every example frameworks.


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


Repository: mesos


Description (updated)
---

See summary.


Diffs (updated)
-

  src/examples/balloon_framework.cpp dfd049b860345adc33c3e774dcdff0320da107f6 
  src/examples/disk_full_framework.cpp 215b6d29cf5f52973f2b211e9fc72ca6ec2afa7a 
  src/examples/docker_no_executor_framework.cpp 
4a58f11fc8892f23ade1d8c872ab9b4fc580d478 
  src/examples/dynamic_reservation_framework.cpp 
f3b1c8c4d2684e827fc10776fef4f2287e315b85 
  src/examples/load_generator_framework.cpp 
abb70f42a3a755afaa1d56b1491058b90958f030 
  src/examples/long_lived_framework.cpp 
2a79dfd3f81e5254922895af45c2b72d80ce5f49 
  src/examples/no_executor_framework.cpp 
7d841c6f364e1b671ec829aa9bff3b1f8ecf55ef 
  src/examples/persistent_volume_framework.cpp 
17140294a1eaeeeb92e9e14fc8638182b3a0682e 
  src/examples/test_framework.cpp 9dbc18b039ee13cc1ec9454bd41cb3cfe30d63b4 
  src/examples/test_http_framework.cpp a0cd84ab6dfdd8d6ed3403c6e06fabae6d5b46c9 


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

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


Testing
---

$ make check


Thanks,

Armand Grillet



Review Request 61458: Added documentation of parallel test execution config flag.

2017-08-07 Thread Benjamin Bannier

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

Review request for mesos and Till Toenshoff.


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


Repository: mesos


Description
---

The '--enable-parallel-test-execution' configure flag was added some
time ago without explicit documentation. This patch adds accompanying
configure flag markdown documentation.


Diffs
-

  docs/configuration.md 041c3dfb9c0c1718770f74dfb33a9f5d6fbe9b61 


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


Testing
---

Viewed generated site with `dev` target.


Thanks,

Benjamin Bannier



Review Request 61455: Tweaked checker logs to avoid repeated information.

2017-08-07 Thread Alexander Rukletsov

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

Review request for mesos and Gastón Kleiman.


Repository: mesos


Description
---

Macro `WSTRINGIFY` already prints some test before printing the exit
code of the command or termination signal. Hence adding "returned"
word prior to invoking this macro is not necessary and might even be
misleading.


Diffs
-

  src/checks/checker_process.cpp d92588e9e7235cf403b9b18cd7cd58e652a9f2a2 
  src/checks/health_checker.cpp 41fd94906407b59b1f27033324279d6a0ee2cf4b 


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


Testing
---

make check


Thanks,

Alexander Rukletsov



Re: Review Request 60646: Modified handling of parallel test configure flag for documentation.

2017-08-07 Thread Benjamin Bannier

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

(Updated Aug. 7, 2017, 12:30 p.m.)


Review request for mesos and Till Toenshoff.


Summary (updated)
-

Modified handling of parallel test configure flag for documentation.


Repository: mesos


Description (updated)
---

This patch modifies the handling of the parallel test configure flag
for documentation purposes. With that we document proper ways to
handle both default-enabled and -disabled flags without using
arguments to 'AC_ARG_ENABLE'; the handling of '--disable-werror' is an
example of a default-enabled flag while
'--enable-parallel-test-execution' documents a default-disabled flag.


Diffs (updated)
-

  configure.ac 5a6e42c61e6752c3742532502dbc753071b31323 


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

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


Testing
---

configure and build stout with different settings 
(`--enable-parallel-test-execution`, `--enable-parallel-test-execution=yes`, 
`--enable-parallel-test-execution=no`, `--disable-parallel-test-execution`), 
`make check`.


Thanks,

Benjamin Bannier