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

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [57898, 58446, 58447, 58448, 58449]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker-build.sh

- Mesos Reviewbot


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

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

[Andrew Schwartzmeyer]

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

(Updated April 14, 2017, 4:12 a.m.)


Review request for mesos, Jeff Coffler, John Kordich, Joseph Wu, and Li Li.


Repository: mesos


Description
---

This unblocks us from building exclusively with VS 2017. The previous
patch to ZooKeeper only added VS 2015 support. This patch replaces it
with a CMake build system that will generate whichever solution we need
for Windows (and can replace the Autotools system on Linux).

We're updating to 3.5.2-alpha as the existing 3.5.1 rebundle was a
source tarball, and so missing the necessary generated files. The most
currently used version was based off a random commit. 3.5.2-alpha is the
latest 3.5.x release of ZooKeeper (3.5.x itself is alpha, 3.5.2 is
semi-stable, in comparison to 3.5.3 which is in RC).


Diffs
-

  3rdparty/CMakeLists.txt bb61ef0514fb164f35b34bb6be1bbebb4d1a1861 
  3rdparty/cmake/Mesos3rdpartyConfigure.cmake 
c60652688a23f8628f133b7890ff39e38fc8ae94 
  3rdparty/cmake/Versions.cmake 912726351ff744dd839b8d1c8d64dcc373d879be 
  3rdparty/zookeeper-3.5.2-alpha.patch PRE-CREATION 


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


Testing (updated)
---

cmake and make check on Linux
stout-tests, libprocess-tests, and mesos-tests on Windows: all tests passed. 
(Note: my machine updated to the Creators Update today, disabling long path 
support; the previous way to enable it doesn't seem to work, so I re-ran the 
failed tests on a machine that hadn't updated and still had long path support, 
they passed).

Also tested the build on a clean machine with nothing but the VS 2017 build 
tools (no IDE, no prior VS installations); mesos-tests builds no problem.


Thanks,

Andrew Schwartzmeyer

Re: Review Request 58443: Unit test for file/directory overwriting support in provisioners.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [58408, 58443]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker-build.sh

- Mesos Reviewbot


On April 14, 2017, 12:41 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58443/
> ---
> 
> (Updated April 14, 2017, 12:41 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-5028
> https://issues.apache.org/jira/browse/MESOS-5028
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The test is based on the following image:
>   https://hub.docker.com/r/chhsiao/overwrite/
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> dbbc3533ae269c31c98017d9d5748ef6e1ce 
> 
> 
> Diff: https://reviews.apache.org/r/58443/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> This test is failed without patch 58408 but will be passed after the patch.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

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

[Andrew Schwartzmeyer]

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



Please note that this patch was also [submitted to 
ZooKeeper](https://issues.apache.org/jira/browse/ZOOKEEPER-2756), though it's 
final form for the upstream project will probably be different (rebased onto 
master and split into multiple patches).

- Andrew Schwartzmeyer


On April 14, 2017, 2:15 a.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58448/
> ---
> 
> (Updated April 14, 2017, 2:15 a.m.)
> 
> 
> Review request for mesos, Jeff Coffler, John Kordich, Joseph Wu, and Li Li.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This unblocks us from building exclusively with VS 2017. The previous
> patch to ZooKeeper only added VS 2015 support. This patch replaces it
> with a CMake build system that will generate whichever solution we need
> for Windows (and can replace the Autotools system on Linux).
> 
> We're updating to 3.5.2-alpha as the existing 3.5.1 rebundle was a
> source tarball, and so missing the necessary generated files. The most
> currently used version was based off a random commit. 3.5.2-alpha is the
> latest 3.5.x release of ZooKeeper (3.5.x itself is alpha, 3.5.2 is
> semi-stable, in comparison to 3.5.3 which is in RC).
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt bb61ef0514fb164f35b34bb6be1bbebb4d1a1861 
>   3rdparty/cmake/Mesos3rdpartyConfigure.cmake 
> c60652688a23f8628f133b7890ff39e38fc8ae94 
>   3rdparty/cmake/Versions.cmake 912726351ff744dd839b8d1c8d64dcc373d879be 
>   3rdparty/zookeeper-3.5.2-alpha.patch PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/58448/diff/1/
> 
> 
> Testing
> ---
> 
> cmake and make check on Linux
> stout-tests, libprocess-tests, and mesos-tests on Windows (last still 
> pending, they're so slow).
> 
> Also testing the build on a clean machine with nothing but the VS 2017 build 
> tools (no IDE, no prior VS installations), results pending.
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

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

[Andrew Schwartzmeyer]

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

(Updated April 14, 2017, 2:15 a.m.)


Review request for mesos, Jeff Coffler, John Kordich, Joseph Wu, and Li Li.


Repository: mesos


Description
---

This unblocks us from building exclusively with VS 2017. The previous
patch to ZooKeeper only added VS 2015 support. This patch replaces it
with a CMake build system that will generate whichever solution we need
for Windows (and can replace the Autotools system on Linux).

We're updating to 3.5.2-alpha as the existing 3.5.1 rebundle was a
source tarball, and so missing the necessary generated files. The most
currently used version was based off a random commit. 3.5.2-alpha is the
latest 3.5.x release of ZooKeeper (3.5.x itself is alpha, 3.5.2 is
semi-stable, in comparison to 3.5.3 which is in RC).


Diffs
-

  3rdparty/CMakeLists.txt bb61ef0514fb164f35b34bb6be1bbebb4d1a1861 
  3rdparty/cmake/Mesos3rdpartyConfigure.cmake 
c60652688a23f8628f133b7890ff39e38fc8ae94 
  3rdparty/cmake/Versions.cmake 912726351ff744dd839b8d1c8d64dcc373d879be 
  3rdparty/zookeeper-3.5.2-alpha.patch PRE-CREATION 


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


Testing (updated)
---

cmake and make check on Linux
stout-tests, libprocess-tests, and mesos-tests on Windows (last still pending, 
they're so slow).

Also testing the build on a clean machine with nothing but the VS 2017 build 
tools (no IDE, no prior VS installations), results pending.


Thanks,

Andrew Schwartzmeyer

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

[Andrew Schwartzmeyer]

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



Documentation does need to be updated, but it will come in my other branch full 
of Widnows documentation updates.

- Andrew Schwartzmeyer


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

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

[Andrew Schwartzmeyer]

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

Review request for mesos, Jeff Coffler, John Kordich, Joseph Wu, and Li Li.


Repository: mesos


Description
---

The `SOURCE_SUBDIR` command to `ExternalProject_Add` was added in CMake
3.7.0, and is necessary to most cleanly build an external CMake built
project where the `CMakeLists.txt` is in a subfolder of the project.


Diffs
-

  CMakeLists.txt ea529ec2d5c2b9be4f19c67c2033c3f4b9073c1f 


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


Testing
---


Thanks,

Andrew Schwartzmeyer

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

[Andrew Schwartzmeyer]

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

Review request for mesos, Jeff Coffler, John Kordich, Joseph Wu, and Li Li.


Repository: mesos


Description
---

This unblocks us from building exclusively with VS 2017. The previous
patch to ZooKeeper only added VS 2015 support. This patch replaces it
with a CMake build system that will generate whichever solution we need
for Windows (and can replace the Autotools system on Linux).

We're updating to 3.5.2-alpha as the existing 3.5.1 rebundle was a
source tarball, and so missing the necessary generated files. The most
currently used version was based off a random commit. 3.5.2-alpha is the
latest 3.5.x release of ZooKeeper (3.5.x itself is alpha, 3.5.2 is
semi-stable, in comparison to 3.5.3 which is in RC).


Diffs
-

  3rdparty/CMakeLists.txt bb61ef0514fb164f35b34bb6be1bbebb4d1a1861 
  3rdparty/cmake/Mesos3rdpartyConfigure.cmake 
c60652688a23f8628f133b7890ff39e38fc8ae94 
  3rdparty/cmake/Versions.cmake 912726351ff744dd839b8d1c8d64dcc373d879be 
  3rdparty/zookeeper-3.5.2-alpha.patch PRE-CREATION 


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


Testing
---

cmake and make check on Linux
stout-tests, libprocess-tests, and mesos-tests on Windows (last still pending, 
they're so slow).


Thanks,

Andrew Schwartzmeyer

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

[Andrew Schwartzmeyer]

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



This pays back a lot of technical debt and confusing build cruft (where the 
version set in Versions.cmake was not necessarily the version used).

- Andrew Schwartzmeyer


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

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

[Andrew Schwartzmeyer]

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

Review request for mesos, Jeff Coffler, John Kordich, Joseph Wu, and Li Li.


Repository: mesos


Description
---

This commit removes duplicate code from `3rdparty/CMakeLists.txt`,
and consolidates platform-specific versions into `Versions.cmake`.


Diffs
-

  3rdparty/CMakeLists.txt bb61ef0514fb164f35b34bb6be1bbebb4d1a1861 
  3rdparty/cmake/Mesos3rdpartyConfigure.cmake 
c60652688a23f8628f133b7890ff39e38fc8ae94 
  3rdparty/cmake/Versions.cmake 912726351ff744dd839b8d1c8d64dcc373d879be 
  cmake/CompilationConfigure.cmake 1c5466960f5ac73b8fc81edf7950cc68ed744301 


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


Testing
---


Thanks,

Andrew Schwartzmeyer

Re: Review Request 58446: Windows: Set CMake generator for Protobuf correctly.

[Andrew Schwartzmeyer]

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



Looking at what I ended up doing for ZooKeeper, I think it may be better to 
build this using the CMake defaults of ExternalProject_Add, but it's not 
strictly necessary.

- Andrew Schwartzmeyer


On April 14, 2017, 2:01 a.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58446/
> ---
> 
> (Updated April 14, 2017, 2:01 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Reuse the variable rather than hardcode it.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt bb61ef0514fb164f35b34bb6be1bbebb4d1a1861 
> 
> 
> Diff: https://reviews.apache.org/r/58446/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Review Request 58446: Windows: Set CMake generator for Protobuf correctly.

[Andrew Schwartzmeyer]

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

Review request for mesos.


Repository: mesos


Description
---

Reuse the variable rather than hardcode it.


Diffs
-

  3rdparty/CMakeLists.txt bb61ef0514fb164f35b34bb6be1bbebb4d1a1861 


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


Testing
---


Thanks,

Andrew Schwartzmeyer

Re: Review Request 57952: Added 'config' plugin to the new CLI.

[Joseph Wu]

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


Ship it!




LGTM.


src/cli_new/lib/cli/plugins/config/main.py
Lines 35 (patched)


How about:

"Interacts with the Mesos CLI configuration file"


- Joseph Wu


On April 12, 2017, 2:19 a.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57952/
> ---
> 
> (Updated April 12, 2017, 2:19 a.m.)
> 
> 
> Review request for mesos, Joseph Wu and Kevin Klues.
> 
> 
> Bugs: MESOS-7269
> https://issues.apache.org/jira/browse/MESOS-7269
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Used to show the plugins available for the user.
> 
> 
> Diffs
> -
> 
>   src/cli_new/bin/settings.py 2f6162edc1722054bc44ad25956e6fe666d36c7f 
>   src/cli_new/lib/cli/plugins/config/__init__.py PRE-CREATION 
>   src/cli_new/lib/cli/plugins/config/main.py PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/57952/diff/6/
> 
> 
> Testing
> ---
> 
> Tested manually, PEP8 and Pylint used to make sure that the code style is 
> correct.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 57951: Moved new CLI settings into a user-defined TOML file.

[Joseph Wu]

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


Fix it, then Ship it!




I can fixup the mistakes in the Readme.


src/cli_new/README.md
Lines 82 (patched)


Oops, conflict marker.


- Joseph Wu


On April 12, 2017, 12:59 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57951/
> ---
> 
> (Updated April 12, 2017, 12:59 p.m.)
> 
> 
> Review request for mesos, Joseph Wu and Kevin Klues.
> 
> 
> Bugs: MESOS-7269
> https://issues.apache.org/jira/browse/MESOS-7269
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> These settings were previously in settings.py.
> We now use a TOML file containing the configuration, this
> format has been chosen because:
>   * It supports comments.
>   * It is well-specified.
>   * It allows logical grouping.
>   * It maps ubiquitous data types.
> The config file environment variable, previously
> `MESOS_CLI_CONFIG_PATH`, is now `MESOS_CLI_CONFIG`. This
> change follows the design doc about the new CLI.
> This environement variable `MESOS_CLI_PLUGINS` is not used
> anymore as plugins can be added using the TOML file instead.
> 
> 
> Diffs
> -
> 
>   src/cli_new/README.md aa118132688253d3cec0b27fd6b394f5bc2bdd94 
>   src/cli_new/bin/settings.py 2f6162edc1722054bc44ad25956e6fe666d36c7f 
>   src/cli_new/pip-requirements.txt e73bbfde98f28693463a46b166197ad1fd53c0cb 
> 
> 
> Diff: https://reviews.apache.org/r/57951/diff/7/
> 
> 
> Testing
> ---
> 
> Tested manually, PEP8 and Pylint used to make sure that the code style is 
> correct.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 58381: Added a Table abstraction to the new CLI.

[Joseph Wu]

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


Ship it!




LGTM.

As we discussed offline, since this code was originally posted here: 
https://reviews.apache.org/r/52948/
the commit will be attributed according to Haris Cloudhary, from whom Kevin 
obtained the code.

- Joseph Wu


On April 12, 2017, 2:14 a.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58381/
> ---
> 
> (Updated April 12, 2017, 2:14 a.m.)
> 
> 
> Review request for mesos and Joseph Wu.
> 
> 
> Bugs: MESOS-7282
> https://issues.apache.org/jira/browse/MESOS-7282
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This will be used by future plugins.
> 
> 
> Diffs
> -
> 
>   src/cli_new/lib/cli/util.py ace07fb64e130e2f02d4ab5607ee1a84161ef88b 
> 
> 
> Diff: https://reviews.apache.org/r/58381/diff/2/
> 
> 
> Testing
> ---
> 
> Tested manually, PEP8 and Pylint used to make sure that the code style is 
> correct.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 58430: Changed some failing Base64 tests to use 'u8' string literals.

[Joseph Wu]

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


Ship it!




Ship It!

- Joseph Wu


On April 13, 2017, 11:48 a.m., John Kordich wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58430/
> ---
> 
> (Updated April 13, 2017, 11:48 a.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer and Joseph Wu.
> 
> 
> Bugs: MESOS-7236
> https://issues.apache.org/jira/browse/MESOS-7236
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Changed some failing Base64 tests to use 'u8' string literals.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/tests/base64_tests.cpp 
> 0473221abb352195e03ae970709eadcfc44d77b0 
> 
> 
> Diff: https://reviews.apache.org/r/58430/diff/1/
> 
> 
> Testing
> ---
> 
> Ran make check on Linux and ran stout-tests on Windows.
> 
> 
> Thanks,
> 
> John Kordich
> 
>

Review Request 58443: Unit test for file/directory overwriting support in provisioners.

[Chun-Hung Hsiao]

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

Review request for mesos, Gilbert Song, Jie Yu, and Vinod Kone.


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


Repository: mesos


Description
---

The test is based on the following image:
  https://hub.docker.com/r/chhsiao/overwrite/


Diffs
-

  src/tests/containerizer/provisioner_docker_tests.cpp 
dbbc3533ae269c31c98017d9d5748ef6e1ce 


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


Testing
---

sudo make check

This test is failed without patch 58408 but will be passed after the patch.


Thanks,

Chun-Hung Hsiao

Re: Review Request 58408: Overwriting Directories with Files in Copy Provisioner.

[Chun-Hung Hsiao]

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

(Updated April 14, 2017, 12:24 a.m.)


Review request for mesos, Gilbert Song, Jie Yu, and Vinod Kone.


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


Repository: mesos


Description
---

When a layer overwrites a directory with a regular file or symbolic link
(or vice versa), the old dir/file need to be removed before copying the
layer into the rootfs. This is processed together with whiteout:
The copy provisioner find all files to remove, including files
marked as whiteout and the files described above, and remove them
before the copy process.


Diffs (updated)
-

  src/slave/containerizer/mesos/provisioner/backends/copy.cpp 
584cc6524f81cc1bc231b105507dbfe51fec991d 


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

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


Testing
---

make check
Manually tested on the following images:
  https://hub.docker.com/r/gilbertsong/whiteout/
  https://hub.docker.com/r/chhsiao/overwrite/


Thanks,

Chun-Hung Hsiao

Re: Review Request 57999: Made Path and Mount root optional in Resource.DiskInfo.Source.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On April 5, 2017, 1:56 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57999/
> ---
> 
> (Updated April 5, 2017, 1:56 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jan Schlicht.
> 
> 
> Bugs: MESOS-7312
> https://issues.apache.org/jira/browse/MESOS-7312
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This commit is preparation for future changes where path or mount
> roots might be calculated lazily, i.e., not known early enough
> anymore.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto dd90465cc3da283c078d4e907cc6a4a0e50309ac 
>   include/mesos/v1/mesos.proto 228623155c7f68c0f24d173aacbc6eb734f1382f 
>   src/common/resources.cpp c26e0f995006dc6b2e70a491cea58fa90347e42a 
>   src/slave/paths.cpp ef22ee167f16030f02d28c8e6bab6c2ca4812d8f 
>   src/slave/slave.cpp 65e4a67888fe908e5b2f6ca2ecc9e3a5b9958b2e 
>   src/v1/resources.cpp a53deafbea399a1bcf729d1c151bc46e9da04e11 
> 
> 
> Diff: https://reviews.apache.org/r/57999/diff/5/
> 
> 
> Testing
> ---
> 
> make check (OS X, Fedora25)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 58374: Updated check tests to authenticate with agent operator API.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On April 12, 2017, 9:21 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58374/
> ---
> 
> (Updated April 12, 2017, 9:21 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Anand Mazumdar, Gastón 
> Kleiman, and Vinod Kone.
> 
> 
> Bugs: MESOS-7377
> https://issues.apache.org/jira/browse/MESOS-7377
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch updates the check tests to enable authentication
> on the agent operator API when Mesos has been built with SSL.
> 
> 
> Diffs
> -
> 
>   src/tests/check_tests.cpp d7fcbf940102efe0841f07e8c4937a78361bc422 
> 
> 
> Diff: https://reviews.apache.org/r/58374/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 58370: Updated a health checker test to enable executor authentication.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On April 12, 2017, 9:21 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58370/
> ---
> 
> (Updated April 12, 2017, 9:21 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Anand Mazumdar, Gastón 
> Kleiman, and Vinod Kone.
> 
> 
> Bugs: MESOS-7377
> https://issues.apache.org/jira/browse/MESOS-7377
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch updates the test
> `HealthCheckTest.DefaultExecutorCommandHealthCheck` to
> enable authentication on the agent operator API when
> Mesos has been built with SSL.
> 
> 
> Diffs
> -
> 
>   src/tests/health_check_tests.cpp c5857b2415faaec4d0557e50cbeb42379f4550ac 
> 
> 
> Diff: https://reviews.apache.org/r/58370/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 58369: Updated default executor to pass authorization header to checkers.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On April 13, 2017, 8:27 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58369/
> ---
> 
> (Updated April 13, 2017, 8:27 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Anand Mazumdar, Gastón 
> Kleiman, and Vinod Kone.
> 
> 
> Bugs: MESOS-7377
> https://issues.apache.org/jira/browse/MESOS-7377
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch updates the default executor to pass the executor's
> authorization header to the `Checker` and `HealthChecker`
> libraries when an authentication token is present in the
> environment.
> 
> 
> Diffs
> -
> 
>   src/launcher/default_executor.cpp 9cc40c6391a931f9cdf6965e64be231980537299 
> 
> 
> Diff: https://reviews.apache.org/r/58369/diff/2/
> 
> 
> Testing
> ---
> 
> Testing details can be found at the end of this chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 58373: Updated 'Checker' to authenticate with agent operator API.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On April 12, 2017, 9:21 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58373/
> ---
> 
> (Updated April 12, 2017, 9:21 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Anand Mazumdar, Gastón 
> Kleiman, and Vinod Kone.
> 
> 
> Bugs: MESOS-7377
> https://issues.apache.org/jira/browse/MESOS-7377
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch updates the `Checker` to permit initialization with
> an authorization header, which it will provide to the agent
> operator API for authentication when present.
> 
> 
> Diffs
> -
> 
>   src/checks/checker.hpp fb939d85dbec2bf7e81e0c11518ccecddc5a7a11 
>   src/checks/checker.cpp 7510bf23977e007d101fab635865b7160c3a5af6 
> 
> 
> Diff: https://reviews.apache.org/r/58373/diff/1/
> 
> 
> Testing
> ---
> 
> Testing details can be found at the end of this chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 58368: Updated 'HealthChecker' to authenticate with the agent.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On April 12, 2017, 9:20 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58368/
> ---
> 
> (Updated April 12, 2017, 9:20 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Anand Mazumdar, Gastón 
> Kleiman, and Vinod Kone.
> 
> 
> Bugs: MESOS-7377
> https://issues.apache.org/jira/browse/MESOS-7377
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch updates the `HealthChecker` to permit initialization
> with an authorization header, which it will provide to the agent
> operator API for authentication when present.
> 
> 
> Diffs
> -
> 
>   src/checks/health_checker.hpp e17f12f75d388443f6592455177f5bdae31b7c0f 
>   src/checks/health_checker.cpp 769278c40b5e505dc49a49a23d8c0dd97f201a53 
> 
> 
> Diff: https://reviews.apache.org/r/58368/diff/1/
> 
> 
> Testing
> ---
> 
> Testing details can be found at the end of this chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 58328: Updated tests to set '--executor_secret_key' as a path.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On April 13, 2017, 8:17 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58328/
> ---
> 
> (Updated April 13, 2017, 8:17 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-6999
> https://issues.apache.org/jira/browse/MESOS-6999
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch updates the test code to generate a secret key file
> and set the agent '--executor_secret_key' flag with its path.
> 
> 
> Diffs
> -
> 
>   src/tests/mesos.hpp fe897c184829910addec95ace174546092a9e2b2 
>   src/tests/mesos.cpp 099ec376878faaa7efe5bc030785db717cb16f59 
> 
> 
> Diff: https://reviews.apache.org/r/58328/diff/2/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 58327: Changed '--executor_secret_key' agent flag to accept a path.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On April 13, 2017, 8:15 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58327/
> ---
> 
> (Updated April 13, 2017, 8:15 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-6999
> https://issues.apache.org/jira/browse/MESOS-6999
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch changes the agent flag '--executor_secret_key' to accept
> a path, so that the secret will not be leaked in logs.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md 452478eab36f352a31f62f0c1f4da2f43f89bef9 
>   src/slave/flags.hpp 171f67e44518e858049d002fcf037715021da265 
>   src/slave/flags.cpp 9365da2c8462a4375a99a86210b9d6ec628510fe 
>   src/slave/slave.cpp f013e9c7d1c089ec72e2e5db986fd52423ebb8fe 
> 
> 
> Diff: https://reviews.apache.org/r/58327/diff/3/
> 
> 
> Testing
> ---
> 
> Testing details are found in the following patch in this chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 58394: Setup new directory for python http client lib in src/python.

[Jason Lai]

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


Ship it!




Ship It!

- Jason Lai


On April 12, 2017, 9:44 p.m., Eric Chung wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58394/
> ---
> 
> (Updated April 12, 2017, 9:44 p.m.)
> 
> 
> Review request for mesos, Jason Lai, Joseph Wu, and Kevin Klues.
> 
> 
> Bugs: MESOS-7310
> https://issues.apache.org/jira/browse/MESOS-7310
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Setup new directory for python http client lib in src/python.
> 
> 
> Diffs
> -
> 
>   src/cli_new/bootstrap 6d62e9adf1d543ed00a3a2cf2484edf1c33ee443 
>   src/python/.gitignore PRE-CREATION 
>   src/python/lib/mesos/__init__.py PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/58394/diff/1/
> 
> 
> Testing
> ---
> 
> Under src/cli_new:
> 1\. ./bootstrap
> 2\. . ./activate
> 3\. python
> 4\. >>> import mesos
> 5\. >>> mesos.\_\_path\_\_
> 6\. verify that the path printed out is indeed at src/python/lib/mesos
> 
> 
> Thanks,
> 
> Eric Chung
> 
>

Re: Review Request 57898: Windows: Add deprecation warning for VS 2015.

[Andrew Schwartzmeyer]

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

(Updated April 13, 2017, 10:03 p.m.)


Review request for mesos, Jeff Coffler, Joseph Wu, and Michael Park.


Changes
---

Update bug since it was pulled out of the task.


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


Repository: mesos


Description
---

Specifically this checks if the generator used did not match "Visual
Studio 15 2017", which Mesos prefers. Note that the `CMAKE_GENERATOR`
variable is expanded fully even when shorthand is used, e.g. `-G"Visual
Studio 15"` will correctly match.


Diffs
-

  cmake/CompilationConfigure.cmake 5936be07557395ad08b06aa376b5a29f9b11c143 


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


Testing
---

Tested using different generators on Windows. Warning is displayed when using 
`-G"Visual Studio 14"`, `-G"Visual Studio 14 2015"`, and `-G"Visual Studio 14 
2015 Win64"`, but not when using `-G"Visual Studio 15"`, `-G"Visual Studio 15 
2017"`, or `-G"Visual Studio 15 2017 Win64"`.


Thanks,

Andrew Schwartzmeyer

Re: Review Request 57254: Added support for hierarchical roles to DRFSorter.

[Neil Conway]

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

(Updated April 13, 2017, 9:42 p.m.)


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


Changes
---

Switch from `std::set` to `std::vector`, move `Client` into a nested struct 
`DRFSorter::Node`, various other changes.


Repository: mesos


Description (updated)
---

This commit replaces the sorter's flat list of clients with a tree; the
tree represents the hierarchical relationship between sorter clients.
Each node in the tree contains a vector of pointers to child nodes. The
tree might contain nodes that do not correspond directly to sorter
clients. For example, adding clients "a/b" and "c/d" results in the
following tree:

root
 -> a
   -> b
 -> c
   -> d

The `sort` member function still only returns one result for each active
client in the sorter. This is implemented by ensuring that each sorter
client is associated with a leaf node in the tree. Note that it is
possible for a leaf node to be transformed into an internal node by a
subsequent insertion; to handle this case, we "implicitly" create an
extra child node, which maintains the invariant that each client has a
leaf node. For example, if the client "a/b/x" is added to the tree
above, the result is:

root
 -> a
   -> b
 -> .
 -> x
 -> c
   -> d

The "." leaf node holds the allocation that has been made to the "a/b"
client itself; the "a/b" node holds the sum of all the allocations that
have been made to the subtree rooted at "a/b", which also includes
"a/b/x".

This commit also introduces a new approach to sorting: rather than
keeping an `std::set` of sorter clients, we now keep a tree of
`std::vector`, which is sorted explicitly via `std::sort`. The previous
implementation tried to optimize the sorting process by updating the
sort order incrementally when a single sorter client was updated; this
commit removes that optimization, and instead re-sorts the entire tree
whenever the sort order is changed.

Re-introducing a version of this optimization would make sense in the
future (MESOS-7390), but benchmarking suggests that this commit results
in a net improvement in sorter performance anyway. The sorter perf
improvement is likely due to the introduction of a secondary hashmap
that allows the leaf node associated with a tree path to be find
efficiently; the previous implementation required a linear scan of a
`std::set`.


Diffs (updated)
-

  src/master/allocator/sorter/drf/metrics.cpp 
15aab32db5ca1a7a14080e9bbb7c65283be3ec20 
  src/master/allocator/sorter/drf/sorter.hpp 
76329220e1115c1de7810fb69b943c78c078be59 
  src/master/allocator/sorter/drf/sorter.cpp 
ed54680cecb637931fc344fbcf8fd3b14cc24295 
  src/master/allocator/sorter/sorter.hpp 
b3029fcf7342406955760da53f1ae736769f308c 
  src/tests/hierarchical_allocator_tests.cpp 
33e7b455f8664858eb4f03727b076a10c80cd6e0 
  src/tests/master_allocator_tests.cpp 119e318f8a01d50e8dae5c30cf5fa6a017c3c625 
  src/tests/sorter_tests.cpp 43bd85798aef0c89751b725ebf35308a5e9e997a 


Diff: https://reviews.apache.org/r/57254/diff/17/

Changes: https://reviews.apache.org/r/57254/diff/16-17/


Testing
---


Thanks,

Neil Conway

Re: Review Request 58428: Added tests for failed executor authorization.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [58327, 58328, 58251, 58252, 58253, 58254, 58255, 58258, 58428]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker-build.sh

- Mesos Reviewbot


On April 13, 2017, 8:21 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58428/
> ---
> 
> (Updated April 13, 2017, 8:21 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-7339
> https://issues.apache.org/jira/browse/MESOS-7339
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds new tests to verify that HTTP executors cannot
> subscribe or launch nested containers when HTTP executor
> authentication is enabled, authorization is enabled, and they
> do not provide a valid executor authentication token
> 
> 
> Diffs
> -
> 
>   src/tests/slave_authorization_tests.cpp 
> 3657e0a3d19d75cef92e5bf90b65ef00c291b032 
> 
> 
> Diff: https://reviews.apache.org/r/58428/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 58408: Overwriting Directories with Files in Copy Provisioner.

[Chun-Hung Hsiao]

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

(Updated April 13, 2017, 8:37 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, and Vinod Kone.


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


Repository: mesos


Description
---

When a layer overwrites a directory with a regular file or symbolic link
(or vice versa), the old dir/file need to be removed before copying the
layer into the rootfs. This is processed together with whiteout:
The copy provisioner find all files to remove, including files
marked as whiteout and the files described above, and remove them
before the copy process.


Diffs
-

  src/slave/containerizer/mesos/provisioner/backends/copy.cpp 
584cc6524f81cc1bc231b105507dbfe51fec991d 


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


Testing (updated)
---

make check
Manually tested on the following images:
  https://hub.docker.com/r/gilbertsong/whiteout/
  https://hub.docker.com/r/chhsiao/overwrite/


Thanks,

Chun-Hung Hsiao

Re: Review Request 58369: Updated default executor to pass authorization header to checkers.

[Greg Mann]

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

(Updated April 13, 2017, 8:27 p.m.)


Review request for mesos, Alexander Rukletsov, Anand Mazumdar, Gastón Kleiman, 
and Vinod Kone.


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


Repository: mesos


Description
---

This patch updates the default executor to pass the executor's
authorization header to the `Checker` and `HealthChecker`
libraries when an authentication token is present in the
environment.


Diffs (updated)
-

  src/launcher/default_executor.cpp 9cc40c6391a931f9cdf6965e64be231980537299 


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

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


Testing
---

Testing details can be found at the end of this chain.


Thanks,

Greg Mann

Review Request 58428: Added tests for failed executor authorization.

[Greg Mann]

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

Review request for mesos, Anand Mazumdar and Vinod Kone.


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


Repository: mesos


Description
---

This patch adds new tests to verify that HTTP executors cannot
subscribe or launch nested containers when HTTP executor
authentication is enabled, authorization is enabled, and they
do not provide a valid executor authentication token


Diffs
-

  src/tests/slave_authorization_tests.cpp 
3657e0a3d19d75cef92e5bf90b65ef00c291b032 


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


Testing
---

`make check`


Thanks,

Greg Mann

Re: Review Request 58258: Added a new agent authorization test which runs a task group.

[Greg Mann]

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

(Updated April 13, 2017, 8:19 p.m.)


Review request for mesos, Adam B, Alexander Rojas, Till Toenshoff, and Vinod 
Kone.


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


Repository: mesos


Description
---

This patch adds a new test,
`SlaveAuthorizerTest.AuthorizeRunTaskGroup`, which
verifies that task groups can be launched when
executor authentication is required and the local
authorizer is loaded.


Diffs (updated)
-

  src/tests/slave_authorization_tests.cpp 
3657e0a3d19d75cef92e5bf90b65ef00c291b032 


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

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


Testing
---

`make check` when configured with SSL enabled.


Thanks,

Greg Mann

Re: Review Request 58328: Updated tests to set '--executor_secret_key' as a path.

[Greg Mann]

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

(Updated April 13, 2017, 8:17 p.m.)


Review request for mesos, Anand Mazumdar and Vinod Kone.


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


Repository: mesos


Description
---

This patch updates the test code to generate a secret key file
and set the agent '--executor_secret_key' flag with its path.


Diffs (updated)
-

  src/tests/mesos.hpp fe897c184829910addec95ace174546092a9e2b2 
  src/tests/mesos.cpp 099ec376878faaa7efe5bc030785db717cb16f59 


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

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


Testing
---

`make check`


Thanks,

Greg Mann

Re: Review Request 58327: Changed '--executor_secret_key' agent flag to accept a path.

[Greg Mann]

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

(Updated April 13, 2017, 8:15 p.m.)


Review request for mesos, Anand Mazumdar and Vinod Kone.


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


Repository: mesos


Description
---

This patch changes the agent flag '--executor_secret_key' to accept
a path, so that the secret will not be leaked in logs.


Diffs (updated)
-

  docs/configuration.md 452478eab36f352a31f62f0c1f4da2f43f89bef9 
  src/slave/flags.hpp 171f67e44518e858049d002fcf037715021da265 
  src/slave/flags.cpp 9365da2c8462a4375a99a86210b9d6ec628510fe 
  src/slave/slave.cpp f013e9c7d1c089ec72e2e5db986fd52423ebb8fe 


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

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


Testing
---

Testing details are found in the following patch in this chain.


Thanks,

Greg Mann

Re: Review Request 58328: Updated tests to set '--executor_secret_key' as a path.

[Greg Mann]

> On April 13, 2017, 6:54 p.m., Greg Mann wrote:
> > src/tests/mesos.cpp
> > Lines 218-235 (patched)
> > 
> >
> > Use `os::write()`

Looks like we don't provide an overload of `os::write` which accepts access 
permission bits.


- Greg


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


On April 11, 2017, 4:37 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58328/
> ---
> 
> (Updated April 11, 2017, 4:37 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-6999
> https://issues.apache.org/jira/browse/MESOS-6999
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch updates the test code to generate a secret key file
> and set the agent '--executor_secret_key' flag with its path.
> 
> 
> Diffs
> -
> 
>   src/tests/mesos.cpp 099ec376878faaa7efe5bc030785db717cb16f59 
> 
> 
> Diff: https://reviews.apache.org/r/58328/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 58430: Changed some failing Base64 tests to use 'u8' string literals.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [58430]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker-build.sh

- Mesos Reviewbot


On April 13, 2017, 6:48 p.m., John Kordich wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58430/
> ---
> 
> (Updated April 13, 2017, 6:48 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer and Joseph Wu.
> 
> 
> Bugs: MESOS-7236
> https://issues.apache.org/jira/browse/MESOS-7236
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Changed some failing Base64 tests to use 'u8' string literals.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/tests/base64_tests.cpp 
> 0473221abb352195e03ae970709eadcfc44d77b0 
> 
> 
> Diff: https://reviews.apache.org/r/58430/diff/1/
> 
> 
> Testing
> ---
> 
> Ran make check on Linux and ran stout-tests on Windows.
> 
> 
> Thanks,
> 
> John Kordich
> 
>

Re: Review Request 58255: Added implicit authorization to the agent executor API.

[Greg Mann]

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



Add TODO to possibly move into authorizer, create ticket for consistent authZ 
story across executors and frameworks.


src/slave/http.cpp
Lines 731 (patched)


Return Forbidden?


- Greg Mann


On April 12, 2017, 7:28 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58255/
> ---
> 
> (Updated April 12, 2017, 7:28 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, and Till Toenshoff.
> 
> 
> Bugs: MESOS-7339
> https://issues.apache.org/jira/browse/MESOS-7339
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch updates the agent handler for the executor API to
> verify the FrameworkID and ExecutorID contained within the
> executor's `Principal`, if present. This effectively performs
> implicit authorization of executor calls.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp 468cf332d79ed7315ecf51955235735dec0a2df1 
> 
> 
> Diff: https://reviews.apache.org/r/58255/diff/2/
> 
> 
> Testing
> ---
> 
> Testing details can be found at the end of this chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 58254: Added implicit executor authorization to the agent operator API.

[Greg Mann]

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




src/authorizer/local/authorizer.cpp
Lines 323-327 (original), 323-327 (patched)


Populate for LAUNCH_NESTED_CONTAINER_SESSION

Add authorization test for this (enable authZ be default in all default 
executor tests)



src/authorizer/local/authorizer.cpp
Line 659 (original), 726-733 (patched)


Comment explaining the purpose of this conditional.


- Greg Mann


On April 13, 2017, 6:56 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58254/
> ---
> 
> (Updated April 13, 2017, 6:56 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, Till Toenshoff, and Vinod 
> Kone.
> 
> 
> Bugs: MESOS-7014
> https://issues.apache.org/jira/browse/MESOS-7014
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch updates the agent handlers for the LAUNCH_, WAIT_,
> and KILL_NESTED_CONTAINER calls of the operator API to set the
> `container_id` field within the authorization object,
> facilitating implicit executor authorization.
> 
> 
> Diffs
> -
> 
>   include/mesos/authorizer/authorizer.proto 
> 736f76d552956f2351ffd40fc51d088dff83f8c8 
>   src/authorizer/local/authorizer.cpp 
> 1c1f912794cfe61112a0e513b217ba3a755f35f1 
>   src/slave/http.cpp 468cf332d79ed7315ecf51955235735dec0a2df1 
> 
> 
> Diff: https://reviews.apache.org/r/58254/diff/4/
> 
> 
> Testing
> ---
> 
> Testing details can be found at the end of this chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 58254: Added implicit executor authorization to the agent operator API.

[Greg Mann]

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

(Updated April 13, 2017, 6:56 p.m.)


Review request for mesos, Adam B, Alexander Rojas, Till Toenshoff, and Vinod 
Kone.


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


Repository: mesos


Description
---

This patch updates the agent handlers for the LAUNCH_, WAIT_,
and KILL_NESTED_CONTAINER calls of the operator API to set the
`container_id` field within the authorization object,
facilitating implicit executor authorization.


Diffs (updated)
-

  include/mesos/authorizer/authorizer.proto 
736f76d552956f2351ffd40fc51d088dff83f8c8 
  src/authorizer/local/authorizer.cpp 1c1f912794cfe61112a0e513b217ba3a755f35f1 
  src/slave/http.cpp 468cf332d79ed7315ecf51955235735dec0a2df1 


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

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


Testing
---

Testing details can be found at the end of this chain.


Thanks,

Greg Mann

Re: Review Request 58328: Updated tests to set '--executor_secret_key' as a path.

[Greg Mann]

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




src/tests/mesos.cpp
Lines 218-235 (patched)


Use `os::write()`


- Greg Mann


On April 11, 2017, 4:37 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58328/
> ---
> 
> (Updated April 11, 2017, 4:37 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-6999
> https://issues.apache.org/jira/browse/MESOS-6999
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch updates the test code to generate a secret key file
> and set the agent '--executor_secret_key' flag with its path.
> 
> 
> Diffs
> -
> 
>   src/tests/mesos.cpp 099ec376878faaa7efe5bc030785db717cb16f59 
> 
> 
> Diff: https://reviews.apache.org/r/58328/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 58327: Changed '--executor_secret_key' agent flag to accept a path.

[Greg Mann]

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




src/slave/slave.cpp
Lines 307-312 (patched)


Add TODO to factor this out.


- Greg Mann


On April 13, 2017, 6:48 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58327/
> ---
> 
> (Updated April 13, 2017, 6:48 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-6999
> https://issues.apache.org/jira/browse/MESOS-6999
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch changes the agent flag '--executor_secret_key' to accept
> a path, so that the secret will not be leaked in logs.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md 452478eab36f352a31f62f0c1f4da2f43f89bef9 
>   src/slave/flags.hpp 171f67e44518e858049d002fcf037715021da265 
>   src/slave/flags.cpp 9365da2c8462a4375a99a86210b9d6ec628510fe 
>   src/slave/slave.cpp f013e9c7d1c089ec72e2e5db986fd52423ebb8fe 
> 
> 
> Diff: https://reviews.apache.org/r/58327/diff/2/
> 
> 
> Testing
> ---
> 
> Testing details are found in the following patch in this chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 58430: Changed some failing Base64 tests to use 'u8' string literals.

[John Kordich via Review Board]

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

(Updated April 13, 2017, 6:48 p.m.)


Review request for mesos, Andrew Schwartzmeyer and Joseph Wu.


Changes
---

These changes will fix the unit test failures on Windows by making sure that 
the string literals are encoded into bytes identically across platforms.


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


Repository: mesos


Description
---

Changed some failing Base64 tests to use 'u8' string literals.


Diffs
-

  3rdparty/stout/tests/base64_tests.cpp 
0473221abb352195e03ae970709eadcfc44d77b0 


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


Testing
---

Ran make check on Linux and ran stout-tests on Windows.


Thanks,

John Kordich

Re: Review Request 58327: Changed '--executor_secret_key' agent flag to accept a path.

[Greg Mann]

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

(Updated April 13, 2017, 6:48 p.m.)


Review request for mesos, Anand Mazumdar and Vinod Kone.


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


Repository: mesos


Description
---

This patch changes the agent flag '--executor_secret_key' to accept
a path, so that the secret will not be leaked in logs.


Diffs (updated)
-

  docs/configuration.md 452478eab36f352a31f62f0c1f4da2f43f89bef9 
  src/slave/flags.hpp 171f67e44518e858049d002fcf037715021da265 
  src/slave/flags.cpp 9365da2c8462a4375a99a86210b9d6ec628510fe 
  src/slave/slave.cpp f013e9c7d1c089ec72e2e5db986fd52423ebb8fe 


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

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


Testing
---

Testing details are found in the following patch in this chain.


Thanks,

Greg Mann

Review Request 58430: Changed some failing Base64 tests to use 'u8' string literals.

[John Kordich via Review Board]

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

Review request for mesos.


Repository: mesos


Description
---

Changed some failing Base64 tests to use 'u8' string literals.


Diffs
-

  3rdparty/stout/tests/base64_tests.cpp 
0473221abb352195e03ae970709eadcfc44d77b0 


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


Testing
---

Ran make check on Linux and ran stout-tests on Windows.


Thanks,

John Kordich

Re: Review Request 58369: Updated default executor to pass authorization header to checkers.

[Greg Mann]

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




src/launcher/default_executor.cpp
Lines 1406-1410 (original), 1410-1414 (patched)


Construct authorization header here.


- Greg Mann


On April 12, 2017, 9:21 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58369/
> ---
> 
> (Updated April 12, 2017, 9:21 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Anand Mazumdar, Gastón 
> Kleiman, and Vinod Kone.
> 
> 
> Bugs: MESOS-7377
> https://issues.apache.org/jira/browse/MESOS-7377
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch updates the default executor to pass the executor's
> authorization header to the `Checker` and `HealthChecker`
> libraries when an authentication token is present in the
> environment.
> 
> 
> Diffs
> -
> 
>   src/launcher/default_executor.cpp 9cc40c6391a931f9cdf6965e64be231980537299 
> 
> 
> Diff: https://reviews.apache.org/r/58369/diff/1/
> 
> 
> Testing
> ---
> 
> Testing details can be found at the end of this chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 58252: Allowed the local authorizer to accept subjects with no value.

[Greg Mann]

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

(Updated April 13, 2017, 6:05 p.m.)


Review request for mesos, Adam B, Alexander Rojas, Till Toenshoff, and Vinod 
Kone.


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


Repository: mesos


Description
---

This patch updates checks in the local authorizer to allow subjects
which specify `claims` instead of a `value`.


Diffs (updated)
-

  src/authorizer/local/authorizer.cpp 1c1f912794cfe61112a0e513b217ba3a755f35f1 


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

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


Testing
---

Testing details can be found at the end of this chain.


Thanks,

Greg Mann

Re: Review Request 58251: Changed 'Principal.claims' to a hashmap.

[Greg Mann]

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

(Updated April 13, 2017, 6:04 p.m.)


Review request for mesos, Adam B, Alexander Rojas, Till Toenshoff, and Vinod 
Kone.


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


Repository: mesos


Description
---

This patch changes the `claims` member of the authentication
`Principal` struct from a `std::map` to a `hashmap`, so that
we can make use of the `contains()` helper during authorization.


Diffs (updated)
-

  3rdparty/libprocess/include/process/authenticator.hpp 
272d92117547512328c09dfc04c6ca4bf7b6b937 


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

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


Testing
---

Testing details can be found at the end of this chain.


Thanks,

Greg Mann

Re: Review Request 58421: Passed `--zk_session_timeout` to ZK master contender and detector.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [58421]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker-build.sh

- Mesos Reviewbot


On April 13, 2017, 3:44 p.m., Ilya Pronin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58421/
> ---
> 
> (Updated April 13, 2017, 3:44 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Vinod Kone.
> 
> 
> Bugs: MESOS-7387
> https://issues.apache.org/jira/browse/MESOS-7387
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently `ZooKeeperMasterContender` and `ZooKeeperMasterDetector` use
> hardcoded session timeouts and do not respect `--zk_session_timeout`
> option. This patch fixes it.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/contender.hpp 2c485ef2f24b3aa1b36b764b4780248835a1d9d5 
>   include/mesos/master/detector.hpp f53c7c0bbeaed0b263d2634dc6eb15481d2d90b2 
>   src/master/contender/contender.cpp cc486a176d14e61a2434ce3677c8741963a39057 
>   src/master/contender/zookeeper.hpp becb93fbc04031c133f42f44f3cf406a27262444 
>   src/master/contender/zookeeper.cpp 1aa8f53fc9b0733ac3edb2b4288b7225fac25828 
>   src/master/detector/detector.cpp 0ba29ca9455b85f73cee6787e251b9442c6f97ea 
>   src/master/detector/zookeeper.hpp 5b531e0f1fed7297bb73c5b02e8ef51d0c27ca38 
>   src/master/detector/zookeeper.cpp a737d2403e480d14f1cd345b48af2833d2fa3284 
>   src/master/main.cpp 90d159e248924f6806271dda1ced6d37dfa5f5c9 
> 
> 
> Diff: https://reviews.apache.org/r/58421/diff/1/
> 
> 
> Testing
> ---
> 
> Ran `make check`.
> 
> Manually verified that all 4 ZK session negotiate 20 sec timeout when 
> `--zk_session_timeout=20secs`.
> ```
> 2017-04-13 15:29:21,313:67863(0x7028d000):ZOO_INFO@zookeeper_init@800: 
> Initiating client connection, host=192.168.99.100:2181 sessionTimeout=2 
> watcher=0x107617130 sessionId=0 sessionPasswd= context=0x7f999ad15f90 
> flags=0
> 2017-04-13 15:29:21,314:67863(0x7031):ZOO_INFO@zookeeper_init@800: 
> Initiating client connection, host=192.168.99.100:2181 sessionTimeout=2 
> watcher=0x107617130 sessionId=0 sessionPasswd= context=0x7f999ad164f0 
> flags=0
> 2017-04-13 15:29:21,316:67863(0x7020a000):ZOO_INFO@zookeeper_init@800: 
> Initiating client connection, host=192.168.99.100:2181 sessionTimeout=2 
> watcher=0x107617130 sessionId=0 sessionPasswd= context=0x7f999ae05900 
> flags=0
> 2017-04-13 15:29:21,316:67863(0x70104000):ZOO_INFO@zookeeper_init@800: 
> Initiating client connection, host=192.168.99.100:2181 sessionTimeout=2 
> watcher=0x107617130 sessionId=0 sessionPasswd= context=0x7f999ae05a80 
> flags=0
> 2017-04-13 15:29:21,327:67863(0x70622000):ZOO_INFO@check_events@1775: 
> session establishment complete on server [192.168.99.100:2181], 
> sessionId=0x15b67b0efd3000c, negotiated timeout=2
> 2017-04-13 15:29:21,333:67863(0x70728000):ZOO_INFO@check_events@1775: 
> session establishment complete on server [192.168.99.100:2181], 
> sessionId=0x15b67b0efd3000d, negotiated timeout=2
> 2017-04-13 15:29:21,337:67863(0x7082e000):ZOO_INFO@check_events@1775: 
> session establishment complete on server [192.168.99.100:2181], 
> sessionId=0x15b67b0efd3000e, negotiated timeout=2
> 2017-04-13 15:29:21,347:67863(0x70934000):ZOO_INFO@check_events@1775: 
> session establishment complete on server [192.168.99.100:2181], 
> sessionId=0x15b67b0efd3000f, negotiated timeout=2
> ```
> 
> 
> Thanks,
> 
> Ilya Pronin
> 
>

Re: Review Request 58410: Removed a duplicate unit test 'ROOT_INTERNET_CURL_Normalize'.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On April 13, 2017, 5:34 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58410/
> ---
> 
> (Updated April 13, 2017, 5:34 a.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Chun-Hung Hsiao, Artem 
> Harutyunyan, Ilya Pronin, Jie Yu, Timothy Chen, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed a duplicate unit test 'ROOT_INTERNET_CURL_Normalize'.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> dbbc3533ae269c31c98017d9d5748ef6e1ce 
> 
> 
> Diff: https://reviews.apache.org/r/58410/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Review Request 58421: Passed `--zk_session_timeout` to ZK master contender and detector.

[Ilya Pronin]

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

Review request for mesos, Benjamin Mahler and Vinod Kone.


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


Repository: mesos


Description
---

Currently `ZooKeeperMasterContender` and `ZooKeeperMasterDetector` use
hardcoded session timeouts and do not respect `--zk_session_timeout`
option. This patch fixes it.


Diffs
-

  include/mesos/master/contender.hpp 2c485ef2f24b3aa1b36b764b4780248835a1d9d5 
  include/mesos/master/detector.hpp f53c7c0bbeaed0b263d2634dc6eb15481d2d90b2 
  src/master/contender/contender.cpp cc486a176d14e61a2434ce3677c8741963a39057 
  src/master/contender/zookeeper.hpp becb93fbc04031c133f42f44f3cf406a27262444 
  src/master/contender/zookeeper.cpp 1aa8f53fc9b0733ac3edb2b4288b7225fac25828 
  src/master/detector/detector.cpp 0ba29ca9455b85f73cee6787e251b9442c6f97ea 
  src/master/detector/zookeeper.hpp 5b531e0f1fed7297bb73c5b02e8ef51d0c27ca38 
  src/master/detector/zookeeper.cpp a737d2403e480d14f1cd345b48af2833d2fa3284 
  src/master/main.cpp 90d159e248924f6806271dda1ced6d37dfa5f5c9 


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


Testing
---

Ran `make check`.

Manually verified that all 4 ZK session negotiate 20 sec timeout when 
`--zk_session_timeout=20secs`.
```
2017-04-13 15:29:21,313:67863(0x7028d000):ZOO_INFO@zookeeper_init@800: 
Initiating client connection, host=192.168.99.100:2181 sessionTimeout=2 
watcher=0x107617130 sessionId=0 sessionPasswd= context=0x7f999ad15f90 
flags=0
2017-04-13 15:29:21,314:67863(0x7031):ZOO_INFO@zookeeper_init@800: 
Initiating client connection, host=192.168.99.100:2181 sessionTimeout=2 
watcher=0x107617130 sessionId=0 sessionPasswd= context=0x7f999ad164f0 
flags=0
2017-04-13 15:29:21,316:67863(0x7020a000):ZOO_INFO@zookeeper_init@800: 
Initiating client connection, host=192.168.99.100:2181 sessionTimeout=2 
watcher=0x107617130 sessionId=0 sessionPasswd= context=0x7f999ae05900 
flags=0
2017-04-13 15:29:21,316:67863(0x70104000):ZOO_INFO@zookeeper_init@800: 
Initiating client connection, host=192.168.99.100:2181 sessionTimeout=2 
watcher=0x107617130 sessionId=0 sessionPasswd= context=0x7f999ae05a80 
flags=0
2017-04-13 15:29:21,327:67863(0x70622000):ZOO_INFO@check_events@1775: 
session establishment complete on server [192.168.99.100:2181], 
sessionId=0x15b67b0efd3000c, negotiated timeout=2
2017-04-13 15:29:21,333:67863(0x70728000):ZOO_INFO@check_events@1775: 
session establishment complete on server [192.168.99.100:2181], 
sessionId=0x15b67b0efd3000d, negotiated timeout=2
2017-04-13 15:29:21,337:67863(0x7082e000):ZOO_INFO@check_events@1775: 
session establishment complete on server [192.168.99.100:2181], 
sessionId=0x15b67b0efd3000e, negotiated timeout=2
2017-04-13 15:29:21,347:67863(0x70934000):ZOO_INFO@check_events@1775: 
session establishment complete on server [192.168.99.100:2181], 
sessionId=0x15b67b0efd3000f, negotiated timeout=2
```


Thanks,

Ilya Pronin

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

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [58287]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker-build.sh

- Mesos Reviewbot


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

Re: Review Request 58137: Added 'mesos config show' command to display the config file.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [57896, 57951, 58381, 57952, 58137]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker-build.sh

- Mesos Reviewbot


On April 12, 2017, 9:21 a.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58137/
> ---
> 
> (Updated April 12, 2017, 9:21 a.m.)
> 
> 
> Review request for mesos, Joseph Wu and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This command displays the contents of the user-defined config.toml file.
> 
> 
> Diffs
> -
> 
>   src/cli_new/lib/cli/plugins/config/main.py PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/58137/diff/5/
> 
> 
> Testing
> ---
> 
> Tested manually, PEP8 and Pylint used to make sure that the code style is 
> correct.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

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

[Andy Pang]

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

(Updated 四月 13, 2017, 12:44 p.m.)


Review request for mesos, Jie Yu and Vinod Kone.


Repository: mesos


Description
---

Print corresponding address when socket shutdown.
Default just print socket 'fd',it's not convenient
to find corresponding address.


Diffs (updated)
-

  3rdparty/libprocess/src/process.cpp 92efa91 


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

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


Testing
---

make check


File Attachments (updated)


process.patch
  
https://reviews.apache.org/media/uploaded/files/2017/04/13/d5736720-1262-484a-8efd-6e1eeada944d__process.patch


Thanks,

Andy Pang

Re: Review Request 58347: Added 'ASSERT_THREADSAFE' in libprocess.

[Jan Schlicht]

> On April 11, 2017, 8:45 p.m., Neil Conway wrote:
> > I'm curious what the goal of these assertions is: to fail the test (at 
> > runtime) if the current version of GTest is not thread-safe, right? It 
> > seems weird to be doing that at runtime, since whether GTest is threadsafe 
> > or not seems to be a compile-time property.
> > 
> > Would it make more sense to just avoid compiling these tests if 
> > `GTEST_IS_THREADSAFE` is not defined?

Yes, we could do something like
```
#if GTEST_IS_THREADSAFE
#define TEST_THREADSAFE(test_case_name, test_name) \
  TEST(test_case_name, test_name)
#else
#define TEST_THREADSAFE(test_case_name, test_name) \
  TEST(test_case_name, DISABLED_##test_name)
#endif
```
similar to the `TEST_TEMP_DISABLED_ON_WINDOWS` macros, or use a `THREADSAFE_` 
prefix in the test name and add a filter for these tests. I prefer the second 
solution, because it allows a test case to have mutiple prefixes and will 
implement that.


> On April 11, 2017, 8:45 p.m., Neil Conway wrote:
> > 3rdparty/libprocess/src/tests/process_tests.cpp
> > Line 294 (original), 294 (patched)
> > 
> >
> > Seems like the reference to `GTEST_IS_THREADSAFE` should be updated, 
> > here and elsewhere.
> 
> Joseph Wu wrote:
> One thing to note: Upgrading gtest to 1.8 will bring thread-safety 
> support to Windows ( https://reviews.apache.org/r/58349/ ), so the macro will 
> actually be defined.

Good point. I'll re-enable these tests.


- Jan


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


On April 11, 2017, 2:27 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58347/
> ---
> 
> (Updated April 11, 2017, 2:27 p.m.)
> 
> 
> Review request for mesos, Neil Conway and Till Toenshoff.
> 
> 
> Bugs: MESOS-7193
> https://issues.apache.org/jira/browse/MESOS-7193
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Using 'GTEST_IS_THREADSAFE' in asserts to fail tests that need
> thread-safety early is problematic, because it may be undefined.
> Instead, a new assertion is introduced that evaluates
> 'GTEST_IS_THREADSAFE' and succeeds or fails accordingly.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/gtest.hpp 
> 27077ac9047447fc4c52cc76ab26420e5bc79418 
>   3rdparty/libprocess/src/tests/future_tests.cpp 
> f21361ed1e354778bcd0357afb71300f05d3ecfd 
>   3rdparty/libprocess/src/tests/io_tests.cpp 
> 302fadc2a1894d3fd7c4f4975af3f2cc6c3a22de 
>   3rdparty/libprocess/src/tests/limiter_tests.cpp 
> b80b1da214f97b50aa7b61b79bbf683fd01116aa 
>   3rdparty/libprocess/src/tests/metrics_tests.cpp 
> d7fdb06060b273e16be27a263b5ee268842aa25c 
>   3rdparty/libprocess/src/tests/process_tests.cpp 
> bf90c7e78fd50ad7e16cc0a69a248ba71e2a7115 
>   3rdparty/libprocess/src/tests/reap_tests.cpp 
> 30518dee6c2fb904a607c7a457a5ec7366aab818 
> 
> 
> Diff: https://reviews.apache.org/r/58347/diff/1/
> 
> 
> Testing
> ---
> 
> libprocess-tests (using automake and CMake with macOS, Linux, Windows)
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 57817: Suppress offers for frameworks on registration.

[Anindya Sinha]

> On April 13, 2017, 12:21 a.m., Vinod Kone wrote:
> > src/master/allocator/mesos/allocator.hpp
> > Line 70 (original), 70-71 (patched)
> > 
> >
> > This is making me think whether we want framework to subscribe with 
> > "inactive" field instead "suppress".
> > 
> > The reason being, "suppressOffers" was originally added instead of 
> > "deactivateFramework" because the former was intended to have some filters. 
> > One such filter has been recently added: you can suppress offers for a 
> > specific role instead of all roles. Do we want to give the same flexibility 
> > for suppression during subscription? 
> > 
> > For example, here it's not clear to me what is the difference between 
> > "active" and "offersSuppressed" booleans as far as the allocator is 
> > concerned.
> > 
> > Thoughts?

I agree that the 2 APIs are very similar and I agree we can also achieve the 
same by an `inactive` (instead of `suppress_offers`) field of `SUBSCRIBE` 
override the definition of an active framework which is currently just based on 
framework state. However, it seemed cleaner to me to *not* update the 
definition of an active framework based on a field in the `SUBSCRIBE` call. 
Basically, `framework->state` is *modifiable* characteristic of a framework, 
whereas `suppress_offers` is an initial setting to register that framework.

I think functionally it would achieve the same so I am fine with using an 
`inactive` field in `SUBSCRIBE` redefine the definition of an active framework.

Let me know what you think and I can adapt accordingly.


- Anindya


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


On April 11, 2017, 11:10 p.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57817/
> ---
> 
> (Updated April 11, 2017, 11:10 p.m.)
> 
> 
> Review request for mesos, James Peach, Vinod Kone, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-7015
> https://issues.apache.org/jira/browse/MESOS-7015
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> If requested in SUBSCRIBE api call, offers are suppressed on
> framework registration.
> 
> 
> Diffs
> -
> 
>   include/mesos/allocator/allocator.hpp 
> 6eda1b8619269c1501a935045b18b1deaf845b33 
>   src/master/allocator/mesos/allocator.hpp 
> 57b54b86c43c7731e64d422d285c4b8ca7e27a60 
>   src/master/allocator/mesos/hierarchical.hpp 
> f84b0574ce9a392c9528c87b04b01dbb2053cff7 
>   src/master/allocator/mesos/hierarchical.cpp 
> 051f749dd5921a322ca930a042c31814616d38f9 
>   src/master/master.hpp d537933d0b467a6f9996951c601b31338bb9d034 
>   src/master/master.cpp 0f4c64c6b102ef201779a331c96b5d78a98281ad 
>   src/tests/allocator.hpp 6b71c574e0e4facd1795ef50ee0869c03b87833d 
>   src/tests/hierarchical_allocator_tests.cpp 
> 33e7b455f8664858eb4f03727b076a10c80cd6e0 
>   src/tests/master_allocator_tests.cpp 
> 119e318f8a01d50e8dae5c30cf5fa6a017c3c625 
>   src/tests/persistent_volume_endpoints_tests.cpp 
> 1237d081d781948975f66a8240ae9bdb5e819a3b 
>   src/tests/reservation_tests.cpp 4504831d77c1bfcf5f2ddf6d28cd45dea2c421ad 
>   src/tests/resource_offers_tests.cpp 
> f0bca1d9e03013ce35215b0ffa6b50b38972dc0c 
>   src/tests/slave_recovery_tests.cpp 53f33a2b0411c8158326074ce043c7b1dbeef5b4 
> 
> 
> Diff: https://reviews.apache.org/r/57817/diff/4/
> 
> 
> Testing
> ---
> 
> All tests passed.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>