date:20170809

Review Request 61544: WIP: Rewrote Mesos CMake build.

2017-08-09 Thread Andrew Schwartzmeyer


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

Review request for mesos, Aaron Wood, Benjamin Bannier, Chun-Hung Hsiao, Jeff 
Coffler, Jie Yu, James Peach, and Joseph Wu.


Repository: mesos


Description
---

WIP: Rewrote Mesos CMake build.


Diffs
-

  cmake/CompilationConfigure.cmake 2109697e02e58c673948f2ae67ef8255d16aa736 
  cmake/MesosConfigure.cmake 5ecad2c0fbd245dd14e9c09869dd6da2ca4e602a 
  src/CMakeLists.txt 98ccaf41bf5e7d14164e1c8b6e49268ac865a52c 
  src/checks/CMakeLists.txt 4854b2765202e10f5b791f08ed1416f7740e2aad 
  src/cli/CMakeLists.txt 056d1d734e4424dac3eb29c323961d3a2e87a8dc 
  src/docker/CMakeLists.txt 2d7c30db5854853625a99227568a27dde281ec2e 
  src/examples/CMakeLists.txt e8b7ea4bd91c20536ce7bd8d0ffdfd4e549b056f 
  src/launcher/CMakeLists.txt 97edc4a28a995da8a5e963aaa62c4c955cc59719 
  src/local/CMakeLists.txt 7c9a718d17772be1c08e132136739cfd9449470d 
  src/log/CMakeLists.txt 2fad6cca467dbde9989d26424a2a6c608cd251e8 
  src/master/CMakeLists.txt 0062fb3e9d67fdfcda1057112d4ba34abbd4be5f 
  src/master/cmake/MasterConfigure.cmake 
173dc36b5a1b7ff23cd0542ea4313ae6e5e0ae3f 
  src/slave/CMakeLists.txt 9dbadca53b9ac34db08cfb9c9e9fea741c01114b 
  src/slave/cmake/AgentConfigure.cmake af2f74f82b68920427f36c0c863b97c0191b8f06 
  src/slave/container_loggers/CMakeLists.txt 
c0774ce3110fd8d2aac6dc00d2b800e336214f96 
  src/slave/containerizer/mesos/CMakeLists.txt 
fa2043efadd1c863c5ad54fdc518397c97488dcb 
  src/slave/qos_controllers/CMakeLists.txt 
65ab338a71276a77e43af962fbc9b76e050efca6 
  src/slave/resource_estimators/CMakeLists.txt 
1d28bd457c7e567219d892f0940903b12c5a877b 
  src/tests/CMakeLists.txt 6dd2716de942adf6cefa5a464ef664f3c3ebb7a3 
  src/tests/cmake/MesosTestsConfigure.cmake 
48b4d2fd74b545c03162c7cb06013baf67298241 
  src/usage/CMakeLists.txt fc87b297832157b679039c68baf9ddc68a292060 


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


Testing
---

Built and ran mesos-tests on CentOS 7 and Windows 10. THIS IS WORK IN PROGRESS 
FOR REVIEW.


Thanks,

Andrew Schwartzmeyer

Re: Review Request 61334: Imported `http_parser` library.

2017-08-09 Thread Andrew Schwartzmeyer


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

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


Review request for mesos.


Changes
---

The `UPDATE` command is unconditionally run, causing what appears to break 
incremental builds. It causes the install step (a noop) to be repeated, but 
that updates the target, and so causes libprocess to be relinked and so forth. 
The patch command is run once and only once, and really, what we're doing here 
(adding a file to the source tree) _is_ patching the source tree.


Repository: mesos


Description (updated)
---

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


Diffs (updated)
-

  3rdparty/CMakeLists.txt f4feaf90b4750338cfa2127524a5e6556a86d063 


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

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


Testing
---


Thanks,

Andrew Schwartzmeyer

Re: Review Request 61365: Added `get_byproducts()` function to fix Ninja generator.

2017-08-09 Thread Andrew Schwartzmeyer


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

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


Review request for mesos and Joseph Wu.


Changes
---

Updated because of a rebase.


Bugs: MESOS-3576, MESOS-5656 and MESOS-6794
https://issues.apache.org/jira/browse/MESOS-3576
https://issues.apache.org/jira/browse/MESOS-5656
https://issues.apache.org/jira/browse/MESOS-6794


Repository: mesos


Description (updated)
---

CMake also supports Ninja as a generator, but Ninja requires that each
use of `ExternalProject_Add` specifies its `BUILD_BYPRODUCTS`.
Interestingly, it doesn't care about interface libraries, and
furthermore, it can't derive the dependency from other
information (despite it being available).

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


Diffs (updated)
-

  3rdparty/CMakeLists.txt f4feaf90b4750338cfa2127524a5e6556a86d063 


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

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


Testing
---

This is the last patch in a long chain of CMake refactoring.

This has been tested on Windows 10, CentOS 7, and macOS. While I've worked on 
it extensively, it could still use yet more testing by others to ensure all 
configurations and environments still work properly.


Thanks,

Andrew Schwartzmeyer

Re: Review Request 61515: Updated `CompilationConfigure.cmake` for imported libraries.

2017-08-09 Thread Andrew Schwartzmeyer


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

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


Review request for mesos and Joseph Wu.


Changes
---

Nothing else is referred to here with the "lib" prefix, so I removed it from 
libapr.


Repository: mesos


Description (updated)
---

Windows and Linux have converged on several libraries (ZooKeeper,
Protobuf, and libevent) such that the same bundled version is used, so
the warning message was updated.

Third-party specific compilation flags were moved to the import of each
library, so this configuration no longer needs to include it.

The minimum CMake version was updated and usupports `string(TIMESTAMP)`,
deprecating the `date` work-around.


Diffs (updated)
-

  cmake/CompilationConfigure.cmake 2109697e02e58c673948f2ae67ef8255d16aa736 


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

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


Testing
---


Thanks,

Andrew Schwartzmeyer

Re: Review Request 61495: Removed table from markdown and added cross-links.

2017-08-09 Thread Till Toenshoff

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

docs/home.md
Lines 70 (patched)

We may want to consider calling these "task state reasons" instead. 

s/mesos/Mesos/

We generally use initial capitals for product and company names.

docs/task-reasons.md
Line 1 (original), 1 (patched)

"Task State Reasons" makes more sense to me.

docs/task-reasons.md
Lines 49 (patched)

Can we avoid HTML code here? We typically used HTML for getting tables 
properly formatted as the apache site rendering otherwise caused issues in the 
resulting HTML code.

If not done already, I would suggest you to play with the site renderer a 
bit to see if the results are actually what you are hoping for.

include/mesos/mesos.proto
Lines 2149-2150 (original)

Avoiding duplication by moving this comment into the implementation seems a 
good move to me.

- Till Toenshoff

On Aug. 9, 2017, 3:41 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61495/
> ---
> 
> (Updated Aug. 9, 2017, 3:41 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Till Toenshoff.
> 
> 
> Bugs: MESOS-5078
> https://issues.apache.org/jira/browse/MESOS-5078
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed table from markdown and added cross-links.
> 
> 
> Diffs
> -
> 
>   docs/home.md ab32838f621de76498262c9dd04e1cf01f8378ca 
>   docs/task-reasons.md PRE-CREATION 
>   include/mesos/mesos.proto f31f5bdc2ace2b261885b252e7c01ceb9b76c461 
>   include/mesos/v1/mesos.proto 66386a84bc21989d1c1237e629d5d04662a368fa 
>   src/master/master.cpp 43cb6977ca58dce1808e4bdb2d109d549622beb9 
> 
> 
> Diff: https://reviews.apache.org/r/61495/diff/2/
> 
> 
> Testing
> ---
> 
> None
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 61511: Improved the readability of some assertions/expectations.

2017-08-09 Thread Gastón Kleiman



> On Aug. 9, 2017, 3:21 p.m., Benjamin Bannier wrote:
> > src/tests/api_tests.cpp
> > Line 1 (original), 1 (patched)
> > 
> >
> > Searching with
> > 
> > $ git grep -E '(EXPECT|ASSERT)_(EQ|NE).*(0u,\s.*size())'
> > 
> > I still get ~50 hits. Since you are at it, would you mind adjusting 
> > also?

I had done a similar search, and as far as I can see, the other matches are 
either false possitives or in stout/libprocess:

```
src/tests/api_tests.cpp:ASSERT_EQ(0u, getState.get_tasks().tasks_size());
src/tests/api_tests.cpp:ASSERT_EQ(0u, 
getState.get_executors().executors_size());
src/tests/api_tests.cpp:ASSERT_EQ(0u, 
getState.get_tasks().completed_tasks_size());
src/tests/api_tests.cpp:ASSERT_EQ(0u, getState.get_tasks().tasks_size());
src/tests/api_tests.cpp:  EXPECT_EQ(0u, 
getState.get_frameworks().frameworks_size());
src/tests/api_tests.cpp:  EXPECT_EQ(0u, getState.get_agents().agents_size());
src/tests/api_tests.cpp:  EXPECT_EQ(0u, getState.get_tasks().tasks_size());
src/tests/api_tests.cpp:  EXPECT_EQ(0u, 
getState.get_executors().executors_size());
src/tests/api_tests.cpp:ASSERT_EQ(0u, 
v1Response->get_agents().agents_size());
src/tests/api_tests.cpp:ASSERT_EQ(0u, 
v1Response->get_agents().recovered_agents_size());
src/tests/api_tests.cpp:  EXPECT_EQ(0u, getState.get_tasks().tasks_size());
src/tests/api_tests.cpp:  EXPECT_EQ(0u, 
getState.get_executors().executors_size());
src/tests/api_tests.cpp:  EXPECT_EQ(0u, 
getState.get_frameworks().frameworks_size());
src/tests/api_tests.cpp:  EXPECT_EQ(0u, getState.get_agents().agents_size());
src/tests/api_tests.cpp:  EXPECT_EQ(0u, getState.get_tasks().tasks_size());
src/tests/api_tests.cpp:  EXPECT_EQ(0u, 
getState.get_executors().executors_size());
src/tests/api_tests.cpp:ASSERT_EQ(0u, 
getState.get_frameworks().frameworks_size());
src/tests/api_tests.cpp:ASSERT_EQ(0u, 
getState.get_tasks().launched_tasks_size());
src/tests/api_tests.cpp:ASSERT_EQ(0u, 
getState.get_executors().executors_size());
src/tests/api_tests.cpp:ASSERT_EQ(0u, 
getState.get_frameworks().completed_frameworks_size());
src/tests/api_tests.cpp:ASSERT_EQ(0u, 
getState.get_tasks().completed_tasks_size());
src/tests/api_tests.cpp:ASSERT_EQ(0u, 
getState.get_executors().completed_executors_size());
src/tests/api_tests.cpp:ASSERT_EQ(0u, 
getState.get_frameworks().frameworks_size());
src/tests/api_tests.cpp:ASSERT_EQ(0u, 
getState.get_tasks().launched_tasks_size());
src/tests/api_tests.cpp:ASSERT_EQ(0u, 
getState.get_executors().executors_size());
```

I don't think we can use `empty()` here.

And in the following match `manifest` is not a real container:

```
src/tests/containerizer/docker_spec_tests.cpp:  EXPECT_EQ(0u, manifest->size());
```

I'll drop this issue and add two more patches for stout and libprocess to the 
chain.


- Gastón


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


On Aug. 9, 2017, 1:01 a.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61511/
> ---
> 
> (Updated Aug. 9, 2017, 1:01 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Jie Yu, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Prefer checking whether a container is empty instead of checking its
> size.
> 
> 
> Diffs
> -
> 
>   src/tests/api_tests.cpp 75f7a585ec75f1d8b78453dd203bb78822ee7bc9 
>   src/tests/container_logger_tests.cpp 
> 2ebff809636fbf68c9e3fb87b98f168338f80d6d 
>   src/tests/containerizer/cgroups_isolator_tests.cpp 
> 67242119f2d10f6f3010c374ea58138e40e4a33e 
>   src/tests/containerizer/cgroups_tests.cpp 
> 506fc7f00dc1ac48476334de930b720b85a691dc 
>   src/tests/containerizer/cpu_isolator_tests.cpp 
> 0e86318bb7e261ac00be19f6405557f29a2e92af 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> c7984592aec2d4e7d1eb7ce077e742c4edc03bb9 
>   src/tests/containerizer/docker_volume_isolator_tests.cpp 
> 866af61f8669163ff3ddd10ed7ecb655568f8250 
>   src/tests/containerizer/environment_secret_isolator_tests.cpp 
> b034ceec7d1bf92db8a1d344835ad48ea2d24952 
>   src/tests/containerizer/io_switchboard_tests.cpp 
> 742143a0f093e415fe98235bbd25342fd65e0483 
>   src/tests/containerizer/linux_capabilities_isolator_tests.cpp 
> 6d95d607b81358953a4afcec03b60e87e7192edd 
>   src/tests/containerizer/linux_filesystem_isolator_tests.cpp 
> f9cab2fd123e48a5b4387fb2b609d70c7bf535cd 
>   src/tests/containerizer/memory_isolator_tests.cpp 
> b7b7acd37f69832e9ba79c9fe7ed0bc473b53e02 
>   src/tests/containerizer/memory_pressure_tests.cpp 
>

Re: Review Request 61517: Refactored OpenSSL library checks in libprocess.

2017-08-09 Thread Chun-Hung Hsiao


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

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


Review request for mesos, Benjamin Bannier, Jie Yu, and Till Toenshoff.


Changes
---

Addressed @tillt's comments.


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


Repository: mesos


Description
---

Since gRPC requires OpenSSL, we checks if libssl and libcrypto exist no
matter if the `--enable-ssl` flag is on. This enables us to have a
non-SSL-enabled libprocess build with gRPC support. Also fixed an error
that a bad linker might link a configure test with libssl unnecessarily,
which would cause a runtime failure if libssl is not in the runtime
library search path.


Diffs (updated)
-

  3rdparty/libprocess/configure.ac 29b69b97428a8d5fead09e507ae0e98a46761464 


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

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


Testing
---

Ran `make check` on the following two configurations:
1. ../configure
2. ../configure --enable-ssl --enable-libevent


Thanks,

Chun-Hung Hsiao

Re: Review Request 61433: Refactored OpenSSL library checks in Mesos.

2017-08-09 Thread Chun-Hung Hsiao


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

(Updated Aug. 9, 2017, 11:47 p.m.)


Review request for mesos, Benjamin Bannier, Jie Yu, and Till Toenshoff.


Changes
---

Addressed @tillt's comments.


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


Repository: mesos


Description
---

Since gRPC requires OpenSSL, we checks if libssl and libcrypto exist no
matter if the `--enable-ssl` flag is on. This enables us to have a
non-SSL-enabled Mesos build with gRPC support. Also fixed errors that
a bad linker might link the configure tests with libssl unnecessarily,
which would cause runtime failures if libssl is not in the runtime
library search path.


Diffs (updated)
-

  configure.ac 307f0aea7f19932befba37c5467851718d317c92 


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

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


Testing
---

Ran `make check` on the following two configurations:
1. ../configure
2. ../configure --enable-ssl --enable-libevent


Thanks,

Chun-Hung Hsiao

Re: Review Request 61517: Refactored OpenSSL library checks in libprocess.

2017-08-09 Thread Till Toenshoff


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


Ship it!





3rdparty/libprocess/configure.ac
Lines 957-959 (patched)


See my (non issue) comment in the previous one.


- Till Toenshoff


On Aug. 9, 2017, 10:47 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61517/
> ---
> 
> (Updated Aug. 9, 2017, 10:47 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Jie Yu, and Till Toenshoff.
> 
> 
> Bugs: MESOS-7870
> https://issues.apache.org/jira/browse/MESOS-7870
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Since gRPC requires OpenSSL, we checks if libssl and libcrypto exist no
> matter if the `--enable-ssl` flag is on. This enables us to have a
> non-SSL-enabled libprocess build with gRPC support. Also fixed an error
> that a bad linker might link a configure test with libssl unnecessarily,
> which would cause a runtime failure if libssl is not in the runtime
> library search path.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/configure.ac 29b69b97428a8d5fead09e507ae0e98a46761464 
> 
> 
> Diff: https://reviews.apache.org/r/61517/diff/2/
> 
> 
> Testing
> ---
> 
> Ran `make check` on the following two configurations:
> 1. ../configure
> 2. ../configure --enable-ssl --enable-libevent
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 61433: Refactored OpenSSL library checks in Mesos.

2017-08-09 Thread Till Toenshoff


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


Ship it!





configure.ac
Lines 1727-1729 (patched)


Your formatting is well readable but not entirely consistent with the other 
checks.

```
AC_CHECK_HEADERS([openssl/ssl.h],
 [AC_CHECK_LIB([ssl],
   [SSL_CTX_new],
   [found_ssl=yes],
   [],
   [-lcrypto])])
```

I am a fan of consistent formatting but leaving it up to you to decide :) - 
hence not an "issue".


- Till Toenshoff


On Aug. 9, 2017, 10:46 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61433/
> ---
> 
> (Updated Aug. 9, 2017, 10:46 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Jie Yu, and Till Toenshoff.
> 
> 
> Bugs: MESOS-7870
> https://issues.apache.org/jira/browse/MESOS-7870
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Since gRPC requires OpenSSL, we checks if libssl and libcrypto exist no
> matter if the `--enable-ssl` flag is on. This enables us to have a
> non-SSL-enabled Mesos build with gRPC support. Also fixed errors that
> a bad linker might link the configure tests with libssl unnecessarily,
> which would cause runtime failures if libssl is not in the runtime
> library search path.
> 
> 
> Diffs
> -
> 
>   configure.ac 5a6e42c61e6752c3742532502dbc753071b31323 
> 
> 
> Diff: https://reviews.apache.org/r/61433/diff/4/
> 
> 
> Testing
> ---
> 
> Ran `make check` on the following two configurations:
> 1. ../configure
> 2. ../configure --enable-ssl --enable-libevent
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 61517: Refactored OpenSSL library checks in libprocess.

2017-08-09 Thread Chun-Hung Hsiao


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

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


Review request for mesos, Benjamin Bannier, Jie Yu, and Till Toenshoff.


Changes
---

Addressed @bbannier's comments.


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


Repository: mesos


Description
---

Since gRPC requires OpenSSL, we checks if libssl and libcrypto exist no
matter if the `--enable-ssl` flag is on. This enables us to have a
non-SSL-enabled libprocess build with gRPC support. Also fixed an error
that a bad linker might link a configure test with libssl unnecessarily,
which would cause a runtime failure if libssl is not in the runtime
library search path.


Diffs (updated)
-

  3rdparty/libprocess/configure.ac 29b69b97428a8d5fead09e507ae0e98a46761464 


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

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


Testing
---

Ran `make check` on the following two configurations:
1. ../configure
2. ../configure --enable-ssl --enable-libevent


Thanks,

Chun-Hung Hsiao

Re: Review Request 61433: Refactored OpenSSL library checks in Mesos.

2017-08-09 Thread Chun-Hung Hsiao


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

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


Review request for mesos, Benjamin Bannier, Jie Yu, and Till Toenshoff.


Changes
---

Addressed @bbannier's comments.


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


Repository: mesos


Description
---

Since gRPC requires OpenSSL, we checks if libssl and libcrypto exist no
matter if the `--enable-ssl` flag is on. This enables us to have a
non-SSL-enabled Mesos build with gRPC support. Also fixed errors that
a bad linker might link the configure tests with libssl unnecessarily,
which would cause runtime failures if libssl is not in the runtime
library search path.


Diffs (updated)
-

  configure.ac 5a6e42c61e6752c3742532502dbc753071b31323 


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

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


Testing
---

Ran `make check` on the following two configurations:
1. ../configure
2. ../configure --enable-ssl --enable-libevent


Thanks,

Chun-Hung Hsiao

Re: Review Request 61535: Updated comments for the agent's 'getExecutorInfo()' helper function.

2017-08-09 Thread Greg Mann


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



Discarding because I included these changes when committing the following 
patch: https://reviews.apache.org/r/61524/

- Greg Mann


On Aug. 9, 2017, 6:58 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61535/
> ---
> 
> (Updated Aug. 9, 2017, 6:58 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Benjamin Mahler.
> 
> 
> Bugs: MESOS-7871
> https://issues.apache.org/jira/browse/MESOS-7871
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch updates comments associated with the getExecutorInfo()
> helper function to more accurately reflect the fact that TaskInfo
> objects launched as part of a task group will have both ExecutorInfo
> and CommandInfo set.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.hpp 1fe93dab1b2bef24721cc1bcffebe1b259e96d79 
>   src/slave/slave.cpp 7381530515f86faf4c3e8f82bcd9483f6cf0498b 
> 
> 
> Diff: https://reviews.apache.org/r/61535/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 61524: Moved task validation from `getExecutorInfo` to `runTask` on agent.

2017-08-09 Thread Greg Mann



> On Aug. 9, 2017, 6:04 p.m., Benjamin Mahler wrote:
> > src/slave/slave.cpp
> > Lines 4987-4989 (original), 4987-4989 (patched)
> > 
> >
> > Perhaps a note here saying that the master injects the executor for 
> > tasks within a task group?
> 
> Greg Mann wrote:
> In the interest of time, I followed up with patches to add a new test and 
> update comments:
> https://reviews.apache.org/r/61534/
> https://reviews.apache.org/r/61535/

I included the comment updates from https://reviews.apache.org/r/61535/ in this 
patch when committing.


- Greg


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


On Aug. 9, 2017, 1:14 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61524/
> ---
> 
> (Updated Aug. 9, 2017, 1:14 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Greg Mann, and Vinod Kone.
> 
> 
> Bugs: MESOS-7871
> https://issues.apache.org/jira/browse/MESOS-7871
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, `getExecutorInfo` was called only in `runTask`, so it was
> validating an invariant related to launching a single task. The
> invariant is that a task should have either CommandInfo or ExecutorInfo
> set but not both. Now `getExecutorInfo` is also called to calculate
> allocated resources which might be related to a task group, which might
> violate the invariant.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp 7381530515f86faf4c3e8f82bcd9483f6cf0498b 
> 
> 
> Diff: https://reviews.apache.org/r/61524/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 61531: Fixed the device number proto 'major' and 'minor' to avoid MACROs.

2017-08-09 Thread Gilbert Song


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

(Updated Aug. 9, 2017, 3:24 p.m.)


Review request for mesos, Ilya Pronin, Jie Yu, James Peach, Qian Zhang, and 
Vinod Kone.


Repository: mesos


Description
---

Fixed the device number proto 'major' and 'minor' to avoid MACROs.


Diffs (updated)
-

  include/mesos/mesos.proto f31f5bdc2ace2b261885b252e7c01ceb9b76c461 
  include/mesos/v1/mesos.proto 66386a84bc21989d1c1237e629d5d04662a368fa 
  src/linux/cgroups.hpp e708747f16fa4579bbd7d0e9e4bf969c05625568 
  src/linux/cgroups.cpp 21f9983cb34a7b37d65b802dc56bc4804077664b 
  src/slave/containerizer/mesos/isolators/cgroups/subsystems/blkio.cpp 
96014b560377b5c716e30781de18d34c2ea8e17b 


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

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


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 60495: Added network ports isolator listen socket utilities.

2017-08-09 Thread James Peach


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

(Updated Aug. 9, 2017, 8:19 p.m.)


Review request for mesos, Qian Zhang and Jiang Yan Xu.


Changes
---

Rebased and addressed review feedback.


Summary (updated)
-

Added network ports isolator listen socket utilities.


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


Repository: mesos


Description
---

Implemented network ports isolator socket utilities to find the
inode numbers of all the listening sockets, and the inodes of the
open sockets for a process. Together, these utilities can tell us
which sockets a process is listening on.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/network/ports.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/network/ports.cpp PRE-CREATION 


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

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


Testing
---

make check (Fedora 26)


Thanks,

James Peach

Review Request 61538: Used common port range interval code in the port_mapping isolator.

2017-08-09 Thread James Peach


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

Review request for mesos, Qian Zhang and Jiang Yan Xu.


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


Repository: mesos


Description
---

Switched the port_mapping isolator over to start using the
common values code to parse ranges into an IntervalSet. Since
that is generic code, we now deal with ports as uint64_t rather
than uint16_t.


Diffs
-

  src/slave/containerizer/mesos/isolators/network/port_mapping.hpp 
6ff3820de6af544c6ec92b0e76fd934715b87a96 
  src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
57d4ccd515b538eece94a228916ec764c83473be 


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


Testing
---

make check (Fedora 26)


Thanks,

James Peach

Re: Review Request 61517: Refactored OpenSSL library checks in libprocess.

2017-08-09 Thread Benjamin Bannier


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


Fix it, then Ship it!





3rdparty/libprocess/configure.ac
Lines 1108-1109 (patched)


Let's focus this comment, see 
https://reviews.apache.org/r/61433/#comment258427.


- Benjamin Bannier


On Aug. 9, 2017, 5 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61517/
> ---
> 
> (Updated Aug. 9, 2017, 5 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Jie Yu, and Till Toenshoff.
> 
> 
> Bugs: MESOS-7870
> https://issues.apache.org/jira/browse/MESOS-7870
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Since gRPC requires OpenSSL, we checks if libssl and libcrypto exist no
> matter if the `--enable-ssl` flag is on. This enables us to have a
> non-SSL-enabled libprocess build with gRPC support. Also fixed an error
> that a bad linker might link a configure test with libssl unnecessarily,
> which would cause a runtime failure if libssl is not in the runtime
> library search path.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/configure.ac 29b69b97428a8d5fead09e507ae0e98a46761464 
> 
> 
> Diff: https://reviews.apache.org/r/61517/diff/1/
> 
> 
> Testing
> ---
> 
> Ran `make check` on the following two configurations:
> 1. ../configure
> 2. ../configure --enable-ssl --enable-libevent
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 61433: Refactored OpenSSL library checks in Mesos.

2017-08-09 Thread Benjamin Bannier


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


Fix it, then Ship it!





configure.ac
Lines 1967-1968 (patched)


We are not working around linker bugs, but our autotools setup where we 
globally mess with `LIBS` not taking into account e.g., setting up library 
lookup paths.

How about just

# NOTE: We clear `LIBS` here to prevent linking in libraries unrelated 
to
# the test. These libraries might not be in the linker lookup paths.


- Benjamin Bannier


On Aug. 9, 2017, 5 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61433/
> ---
> 
> (Updated Aug. 9, 2017, 5 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Jie Yu, and Till Toenshoff.
> 
> 
> Bugs: MESOS-7870
> https://issues.apache.org/jira/browse/MESOS-7870
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Since gRPC requires OpenSSL, we checks if libssl and libcrypto exist no
> matter if the `--enable-ssl` flag is on. This enables us to have a
> non-SSL-enabled Mesos build with gRPC support. Also fixed errors that
> a bad linker might link the configure tests with libssl unnecessarily,
> which would cause runtime failures if libssl is not in the runtime
> library search path.
> 
> 
> Diffs
> -
> 
>   configure.ac 5a6e42c61e6752c3742532502dbc753071b31323 
> 
> 
> Diff: https://reviews.apache.org/r/61433/diff/3/
> 
> 
> Testing
> ---
> 
> Ran `make check` on the following two configurations:
> 1. ../configure
> 2. ../configure --enable-ssl --enable-libevent
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 61531: Fixed the device number proto 'major' and 'minor' to avoid MACROs.

2017-08-09 Thread James Peach


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




src/slave/containerizer/mesos/isolators/cgroups/subsystems/blkio.cpp
Line 354 (original), 354 (patched)


You also need to include `` for this usage.


- James Peach


On Aug. 9, 2017, 7:05 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61531/
> ---
> 
> (Updated Aug. 9, 2017, 7:05 p.m.)
> 
> 
> Review request for mesos, Ilya Pronin, Jie Yu, James Peach, Qian Zhang, and 
> Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed the device number proto 'major' and 'minor' to avoid MACROs.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto f31f5bdc2ace2b261885b252e7c01ceb9b76c461 
>   include/mesos/v1/mesos.proto 66386a84bc21989d1c1237e629d5d04662a368fa 
>   src/linux/cgroups.hpp e708747f16fa4579bbd7d0e9e4bf969c05625568 
>   src/linux/cgroups.cpp 21f9983cb34a7b37d65b802dc56bc4804077664b 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/blkio.cpp 
> 96014b560377b5c716e30781de18d34c2ea8e17b 
> 
> 
> Diff: https://reviews.apache.org/r/61531/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 61531: Fixed the device number proto 'major' and 'minor' to avoid MACROs.

2017-08-09 Thread Gilbert Song



> On Aug. 9, 2017, 11:07 a.m., James Peach wrote:
> > We still need to switch to using the `major` and `minor` macros from 
> > `sys/sysmacros.h` so avoid the deprecation warning. 
> > [Here's](https://paste.fedoraproject.org/paste/QvCDIi9ZUeLG2MhVIDwhOA) what 
> > I would recommend.
> > 
> > ```
> > In file included from 
> > ../../src/slave/containerizer/mesos/linux_launcher.cpp:34:0:
> > ../../src/linux/cgroups.hpp:418:13: error: In the GNU C Library, "major" is 
> > defined
> >  by . For historical compatibility, it is
> >  currently defined by  as well, but we plan to
> >  remove this soon. To use "major", include 
> >  directly. If you did not intend to use a system-defined macro
> >  "major", you should undefine it after including . [-Werror]
> >inline unsigned int getMajor() const { return major(value); }
> >  ^~~
> > ```
> 
> James Peach wrote:
> We also need `sys/sysmacros.h` in 
> `src/slave/containerizer/mesos/isolators/cgroups/subsystems/blkio.cpp`.

Thanks for the great comment, James!


- Gilbert


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


On Aug. 9, 2017, 12:05 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61531/
> ---
> 
> (Updated Aug. 9, 2017, 12:05 p.m.)
> 
> 
> Review request for mesos, Ilya Pronin, Jie Yu, James Peach, Qian Zhang, and 
> Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed the device number proto 'major' and 'minor' to avoid MACROs.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto f31f5bdc2ace2b261885b252e7c01ceb9b76c461 
>   include/mesos/v1/mesos.proto 66386a84bc21989d1c1237e629d5d04662a368fa 
>   src/linux/cgroups.hpp e708747f16fa4579bbd7d0e9e4bf969c05625568 
>   src/linux/cgroups.cpp 21f9983cb34a7b37d65b802dc56bc4804077664b 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/blkio.cpp 
> 96014b560377b5c716e30781de18d34c2ea8e17b 
> 
> 
> Diff: https://reviews.apache.org/r/61531/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 61531: Fixed the device number proto 'major' and 'minor' to avoid MACROs.

2017-08-09 Thread Gilbert Song


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

(Updated Aug. 9, 2017, 12:05 p.m.)


Review request for mesos, Ilya Pronin, Jie Yu, James Peach, Qian Zhang, and 
Vinod Kone.


Changes
---

Addressed James' comment.


Summary (updated)
-

Fixed the device number proto 'major' and 'minor' to avoid MACROs.


Repository: mesos


Description
---

Fixed the device number proto 'major' and 'minor' to avoid MACROs.


Diffs (updated)
-

  include/mesos/mesos.proto f31f5bdc2ace2b261885b252e7c01ceb9b76c461 
  include/mesos/v1/mesos.proto 66386a84bc21989d1c1237e629d5d04662a368fa 
  src/linux/cgroups.hpp e708747f16fa4579bbd7d0e9e4bf969c05625568 
  src/linux/cgroups.cpp 21f9983cb34a7b37d65b802dc56bc4804077664b 
  src/slave/containerizer/mesos/isolators/cgroups/subsystems/blkio.cpp 
96014b560377b5c716e30781de18d34c2ea8e17b 


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

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


Testing (updated)
---

make check


Thanks,

Gilbert Song

Re: Review Request 61535: Updated comments for the agent's 'getExecutorInfo()' helper function.

2017-08-09 Thread Greg Mann


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

(Updated Aug. 9, 2017, 6:58 p.m.)


Review request for mesos, Andrei Budnik and Benjamin Mahler.


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


Repository: mesos


Description (updated)
---

This patch updates comments associated with the getExecutorInfo()
helper function to more accurately reflect the fact that TaskInfo
objects launched as part of a task group will have both ExecutorInfo
and CommandInfo set.


Diffs
-

  src/slave/slave.hpp 1fe93dab1b2bef24721cc1bcffebe1b259e96d79 
  src/slave/slave.cpp 7381530515f86faf4c3e8f82bcd9483f6cf0498b 


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


Testing
---


Thanks,

Greg Mann

Re: Review Request 61535: Updated comments for the agent's 'getExecutorInfo()' helper function.

2017-08-09 Thread Greg Mann


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

(Updated Aug. 9, 2017, 6:57 p.m.)


Review request for mesos, Andrei Budnik and Benjamin Mahler.


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


Repository: mesos


Description (updated)
---

Updated comments for the agent's 'getExecutorInfo()' helper function.


Diffs (updated)
-

  src/slave/slave.hpp 1fe93dab1b2bef24721cc1bcffebe1b259e96d79 
  src/slave/slave.cpp 7381530515f86faf4c3e8f82bcd9483f6cf0498b 


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

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


Testing
---


Thanks,

Greg Mann

Re: Review Request 61534: Added test to verify the fix for a failed agent assertion.

2017-08-09 Thread Greg Mann


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

(Updated Aug. 9, 2017, 6:56 p.m.)


Review request for mesos, Andrei Budnik and Benjamin Mahler.


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


Repository: mesos


Description (updated)
---

This patch adds 'SlaveTest.GetStateTaskGroupPending', which confirms
the fix for MESOS-7871. The test verifies that requests to the agent's
'/state' endpoint are successful when there are pending tasks on the
agent which were launched as part of a task group.


Diffs
-

  src/tests/slave_tests.cpp 1d9d142ed9e801b79535a2c28f5a94ffbf1bf160 


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


Testing
---

`bin/mesos-tests.sh --gtest_filter="*GetStateTaskGroupPending*" 
--gtest_repeat=1000`


Thanks,

Greg Mann

Re: Review Request 61534: Added test to verify the fix for a failed agent assertion.

2017-08-09 Thread Greg Mann


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

(Updated Aug. 9, 2017, 6:55 p.m.)


Review request for mesos, Andrei Budnik and Benjamin Mahler.


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


Repository: mesos


Description (updated)
---

Added test to verify the fix for a failed agent assertion.


Diffs (updated)
-

  src/tests/slave_tests.cpp 1d9d142ed9e801b79535a2c28f5a94ffbf1bf160 


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

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


Testing
---

`bin/mesos-tests.sh --gtest_filter="*GetStateTaskGroupPending*" 
--gtest_repeat=1000`


Thanks,

Greg Mann

Re: Review Request 61535: Updated comments for the agent's 'getExecutorInfo()' helper function.

2017-08-09 Thread Benjamin Mahler


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


Ship it!





src/slave/slave.hpp
Lines 399-400 (original), 399-401 (patched)


then we generate an ExecutorInfo corresponding to the command executor

suitable for command executor sounds a bit odd?


- Benjamin Mahler


On Aug. 9, 2017, 6:33 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61535/
> ---
> 
> (Updated Aug. 9, 2017, 6:33 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Benjamin Mahler.
> 
> 
> Bugs: MESOS-7871
> https://issues.apache.org/jira/browse/MESOS-7871
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch updates comments associated with the `getExecutorInfo()`
> helper function to more accurately reflect the fact that `TaskInfo`
> objects launched as part of a task group will have both `ExecutorInfo`
> and `CommandInfo` set.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.hpp 1fe93dab1b2bef24721cc1bcffebe1b259e96d79 
>   src/slave/slave.cpp 7381530515f86faf4c3e8f82bcd9483f6cf0498b 
> 
> 
> Diff: https://reviews.apache.org/r/61535/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 61524: Moved task validation from `getExecutorInfo` to `runTask` on agent.

2017-08-09 Thread Greg Mann



> On Aug. 9, 2017, 6:04 p.m., Benjamin Mahler wrote:
> > src/slave/slave.cpp
> > Lines 4987-4989 (original), 4987-4989 (patched)
> > 
> >
> > Perhaps a note here saying that the master injects the executor for 
> > tasks within a task group?

In the interest of time, I followed up with patches to add a new test and 
update comments:
https://reviews.apache.org/r/61534/
https://reviews.apache.org/r/61535/


- Greg


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


On Aug. 9, 2017, 1:14 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61524/
> ---
> 
> (Updated Aug. 9, 2017, 1:14 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Greg Mann, and Vinod Kone.
> 
> 
> Bugs: MESOS-7871
> https://issues.apache.org/jira/browse/MESOS-7871
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, `getExecutorInfo` was called only in `runTask`, so it was
> validating an invariant related to launching a single task. The
> invariant is that a task should have either CommandInfo or ExecutorInfo
> set but not both. Now `getExecutorInfo` is also called to calculate
> allocated resources which might be related to a task group, which might
> violate the invariant.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp 7381530515f86faf4c3e8f82bcd9483f6cf0498b 
> 
> 
> Diff: https://reviews.apache.org/r/61524/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 61534: Added test to verify the fix for a failed agent assertion.

2017-08-09 Thread Benjamin Mahler


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


Fix it, then Ship it!





src/tests/slave_tests.cpp
Lines 1825 (patched)


Maybe clarify here that we just want to make sure it doesn't crsah? Ideally 
we would be looking to make sure the pending tasks show up (which I hope we 
already have a test for for non task groups?)


- Benjamin Mahler


On Aug. 9, 2017, 6:28 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61534/
> ---
> 
> (Updated Aug. 9, 2017, 6:28 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Benjamin Mahler.
> 
> 
> Bugs: MESOS-7871
> https://issues.apache.org/jira/browse/MESOS-7871
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds 'SlaveTest.GetStateTaskGroupPending', which confirms
> the fix for MESOS-7871. The test verifies that requests to the agent's
> '/state' endpoint are successful when there are pending tasks on the
> agent which were launched as part of a task group.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_tests.cpp 1d9d142ed9e801b79535a2c28f5a94ffbf1bf160 
> 
> 
> Diff: https://reviews.apache.org/r/61534/diff/1/
> 
> 
> Testing
> ---
> 
> `bin/mesos-tests.sh --gtest_filter="*GetStateTaskGroupPending*" 
> --gtest_repeat=1000`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 61524: Moved task validation from `getExecutorInfo` to `runTask` on agent.

2017-08-09 Thread Greg Mann


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


Ship it!




Ship It!

- Greg Mann


On Aug. 9, 2017, 1:14 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61524/
> ---
> 
> (Updated Aug. 9, 2017, 1:14 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Greg Mann, and Vinod Kone.
> 
> 
> Bugs: MESOS-7871
> https://issues.apache.org/jira/browse/MESOS-7871
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, `getExecutorInfo` was called only in `runTask`, so it was
> validating an invariant related to launching a single task. The
> invariant is that a task should have either CommandInfo or ExecutorInfo
> set but not both. Now `getExecutorInfo` is also called to calculate
> allocated resources which might be related to a task group, which might
> violate the invariant.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp 7381530515f86faf4c3e8f82bcd9483f6cf0498b 
> 
> 
> Diff: https://reviews.apache.org/r/61524/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Review Request 61535: Updated comments for the agent's 'getExecutorInfo()' helper function.

2017-08-09 Thread Greg Mann


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

Review request for mesos, Andrei Budnik and Benjamin Mahler.


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


Repository: mesos


Description
---

This patch updates comments associated with the `getExecutorInfo()`
helper function to more accurately reflect the fact that `TaskInfo`
objects launched as part of a task group will have both `ExecutorInfo`
and `CommandInfo` set.


Diffs
-

  src/slave/slave.hpp 1fe93dab1b2bef24721cc1bcffebe1b259e96d79 
  src/slave/slave.cpp 7381530515f86faf4c3e8f82bcd9483f6cf0498b 


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


Testing
---


Thanks,

Greg Mann

Review Request 61534: Added test to verify the fix for a failed agent assertion.

2017-08-09 Thread Greg Mann


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

Review request for mesos, Andrei Budnik and Benjamin Mahler.


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


Repository: mesos


Description
---

This patch adds 'SlaveTest.GetStateTaskGroupPending', which confirms
the fix for MESOS-7871. The test verifies that requests to the agent's
'/state' endpoint are successful when there are pending tasks on the
agent which were launched as part of a task group.


Diffs
-

  src/tests/slave_tests.cpp 1d9d142ed9e801b79535a2c28f5a94ffbf1bf160 


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


Testing
---

`bin/mesos-tests.sh --gtest_filter="*GetStateTaskGroupPending*" 
--gtest_repeat=1000`


Thanks,

Greg Mann

Re: Review Request 61531: [WIP] Fixed the device number proto 'major' and 'minor' to avoid MACROs.

2017-08-09 Thread James Peach



> On Aug. 9, 2017, 6:07 p.m., James Peach wrote:
> > We still need to switch to using the `major` and `minor` macros from 
> > `sys/sysmacros.h` so avoid the deprecation warning. 
> > [Here's](https://paste.fedoraproject.org/paste/QvCDIi9ZUeLG2MhVIDwhOA) what 
> > I would recommend.
> > 
> > ```
> > In file included from 
> > ../../src/slave/containerizer/mesos/linux_launcher.cpp:34:0:
> > ../../src/linux/cgroups.hpp:418:13: error: In the GNU C Library, "major" is 
> > defined
> >  by . For historical compatibility, it is
> >  currently defined by  as well, but we plan to
> >  remove this soon. To use "major", include 
> >  directly. If you did not intend to use a system-defined macro
> >  "major", you should undefine it after including . [-Werror]
> >inline unsigned int getMajor() const { return major(value); }
> >  ^~~
> > ```

We also need `sys/sysmacros.h` in 
`src/slave/containerizer/mesos/isolators/cgroups/subsystems/blkio.cpp`.


- James


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


On Aug. 9, 2017, 5:38 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61531/
> ---
> 
> (Updated Aug. 9, 2017, 5:38 p.m.)
> 
> 
> Review request for mesos, Ilya Pronin, Jie Yu, James Peach, Qian Zhang, and 
> Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed the device number proto 'major' and 'minor' to avoid MACROs.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto f31f5bdc2ace2b261885b252e7c01ceb9b76c461 
>   include/mesos/v1/mesos.proto 66386a84bc21989d1c1237e629d5d04662a368fa 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/blkio.cpp 
> 96014b560377b5c716e30781de18d34c2ea8e17b 
> 
> 
> Diff: https://reviews.apache.org/r/61531/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 61531: [WIP] Fixed the device number proto 'major' and 'minor' to avoid MACROs.

2017-08-09 Thread James Peach

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

We still need to switch to using the `major` and `minor` macros from 
`sys/sysmacros.h` so avoid the deprecation warning. 
[Here's](https://paste.fedoraproject.org/paste/QvCDIi9ZUeLG2MhVIDwhOA) what I 
would recommend.

```
In file included from 
../../src/slave/containerizer/mesos/linux_launcher.cpp:34:0:
../../src/linux/cgroups.hpp:418:13: error: In the GNU C Library, "major" is 
defined
 by . For historical compatibility, it is
 currently defined by  as well, but we plan to
 remove this soon. To use "major", include 
 directly. If you did not intend to use a system-defined macro
 "major", you should undefine it after including . [-Werror]
   inline unsigned int getMajor() const { return major(value); }
 ^~~
```

- James Peach

On Aug. 9, 2017, 5:38 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61531/
> ---
> 
> (Updated Aug. 9, 2017, 5:38 p.m.)
> 
> 
> Review request for mesos, Ilya Pronin, Jie Yu, James Peach, Qian Zhang, and 
> Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed the device number proto 'major' and 'minor' to avoid MACROs.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto f31f5bdc2ace2b261885b252e7c01ceb9b76c461 
>   include/mesos/v1/mesos.proto 66386a84bc21989d1c1237e629d5d04662a368fa 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/blkio.cpp 
> 96014b560377b5c716e30781de18d34c2ea8e17b 
> 
> 
> Diff: https://reviews.apache.org/r/61531/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 61524: Moved task validation from `getExecutorInfo` to `runTask` on agent.

2017-08-09 Thread Benjamin Mahler


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


Ship it!




Can we also add a test that ensures that getting the metrics succeeds when task 
groups were launched?


src/slave/slave.cpp
Lines 4987-4989 (original), 4987-4989 (patched)


Perhaps a note here saying that the master injects the executor for tasks 
within a task group?


- Benjamin Mahler


On Aug. 9, 2017, 1:14 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61524/
> ---
> 
> (Updated Aug. 9, 2017, 1:14 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Greg Mann, and Vinod Kone.
> 
> 
> Bugs: MESOS-7871
> https://issues.apache.org/jira/browse/MESOS-7871
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, `getExecutorInfo` was called only in `runTask`, so it was
> validating an invariant related to launching a single task. The
> invariant is that a task should have either CommandInfo or ExecutorInfo
> set but not both. Now `getExecutorInfo` is also called to calculate
> allocated resources which might be related to a task group, which might
> violate the invariant.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp 7381530515f86faf4c3e8f82bcd9483f6cf0498b 
> 
> 
> Diff: https://reviews.apache.org/r/61524/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

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

2017-08-09 Thread Eric Chung


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

(Updated Aug. 9, 2017, 5:54 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 (updated)
---

install tox
cd src/python/lib
tox


Thanks,

Eric Chung

Review Request 61531: [WIP] Fixed the device number proto 'major' and 'minor' to avoid MACROs.

2017-08-09 Thread Gilbert Song


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

Review request for mesos, Ilya Pronin, Jie Yu, James Peach, Qian Zhang, and 
Vinod Kone.


Repository: mesos


Description
---

Fixed the device number proto 'major' and 'minor' to avoid MACROs.


Diffs
-

  include/mesos/mesos.proto f31f5bdc2ace2b261885b252e7c01ceb9b76c461 
  include/mesos/v1/mesos.proto 66386a84bc21989d1c1237e629d5d04662a368fa 
  src/slave/containerizer/mesos/isolators/cgroups/subsystems/blkio.cpp 
96014b560377b5c716e30781de18d34c2ea8e17b 


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


Testing
---


Thanks,

Gilbert Song

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

2017-08-09 Thread Avinash sridharan

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

docs/health-checks.md
Line 29 (original), 29 (patched)

s/defines/which defines?

docs/health-checks.md
Lines 162 (patched)

s/the following/to the following?

docs/health-checks.md
Lines 204 (patched)

I think we also need to mention that the expectation here is that task is 
listening on the `loopback` interface along with any other routeable interface 
to which it might be listening.

docs/health-checks.md
Lines 208 (patched)

how is the `host` resolved? It's not necessary that it will resolve to 
`127.0.0.1`?

docs/health-checks.md
Lines 232 (patched)

Ditto to the comments on the `HTTP Checks` section.

docs/health-checks.md
Line 265 (original), 494 (patched)

Why do we need to share the `mnt` namespace? This is already done by the 
executor for `MesosContainerizer` so why does the `checker` need to do this if 
it is running as part of the executor?

docs/health-checks.md
Line 267 (original), 496 (patched)

A bit confused here? I thought only `Health checks` are supported for 
`docker executor` and not `checks`?

- Avinash sridharan

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 61530: Enabled retries for `killTasks` in docker executor.

2017-08-09 Thread Andrei Budnik


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

Review request for mesos and Alexander Rukletsov.


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


Repository: mesos


Description
---

Previously, after `docker stop` command failure, docker executor
neither allowed a scheduler to retry `killTask` command, nor retried to
kill a task when task killing was triggered by a failed health check.


Diffs
-

  src/docker/executor.cpp 26f12ec002f754fab0d34c01472cf95b499d8007 


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


Testing
---

sudo make check

Manual testing:

Emulating `docker stop` errors:
===
1. Add `return fmt.Errorf("Emulating error!")` to 
https://github.com/docker/docker-ce/blob/master/components/engine/daemon/stop.go#L21
2. build docker from sources: 
http://oyvindsk.com/writing/docker-build-from-source
3. stop docker service and launch dockerd binary, like: `sudo 
./bundles/17.06.0-dev/binary-daemon/dockerd`

Emulating docker daemon hang:
=
1. `ps aux|grep dockerd` - 2 processes will be found
2. `sudo kill -STOP  ` - send SIGSTOP to docker daemon processes 
just before sending `docker stop`

Emulating health check failure in docker executor:
==
1. Add
```c++
  static int fake = 0;
  if (++fake > 10) {
failure();
return;
  }
```
to `HealthChecker::processCheckResult()` in `src/checks/health_checker.cpp`
2. Add
```c++
   HealthCheck healthCheck;
   healthCheck.set_type(HealthCheck::COMMAND);
   healthCheck.mutable_command()->set_value("exit 0");
   healthCheck.set_delay_seconds(0);
   healthCheck.set_interval_seconds(0);
   healthCheck.set_grace_period_seconds(1);
   _task.mutable_health_check()->CopyFrom(healthCheck);
```
to `CommandScheduler::offers()` in `src/cli/execute.cpp`
3. compile mesos
4. run mesos agent: `sudo GLOG_v=1 ./bin/mesos-agent.sh 
--resources="cpus:1;mem:100" 
--work_dir='/home/some_user/mesos/build/var/agent-1' 
--containerizers="docker,mesos" --master="127.0.1.1:5050"`
5. launch docker executor: `./src/mesos-execute --master="127.0.1.1:5050" 
--name="a" --containerizer=docker --docker_image="ubuntu:xenial" 
--command="while true; do : ; done"`


Thanks,

Andrei Budnik

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

2017-08-09 Thread Till Toenshoff


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


Ship it!




Ship It!

- Till Toenshoff


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 60646: Modified handling of parallel test configure flag for documentation.

2017-08-09 Thread Till Toenshoff


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


Ship it!




Ship It!

- Till Toenshoff


On Aug. 7, 2017, 10:30 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60646/
> ---
> 
> (Updated Aug. 7, 2017, 10:30 a.m.)
> 
> 
> Review request for mesos and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> 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
> -
> 
>   configure.ac 5a6e42c61e6752c3742532502dbc753071b31323 
> 
> 
> Diff: https://reviews.apache.org/r/60646/diff/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
> 
>

Re: Review Request 61495: Removed table from markdown and added cross-links.

2017-08-09 Thread Benno Evers


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

(Updated Aug. 9, 2017, 3:41 p.m.)


Review request for mesos, Alexander Rukletsov and Till Toenshoff.


Changes
---

Removed table from markdown and added cross-links.


Summary (updated)
-

Removed table from markdown and added cross-links.


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


Repository: mesos


Description (updated)
---

Removed table from markdown and added cross-links.


Diffs (updated)
-

  docs/home.md ab32838f621de76498262c9dd04e1cf01f8378ca 
  docs/task-reasons.md PRE-CREATION 
  include/mesos/mesos.proto f31f5bdc2ace2b261885b252e7c01ceb9b76c461 
  include/mesos/v1/mesos.proto 66386a84bc21989d1c1237e629d5d04662a368fa 
  src/master/master.cpp 43cb6977ca58dce1808e4bdb2d109d549622beb9 


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

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


Testing
---

None


Thanks,

Benno Evers

Re: Review Request 61511: Improved the readability of some assertions/expectations.

2017-08-09 Thread Benjamin Bannier


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


Fix it, then Ship it!




Thanks for the cleanup Gaston!

Until looking at your patch I didn't realize how widespread the mistake of 
possibly using junk elements from possibly empty containers was, e.g.,

 EXPECT_NE(0u, offers.size()); // or equivalent: 
EXPECT_FALSE(offers.empty())
 
 // Do something with offers[0] which for e.g., 
 // `vector` doesn't do bounds checks.

The correct pattern here would be to do a hard assert for a non-empty 
collection so we don't run into undefined behavior should we against our 
expectations receive an empty collection. Could you please file a ticket for 
that?


src/tests/api_tests.cpp
Line 1 (original), 1 (patched)


Searching with

$ git grep -E '(EXPECT|ASSERT)_(EQ|NE).*(0u,\s.*size())'

I still get ~50 hits. Since you are at it, would you mind adjusting also?


- Benjamin Bannier


On Aug. 9, 2017, 3:01 a.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61511/
> ---
> 
> (Updated Aug. 9, 2017, 3:01 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Jie Yu, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Prefer checking whether a container is empty instead of checking its
> size.
> 
> 
> Diffs
> -
> 
>   src/tests/api_tests.cpp 75f7a585ec75f1d8b78453dd203bb78822ee7bc9 
>   src/tests/container_logger_tests.cpp 
> 2ebff809636fbf68c9e3fb87b98f168338f80d6d 
>   src/tests/containerizer/cgroups_isolator_tests.cpp 
> 67242119f2d10f6f3010c374ea58138e40e4a33e 
>   src/tests/containerizer/cgroups_tests.cpp 
> 506fc7f00dc1ac48476334de930b720b85a691dc 
>   src/tests/containerizer/cpu_isolator_tests.cpp 
> 0e86318bb7e261ac00be19f6405557f29a2e92af 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> c7984592aec2d4e7d1eb7ce077e742c4edc03bb9 
>   src/tests/containerizer/docker_volume_isolator_tests.cpp 
> 866af61f8669163ff3ddd10ed7ecb655568f8250 
>   src/tests/containerizer/environment_secret_isolator_tests.cpp 
> b034ceec7d1bf92db8a1d344835ad48ea2d24952 
>   src/tests/containerizer/io_switchboard_tests.cpp 
> 742143a0f093e415fe98235bbd25342fd65e0483 
>   src/tests/containerizer/linux_capabilities_isolator_tests.cpp 
> 6d95d607b81358953a4afcec03b60e87e7192edd 
>   src/tests/containerizer/linux_filesystem_isolator_tests.cpp 
> f9cab2fd123e48a5b4387fb2b609d70c7bf535cd 
>   src/tests/containerizer/memory_isolator_tests.cpp 
> b7b7acd37f69832e9ba79c9fe7ed0bc473b53e02 
>   src/tests/containerizer/memory_pressure_tests.cpp 
> 8a43c4fada8bdcedc6e1e24833d0d6c94cf927d2 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> 1fc56c4a8ca95bb47af968ad72dfef69455e7d46 
>   src/tests/containerizer/port_mapping_tests.cpp 
> 84b39b15880c7b5d9b7967f1e686baa59f52a015 
>   src/tests/containerizer/posix_rlimits_isolator_tests.cpp 
> bf25049515e8b8d0c085be24c4de22b3ade261b2 
>   src/tests/containerizer/routing_tests.cpp 
> d05b3b11dc5fcd54d956990f252509865168e6b6 
>   src/tests/dynamic_weights_tests.cpp 
> 3c86325a0749112606683bffdf305661170493e5 
>   src/tests/exception_tests.cpp b5b657c2d1b31b59ab1554bf854ef1a69b1517ed 
>   src/tests/fault_tolerance_tests.cpp 
> 5b8213531f5688e94063937af19e674508f0dd8c 
>   src/tests/fetcher_cache_tests.cpp 6d212cd460322ad6e97f4cf6ef537323275b7da0 
>   src/tests/group_tests.cpp f2e37cc7184dd328466b554a16da3173cfbca4a7 
>   src/tests/health_check_tests.cpp e824d1b2c2b17423394da0b089ae8940450378b7 
>   src/tests/hook_tests.cpp 04edbf809b7289a7d5336e4f00f23dfb55d3dbf8 
>   src/tests/master_authorization_tests.cpp 
> 8d54472d8a31956e6e8bb5fe7ffbf47dc793c0bc 
>   src/tests/master_maintenance_tests.cpp 
> e7a80ff4199927df8bf0fb54458d357ae444260d 
>   src/tests/master_slave_reconciliation_tests.cpp 
> 9c31eeae1a1b67af142a01e6c548b509ba06740c 
>   src/tests/master_tests.cpp e6ed02e07a9860ca3a56bba9c502a09444c5b26e 
>   src/tests/master_validation_tests.cpp 
> 813fb25007973f3499b94dcd0e9d2184ba08634c 
>   src/tests/oversubscription_tests.cpp 
> 54535a35e33dde0db3b547b9e31b4545d2900b67 
>   src/tests/partition_tests.cpp 62a84f797201ccd18b71490949e3130d2b9c3668 
>   src/tests/persistent_volume_tests.cpp 
> 7b0f436aa270f64cde93adc58833eef97c73dc26 
>   src/tests/reconciliation_tests.cpp f1635fd627d9a7944cb9d4b0117fcded7b6693e1 
>   src/tests/registrar_zookeeper_tests.cpp 
> cc082d320b4b150717728ec48edd0b26169cecd7 
>   src/tests/resource_offers_tests.cpp 
> e1fcab4b8fbb625876cf246505db2d05ac5d5710 
>   src/tests/scheduler_driver_tests.cpp 
> 47ad1dc9a4e5e0c5444fcc3a90d4645c3b6013ca 
>

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

2017-08-09 Thread Mesos Reviewbot


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



Patch looks great!

Reviews applied: [61434]

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 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 61524: Moved task validation from `getExecutorInfo` to `runTask` on agent.

2017-08-09 Thread Andrei Budnik


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

Review request for mesos, Benjamin Mahler, Greg Mann, and Vinod Kone.


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


Repository: mesos


Description
---

Previously, `getExecutorInfo` was called only in `runTask`, so it was
validating an invariant related to launching a single task. The
invariant is that a task should have either CommandInfo or ExecutorInfo
set but not both. Now `getExecutorInfo` is also called to calculate
allocated resources which might be related to a task group, which might
violate the invariant.


Diffs
-

  src/slave/slave.cpp 7381530515f86faf4c3e8f82bcd9483f6cf0498b 


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


Testing
---

sudo make check


Thanks,

Andrei Budnik

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

2017-08-09 Thread Armand Grillet


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


Ship it!




Testing done:
```
install tox
cd src/python/lib
tox
```

- Armand Grillet


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 61137: Cleaned up style in example frameworks.

2017-08-09 Thread Benno Evers


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


Ship it!




- Benno Evers


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
> 
>

47 matches

Mail list logo