Re: Review Request 38519: Change function quiesce() to suppress()

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [38514, 38516, 38544, 38519]

All tests passed.

- Mesos ReviewBot


On Sept. 21, 2015, 6:06 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38519/
> ---
> 
> (Updated Sept. 21, 2015, 6:06 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-3037
> https://issues.apache.org/jira/browse/MESOS-3037
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Change function quiesce() to suppress()
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 607d8e1b6efe0115b09be0a0c73edbd8e64d8415 
>   src/master/master.hpp 0c1e81cb807ab04e3d836272c0a197362d811d4c 
>   src/master/master.cpp 6c0db210747c7d88179556abaade65743bd8cb3a 
> 
> Diff: https://reviews.apache.org/r/38519/diff/
> 
> 
> Testing
> ---
> 
> Platform: Ubuntu:14.04
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 37993: WIP: Add explanatory comments for Allocator interface

[Guangya Liu]

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

(Updated 九月 21, 2015, 7:25 a.m.)


Review request for mesos, Adam B, Alexander Rukletsov, and Michael Park.


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


Repository: mesos


Description
---

Add explanatory comments for Allocator interface


Diffs (updated)
-

  include/mesos/master/allocator.hpp 3fea47ffcc69531308068e2701502e481605b912 

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


Testing
---


Thanks,

Guangya Liu

Re: Review Request 38544: libprocess: Updated macro of suppress to SUPPRESS

[Guangya Liu]

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

(Updated 九月 21, 2015, 7:27 a.m.)


Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

The macro suppress conflicts with suppress() for a call in master,
this patch is updating macro suppress to SUPPRESS to resolve the
conflict.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/sendfile.hpp 
50ad49e7cc3a2d2c0d731c177109629e23660041 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/signals.hpp 
7a790242368a06975cdabe60301d1f341849f09c 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/signals.hpp 
8361a13907ec8044d52545088a767e337dbb1b37 
  3rdparty/libprocess/3rdparty/stout/tests/os/signals_tests.cpp 
de86232cd4a646c63cdb41d18cd4375615cb42e4 

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


Testing
---


Thanks,

Guangya Liu

Re: Review Request 37993: Add explanatory comments for Allocator interface

[Guangya Liu]

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

(Updated 九月 21, 2015, 7:28 a.m.)


Review request for mesos, Adam B, Alexander Rukletsov, and Michael Park.


Summary (updated)
-

Add explanatory comments for Allocator interface


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


Repository: mesos


Description
---

Add explanatory comments for Allocator interface


Diffs
-

  include/mesos/master/allocator.hpp 3fea47ffcc69531308068e2701502e481605b912 

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


Testing
---


Thanks,

Guangya Liu

Re: Review Request 38279: Enabled resources/attributes discovery

[Guangya Liu]

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



src/tests/hook_tests.cpp (line 659)


s/VerifySlaveResourcesHook/VerifySlaveResourcesDiscoverHook/g


- Guangya Liu


On 九月 19, 2015, 12:51 a.m., Felix Abecassis wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38279/
> ---
> 
> (Updated 九月 19, 2015, 12:51 a.m.)
> 
> 
> Review request for mesos, Connor Doyle and Niklas Nielsen.
> 
> 
> Bugs: MESOS-3366
> https://issues.apache.org/jira/browse/MESOS-3366
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> First API draft for MESOS-3366.
> 
> 1) Only supports resources for now, we can add another hook for attributes 
> with a very similar code.
> 2) The callback currently receives the full SlaveInfo structure and construct 
> a new Resources object.
> 3) If there is multiple callbacks, each callback will see the changes made by 
> previous callbacks and are free to override or merge the detected resources 
> as they see fit.
> 
> 
> Diffs
> -
> 
>   include/mesos/hook.hpp 2353602 
>   src/examples/test_hook_module.cpp 0dc74d6 
>   src/hook/manager.hpp a517c05 
>   src/hook/manager.cpp 691976e 
>   src/slave/slave.cpp ad710d7 
>   src/tests/hook_tests.cpp b23a587 
> 
> Diff: https://reviews.apache.org/r/38279/diff/
> 
> 
> Testing
> ---
> 
> make clean && make && make check
> 
> 
> Thanks,
> 
> Felix Abecassis
> 
>

Re: Review Request 37993: Add explanatory comments for Allocator interface

[Jian Qiu]

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



include/mesos/master/allocator.hpp (line 80)


needs adding explanation for @param inverseOfferCallback


- Jian Qiu


On Sept. 21, 2015, 7:28 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37993/
> ---
> 
> (Updated Sept. 21, 2015, 7:28 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, and Michael Park.
> 
> 
> Bugs: MESOS-2224
> https://issues.apache.org/jira/browse/MESOS-2224
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add explanatory comments for Allocator interface
> 
> 
> Diffs
> -
> 
>   include/mesos/master/allocator.hpp 3fea47ffcc69531308068e2701502e481605b912 
> 
> Diff: https://reviews.apache.org/r/37993/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 37993: Add explanatory comments for Allocator interface

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [37993]

All tests passed.

- Mesos ReviewBot


On Sept. 21, 2015, 7:28 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37993/
> ---
> 
> (Updated Sept. 21, 2015, 7:28 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, and Michael Park.
> 
> 
> Bugs: MESOS-2224
> https://issues.apache.org/jira/browse/MESOS-2224
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add explanatory comments for Allocator interface
> 
> 
> Diffs
> -
> 
>   include/mesos/master/allocator.hpp 3fea47ffcc69531308068e2701502e481605b912 
> 
> Diff: https://reviews.apache.org/r/37993/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Review Request 38549: [1/2]CMake: Add libevent version, configure Windows to use as default.

[Alex Clemmer]

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

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


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


Repository: mesos


Description
---

Mesos will need to use libevent to build the process library on Windows.

This commit will add a default version of libevent, which we will
eventually retrieve from the 3rdparty GitHub repository, which for now
is our "official" distribution channel for out-of-band third-party
dependencies (especially on Windows, which has no package manager).


Diffs
-

  3rdparty/cmake/Versions.cmake PRE-CREATION 
  CMakeLists.txt 3b6f4af337466d33cb915959a5995e4307b27be3 

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


Testing
---

Compiled and ran made sure libprocess and stout tests ran and passed on the 
following platforms:

* OS X 10.10
* Windows 10
* Ubuntu 14.04.2


Thanks,

Alex Clemmer

Review Request 38550: [2/2]CMake: Integrate libevent into Windows builds.

[Alex Clemmer]

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

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


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


Repository: mesos


Description
---

Since there is no robust standalone libev support on Windows, Mesos will
need to use libevent to support Windows, at least in the short term.

This commit will add logic to configure, build, and install libevent for
all Windows builds.


Diffs
-

  3rdparty/libprocess/3rdparty/CMakeLists.txt 
b9c9fae7d448906e9c9f5ab0ee3fe138a0171a7d 
  3rdparty/libprocess/cmake/ProcessConfigure.cmake 
a5f8d399e151acad87bb72ecb1f7372b2c467423 

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


Testing
---

Compiled and ran made sure libprocess and stout tests ran and passed on the 
following platforms:

* OS X 10.10
* Windows 10
* Ubuntu 14.04.2


Thanks,

Alex Clemmer

Review Request 38552: [2/2]CMake: Add Windows-specific build targets for APR.

[Alex Clemmer]

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

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


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


Repository: mesos


Description
---

APR is required for Mesos to run. Normally we expect that a user has
obtained APR with their favorite package manager, but on Windows, we
cannot really expect that this is the case. So, we have to rope in the
dependency manually in the CMake build system.

This commit will introduce the necessary build targets to obtain,
configure, build, and install APR for Windows builds. It will ignore APR
and assume it's there for Linux builds.


Diffs
-

  3rdparty/libprocess/3rdparty/CMakeLists.txt 
b9c9fae7d448906e9c9f5ab0ee3fe138a0171a7d 
  3rdparty/libprocess/cmake/ProcessConfigure.cmake 
a5f8d399e151acad87bb72ecb1f7372b2c467423 

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


Testing
---

Compiled and ran made sure libprocess and stout tests ran and passed on the 
following platforms:

* OS X 10.10
* Windows 10
* Ubuntu 14.04.2


Thanks,

Alex Clemmer

Review Request 38551: [1/2]CMake: Add version info for APR we need to build Windows.

[Alex Clemmer]

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

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


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


Repository: mesos


Description
---

[1/2]CMake: Add version info for APR we need to build Windows.


Diffs
-

  3rdparty/cmake/Versions.cmake PRE-CREATION 

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


Testing
---

Compiled and ran made sure libprocess and stout tests ran and passed on the 
following platforms:

* OS X 10.10
* Windows 10
* Ubuntu 14.04.2


Thanks,

Alex Clemmer

Re: Review Request 38550: [2/2]CMake: Integrate libevent into Windows builds.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [38456, 38457, 38529, 38530, 38531, 38538, 38539, 38540, 
38541, 38542, 38549, 38550]

All tests passed.

- Mesos ReviewBot


On Sept. 21, 2015, 10:43 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38550/
> ---
> 
> (Updated Sept. 21, 2015, 10:43 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Bugs: MESOS-3380
> https://issues.apache.org/jira/browse/MESOS-3380
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Since there is no robust standalone libev support on Windows, Mesos will
> need to use libevent to support Windows, at least in the short term.
> 
> This commit will add logic to configure, build, and install libevent for
> all Windows builds.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/CMakeLists.txt 
> b9c9fae7d448906e9c9f5ab0ee3fe138a0171a7d 
>   3rdparty/libprocess/cmake/ProcessConfigure.cmake 
> a5f8d399e151acad87bb72ecb1f7372b2c467423 
> 
> Diff: https://reviews.apache.org/r/38550/diff/
> 
> 
> Testing
> ---
> 
> Compiled and ran made sure libprocess and stout tests ran and passed on the 
> following platforms:
> 
> * OS X 10.10
> * Windows 10
> * Ubuntu 14.04.2
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 38552: [2/2]CMake: Add Windows-specific build targets for APR.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [38456, 38457, 38529, 38530, 38531, 38538, 38539, 38540, 
38541, 38542, 38549, 38550, 38551, 38552]

All tests passed.

- Mesos ReviewBot


On Sept. 21, 2015, 11:21 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38552/
> ---
> 
> (Updated Sept. 21, 2015, 11:21 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Bugs: MESOS-3383
> https://issues.apache.org/jira/browse/MESOS-3383
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> APR is required for Mesos to run. Normally we expect that a user has
> obtained APR with their favorite package manager, but on Windows, we
> cannot really expect that this is the case. So, we have to rope in the
> dependency manually in the CMake build system.
> 
> This commit will introduce the necessary build targets to obtain,
> configure, build, and install APR for Windows builds. It will ignore APR
> and assume it's there for Linux builds.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/CMakeLists.txt 
> b9c9fae7d448906e9c9f5ab0ee3fe138a0171a7d 
>   3rdparty/libprocess/cmake/ProcessConfigure.cmake 
> a5f8d399e151acad87bb72ecb1f7372b2c467423 
> 
> Diff: https://reviews.apache.org/r/38552/diff/
> 
> 
> Testing
> ---
> 
> Compiled and ran made sure libprocess and stout tests ran and passed on the 
> following platforms:
> 
> * OS X 10.10
> * Windows 10
> * Ubuntu 14.04.2
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 37821: Join threads in libprocess when shutting down.

[Greg Mann]

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

(Updated Sept. 21, 2015, 3:53 p.m.)


Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Joris Van 
Remoortere, Joseph Wu, and Michael Park.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Join threads in libprocess when shutting down.


Diffs (updated)
-

  3rdparty/libprocess/src/event_loop.hpp 
36a4cd2b1ff59f6922173ad17115bf80cc3c8f30 
  3rdparty/libprocess/src/libev.cpp 97a2694f9b10bc61841443b21f4f96055493e840 
  3rdparty/libprocess/src/libevent.cpp ee7906470069b0391dde7cd685b1d4eb3a158c03 
  3rdparty/libprocess/src/process.cpp 4afa30569b4d235637b49a624602e6b199c32e0e 

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


Testing
---

After configuring with both "../configure" and "../configure --enable-libevent 
--enable-ssl":

make check


Also, to check for race conditions related to the initialization/shutdown of 
libprocess, try something like:

for n in {1..1000}; do echo $n; 3rdparty/libprocess/tests 
--gtest_filter=ProcessTest.Spawn; done


Thanks,

Greg Mann

Re: Review Request 38457: CMake: Fix MESOS-3250, a dynamic load error in Stout tests on OS X.

[Marco Massenzio]

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

Ship it!


Ship It!

- Marco Massenzio


On Sept. 21, 2015, 2:43 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38457/
> ---
> 
> (Updated Sept. 21, 2015, 2:43 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Bugs: MESOS-3250
> https://issues.apache.org/jira/browse/MESOS-3250
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> A few third-party libraries (libev, gmock) do not have `make install`
> commands available, so below we have to add our own "install" commands.
> 
> The reason is: if we do not, we get runtime library load problems on OS
> X. In particular, `dydl` will look for these libraries at the prefix we
> passed to `configure` (or in `/usr/local` if we did not pass a prefix
> in), but since they don't have a `make install` step, they never get
> placed in the prefix folder.
> 
> Our solution is to:
>   (1) make a lib directory inside the Mesos folder for each of the
>   libraries that has no install step, and
>   (2) copy all such libraries into their respective directories.
> 
> (Note that step (1) is not only convenient, but important: make will add
> a `lib` to the end of your prefix path when linking, and since the built
> libraries end up in a `.libs` folder, it's not enough to simply pass the
> build directory into `configure` as a prefix; so if we're going to move
> the libraries, we might as well move them to a library folder.)
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/CMakeLists.txt 
> b9c9fae7d448906e9c9f5ab0ee3fe138a0171a7d 
> 
> Diff: https://reviews.apache.org/r/38457/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 37821: Join threads in libprocess when shutting down.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [37821]

All tests passed.

- Mesos ReviewBot


On Sept. 21, 2015, 3:53 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37821/
> ---
> 
> (Updated Sept. 21, 2015, 3:53 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Joris Van 
> Remoortere, Joseph Wu, and Michael Park.
> 
> 
> Bugs: MESOS-3158
> https://issues.apache.org/jira/browse/MESOS-3158
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Join threads in libprocess when shutting down.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/event_loop.hpp 
> 36a4cd2b1ff59f6922173ad17115bf80cc3c8f30 
>   3rdparty/libprocess/src/libev.cpp 97a2694f9b10bc61841443b21f4f96055493e840 
>   3rdparty/libprocess/src/libevent.cpp 
> ee7906470069b0391dde7cd685b1d4eb3a158c03 
>   3rdparty/libprocess/src/process.cpp 
> 4afa30569b4d235637b49a624602e6b199c32e0e 
> 
> Diff: https://reviews.apache.org/r/37821/diff/
> 
> 
> Testing
> ---
> 
> After configuring with both "../configure" and "../configure 
> --enable-libevent --enable-ssl":
> 
> make check
> 
> 
> Also, to check for race conditions related to the initialization/shutdown of 
> libprocess, try something like:
> 
> for n in {1..1000}; do echo $n; 3rdparty/libprocess/tests 
> --gtest_filter=ProcessTest.Spawn; done
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 38137: Added Docker provisioner, store and local puller

[Timothy Chen]

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

(Updated Sept. 21, 2015, 5:03 p.m.)


Review request for mesos, Jie Yu, Jojy Varghese, Till Toenshoff, and Jiang Yan 
Xu.


Summary (updated)
-

Added Docker provisioner, store and local puller


Repository: mesos


Description
---

Joining all the commits around provisioner and local store into one review so 
it's easier to review, as patches
are changing code on top of each other.

All the commits are going to committed together.


Diffs
-

  src/Makefile.am 8963cea 
  src/slave/containerizer/provisioner.hpp 9e0e0b8 
  src/slave/containerizer/provisioner.cpp 2ac9008 
  src/slave/containerizer/provisioners/docker/local_store.hpp PRE-CREATION 
  src/slave/containerizer/provisioners/docker/local_store.cpp PRE-CREATION 
  src/slave/containerizer/provisioners/docker/message.hpp PRE-CREATION 
  src/slave/containerizer/provisioners/docker/message.proto PRE-CREATION 
  src/slave/containerizer/provisioners/docker/metadata_manager.hpp PRE-CREATION 
  src/slave/containerizer/provisioners/docker/metadata_manager.cpp PRE-CREATION 
  src/slave/containerizer/provisioners/docker/paths.hpp PRE-CREATION 
  src/slave/containerizer/provisioners/docker/paths.cpp PRE-CREATION 
  src/slave/containerizer/provisioners/docker/provisioner.hpp PRE-CREATION 
  src/slave/containerizer/provisioners/docker/provisioner.cpp PRE-CREATION 
  src/slave/containerizer/provisioners/docker/store.hpp PRE-CREATION 
  src/slave/containerizer/provisioners/docker/store.cpp PRE-CREATION 
  src/slave/flags.hpp 799c963 
  src/slave/flags.cpp b676bac 
  src/tests/containerizer/docker_provisioner_tests.cpp PRE-CREATION 

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


Testing
---

make check


Thanks,

Timothy Chen

Re: Review Request 38137: Added Docker provisioner, store and local puller

[Timothy Chen]

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

(Updated Sept. 21, 2015, 5:06 p.m.)


Review request for mesos, Jie Yu, Jojy Varghese, Till Toenshoff, and Jiang Yan 
Xu.


Repository: mesos


Description
---

Joining all the commits around provisioner and local store into one review so 
it's easier to review, as patches
are changing code on top of each other.

All the commits are going to committed together.


Diffs (updated)
-

  src/Makefile.am 834f69a 
  src/slave/containerizer/provisioner.cpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/local_puller.hpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/local_puller.cpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/message.hpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/message.proto PRE-CREATION 
  src/slave/containerizer/provisioner/docker/metadata_manager.hpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/metadata_manager.cpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/paths.hpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/paths.cpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/puller.hpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/puller.cpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/store.hpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/store.cpp PRE-CREATION 
  src/slave/flags.hpp e31a418 
  src/slave/flags.cpp add4196 
  src/tests/containerizer/provisioner_docker_tests.cpp 1b0c304 

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


Testing
---

make check


Thanks,

Timothy Chen

Re: Review Request 38051: Only update the task status when its old status is not terminal.

[Vinod Kone]

> On Sept. 17, 2015, 10:25 p.m., Vinod Kone wrote:
> > Can you write a test for this?
> 
> Yong Qiao Wang wrote:
> I find the code changes in this patch does not be tested with an 
> end-to-end case except to check the error log messages of master, so my test 
> strategy are:
> 
> 1. Change the python test executor to send the TASK_FINASHED with two 
> times;
> 2. Check the error log message of maser;
> 
> Test passwd!
> 
> Vinod, I am not familiar with current test framework of mesos, cloud you 
> provide some similar test code, and I can refer to write my test case for 
> this. Thanks!
> 
> Vinod Kone wrote:
> Instaed of updating python executor, write a self-contained unit test. 
> Take a look at "UnacknowledgedTerminalTask" and 
> "ReleaseResourcesForTerminalTaskWithPendingUpdates" in 
> src/tests/master_tests.cpp for inspiration on how to write such a test.
> 
> Also, instead of sending TASK_FINISHED multiple times, have the executor 
> send TASK_FINISHED followed by a TASK_KILLED. That way you can check that the 
> status of the task hasn't changed in the master.
> 
> Yong Qiao Wang wrote:
> Thanks Vinod. In Master::statusUpdate function, I find if the status 
> update is invalid(such as out-of-order updates), we still forward the updates 
> to related framework and update the metrics 
> (metrics->valid_status_updates++), is it reasonable?

yes.


- Vinod


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


On Sept. 18, 2015, 7:15 a.m., Yong Qiao Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38051/
> ---
> 
> (Updated Sept. 18, 2015, 7:15 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-2864
> https://issues.apache.org/jira/browse/MESOS-2864
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Only update the task status when its old status is not terminal.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 1c4e7af 
> 
> Diff: https://reviews.apache.org/r/38051/diff/
> 
> 
> Testing
> ---
> 
> UT:
> 
> 1. make successfully!
> 2. make check successfully!
> 3. Run test framework successfully!
> 
> 
> Thanks,
> 
> Yong Qiao Wang
> 
>

Re: Review Request 38535: Fix ExamplesTest errors when user is root.

[Jie Yu]

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



src/slave/containerizer/linux_launcher.cpp (lines 184 - 186)


Could you please explain where the bug is in the description. I don't 
understand why this code is buggy.


- Jie Yu


On Sept. 20, 2015, 5:04 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38535/
> ---
> 
> (Updated Sept. 20, 2015, 5:04 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Joris Van Remoortere, Kapil Arya, and 
> Michael Park.
> 
> 
> Bugs: MESOS-3474
> https://issues.apache.org/jira/browse/MESOS-3474
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix ExamplesTest errors when user is root.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/linux_launcher.cpp 
> dde552ff2a9d29c31f7676180fffa19d1fb0ba63 
> 
> Diff: https://reviews.apache.org/r/38535/diff/
> 
> 
> Testing
> ---
> 
> sudo ./bin/mesos-tests.sh --gtest_filter="ExamplesTest.*" --verbose
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 38137: Added Docker provisioner, store and local puller

[Mesos ReviewBot]

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


Bad patch!

Reviews applied: [38137]

Failed command: make -j3 distcheck

Error:
 make  dist-gzip am__post_remove_distdir='@:'
make[1]: Entering directory 
`/home/jenkins/jenkins-slave/workspace/mesos-reviewbot'
if test -d "mesos-0.25.0"; then find "mesos-0.25.0" -type d ! -perm -200 -exec 
chmod u+w {} ';' && rm -rf "mesos-0.25.0" || { sleep 5 && rm -rf 
"mesos-0.25.0"; }; else :; fi
test -d "mesos-0.25.0" || mkdir "mesos-0.25.0"
 (cd 3rdparty && make  top_distdir=../mesos-0.25.0 
distdir=../mesos-0.25.0/3rdparty \
 am__remove_distdir=: am__skip_length_check=: am__skip_mode_fix=: distdir)
make[2]: Entering directory 
`/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty'
 (cd libprocess && make  top_distdir=../../mesos-0.25.0 
distdir=../../mesos-0.25.0/3rdparty/libprocess \
 am__remove_distdir=: am__skip_length_check=: am__skip_mode_fix=: distdir)
make[3]: Entering directory 
`/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess'
:
test -d "../../mesos-0.25.0/3rdparty/libprocess" || mkdir 
"../../mesos-0.25.0/3rdparty/libprocess"
 (cd 3rdparty && make  top_distdir=../../../mesos-0.25.0 
distdir=../../../mesos-0.25.0/3rdparty/libprocess/3rdparty \
 am__remove_distdir=: am__skip_length_check=: am__skip_mode_fix=: distdir)
make[4]: Entering directory 
`/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/3rdparty'
 (cd stout && make  top_distdir=../../../../mesos-0.25.0 
distdir=../../../../mesos-0.25.0/3rdparty/libprocess/3rdparty/stout \
 am__remove_distdir=: am__skip_length_check=: am__skip_mode_fix=: distdir)
make[5]: Entering directory 
`/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/3rdparty/stout'
:
test -d "../../../../mesos-0.25.0/3rdparty/libprocess/3rdparty/stout" || mkdir 
"../../../../mesos-0.25.0/3rdparty/libprocess/3rdparty/stout"
 (cd include && make  top_distdir=../../../../../mesos-0.25.0 
distdir=../../../../../mesos-0.25.0/3rdparty/libprocess/3rdparty/stout/include \
 am__remove_distdir=: am__skip_length_check=: am__skip_mode_fix=: distdir)
make[6]: Entering directory 
`/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/3rdparty/stout/include'
make[6]: Leaving directory 
`/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/3rdparty/stout/include'
test -n ":" \
|| find "../../../../mesos-0.25.0/3rdparty/libprocess/3rdparty/stout" 
-type d ! -perm -755 \
-exec chmod u+rwx,go+rx {} \; -o \
  ! -type d ! -perm -444 -links 1 -exec chmod a+r {} \; -o \
  ! -type d ! -perm -400 -exec chmod a+r {} \; -o \
  ! -type d ! -perm -444 -exec /bin/bash 
/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/3rdparty/stout/install-sh
 -c -m a+r {} {} \; \
|| chmod -R a+r 
"../../../../mesos-0.25.0/3rdparty/libprocess/3rdparty/stout"
make[5]: Leaving directory 
`/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/3rdparty/stout'
make[4]: Leaving directory 
`/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/3rdparty'
 (cd include && make  top_distdir=../../../mesos-0.25.0 
distdir=../../../mesos-0.25.0/3rdparty/libprocess/include \
 am__remove_distdir=: am__skip_length_check=: am__skip_mode_fix=: distdir)
make[4]: Entering directory 
`/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/include'
make[4]: Leaving directory 
`/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/include'
test -n ":" \
|| find "../../mesos-0.25.0/3rdparty/libprocess" -type d ! -perm -755 \
-exec chmod u+rwx,go+rx {} \; -o \
  ! -type d ! -perm -444 -links 1 -exec chmod a+r {} \; -o \
  ! -type d ! -perm -400 -exec chmod a+r {} \; -o \
  ! -type d ! -perm -444 -exec /bin/bash 
/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/install-sh
 -c -m a+r {} {} \; \
|| chmod -R a+r "../../mesos-0.25.0/3rdparty/libprocess"
make[3]: Leaving directory 
`/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess'
make[2]: Leaving directory 
`/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty'
 (cd src && make  top_distdir=../mesos-0.25.0 distdir=../mesos-0.25.0/src \
 am__remove_distdir=: am__skip_length_check=: am__skip_mode_fix=: distdir)
make[2]: Entering directory 
`/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/src'
make[2]: *** No rule to make target `messages/docker_provisioner.proto', needed 
by `distdir'.  Stop.
make[2]: Leaving directory 
`/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/src'
make[1]: *** [distdir] Error 1
make[1]: Leaving directory 
`/home/jenkins/jenkins-sla

Re: Review Request 38514: Update QUIESCE to SUPPRESS in Mesos Call

[Vinod Kone]

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

Ship it!


Ship It!

- Vinod Kone


On Sept. 21, 2015, 6:05 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38514/
> ---
> 
> (Updated Sept. 21, 2015, 6:05 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-3037
> https://issues.apache.org/jira/browse/MESOS-3037
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> A comment from Vinod: After some discussion with other committers,
> most of them felt SUPPRESS is better than QUIESCE.
> 
> 
> Diffs
> -
> 
>   include/mesos/scheduler/scheduler.proto 
> 89ddc10d5873cdf09bf5084c4869bc7c29933eab 
>   include/mesos/v1/scheduler/scheduler.proto 
> bc19b8dfb2ded26a30a28e7370f06816e0fb1999 
>   src/master/http.cpp 607d8e1b6efe0115b09be0a0c73edbd8e64d8415 
>   src/master/master.cpp 6c0db210747c7d88179556abaade65743bd8cb3a 
>   src/master/validation.cpp 0f3fc1a20b065a88712cceabab0ceffaec598533 
>   src/sched/sched.cpp 84c2edb2c660f976fd57d1407dd2e9d5376500aa 
>   src/tests/scheduler_tests.cpp 0f892f9d423f0bc72cdab22452e0ad7965dda444 
> 
> Diff: https://reviews.apache.org/r/38514/diff/
> 
> 
> Testing
> ---
> 
> Platform: Ubuntu:14.04
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 38544: libprocess: Updated macro of suppress to SUPPRESS

[Vinod Kone]

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

Ship it!


Ship It!

- Vinod Kone


On Sept. 21, 2015, 7:27 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38544/
> ---
> 
> (Updated Sept. 21, 2015, 7:27 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-3037
> https://issues.apache.org/jira/browse/MESOS-3037
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The macro suppress conflicts with suppress() for a call in master,
> this patch is updating macro suppress to SUPPRESS to resolve the
> conflict.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/sendfile.hpp 
> 50ad49e7cc3a2d2c0d731c177109629e23660041 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/signals.hpp 
> 7a790242368a06975cdabe60301d1f341849f09c 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/signals.hpp 
> 8361a13907ec8044d52545088a767e337dbb1b37 
>   3rdparty/libprocess/3rdparty/stout/tests/os/signals_tests.cpp 
> de86232cd4a646c63cdb41d18cd4375615cb42e4 
> 
> Diff: https://reviews.apache.org/r/38544/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 38528: Disable perf test when perf version is not support.

[Paul Brett]

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


The PerfTests ROOT_Events and ROOT_Sample both require a supported version of 
perf to be installed, however the Parse test does not.  Could we use the 
supported() test to disable all events matching both ROOT and Perf?

- Paul Brett


On Sept. 20, 2015, 2:49 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38528/
> ---
> 
> (Updated Sept. 20, 2015, 2:49 a.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere and Michael Park.
> 
> 
> Bugs: MESOS-3471
> https://issues.apache.org/jira/browse/MESOS-3471
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Disable perf test when perf version is not support.
> 
> 
> Diffs
> -
> 
>   src/tests/environment.cpp e40cde23daba25b0b61b567ee73682c67e7acbdc 
> 
> Diff: https://reviews.apache.org/r/38528/diff/
> 
> 
> Testing
> ---
> 
> # On CentOS 6.6
> sudo make -j8 check
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 38137: Added Docker provisioner, store and local puller

[Timothy Chen]

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

(Updated Sept. 21, 2015, 5:33 p.m.)


Review request for mesos, Jie Yu, Jojy Varghese, Till Toenshoff, and Jiang Yan 
Xu.


Repository: mesos


Description
---

Joining all the commits around provisioner and local store into one review so 
it's easier to review, as patches
are changing code on top of each other.

All the commits are going to committed together.


Diffs (updated)
-

  src/Makefile.am 834f69a 
  src/slave/containerizer/provisioner.cpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/local_puller.hpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/local_puller.cpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/message.hpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/message.proto PRE-CREATION 
  src/slave/containerizer/provisioner/docker/metadata_manager.hpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/metadata_manager.cpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/paths.hpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/paths.cpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/puller.hpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/puller.cpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/store.hpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/store.cpp PRE-CREATION 
  src/slave/flags.hpp e31a418 
  src/slave/flags.cpp add4196 
  src/tests/containerizer/provisioner_docker_tests.cpp 1b0c304 

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


Testing
---

make check


Thanks,

Timothy Chen

Re: Review Request 38137: Added Docker provisioner, store and local puller

[Mesos ReviewBot]

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


Bad patch!

Reviews applied: [38137]

Failed command: make -j3 distcheck

Error:
 make  dist-gzip am__post_remove_distdir='@:'
make[1]: Entering directory 
`/home/jenkins/jenkins-slave/workspace/mesos-reviewbot'
if test -d "mesos-0.25.0"; then find "mesos-0.25.0" -type d ! -perm -200 -exec 
chmod u+w {} ';' && rm -rf "mesos-0.25.0" || { sleep 5 && rm -rf 
"mesos-0.25.0"; }; else :; fi
test -d "mesos-0.25.0" || mkdir "mesos-0.25.0"
 (cd 3rdparty && make  top_distdir=../mesos-0.25.0 
distdir=../mesos-0.25.0/3rdparty \
 am__remove_distdir=: am__skip_length_check=: am__skip_mode_fix=: distdir)
make[2]: Entering directory 
`/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty'
 (cd libprocess && make  top_distdir=../../mesos-0.25.0 
distdir=../../mesos-0.25.0/3rdparty/libprocess \
 am__remove_distdir=: am__skip_length_check=: am__skip_mode_fix=: distdir)
make[3]: Entering directory 
`/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess'
:
test -d "../../mesos-0.25.0/3rdparty/libprocess" || mkdir 
"../../mesos-0.25.0/3rdparty/libprocess"
 (cd 3rdparty && make  top_distdir=../../../mesos-0.25.0 
distdir=../../../mesos-0.25.0/3rdparty/libprocess/3rdparty \
 am__remove_distdir=: am__skip_length_check=: am__skip_mode_fix=: distdir)
make[4]: Entering directory 
`/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/3rdparty'
 (cd stout && make  top_distdir=../../../../mesos-0.25.0 
distdir=../../../../mesos-0.25.0/3rdparty/libprocess/3rdparty/stout \
 am__remove_distdir=: am__skip_length_check=: am__skip_mode_fix=: distdir)
make[5]: Entering directory 
`/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/3rdparty/stout'
:
test -d "../../../../mesos-0.25.0/3rdparty/libprocess/3rdparty/stout" || mkdir 
"../../../../mesos-0.25.0/3rdparty/libprocess/3rdparty/stout"
 (cd include && make  top_distdir=../../../../../mesos-0.25.0 
distdir=../../../../../mesos-0.25.0/3rdparty/libprocess/3rdparty/stout/include \
 am__remove_distdir=: am__skip_length_check=: am__skip_mode_fix=: distdir)
make[6]: Entering directory 
`/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/3rdparty/stout/include'
make[6]: Leaving directory 
`/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/3rdparty/stout/include'
test -n ":" \
|| find "../../../../mesos-0.25.0/3rdparty/libprocess/3rdparty/stout" 
-type d ! -perm -755 \
-exec chmod u+rwx,go+rx {} \; -o \
  ! -type d ! -perm -444 -links 1 -exec chmod a+r {} \; -o \
  ! -type d ! -perm -400 -exec chmod a+r {} \; -o \
  ! -type d ! -perm -444 -exec /bin/bash 
/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/3rdparty/stout/install-sh
 -c -m a+r {} {} \; \
|| chmod -R a+r 
"../../../../mesos-0.25.0/3rdparty/libprocess/3rdparty/stout"
make[5]: Leaving directory 
`/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/3rdparty/stout'
make[4]: Leaving directory 
`/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/3rdparty'
 (cd include && make  top_distdir=../../../mesos-0.25.0 
distdir=../../../mesos-0.25.0/3rdparty/libprocess/include \
 am__remove_distdir=: am__skip_length_check=: am__skip_mode_fix=: distdir)
make[4]: Entering directory 
`/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/include'
make[4]: Leaving directory 
`/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/include'
test -n ":" \
|| find "../../mesos-0.25.0/3rdparty/libprocess" -type d ! -perm -755 \
-exec chmod u+rwx,go+rx {} \; -o \
  ! -type d ! -perm -444 -links 1 -exec chmod a+r {} \; -o \
  ! -type d ! -perm -400 -exec chmod a+r {} \; -o \
  ! -type d ! -perm -444 -exec /bin/bash 
/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/install-sh
 -c -m a+r {} {} \; \
|| chmod -R a+r "../../mesos-0.25.0/3rdparty/libprocess"
make[3]: Leaving directory 
`/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess'
make[2]: Leaving directory 
`/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty'
 (cd src && make  top_distdir=../mesos-0.25.0 distdir=../mesos-0.25.0/src \
 am__remove_distdir=: am__skip_length_check=: am__skip_mode_fix=: distdir)
make[2]: Entering directory 
`/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/src'
make[2]: *** No rule to make target 
`slave/containerizer/provisioner/docker.hpp', needed by `distdir'.  Stop.
make[2]: Leaving directory 
`/home/jenkins/jenkins-slave/workspace/mesos-reviewbot/src'
make[1]: *** [distdir] Error 1
make[1]: Leaving directory 
`/home/jenkins/je

Re: Review Request 38535: Fix ExamplesTest errors when user is root.

[haosdent huang]

> On Sept. 21, 2015, 5:16 p.m., Jie Yu wrote:
> > src/slave/containerizer/linux_launcher.cpp, lines 184-186
> > 
> >
> > Could you please explain where the bug is in the description. I don't 
> > understand why this code is buggy.

Because we have multi slaves, this share memory would be override in different 
threads. So that all threads would got the same "pipes" and same execute 
"function" pointer.


- haosdent


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


On Sept. 20, 2015, 5:04 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38535/
> ---
> 
> (Updated Sept. 20, 2015, 5:04 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Joris Van Remoortere, Kapil Arya, and 
> Michael Park.
> 
> 
> Bugs: MESOS-3474
> https://issues.apache.org/jira/browse/MESOS-3474
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix ExamplesTest errors when user is root.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/linux_launcher.cpp 
> dde552ff2a9d29c31f7676180fffa19d1fb0ba63 
> 
> Diff: https://reviews.apache.org/r/38535/diff/
> 
> 
> Testing
> ---
> 
> sudo ./bin/mesos-tests.sh --gtest_filter="ExamplesTest.*" --verbose
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 38137: Added Docker provisioner, store and local puller

[Timothy Chen]

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

(Updated Sept. 21, 2015, 5:50 p.m.)


Review request for mesos, Jie Yu, Jojy Varghese, Till Toenshoff, and Jiang Yan 
Xu.


Repository: mesos


Description
---

Joining all the commits around provisioner and local store into one review so 
it's easier to review, as patches
are changing code on top of each other.

All the commits are going to committed together.


Diffs (updated)
-

  src/Makefile.am 834f69a 
  src/slave/containerizer/provisioner.cpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/local_puller.hpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/local_puller.cpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/message.hpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/message.proto PRE-CREATION 
  src/slave/containerizer/provisioner/docker/metadata_manager.hpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/metadata_manager.cpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/paths.hpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/paths.cpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/puller.hpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/puller.cpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/store.hpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/store.cpp PRE-CREATION 
  src/slave/flags.hpp e31a418 
  src/slave/flags.cpp add4196 
  src/tests/containerizer/provisioner_docker_tests.cpp 1b0c304 

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


Testing
---

make check


Thanks,

Timothy Chen

Re: Review Request 38535: Fix ExamplesTest errors when user is root.

[haosdent huang]

> On Sept. 21, 2015, 5:16 p.m., Jie Yu wrote:
> > src/slave/containerizer/linux_launcher.cpp, lines 184-186
> > 
> >
> > Could you please explain where the bug is in the description. I don't 
> > understand why this code is buggy.
> 
> haosdent huang wrote:
> Because we have multi slaves, this share memory would be override in 
> different threads. So that all threads would got the same "pipes" and same 
> execute "function" pointer.

And I some trace code here before, when we run LinuxLauncher parallels. In 
clone method, the pointers to func are different. And then go into childMain 
function, the pointers func become the last one. Only after change to malloc a 
new stack in clone, the local variables in childMain would become different.


- haosdent


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


On Sept. 20, 2015, 5:04 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38535/
> ---
> 
> (Updated Sept. 20, 2015, 5:04 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Joris Van Remoortere, Kapil Arya, and 
> Michael Park.
> 
> 
> Bugs: MESOS-3474
> https://issues.apache.org/jira/browse/MESOS-3474
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix ExamplesTest errors when user is root.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/linux_launcher.cpp 
> dde552ff2a9d29c31f7676180fffa19d1fb0ba63 
> 
> Diff: https://reviews.apache.org/r/38535/diff/
> 
> 
> Testing
> ---
> 
> sudo ./bin/mesos-tests.sh --gtest_filter="ExamplesTest.*" --verbose
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 38529: CMake: Only compile proc_tests.cpp for Linux platforms.

[Joseph Wu]

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

Ship it!


Ship It!

This is required because `stout/proc.hpp` is Linux-only, right?

- Joseph Wu


On Sept. 19, 2015, 7:07 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38529/
> ---
> 
> (Updated Sept. 19, 2015, 7:07 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Bugs: MESOS-3345
> https://issues.apache.org/jira/browse/MESOS-3345
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> CMake: Only compile proc_tests.cpp for Linux platforms.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt 
> baa648aacfd5204c33e102b58126862729fbb612 
> 
> Diff: https://reviews.apache.org/r/38529/diff/
> 
> 
> Testing
> ---
> 
> Compiled and ran made sure libprocess and stout tests ran and passed on the 
> following platforms:
> 
> * OS X 10.10
> * Windows 10
> * Ubuntu 14.04.2
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 38497: Introduced Executor HTTP endpoint on Agent

[Isabel Jimenez]

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

Ship it!


Ship It!

- Isabel Jimenez


On Sept. 18, 2015, 5:18 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38497/
> ---
> 
> (Updated Sept. 18, 2015, 5:18 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Isabel Jimenez, and Vinod Kone.
> 
> 
> Bugs: MESOS-2907
> https://issues.apache.org/jira/browse/MESOS-2907
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change introduces a stub endpoint on Agent. As of now, it doesn't do 
> much except validating the `Content-Type`, `Accept` headers among other 
> trivial validations. Most of the functionality already existed in 
> `src/master/http.cpp`
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp 101aa06e981eaa43bf6c68268b057f3d15e33e2e 
>   src/slave/slave.hpp 32e18305f3b630ab5f4e7aca2ef14cdb14c10721 
>   src/slave/slave.cpp ad710d7b930a2f115d503ceb8f8fd7421ad30287 
> 
> Diff: https://reviews.apache.org/r/38497/diff/
> 
> 
> Testing
> ---
> 
> make check + tests in r38499
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 38535: Fix ExamplesTest errors when user is root.

[Jie Yu]

> On Sept. 21, 2015, 5:16 p.m., Jie Yu wrote:
> > src/slave/containerizer/linux_launcher.cpp, lines 184-186
> > 
> >
> > Could you please explain where the bug is in the description. I don't 
> > understand why this code is buggy.
> 
> haosdent huang wrote:
> Because we have multi slaves, this share memory would be override in 
> different threads. So that all threads would got the same "pipes" and same 
> execute "function" pointer.
> 
> haosdent huang wrote:
> And I some trace code here before, when we run LinuxLauncher parallels. 
> In clone method, the pointers to func are different. And then go into 
> childMain function, the pointers func become the last one. Only after change 
> to malloc a new stack in clone, the local variables in childMain would become 
> different.

Thanks haosdent! Just chatted with Kapil about this as well. Looks like glibc 
writes to the stack before calling system call 'clone'. That explains why it's 
problematic in multiple slave scenarios. Thanks for the patch. Mind adding some 
comments about why we need to do that? Thanks!


- Jie


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


On Sept. 20, 2015, 5:04 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38535/
> ---
> 
> (Updated Sept. 20, 2015, 5:04 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Joris Van Remoortere, Kapil Arya, and 
> Michael Park.
> 
> 
> Bugs: MESOS-3474
> https://issues.apache.org/jira/browse/MESOS-3474
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix ExamplesTest errors when user is root.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/linux_launcher.cpp 
> dde552ff2a9d29c31f7676180fffa19d1fb0ba63 
> 
> Diff: https://reviews.apache.org/r/38535/diff/
> 
> 
> Testing
> ---
> 
> sudo ./bin/mesos-tests.sh --gtest_filter="ExamplesTest.*" --verbose
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 38530: CMake: Add preprocessor definitions required to build picojson 1.3.0.

[Joseph Wu]

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

Ship it!



cmake/MesosConfigure.cmake (lines 121 - 123)


Can you also copy in the note?

```
# NOTE: PicoJson requires __STDC_FORMAT_MACROS to be defined before 
importing
# 'inttypes.h'.  Since other libraries may also import this header, it must
# be globally defined so that PicoJson has access to the macros, regardless
# of the order of inclusion.
```


- Joseph Wu


On Sept. 19, 2015, 7:07 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38530/
> ---
> 
> (Updated Sept. 19, 2015, 7:07 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Bugs: MESOS-3345
> https://issues.apache.org/jira/browse/MESOS-3345
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> CMake: Add preprocessor definitions required to build picojson 1.3.0.
> 
> 
> Diffs
> -
> 
>   cmake/MesosConfigure.cmake b530da4c1e6f202b682ad7d6892da95d2181f8c8 
> 
> Diff: https://reviews.apache.org/r/38530/diff/
> 
> 
> Testing
> ---
> 
> Compiled and ran made sure libprocess and stout tests ran and passed on the 
> following platforms:
> 
> * OS X 10.10
> * Windows 10
> * Ubuntu 14.04.2
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 38527: Fix UserCgroupIsolatorTest failed on CentOS 6.6.

[Cong Wang]

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


What exactly is the failure? You don't provide in description or a ticket.


src/tests/containerizer/isolator_tests.cpp (line 1214)


You can just use initializer list here.


- Cong Wang


On Sept. 19, 2015, 7:23 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38527/
> ---
> 
> (Updated Sept. 19, 2015, 7:23 p.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix UserCgroupIsolatorTest failed on CentOS 6.6.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/isolator_tests.cpp 
> a25ae97a519feb8ead6177da160df8a276ca15bf 
> 
> Diff: https://reviews.apache.org/r/38527/diff/
> 
> 
> Testing
> ---
> 
> sudo ./bin/mesos-tests.sh --gtest_filter="UserCgroupIsolatorTest*" --verbose
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 38473: Add flag to disable hostname lookup.

[Marco Massenzio]

> On Sept. 18, 2015, 10:57 p.m., Cong Wang wrote:
> > Why not just set --host_name=$LIBPROCESS_IP for your case since you anyway 
> > need to provide a flag?
> 
> Guangya Liu wrote:
> I also have the same question with Cong, @Marco, can you please show more 
> detail for why not using the solution as above? What is the benefit of this 
> compared to using --host_name=$LIBPROCESS_IP? Thanks.
> 
> Marco Massenzio wrote:
> `$LIBPROCESS_IP` may not be set by the time we run the binary - it is 
> actually set in the `main()` method (of both Master/Agent) after parsing the 
> `--ip` (and `--ip_discovery_command`) flags.
> 
> The reason I use `LIBPROCESS_IP` here is to ensure that the IP used by 
> `libprocess` (which is initialized before creating the `Master` or `Slave` 
> object) is consistent with what the `Master/Agent` will be configured with.
> 
> Please also refer to the conversation about the introduction of 
> `--ip_discovery_command` as to why we need a "dynamic" way of configuring 
> hostname/IP - it all boils down to make Mesos play nice in various Cloud (and 
> Enterprise data centers) where the DNS resolution (and hostname configuration 
> of VMs) is vastly different and may not necessarily conform to the generally 
> assumed scenario, where IP/hostname are kept in sync / resolved by the OS 
> and/or external services (DHCP/DNS).
> 
> Qian Zhang wrote:
> If you want to set hostname to the chosen IP, then I think you can just 
> use the existing flag "--hostname" (set it to the chosen IP manually). And if 
> "--hostname" is set, then I think the hostname lookup will be bypassed. So 
> why do we need to introduce a new flag "--no_hostname_lookup"?

Please refer to the conversation on 
https://issues.apache.org/jira/browse/MESOS-3154 for why this may not be 
possible in the above-mentioned environments.
(and, yes, we do know about the `--hostname` flag - unfortunately, under 
certain scenarios, it's not possible to set it to a "static" value)


- Marco


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


On Sept. 19, 2015, 1:25 a.m., Marco Massenzio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38473/
> ---
> 
> (Updated Sept. 19, 2015, 1:25 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Cody Maloney, and Neil Conway.
> 
> 
> Bugs: MESOS-3457
> https://issues.apache.org/jira/browse/MESOS-3457
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Under certain circumstances, dynamic lookup of hostname, while
> successful, provides undesirable results; we would also like, in
> those circumstances, be able to just set the hostname to the chosen
> IP address (possibly set via the --ip_discovery_command method).
> 
> The flag we add here, --[no]-hostname_lookup is `true` by default
> (which is the existing behavior) and will work under most
> circumstances: however, by disabling lookup (using, for example,
> --no_hostname_lookup) the hostname used will be the same as the
> one configured (possibly, via --ip or MESOS_IP) in `LIBPROCESS_IP`.
> 
> This change affects both Master/Agent nodes.
> 
> WARNING - the `Address::hostname()` method always does a dynamic
> hostname lookup, which may in fact return inconsistent results
> wrt the Master's configuration (this is *not* affected by
> this change) and should be avoided; use instead
> `Master::info()::hostname()` which is always consistent with
> the Master's configuration.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md dd7f4aa806a3c1a8653a0fda9a326a3707308e7c 
>   src/master/flags.hpp e4b1df3f5a33049defff4688463274067f1f1ebf 
>   src/master/flags.cpp 80879611fbcfd764c9fc8f60a31613a9c8fc2364 
>   src/master/master.cpp ca4d5876dcd427964111428edc22d567ddaede0b 
>   src/slave/flags.hpp e31a4183170c3442ac4a15365c229391e7e91480 
>   src/slave/flags.cpp add4196dfd06c0f602ff5ebd39960dc05c4cd11f 
>   src/slave/slave.cpp ad710d7b930a2f115d503ceb8f8fd7421ad30287 
>   src/tests/cluster.hpp 114583de8c867495a2b7a953e6826708838e5d23 
>   src/tests/master_tests.cpp a477794df37c658b80d79dea8555b001415f7b6a 
>   src/tests/mesos.hpp 3db97aca921c9216d90384e1eb17030849516454 
> 
> Diff: https://reviews.apache.org/r/38473/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Marco Massenzio
> 
>

Re: Review Request 38531: CMake: Update CMake config to build Mesos against picojson v1.3.0.

[Joseph Wu]

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

Ship it!


Ship It!

- Joseph Wu


On Sept. 19, 2015, 7:07 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38531/
> ---
> 
> (Updated Sept. 19, 2015, 7:07 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Bugs: MESOS-3345
> https://issues.apache.org/jira/browse/MESOS-3345
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> CMake: Update CMake config to build Mesos against picojson v1.3.0.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/cmake/ProcessConfigure.cmake 
> a5f8d399e151acad87bb72ecb1f7372b2c467423 
> 
> Diff: https://reviews.apache.org/r/38531/diff/
> 
> 
> Testing
> ---
> 
> Compiled and ran made sure libprocess and stout tests ran and passed on the 
> following platforms:
> 
> * OS X 10.10
> * Windows 10
> * Ubuntu 14.04.2
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 38528: Disable perf test when perf version is not support.

[haosdent huang]

> On Sept. 21, 2015, 5:32 p.m., Paul Brett wrote:
> > The PerfTests ROOT_Events and ROOT_Sample both require a supported version 
> > of perf to be installed, however the Parse test does not.  Could we use the 
> > supported() test to disable all events matching both ROOT and Perf?

I am not ideas. Just see a lot of failures in CentOS 6.6. Should we create a 
new filter, which name is "PERF_TEST_"? Also, there are not only 
"PerfTest.ROOT_Events" depends on "supported". I forgot 7~8 test cases is 
failed because of perf events, most of our cgroup test seems need perf_event.


- haosdent


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


On Sept. 21, 2015, 5:47 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38528/
> ---
> 
> (Updated Sept. 21, 2015, 5:47 p.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere, Michael Park, and Paul Brett.
> 
> 
> Bugs: MESOS-3471
> https://issues.apache.org/jira/browse/MESOS-3471
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Disable perf test when perf version is not support.
> 
> 
> Diffs
> -
> 
>   src/tests/environment.cpp e40cde23daba25b0b61b567ee73682c67e7acbdc 
> 
> Diff: https://reviews.apache.org/r/38528/diff/
> 
> 
> Testing
> ---
> 
> # On CentOS 6.6
> sudo make -j8 check
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 38538: [VIA HAOSDENT] CMake: Add `CMAKE_NOOP` to common definitions file.

[Joseph Wu]

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

Ship it!


Ship It!

- Joseph Wu


On Sept. 20, 2015, 7:33 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38538/
> ---
> 
> (Updated Sept. 20, 2015, 7:33 p.m.)
> 
> 
> Review request for mesos, haosdent huang, Artem Harutyunyan, Joris Van 
> Remoortere, and Joseph Wu.
> 
> 
> Bugs: MESOS-3434
> https://issues.apache.org/jira/browse/MESOS-3434
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This was originally part of this review[1] by haosdent. It was split out
> and moved into the root folder by hausdorff, because we will later reuse
> it to build other third-party libraries on Windows that are not only in
> libprocess, and hence it belongs outside of the commits that touch
> libprocess.
> 
> But, the code is essentially haosdent's, so the credit should go to him.
> 
> [1] Original review: https://reviews.apache.org/r/37273
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 3b6f4af337466d33cb915959a5995e4307b27be3 
>   cmake/Common.cmake PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38538/diff/
> 
> 
> Testing
> ---
> 
> Compiled and ran made sure libprocess and stout tests ran and passed on the 
> following platforms:
> 
> * OS X 10.10
> * Windows 10
> * Ubuntu 14.04.2
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 38530: CMake: Add preprocessor definitions required to build picojson 1.3.0.

[Alex Clemmer]

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

(Updated Sept. 21, 2015, 6:06 p.m.)


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


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


Repository: mesos


Description
---

CMake: Add preprocessor definitions required to build picojson 1.3.0.


Diffs (updated)
-

  cmake/MesosConfigure.cmake b530da4c1e6f202b682ad7d6892da95d2181f8c8 

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


Testing
---

Compiled and ran made sure libprocess and stout tests ran and passed on the 
following platforms:

* OS X 10.10
* Windows 10
* Ubuntu 14.04.2


Thanks,

Alex Clemmer

Re: Review Request 38527: Fix UserCgroupIsolatorTest failed on CentOS 6.6.

[haosdent huang]

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

(Updated Sept. 21, 2015, 6:06 p.m.)


Review request for mesos, Joris Van Remoortere, Michael Park, and Cong Wang.


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


Repository: mesos


Description
---

Fix UserCgroupIsolatorTest failed on CentOS 6.6.


Diffs
-

  src/tests/containerizer/isolator_tests.cpp 
a25ae97a519feb8ead6177da160df8a276ca15bf 

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


Testing
---

sudo ./bin/mesos-tests.sh --gtest_filter="UserCgroupIsolatorTest*" --verbose


Thanks,

haosdent huang

Re: Review Request 38499: Tests for executor endpoint on agent

[Isabel Jimenez]

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

Ship it!


Ship It!

- Isabel Jimenez


On Sept. 18, 2015, 5:19 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38499/
> ---
> 
> (Updated Sept. 18, 2015, 5:19 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Isabel Jimenez, and Vinod Kone.
> 
> 
> Bugs: MESOS-2907
> https://issues.apache.org/jira/browse/MESOS-2907
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change introduces basic validation tests for the Executor HTTP endpoint 
> on Agent.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 834f69afe87a093c6d275916f7781d470535a728 
>   src/tests/executor_http_api_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38499/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 38539: [VIA HAOSDENT] [1/2]Add CMake macro VsBuildCommand in libprocess.

[Joseph Wu]

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

Ship it!


Definitely looks a lot cleaner with the NOOP macro :)


3rdparty/libprocess/cmake/macros/VsBuildCommand.bat (line 19)


Period at the end.



3rdparty/libprocess/cmake/macros/VsBuildCommand.bat (line 52)


s/not/does not/
s/current/the current/

And a period at the end.


- Joseph Wu


On Sept. 20, 2015, 7:40 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38539/
> ---
> 
> (Updated Sept. 20, 2015, 7:40 p.m.)
> 
> 
> Review request for mesos, haosdent huang, Artem Harutyunyan, Joris Van 
> Remoortere, and Joseph Wu.
> 
> 
> Bugs: MESOS-3434
> https://issues.apache.org/jira/browse/MESOS-3434
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add CMake macro VsBuildCommand in libprocess.
> 
> This commit is the product of two reviews; the first one[1], by
> haosdent, was lightly rebased by hausdorff to target a later HEAD, and
> then augmented to fix the broken Ubuntu 14.04 build that this commit
> caused. Other than those things, the review is functionally identical to
> the original, and all credit for the work should go to haosdent.
> 
> [1] Original review: https://reviews.apache.org/r/37273
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/CMakeLists.txt 
> b9c9fae7d448906e9c9f5ab0ee3fe138a0171a7d 
>   3rdparty/libprocess/cmake/macros/VsBuildCommand.bat PRE-CREATION 
>   3rdparty/libprocess/cmake/macros/VsBuildCommand.cmake PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38539/diff/
> 
> 
> Testing
> ---
> 
> Compiled and ran made sure libprocess and stout tests ran and passed on the 
> following platforms:
> 
> * OS X 10.10
> * Windows 10
> * Ubuntu 14.04.2
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 38540: [VIA HAOSDENT] [2/2]Generate make batch file to build project in windows.

[Joseph Wu]

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

Ship it!


Ship It!

- Joseph Wu


On Sept. 20, 2015, 7:41 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38540/
> ---
> 
> (Updated Sept. 20, 2015, 7:41 p.m.)
> 
> 
> Review request for mesos, haosdent huang, Artem Harutyunyan, Joris Van 
> Remoortere, and Joseph Wu.
> 
> 
> Bugs: MESOS-3434
> https://issues.apache.org/jira/browse/MESOS-3434
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Generate make batch file to build project in windows.
> 
> This commit is the product of two reviews; the first one[1], by
> haosdent, was lightly rebased by hausdorff to target a later HEAD, and
> then augmented to fix the broken Ubuntu 14.04 build that this commit
> caused. Other than those things, the review is functionally identical to
> the original, and all credit for the work should go to haosdent.
> 
> [1] Original review: https://reviews.apache.org/r/37275
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 3b6f4af337466d33cb915959a5995e4307b27be3 
>   cmake/MesosConfigure.cmake b530da4c1e6f202b682ad7d6892da95d2181f8c8 
> 
> Diff: https://reviews.apache.org/r/38540/diff/
> 
> 
> Testing
> ---
> 
> Compiled and ran made sure libprocess and stout tests ran and passed on the 
> following platforms:
> 
> * OS X 10.10
> * Windows 10
> * Ubuntu 14.04.2
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 38473: Add flag to disable hostname lookup.

[Cong Wang]

> On Sept. 18, 2015, 10:57 p.m., Cong Wang wrote:
> > Why not just set --host_name=$LIBPROCESS_IP for your case since you anyway 
> > need to provide a flag?
> 
> Guangya Liu wrote:
> I also have the same question with Cong, @Marco, can you please show more 
> detail for why not using the solution as above? What is the benefit of this 
> compared to using --host_name=$LIBPROCESS_IP? Thanks.
> 
> Marco Massenzio wrote:
> `$LIBPROCESS_IP` may not be set by the time we run the binary - it is 
> actually set in the `main()` method (of both Master/Agent) after parsing the 
> `--ip` (and `--ip_discovery_command`) flags.
> 
> The reason I use `LIBPROCESS_IP` here is to ensure that the IP used by 
> `libprocess` (which is initialized before creating the `Master` or `Slave` 
> object) is consistent with what the `Master/Agent` will be configured with.
> 
> Please also refer to the conversation about the introduction of 
> `--ip_discovery_command` as to why we need a "dynamic" way of configuring 
> hostname/IP - it all boils down to make Mesos play nice in various Cloud (and 
> Enterprise data centers) where the DNS resolution (and hostname configuration 
> of VMs) is vastly different and may not necessarily conform to the generally 
> assumed scenario, where IP/hostname are kept in sync / resolved by the OS 
> and/or external services (DHCP/DNS).
> 
> Qian Zhang wrote:
> If you want to set hostname to the chosen IP, then I think you can just 
> use the existing flag "--hostname" (set it to the chosen IP manually). And if 
> "--hostname" is set, then I think the hostname lookup will be bypassed. So 
> why do we need to introduce a new flag "--no_hostname_lookup"?
> 
> Marco Massenzio wrote:
> Please refer to the conversation on 
> https://issues.apache.org/jira/browse/MESOS-3154 for why this may not be 
> possible in the above-mentioned environments.
> (and, yes, we do know about the `--hostname` flag - unfortunately, under 
> certain scenarios, it's not possible to set it to a "static" value)

Makes sense for me, can you document it in configuration.md too? Thanks.


- Cong


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


On Sept. 19, 2015, 1:25 a.m., Marco Massenzio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38473/
> ---
> 
> (Updated Sept. 19, 2015, 1:25 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Cody Maloney, and Neil Conway.
> 
> 
> Bugs: MESOS-3457
> https://issues.apache.org/jira/browse/MESOS-3457
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Under certain circumstances, dynamic lookup of hostname, while
> successful, provides undesirable results; we would also like, in
> those circumstances, be able to just set the hostname to the chosen
> IP address (possibly set via the --ip_discovery_command method).
> 
> The flag we add here, --[no]-hostname_lookup is `true` by default
> (which is the existing behavior) and will work under most
> circumstances: however, by disabling lookup (using, for example,
> --no_hostname_lookup) the hostname used will be the same as the
> one configured (possibly, via --ip or MESOS_IP) in `LIBPROCESS_IP`.
> 
> This change affects both Master/Agent nodes.
> 
> WARNING - the `Address::hostname()` method always does a dynamic
> hostname lookup, which may in fact return inconsistent results
> wrt the Master's configuration (this is *not* affected by
> this change) and should be avoided; use instead
> `Master::info()::hostname()` which is always consistent with
> the Master's configuration.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md dd7f4aa806a3c1a8653a0fda9a326a3707308e7c 
>   src/master/flags.hpp e4b1df3f5a33049defff4688463274067f1f1ebf 
>   src/master/flags.cpp 80879611fbcfd764c9fc8f60a31613a9c8fc2364 
>   src/master/master.cpp ca4d5876dcd427964111428edc22d567ddaede0b 
>   src/slave/flags.hpp e31a4183170c3442ac4a15365c229391e7e91480 
>   src/slave/flags.cpp add4196dfd06c0f602ff5ebd39960dc05c4cd11f 
>   src/slave/slave.cpp ad710d7b930a2f115d503ceb8f8fd7421ad30287 
>   src/tests/cluster.hpp 114583de8c867495a2b7a953e6826708838e5d23 
>   src/tests/master_tests.cpp a477794df37c658b80d79dea8555b001415f7b6a 
>   src/tests/mesos.hpp 3db97aca921c9216d90384e1eb17030849516454 
> 
> Diff: https://reviews.apache.org/r/38473/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Marco Massenzio
> 
>

Re: Review Request 38527: Fix UserCgroupIsolatorTest failed on CentOS 6.6.

[haosdent huang]

> On Sept. 21, 2015, 5:59 p.m., Cong Wang wrote:
> > What exactly is the failure? You don't provide in description or a ticket.

Hi, @wangcong. I add ticket filed just now. In current other test cases, we use 
this way to get cgroup hierarchy. Because CentOS 6 is use "/cgroup" as root 
hierarchy while mesos use "/sys/fs/cgroup" as default hierarchy in flags.


- haosdent


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


On Sept. 21, 2015, 6:06 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38527/
> ---
> 
> (Updated Sept. 21, 2015, 6:06 p.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere, Michael Park, and Cong Wang.
> 
> 
> Bugs: MESOS-3470
> https://issues.apache.org/jira/browse/MESOS-3470
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix UserCgroupIsolatorTest failed on CentOS 6.6.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/isolator_tests.cpp 
> a25ae97a519feb8ead6177da160df8a276ca15bf 
> 
> Diff: https://reviews.apache.org/r/38527/diff/
> 
> 
> Testing
> ---
> 
> sudo ./bin/mesos-tests.sh --gtest_filter="UserCgroupIsolatorTest*" --verbose
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 38541: CMake: Add `Versions.cmake` as an analog to `versions.am`.

[Joseph Wu]

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

Ship it!



3rdparty/cmake/Versions.cmake (lines 4 - 8)


This matches the order from `versions.am`, but you could alphabetize.


- Joseph Wu


On Sept. 20, 2015, 8:23 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38541/
> ---
> 
> (Updated Sept. 20, 2015, 8:23 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> As we prepare to add more third-party dependencies, it will be helpful
> to have all the version information of our third-party dependencies in
> one place, rather than scattered everywhere in the project.
> 
> This commit will introduce `Versions.cmake`, which will hold all this
> information.
> 
> 
> Diffs
> -
> 
>   3rdparty/cmake/Versions.cmake PRE-CREATION 
>   CMakeLists.txt 3b6f4af337466d33cb915959a5995e4307b27be3 
> 
> Diff: https://reviews.apache.org/r/38541/diff/
> 
> 
> Testing
> ---
> 
> Compiled and ran made sure libprocess and stout tests ran and passed on the 
> following platforms:
> 
> * OS X 10.10
> * Windows 10
> * Ubuntu 14.04.2
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 38527: Fix UserCgroupIsolatorTest failed on CentOS 6.6.

[haosdent huang]

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

(Updated Sept. 21, 2015, 6:12 p.m.)


Review request for mesos, Joris Van Remoortere, Michael Park, and Cong Wang.


Changes
---

Update according @wangcong reviews.


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


Repository: mesos


Description
---

Fix UserCgroupIsolatorTest failed on CentOS 6.6.


Diffs (updated)
-

  src/tests/containerizer/isolator_tests.cpp 
a25ae97a519feb8ead6177da160df8a276ca15bf 

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


Testing
---

sudo ./bin/mesos-tests.sh --gtest_filter="UserCgroupIsolatorTest*" --verbose


Thanks,

haosdent huang

Re: Review Request 38539: [VIA HAOSDENT] [1/2]Add CMake macro VsBuildCommand in libprocess.

[Alex Clemmer]

> On Sept. 21, 2015, 6:08 p.m., Joseph Wu wrote:
> > 3rdparty/libprocess/cmake/macros/VsBuildCommand.bat, line 52
> > 
> >
> > s/not/does not/
> > s/current/the current/
> > 
> > And a period at the end.

I'd like to not change "current" to "the current" because that makes the 
comment go over 80 columns, and it doesn't add clarity.

The other changes have been applied though.


- Alex


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


On Sept. 21, 2015, 2:40 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38539/
> ---
> 
> (Updated Sept. 21, 2015, 2:40 a.m.)
> 
> 
> Review request for mesos, haosdent huang, Artem Harutyunyan, Joris Van 
> Remoortere, and Joseph Wu.
> 
> 
> Bugs: MESOS-3434
> https://issues.apache.org/jira/browse/MESOS-3434
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add CMake macro VsBuildCommand in libprocess.
> 
> This commit is the product of two reviews; the first one[1], by
> haosdent, was lightly rebased by hausdorff to target a later HEAD, and
> then augmented to fix the broken Ubuntu 14.04 build that this commit
> caused. Other than those things, the review is functionally identical to
> the original, and all credit for the work should go to haosdent.
> 
> [1] Original review: https://reviews.apache.org/r/37273
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/CMakeLists.txt 
> b9c9fae7d448906e9c9f5ab0ee3fe138a0171a7d 
>   3rdparty/libprocess/cmake/macros/VsBuildCommand.bat PRE-CREATION 
>   3rdparty/libprocess/cmake/macros/VsBuildCommand.cmake PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38539/diff/
> 
> 
> Testing
> ---
> 
> Compiled and ran made sure libprocess and stout tests ran and passed on the 
> following platforms:
> 
> * OS X 10.10
> * Windows 10
> * Ubuntu 14.04.2
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 38539: [VIA HAOSDENT] [1/2]Add CMake macro VsBuildCommand in libprocess.

[Alex Clemmer]

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

(Updated Sept. 21, 2015, 6:14 p.m.)


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


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


Repository: mesos


Description
---

Add CMake macro VsBuildCommand in libprocess.

This commit is the product of two reviews; the first one[1], by
haosdent, was lightly rebased by hausdorff to target a later HEAD, and
then augmented to fix the broken Ubuntu 14.04 build that this commit
caused. Other than those things, the review is functionally identical to
the original, and all credit for the work should go to haosdent.

[1] Original review: https://reviews.apache.org/r/37273


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/CMakeLists.txt 
b9c9fae7d448906e9c9f5ab0ee3fe138a0171a7d 
  3rdparty/libprocess/cmake/macros/VsBuildCommand.bat PRE-CREATION 
  3rdparty/libprocess/cmake/macros/VsBuildCommand.cmake PRE-CREATION 

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


Testing
---

Compiled and ran made sure libprocess and stout tests ran and passed on the 
following platforms:

* OS X 10.10
* Windows 10
* Ubuntu 14.04.2


Thanks,

Alex Clemmer

Re: Review Request 38519: Change function quiesce() to suppress()

[Vinod Kone]

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



src/master/http.cpp (line 486)


doesn't look you changed this to 'suppress' from 'suppressRes'?


- Vinod Kone


On Sept. 21, 2015, 6:06 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38519/
> ---
> 
> (Updated Sept. 21, 2015, 6:06 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-3037
> https://issues.apache.org/jira/browse/MESOS-3037
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Change function quiesce() to suppress()
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 607d8e1b6efe0115b09be0a0c73edbd8e64d8415 
>   src/master/master.hpp 0c1e81cb807ab04e3d836272c0a197362d811d4c 
>   src/master/master.cpp 6c0db210747c7d88179556abaade65743bd8cb3a 
> 
> Diff: https://reviews.apache.org/r/38519/diff/
> 
> 
> Testing
> ---
> 
> Platform: Ubuntu:14.04
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 38542: CMake: Use version info from `Versions.cmake` instead of magic strings.

[Joseph Wu]

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



3rdparty/libprocess/cmake/ProcessTestsConfigure.cmake (line 40)


Did you miss this one?  Or was it left out intentionally?


- Joseph Wu


On Sept. 20, 2015, 8:23 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38542/
> ---
> 
> (Updated Sept. 20, 2015, 8:23 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently we configure the version information of third-party
> dependencies in the CMake build system from magic strings.
> 
> This commit will transition away from the magic string solution and
> towards the variables we define in `Versions.cmake`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/cmake/ProcessConfigure.cmake 
> a5f8d399e151acad87bb72ecb1f7372b2c467423 
>   3rdparty/libprocess/cmake/ProcessTestsConfigure.cmake 
> 9e4dcb83a8cc4e95a2a38573944f6b38e2eac76e 
> 
> Diff: https://reviews.apache.org/r/38542/diff/
> 
> 
> Testing
> ---
> 
> Compiled and ran made sure libprocess and stout tests ran and passed on the 
> following platforms:
> 
> * OS X 10.10
> * Windows 10
> * Ubuntu 14.04.2
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 38549: [1/2]CMake: Add libevent version, configure Windows to use as default.

[Joseph Wu]

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

Ship it!


Ship It!

- Joseph Wu


On Sept. 21, 2015, 3:43 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38549/
> ---
> 
> (Updated Sept. 21, 2015, 3:43 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Bugs: MESOS-3380
> https://issues.apache.org/jira/browse/MESOS-3380
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Mesos will need to use libevent to build the process library on Windows.
> 
> This commit will add a default version of libevent, which we will
> eventually retrieve from the 3rdparty GitHub repository, which for now
> is our "official" distribution channel for out-of-band third-party
> dependencies (especially on Windows, which has no package manager).
> 
> 
> Diffs
> -
> 
>   3rdparty/cmake/Versions.cmake PRE-CREATION 
>   CMakeLists.txt 3b6f4af337466d33cb915959a5995e4307b27be3 
> 
> Diff: https://reviews.apache.org/r/38549/diff/
> 
> 
> Testing
> ---
> 
> Compiled and ran made sure libprocess and stout tests ran and passed on the 
> following platforms:
> 
> * OS X 10.10
> * Windows 10
> * Ubuntu 14.04.2
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 38550: [2/2]CMake: Integrate libevent into Windows builds.

[Joseph Wu]

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

Ship it!


Ship It!

- Joseph Wu


On Sept. 21, 2015, 3:43 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38550/
> ---
> 
> (Updated Sept. 21, 2015, 3:43 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Bugs: MESOS-3380
> https://issues.apache.org/jira/browse/MESOS-3380
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Since there is no robust standalone libev support on Windows, Mesos will
> need to use libevent to support Windows, at least in the short term.
> 
> This commit will add logic to configure, build, and install libevent for
> all Windows builds.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/CMakeLists.txt 
> b9c9fae7d448906e9c9f5ab0ee3fe138a0171a7d 
>   3rdparty/libprocess/cmake/ProcessConfigure.cmake 
> a5f8d399e151acad87bb72ecb1f7372b2c467423 
> 
> Diff: https://reviews.apache.org/r/38550/diff/
> 
> 
> Testing
> ---
> 
> Compiled and ran made sure libprocess and stout tests ran and passed on the 
> following platforms:
> 
> * OS X 10.10
> * Windows 10
> * Ubuntu 14.04.2
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 38551: [1/2]CMake: Add version info for APR we need to build Windows.

[Joseph Wu]

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

Ship it!


Ship It!

- Joseph Wu


On Sept. 21, 2015, 4:21 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38551/
> ---
> 
> (Updated Sept. 21, 2015, 4:21 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Bugs: MESOS-3383
> https://issues.apache.org/jira/browse/MESOS-3383
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> [1/2]CMake: Add version info for APR we need to build Windows.
> 
> 
> Diffs
> -
> 
>   3rdparty/cmake/Versions.cmake PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38551/diff/
> 
> 
> Testing
> ---
> 
> Compiled and ran made sure libprocess and stout tests ran and passed on the 
> following platforms:
> 
> * OS X 10.10
> * Windows 10
> * Ubuntu 14.04.2
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 38552: [2/2]CMake: Add Windows-specific build targets for APR.

[Joseph Wu]

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

Ship it!


Ship It!

- Joseph Wu


On Sept. 21, 2015, 4:21 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38552/
> ---
> 
> (Updated Sept. 21, 2015, 4:21 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Bugs: MESOS-3383
> https://issues.apache.org/jira/browse/MESOS-3383
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> APR is required for Mesos to run. Normally we expect that a user has
> obtained APR with their favorite package manager, but on Windows, we
> cannot really expect that this is the case. So, we have to rope in the
> dependency manually in the CMake build system.
> 
> This commit will introduce the necessary build targets to obtain,
> configure, build, and install APR for Windows builds. It will ignore APR
> and assume it's there for Linux builds.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/CMakeLists.txt 
> b9c9fae7d448906e9c9f5ab0ee3fe138a0171a7d 
>   3rdparty/libprocess/cmake/ProcessConfigure.cmake 
> a5f8d399e151acad87bb72ecb1f7372b2c467423 
> 
> Diff: https://reviews.apache.org/r/38552/diff/
> 
> 
> Testing
> ---
> 
> Compiled and ran made sure libprocess and stout tests ran and passed on the 
> following platforms:
> 
> * OS X 10.10
> * Windows 10
> * Ubuntu 14.04.2
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 38542: CMake: Use version info from `Versions.cmake` instead of magic strings.

[Alex Clemmer]

> On Sept. 21, 2015, 6:15 p.m., Joseph Wu wrote:
> > 3rdparty/libprocess/cmake/ProcessTestsConfigure.cmake, line 40
> > 
> >
> > Did you miss this one?  Or was it left out intentionally?

We don't seem to be using it yet, so I left it out.


- Alex


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


On Sept. 21, 2015, 3:23 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38542/
> ---
> 
> (Updated Sept. 21, 2015, 3:23 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently we configure the version information of third-party
> dependencies in the CMake build system from magic strings.
> 
> This commit will transition away from the magic string solution and
> towards the variables we define in `Versions.cmake`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/cmake/ProcessConfigure.cmake 
> a5f8d399e151acad87bb72ecb1f7372b2c467423 
>   3rdparty/libprocess/cmake/ProcessTestsConfigure.cmake 
> 9e4dcb83a8cc4e95a2a38573944f6b38e2eac76e 
> 
> Diff: https://reviews.apache.org/r/38542/diff/
> 
> 
> Testing
> ---
> 
> Compiled and ran made sure libprocess and stout tests ran and passed on the 
> following platforms:
> 
> * OS X 10.10
> * Windows 10
> * Ubuntu 14.04.2
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 38541: CMake: Add `Versions.cmake` as an analog to `versions.am`.

[Alex Clemmer]

> On Sept. 21, 2015, 6:13 p.m., Joseph Wu wrote:
> > 3rdparty/cmake/Versions.cmake, lines 4-8
> > 
> >
> > This matches the order from `versions.am`, but you could alphabetize.

Now that you point it out, we _should_ alphabetize! It doesn't really matter 
what our style is in `versions.am`, because this isn't autoconf! :)


- Alex


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


On Sept. 21, 2015, 3:23 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38541/
> ---
> 
> (Updated Sept. 21, 2015, 3:23 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> As we prepare to add more third-party dependencies, it will be helpful
> to have all the version information of our third-party dependencies in
> one place, rather than scattered everywhere in the project.
> 
> This commit will introduce `Versions.cmake`, which will hold all this
> information.
> 
> 
> Diffs
> -
> 
>   3rdparty/cmake/Versions.cmake PRE-CREATION 
>   CMakeLists.txt 3b6f4af337466d33cb915959a5995e4307b27be3 
> 
> Diff: https://reviews.apache.org/r/38541/diff/
> 
> 
> Testing
> ---
> 
> Compiled and ran made sure libprocess and stout tests ran and passed on the 
> following platforms:
> 
> * OS X 10.10
> * Windows 10
> * Ubuntu 14.04.2
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 38541: CMake: Add `Versions.cmake` as an analog to `versions.am`.

[Alex Clemmer]

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

(Updated Sept. 21, 2015, 6:25 p.m.)


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


Repository: mesos


Description
---

As we prepare to add more third-party dependencies, it will be helpful
to have all the version information of our third-party dependencies in
one place, rather than scattered everywhere in the project.

This commit will introduce `Versions.cmake`, which will hold all this
information.


Diffs (updated)
-

  3rdparty/cmake/Versions.cmake PRE-CREATION 
  CMakeLists.txt 3b6f4af337466d33cb915959a5995e4307b27be3 

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


Testing
---

Compiled and ran made sure libprocess and stout tests ran and passed on the 
following platforms:

* OS X 10.10
* Windows 10
* Ubuntu 14.04.2


Thanks,

Alex Clemmer

Review Request 38564: Add a new callback enabling custom attribute discovery logic

[Felix Abecassis]

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

Review request for mesos.


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


Repository: mesos


Description
---

Add a new callback enabling custom attribute discovery logic


Diffs
-

  include/mesos/hook.hpp 2353602c8ac8b89e3fa86bda7d1e262a3debef80 
  src/examples/test_hook_module.cpp 0dc74d60576af6f88cbdc1c9a6f82348c5761d2f 
  src/hook/manager.hpp a517c05855b07831a9728c1191243d114781b70a 
  src/hook/manager.cpp 691976e94d5ac9fe4f8ba583214f24900d14248c 
  src/slave/slave.cpp ad710d7b930a2f115d503ceb8f8fd7421ad30287 
  src/tests/hook_tests.cpp b23a587c683c391ca860b3b7d876902987f4d158 

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


Testing
---


Thanks,

Felix Abecassis

Re: Review Request 38551: [1/2]CMake: Add version info for APR we need to build Windows.

[Alex Clemmer]

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

(Updated Sept. 21, 2015, 6:30 p.m.)


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


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


Repository: mesos


Description
---

[1/2]CMake: Add version info for APR we need to build Windows.


Diffs (updated)
-

  3rdparty/cmake/Versions.cmake PRE-CREATION 

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


Testing
---

Compiled and ran made sure libprocess and stout tests ran and passed on the 
following platforms:

* OS X 10.10
* Windows 10
* Ubuntu 14.04.2


Thanks,

Alex Clemmer

Re: Review Request 38549: [1/2]CMake: Add libevent version, configure Windows to use as default.

[Alex Clemmer]

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

(Updated Sept. 21, 2015, 6:31 p.m.)


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


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


Repository: mesos


Description
---

Mesos will need to use libevent to build the process library on Windows.

This commit will add a default version of libevent, which we will
eventually retrieve from the 3rdparty GitHub repository, which for now
is our "official" distribution channel for out-of-band third-party
dependencies (especially on Windows, which has no package manager).


Diffs (updated)
-

  3rdparty/cmake/Versions.cmake PRE-CREATION 
  CMakeLists.txt 3b6f4af337466d33cb915959a5995e4307b27be3 

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


Testing
---

Compiled and ran made sure libprocess and stout tests ran and passed on the 
following platforms:

* OS X 10.10
* Windows 10
* Ubuntu 14.04.2


Thanks,

Alex Clemmer

Re: Review Request 38529: CMake: Only compile proc_tests.cpp for Linux platforms.

[Alex Clemmer]

> On Sept. 21, 2015, 5:55 p.m., Joseph Wu wrote:
> > Ship It!
> > 
> > This is required because `stout/proc.hpp` is Linux-only, right?

Correct!


- Alex


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


On Sept. 20, 2015, 2:07 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38529/
> ---
> 
> (Updated Sept. 20, 2015, 2:07 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Bugs: MESOS-3345
> https://issues.apache.org/jira/browse/MESOS-3345
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> CMake: Only compile proc_tests.cpp for Linux platforms.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt 
> baa648aacfd5204c33e102b58126862729fbb612 
> 
> Diff: https://reviews.apache.org/r/38529/diff/
> 
> 
> Testing
> ---
> 
> Compiled and ran made sure libprocess and stout tests ran and passed on the 
> following platforms:
> 
> * OS X 10.10
> * Windows 10
> * Ubuntu 14.04.2
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 38473: Add flag to disable hostname lookup.

[Marco Massenzio]

> On Sept. 18, 2015, 10:57 p.m., Cong Wang wrote:
> > Why not just set --host_name=$LIBPROCESS_IP for your case since you anyway 
> > need to provide a flag?
> 
> Guangya Liu wrote:
> I also have the same question with Cong, @Marco, can you please show more 
> detail for why not using the solution as above? What is the benefit of this 
> compared to using --host_name=$LIBPROCESS_IP? Thanks.
> 
> Marco Massenzio wrote:
> `$LIBPROCESS_IP` may not be set by the time we run the binary - it is 
> actually set in the `main()` method (of both Master/Agent) after parsing the 
> `--ip` (and `--ip_discovery_command`) flags.
> 
> The reason I use `LIBPROCESS_IP` here is to ensure that the IP used by 
> `libprocess` (which is initialized before creating the `Master` or `Slave` 
> object) is consistent with what the `Master/Agent` will be configured with.
> 
> Please also refer to the conversation about the introduction of 
> `--ip_discovery_command` as to why we need a "dynamic" way of configuring 
> hostname/IP - it all boils down to make Mesos play nice in various Cloud (and 
> Enterprise data centers) where the DNS resolution (and hostname configuration 
> of VMs) is vastly different and may not necessarily conform to the generally 
> assumed scenario, where IP/hostname are kept in sync / resolved by the OS 
> and/or external services (DHCP/DNS).
> 
> Qian Zhang wrote:
> If you want to set hostname to the chosen IP, then I think you can just 
> use the existing flag "--hostname" (set it to the chosen IP manually). And if 
> "--hostname" is set, then I think the hostname lookup will be bypassed. So 
> why do we need to introduce a new flag "--no_hostname_lookup"?
> 
> Marco Massenzio wrote:
> Please refer to the conversation on 
> https://issues.apache.org/jira/browse/MESOS-3154 for why this may not be 
> possible in the above-mentioned environments.
> (and, yes, we do know about the `--hostname` flag - unfortunately, under 
> certain scenarios, it's not possible to set it to a "static" value)
> 
> Cong Wang wrote:
> Makes sense for me, can you document it in configuration.md too? Thanks.

>From what I've seen, configuration.md is essentially a verbatim list of what 
>is emitted by --help, not sure what more to add there; the 
>`--no-hostname_lookup` flag will be a no-op for anyone who doesn't know how to 
>use - and completely obvious to those (such our SRE team) who need it :)

But I will definitely add this to the Jira (which will be referenced to by this 
commit) so that folks can understand where did this come from, no problem!


- Marco


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


On Sept. 19, 2015, 1:25 a.m., Marco Massenzio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38473/
> ---
> 
> (Updated Sept. 19, 2015, 1:25 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Cody Maloney, and Neil Conway.
> 
> 
> Bugs: MESOS-3457
> https://issues.apache.org/jira/browse/MESOS-3457
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Under certain circumstances, dynamic lookup of hostname, while
> successful, provides undesirable results; we would also like, in
> those circumstances, be able to just set the hostname to the chosen
> IP address (possibly set via the --ip_discovery_command method).
> 
> The flag we add here, --[no]-hostname_lookup is `true` by default
> (which is the existing behavior) and will work under most
> circumstances: however, by disabling lookup (using, for example,
> --no_hostname_lookup) the hostname used will be the same as the
> one configured (possibly, via --ip or MESOS_IP) in `LIBPROCESS_IP`.
> 
> This change affects both Master/Agent nodes.
> 
> WARNING - the `Address::hostname()` method always does a dynamic
> hostname lookup, which may in fact return inconsistent results
> wrt the Master's configuration (this is *not* affected by
> this change) and should be avoided; use instead
> `Master::info()::hostname()` which is always consistent with
> the Master's configuration.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md dd7f4aa806a3c1a8653a0fda9a326a3707308e7c 
>   src/master/flags.hpp e4b1df3f5a33049defff4688463274067f1f1ebf 
>   src/master/flags.cpp 80879611fbcfd764c9fc8f60a31613a9c8fc2364 
>   src/master/master.cpp ca4d5876dcd427964111428edc22d567ddaede0b 
>   src/slave/flags.hpp e31a4183170c3442ac4a15365c229391e7e91480 
>   src/slave/flags.cpp add4196dfd06c0f602ff5ebd39960dc05c4cd11f 
>   src/slave/slave.cpp ad710d7b930a2f115d503ceb8f8fd7421ad30287 
>   src/tests/cluster.hpp 114583de8c867495a2b7a953e6826708838e5d23 
>   src/tests/master_tests.cpp a477794df37c658b80d7

Re: Review Request 38527: Fix UserCgroupIsolatorTest failed on CentOS 6.6.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [38527]

All tests passed.

- Mesos ReviewBot


On Sept. 21, 2015, 6:12 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38527/
> ---
> 
> (Updated Sept. 21, 2015, 6:12 p.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere, Michael Park, and Cong Wang.
> 
> 
> Bugs: MESOS-3470
> https://issues.apache.org/jira/browse/MESOS-3470
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix UserCgroupIsolatorTest failed on CentOS 6.6.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/isolator_tests.cpp 
> a25ae97a519feb8ead6177da160df8a276ca15bf 
> 
> Diff: https://reviews.apache.org/r/38527/diff/
> 
> 
> Testing
> ---
> 
> sudo ./bin/mesos-tests.sh --gtest_filter="UserCgroupIsolatorTest*" --verbose
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 38564: Add a new callback enabling custom attribute discovery logic

[Mesos ReviewBot]

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


Bad patch!

Reviews applied: [38564]

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

Error:
 2015-09-21 19:25:39 URL:https://reviews.apache.org/r/38564/diff/raw/ 
[4673/4673] -> "38564.patch" [1]
error: patch failed: src/examples/test_hook_module.cpp:229
error: src/examples/test_hook_module.cpp: patch does not apply
error: patch failed: src/hook/manager.hpp:75
error: src/hook/manager.hpp: patch does not apply
error: patch failed: src/slave/slave.cpp:385
error: src/slave/slave.cpp: patch does not apply
error: patch failed: src/tests/hook_tests.cpp:698
error: src/tests/hook_tests.cpp: patch does not apply
Failed to apply patch

- Mesos ReviewBot


On Sept. 21, 2015, 6:30 p.m., Felix Abecassis wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38564/
> ---
> 
> (Updated Sept. 21, 2015, 6:30 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-3366
> https://issues.apache.org/jira/browse/MESOS-3366
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add a new callback enabling custom attribute discovery logic
> 
> 
> Diffs
> -
> 
>   include/mesos/hook.hpp 2353602c8ac8b89e3fa86bda7d1e262a3debef80 
>   src/examples/test_hook_module.cpp 0dc74d60576af6f88cbdc1c9a6f82348c5761d2f 
>   src/hook/manager.hpp a517c05855b07831a9728c1191243d114781b70a 
>   src/hook/manager.cpp 691976e94d5ac9fe4f8ba583214f24900d14248c 
>   src/slave/slave.cpp ad710d7b930a2f115d503ceb8f8fd7421ad30287 
>   src/tests/hook_tests.cpp b23a587c683c391ca860b3b7d876902987f4d158 
> 
> Diff: https://reviews.apache.org/r/38564/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Felix Abecassis
> 
>

Re: Review Request 38519: Change function quiesce() to suppress()

[Vinod Kone]

> On Sept. 21, 2015, 6:15 p.m., Vinod Kone wrote:
> > src/master/http.cpp, line 486
> > 
> >
> > doesn't look you changed this to 'suppress' from 'suppressRes'?

i'll fix this for you and commit the chain.


- Vinod


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


On Sept. 21, 2015, 6:06 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38519/
> ---
> 
> (Updated Sept. 21, 2015, 6:06 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-3037
> https://issues.apache.org/jira/browse/MESOS-3037
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Change function quiesce() to suppress()
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 607d8e1b6efe0115b09be0a0c73edbd8e64d8415 
>   src/master/master.hpp 0c1e81cb807ab04e3d836272c0a197362d811d4c 
>   src/master/master.cpp 6c0db210747c7d88179556abaade65743bd8cb3a 
> 
> Diff: https://reviews.apache.org/r/38519/diff/
> 
> 
> Testing
> ---
> 
> Platform: Ubuntu:14.04
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 38473: Add flag to disable hostname lookup.

[Benjamin Hindman]

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



src/master/master.cpp (line 345)


Why are you reading LIBPROCESS_IP here? Why not `flags.ip`? There appears 
to be a very non-obvious global invariant and I'd like to capture the 
dependency here as a comment (for posterity!) as well as where we're setting 
LIBPROCESS_IP in the main.cpp files.



src/master/master.cpp (line 349)


In circumstances where a user can easily avoid the master from crashing we 
prefer NOT to use LOG(FATAL) because it prints a stack trace which can hide the 
actual error message. Instead, an EXIT(EXIT_FAILURE) is a good thing to use 
here.

Same for the LOG(FATAL) in the slave below.



src/tests/cluster.hpp (line 126)


Thank you for the doxygen 
comments!!!



src/tests/master_tests.cpp (line 1098)


This should be virtual because you're extending `MasterTest`.



src/tests/master_tests.cpp (line 1121)


Why not just do `cluster.find`? Not sure I understand the need for this 
alias.



src/tests/master_tests.cpp (lines 1122 - 1123)


What does someone running the test get from this extra output information?



src/tests/master_tests.cpp (line 1126)


Indention.


- Benjamin Hindman


On Sept. 19, 2015, 1:25 a.m., Marco Massenzio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38473/
> ---
> 
> (Updated Sept. 19, 2015, 1:25 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Cody Maloney, and Neil Conway.
> 
> 
> Bugs: MESOS-3457
> https://issues.apache.org/jira/browse/MESOS-3457
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Under certain circumstances, dynamic lookup of hostname, while
> successful, provides undesirable results; we would also like, in
> those circumstances, be able to just set the hostname to the chosen
> IP address (possibly set via the --ip_discovery_command method).
> 
> The flag we add here, --[no]-hostname_lookup is `true` by default
> (which is the existing behavior) and will work under most
> circumstances: however, by disabling lookup (using, for example,
> --no_hostname_lookup) the hostname used will be the same as the
> one configured (possibly, via --ip or MESOS_IP) in `LIBPROCESS_IP`.
> 
> This change affects both Master/Agent nodes.
> 
> WARNING - the `Address::hostname()` method always does a dynamic
> hostname lookup, which may in fact return inconsistent results
> wrt the Master's configuration (this is *not* affected by
> this change) and should be avoided; use instead
> `Master::info()::hostname()` which is always consistent with
> the Master's configuration.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md dd7f4aa806a3c1a8653a0fda9a326a3707308e7c 
>   src/master/flags.hpp e4b1df3f5a33049defff4688463274067f1f1ebf 
>   src/master/flags.cpp 80879611fbcfd764c9fc8f60a31613a9c8fc2364 
>   src/master/master.cpp ca4d5876dcd427964111428edc22d567ddaede0b 
>   src/slave/flags.hpp e31a4183170c3442ac4a15365c229391e7e91480 
>   src/slave/flags.cpp add4196dfd06c0f602ff5ebd39960dc05c4cd11f 
>   src/slave/slave.cpp ad710d7b930a2f115d503ceb8f8fd7421ad30287 
>   src/tests/cluster.hpp 114583de8c867495a2b7a953e6826708838e5d23 
>   src/tests/master_tests.cpp a477794df37c658b80d79dea8555b001415f7b6a 
>   src/tests/mesos.hpp 3db97aca921c9216d90384e1eb17030849516454 
> 
> Diff: https://reviews.apache.org/r/38473/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Marco Massenzio
> 
>

Re: Review Request 38473: Add flag to disable hostname lookup.

[Marco Massenzio]

> On Sept. 21, 2015, 7:39 p.m., Benjamin Hindman wrote:
> > src/master/master.cpp, line 345
> > 
> >
> > Why are you reading LIBPROCESS_IP here? Why not `flags.ip`? There 
> > appears to be a very non-obvious global invariant and I'd like to capture 
> > the dependency here as a comment (for posterity!) as well as where we're 
> > setting LIBPROCESS_IP in the main.cpp files.

I've added the following comment, please let me know if (a) I'm getting this 
wrong and (b) whether it requires a more detailed explanation (but the 
underlying rationale is - libprocess will use that (and not the flags) and we 
need to be consistent with it).
```
  // We need to use LIBPROCESS_IP here, instead of 'flags.ip' because the
  // latter may not have been set, and the IP may have been set by other
  // means (eg, auto-discovery; the --ip_discovery_command) and we need to
  // be consistent with what 'libprocess' is initialized with.
```


> On Sept. 21, 2015, 7:39 p.m., Benjamin Hindman wrote:
> > src/master/master.cpp, line 349
> > 
> >
> > In circumstances where a user can easily avoid the master from crashing 
> > we prefer NOT to use LOG(FATAL) because it prints a stack trace which can 
> > hide the actual error message. Instead, an EXIT(EXIT_FAILURE) is a good 
> > thing to use here.
> > 
> > Same for the LOG(FATAL) in the slave below.

ah! I was trying to be "consistent" with the (existing) code above.
Would you like me to change that one too?


> On Sept. 21, 2015, 7:39 p.m., Benjamin Hindman wrote:
> > src/tests/master_tests.cpp, line 1121
> > 
> >
> > Why not just do `cluster.find`? Not sure I understand the need for this 
> > alias.

You are right, no need - I started out adding this, then eventually gone down 
the rabbit hole of same-named classes and hadn't realized that I could actually 
get rid of the alias.

It's gone.


> On Sept. 21, 2015, 7:39 p.m., Benjamin Hindman wrote:
> > src/tests/master_tests.cpp, lines 1122-1123
> > 
> >
> > What does someone running the test get from this extra output 
> > information?

Mostly the hostname that was set up and then not found via the UPID lookup - it 
may help debug weird network configuration issues and/or situations where we 
have broken the hostname lookup/configuration.
(I think? I like to add extra info when tests go wrong, so as to take out (some 
of) the guesswork from the poor soul who'll have to fix my code many 
months/years in the future... I believe in karma :) )

Anyways, figured out that (a) this needs to be an `ASSERT_EQ` (as we 
`master.get()` in the following EXPECT) and (b) the comment here may actually 
be unnecessary, so I've removed it.


- Marco


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


On Sept. 19, 2015, 1:25 a.m., Marco Massenzio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38473/
> ---
> 
> (Updated Sept. 19, 2015, 1:25 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Cody Maloney, and Neil Conway.
> 
> 
> Bugs: MESOS-3457
> https://issues.apache.org/jira/browse/MESOS-3457
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Under certain circumstances, dynamic lookup of hostname, while
> successful, provides undesirable results; we would also like, in
> those circumstances, be able to just set the hostname to the chosen
> IP address (possibly set via the --ip_discovery_command method).
> 
> The flag we add here, --[no]-hostname_lookup is `true` by default
> (which is the existing behavior) and will work under most
> circumstances: however, by disabling lookup (using, for example,
> --no_hostname_lookup) the hostname used will be the same as the
> one configured (possibly, via --ip or MESOS_IP) in `LIBPROCESS_IP`.
> 
> This change affects both Master/Agent nodes.
> 
> WARNING - the `Address::hostname()` method always does a dynamic
> hostname lookup, which may in fact return inconsistent results
> wrt the Master's configuration (this is *not* affected by
> this change) and should be avoided; use instead
> `Master::info()::hostname()` which is always consistent with
> the Master's configuration.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md dd7f4aa806a3c1a8653a0fda9a326a3707308e7c 
>   src/master/flags.hpp e4b1df3f5a33049defff4688463274067f1f1ebf 
>   src/master/flags.cpp 80879611fbcfd764c9fc8f60a31613a9c8fc2364 
>  

Re: Review Request 38473: Add flag to disable hostname lookup.

[Marco Massenzio]

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

(Updated Sept. 21, 2015, 8:05 p.m.)


Review request for mesos, Benjamin Hindman, Cody Maloney, and Neil Conway.


Changes
---

benh comments


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


Repository: mesos


Description
---

Under certain circumstances, dynamic lookup of hostname, while
successful, provides undesirable results; we would also like, in
those circumstances, be able to just set the hostname to the chosen
IP address (possibly set via the --ip_discovery_command method).

The flag we add here, --[no]-hostname_lookup is `true` by default
(which is the existing behavior) and will work under most
circumstances: however, by disabling lookup (using, for example,
--no_hostname_lookup) the hostname used will be the same as the
one configured (possibly, via --ip or MESOS_IP) in `LIBPROCESS_IP`.

This change affects both Master/Agent nodes.

WARNING - the `Address::hostname()` method always does a dynamic
hostname lookup, which may in fact return inconsistent results
wrt the Master's configuration (this is *not* affected by
this change) and should be avoided; use instead
`Master::info()::hostname()` which is always consistent with
the Master's configuration.


Diffs (updated)
-

  docs/configuration.md dd7f4aa806a3c1a8653a0fda9a326a3707308e7c 
  src/master/flags.hpp e4b1df3f5a33049defff4688463274067f1f1ebf 
  src/master/flags.cpp 80879611fbcfd764c9fc8f60a31613a9c8fc2364 
  src/master/master.cpp 90ef8c663c90ffbdcb4aa2377bfba65ea5d3fda9 
  src/slave/flags.hpp e31a4183170c3442ac4a15365c229391e7e91480 
  src/slave/flags.cpp add4196dfd06c0f602ff5ebd39960dc05c4cd11f 
  src/slave/slave.cpp ad710d7b930a2f115d503ceb8f8fd7421ad30287 
  src/tests/cluster.hpp 114583de8c867495a2b7a953e6826708838e5d23 
  src/tests/master_tests.cpp f26344d39543f65f2b0a94b8ff566836c8256bf7 

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


Testing
---

make check


Thanks,

Marco Massenzio

Review Request 38568: Maintenance Primitives: Fix the formatting of the user doc.

[Joseph Wu]

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

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


Repository: mesos


Description
---

The website's markdown renderer is slightly different than the Gist renderer.
This fixes the markdown to show correctly on the website.


Diffs
-

  docs/maintenance.md 465dcfb72777611c2a4be2aa38c8cd7e869a2baa 

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


Testing
---

Rendered with: https://github.com/mesosphere/mesos-website-container


Thanks,

Joseph Wu

Re: Review Request 38279: Enabled resources/attributes discovery

[Connor Doyle]

> On Sept. 14, 2015, 5:34 p.m., Connor Doyle wrote:
> > src/hook/manager.cpp, line 261
> > 
> >
> > Please add a comment describing how the order of hook execution is 
> > determined.  e.g. order in which they appear in the modules json passed to 
> > the slave, etc.
> 
> Felix Abecassis wrote:
> Actually, it seems there is no predefined order right now since 
> src/hook/manager.cpp is using a hashmap, should I open another issue to 
> discuss how to fix this?

Yes, please do.  A HashMap might be overkill for that use case.  Maybe the hook 
manager can get by with a simpler order-preserving data structure.  As modules 
become more common, operators will probably want to control the invocation 
order.  For now the flag value is the only control surface exposed.


- Connor


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


On Sept. 19, 2015, 12:51 a.m., Felix Abecassis wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38279/
> ---
> 
> (Updated Sept. 19, 2015, 12:51 a.m.)
> 
> 
> Review request for mesos, Connor Doyle and Niklas Nielsen.
> 
> 
> Bugs: MESOS-3366
> https://issues.apache.org/jira/browse/MESOS-3366
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> First API draft for MESOS-3366.
> 
> 1) Only supports resources for now, we can add another hook for attributes 
> with a very similar code.
> 2) The callback currently receives the full SlaveInfo structure and construct 
> a new Resources object.
> 3) If there is multiple callbacks, each callback will see the changes made by 
> previous callbacks and are free to override or merge the detected resources 
> as they see fit.
> 
> 
> Diffs
> -
> 
>   include/mesos/hook.hpp 2353602 
>   src/examples/test_hook_module.cpp 0dc74d6 
>   src/hook/manager.hpp a517c05 
>   src/hook/manager.cpp 691976e 
>   src/slave/slave.cpp ad710d7 
>   src/tests/hook_tests.cpp b23a587 
> 
> Diff: https://reviews.apache.org/r/38279/diff/
> 
> 
> Testing
> ---
> 
> make clean && make && make check
> 
> 
> Thanks,
> 
> Felix Abecassis
> 
>

Review Request 38569: Made slave's work_dir a shared mount in LinuxFilesystemIsolator.

[Jie Yu]

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

Review request for mesos, Jojy Varghese, Timothy Chen, and Jiang Yan Xu.


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


Repository: mesos


Description
---

Made slave's work_dir a shared mount in LinuxFilesystemIsolator.

See ticket for motivation.


Diffs
-

  src/slave/containerizer/isolators/filesystem/linux.cpp 
f1e6f7519bdeeff7790fff63e7a9cb3075001758 
  src/tests/environment.cpp e40cde23daba25b0b61b567ee73682c67e7acbdc 
  src/tests/utils.cpp 498c9aa1442c4e937a74364073b2267dde6ffcb2 

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


Testing
---

sudo make check


Thanks,

Jie Yu

Re: Review Request 38535: Fix ExamplesTest errors when user is root.

[Jie Yu]

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



src/slave/containerizer/linux_launcher.cpp (line 200)


Should this be the following?
```
delete[] stack;
```


- Jie Yu


On Sept. 20, 2015, 5:04 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38535/
> ---
> 
> (Updated Sept. 20, 2015, 5:04 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Joris Van Remoortere, Kapil Arya, and 
> Michael Park.
> 
> 
> Bugs: MESOS-3474
> https://issues.apache.org/jira/browse/MESOS-3474
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix ExamplesTest errors when user is root.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/linux_launcher.cpp 
> dde552ff2a9d29c31f7676180fffa19d1fb0ba63 
> 
> Diff: https://reviews.apache.org/r/38535/diff/
> 
> 
> Testing
> ---
> 
> sudo ./bin/mesos-tests.sh --gtest_filter="ExamplesTest.*" --verbose
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 38499: Tests for executor endpoint on agent

[Vinod Kone]

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

Ship it!



src/tests/executor_http_api_tests.cpp (line 79)


s/then/than/



src/tests/executor_http_api_tests.cpp (line 295)


s/json/JSON/



src/tests/executor_http_api_tests.cpp (line 340)


s/json/JSON/



src/tests/executor_http_api_tests.cpp (line 361)


s/a non-supported/an un-supported/


- Vinod Kone


On Sept. 18, 2015, 5:19 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38499/
> ---
> 
> (Updated Sept. 18, 2015, 5:19 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Isabel Jimenez, and Vinod Kone.
> 
> 
> Bugs: MESOS-2907
> https://issues.apache.org/jira/browse/MESOS-2907
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change introduces basic validation tests for the Executor HTTP endpoint 
> on Agent.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 834f69afe87a093c6d275916f7781d470535a728 
>   src/tests/executor_http_api_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38499/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 38568: Maintenance Primitives: Fix the formatting of the user doc.

[Joseph Wu]

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

(Updated Sept. 21, 2015, 2:45 p.m.)


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


Changes
---

Add link to home page.


Repository: mesos


Description (updated)
---

Also adds a link to this document from the documentation home page.

The website's markdown renderer is slightly different than the Gist renderer.
This fixes the markdown to show correctly on the website.


Diffs (updated)
-

  docs/home.md 3dff97ba35fafe8fca7e18866f0c7e8d526022e1 
  docs/maintenance.md 465dcfb72777611c2a4be2aa38c8cd7e869a2baa 

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


Testing
---

Rendered with: https://github.com/mesosphere/mesos-website-container


Thanks,

Joseph Wu

Review Request 38570: Change documentation image links to absolute paths.

[Joseph Wu]

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

Review request for mesos, Adam B, Artem Harutyunyan, and Vinod Kone.


Repository: mesos


Description
---

Also removes extraneous `?raw=true` from the links.


Diffs
-

  docs/external-containerizer.md 96932189b74dce1ae28e0bd73d5543d1afaffb0b 
  docs/fetcher-cache-internals.md d17b41e7113ce72679734fc0bed16614011b6917 
  docs/maintenance.md 465dcfb72777611c2a4be2aa38c8cd7e869a2baa 
  docs/oversubscription.md b1a6c9979025ce33190792a07b8e9618c518430e 

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


Testing
---

Rendered with: https://github.com/mesosphere/mesos-website-container

Confirmed that images show up on the modified docs.


Thanks,

Joseph Wu

Re: Review Request 38499: Tests for executor endpoint on agent

[Anand Mazumdar]

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

(Updated Sept. 21, 2015, 9:53 p.m.)


Review request for mesos, Ben Mahler, Isabel Jimenez, and Vinod Kone.


Changes
---

Review comments from Vinod


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


Repository: mesos


Description
---

This change introduces basic validation tests for the Executor HTTP endpoint on 
Agent.


Diffs (updated)
-

  src/Makefile.am 834f69afe87a093c6d275916f7781d470535a728 
  src/tests/executor_http_api_tests.cpp PRE-CREATION 

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


Testing
---

make check


Thanks,

Anand Mazumdar

Re: Review Request 38499: Added tests for executor endpoint on agent

[Anand Mazumdar]

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

(Updated Sept. 21, 2015, 9:54 p.m.)


Review request for mesos, Ben Mahler, Isabel Jimenez, and Vinod Kone.


Changes
---

Changed summary


Summary (updated)
-

Added tests for executor endpoint on agent


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


Repository: mesos


Description
---

This change introduces basic validation tests for the Executor HTTP endpoint on 
Agent.


Diffs
-

  src/Makefile.am 834f69afe87a093c6d275916f7781d470535a728 
  src/tests/executor_http_api_tests.cpp PRE-CREATION 

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


Testing
---

make check


Thanks,

Anand Mazumdar

Re: Review Request 38519: Change function quiesce() to suppress()

[Guangya Liu]

> On 九月 21, 2015, 6:15 p.m., Vinod Kone wrote:
> > src/master/http.cpp, line 486
> > 
> >
> > doesn't look you changed this to 'suppress' from 'suppressRes'?
> 
> Vinod Kone wrote:
> i'll fix this for you and commit the chain.

Thanks Vinod for the update, it seems I made some mistake when rebase the code.


- Guangya


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


On 九月 21, 2015, 6:06 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38519/
> ---
> 
> (Updated 九月 21, 2015, 6:06 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-3037
> https://issues.apache.org/jira/browse/MESOS-3037
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Change function quiesce() to suppress()
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 607d8e1b6efe0115b09be0a0c73edbd8e64d8415 
>   src/master/master.hpp 0c1e81cb807ab04e3d836272c0a197362d811d4c 
>   src/master/master.cpp 6c0db210747c7d88179556abaade65743bd8cb3a 
> 
> Diff: https://reviews.apache.org/r/38519/diff/
> 
> 
> Testing
> ---
> 
> Platform: Ubuntu:14.04
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 38535: Fix ExamplesTest errors when user is root.

[Jie Yu]

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


OK, I'll address the comments for you since this is a blocker for the release.

- Jie Yu


On Sept. 20, 2015, 5:04 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38535/
> ---
> 
> (Updated Sept. 20, 2015, 5:04 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Joris Van Remoortere, Kapil Arya, and 
> Michael Park.
> 
> 
> Bugs: MESOS-3474
> https://issues.apache.org/jira/browse/MESOS-3474
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix ExamplesTest errors when user is root.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/linux_launcher.cpp 
> dde552ff2a9d29c31f7676180fffa19d1fb0ba63 
> 
> Diff: https://reviews.apache.org/r/38535/diff/
> 
> 
> Testing
> ---
> 
> sudo ./bin/mesos-tests.sh --gtest_filter="ExamplesTest.*" --verbose
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 38535: Fix ExamplesTest errors when user is root.

[Jie Yu]

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


commit 57361f10ccf1e026dbb691e67277cb0cb71c8ea6
Author: haosdent huang 
Date:   Mon Sep 21 14:27:34 2015 -0700

Allocated the stack used by clone dynamically.

This is to address the issue when running multiple slaves during tests.
The glibc 'clone' will modify the stack passed to it, therefore we
cannot use a shared stack.

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

- Jie Yu


On Sept. 20, 2015, 5:04 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38535/
> ---
> 
> (Updated Sept. 20, 2015, 5:04 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Joris Van Remoortere, Kapil Arya, and 
> Michael Park.
> 
> 
> Bugs: MESOS-3474
> https://issues.apache.org/jira/browse/MESOS-3474
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix ExamplesTest errors when user is root.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/linux_launcher.cpp 
> dde552ff2a9d29c31f7676180fffa19d1fb0ba63 
> 
> Diff: https://reviews.apache.org/r/38535/diff/
> 
> 
> Testing
> ---
> 
> sudo ./bin/mesos-tests.sh --gtest_filter="ExamplesTest.*" --verbose
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 38564: Add a new callback enabling custom attribute discovery logic

[Guangya Liu]

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


I think that you need make this patch depend on 
https://reviews.apache.org/r/38279/ to make this works.

- Guangya Liu


On 九月 21, 2015, 6:30 p.m., Felix Abecassis wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38564/
> ---
> 
> (Updated 九月 21, 2015, 6:30 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-3366
> https://issues.apache.org/jira/browse/MESOS-3366
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add a new callback enabling custom attribute discovery logic
> 
> 
> Diffs
> -
> 
>   include/mesos/hook.hpp 2353602c8ac8b89e3fa86bda7d1e262a3debef80 
>   src/examples/test_hook_module.cpp 0dc74d60576af6f88cbdc1c9a6f82348c5761d2f 
>   src/hook/manager.hpp a517c05855b07831a9728c1191243d114781b70a 
>   src/hook/manager.cpp 691976e94d5ac9fe4f8ba583214f24900d14248c 
>   src/slave/slave.cpp ad710d7b930a2f115d503ceb8f8fd7421ad30287 
>   src/tests/hook_tests.cpp b23a587c683c391ca860b3b7d876902987f4d158 
> 
> Diff: https://reviews.apache.org/r/38564/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Felix Abecassis
> 
>

Review Request 38573: Added changelog for 0.25.0 release

[Guangya Liu]

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

Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

This is a WIP CHANGLOG and only including one item of "add a
suppress call to the scheduler", it should be updated in future
to add more features in 0.25.0.


Diffs
-

  CHANGELOG bed2c1eb161dffd3dc20a2e145999db07c72fef8 

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


Testing
---


Thanks,

Guangya Liu

Review Request 38574: Fixed race in hook self-message loop and reenabled VerifySlaveLaunchExecutorHook test

[Niklas Nielsen]

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

Review request for mesos, Jie Yu and Kapil Arya.


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


Repository: mesos


Description
---

Fixed race in hook self-message loop and reenabled 
VerifySlaveLaunchExecutorHook test


Diffs
-

  src/examples/test_hook_module.cpp 0dc74d60576af6f88cbdc1c9a6f82348c5761d2f 
  src/tests/hook_tests.cpp b23a587c683c391ca860b3b7d876902987f4d158 

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


Testing
---

make check (gtest_repeat=100 and gtest_shuffle=1)


Thanks,

Niklas Nielsen

Review Request 38575: Added masterSlaveLostHook

[Niklas Nielsen]

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

Review request for mesos, Joris Van Remoortere and Kapil Arya.


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


Repository: mesos


Description
---

Added masterSlaveLostHook


Diffs
-

  include/mesos/hook.hpp 2353602c8ac8b89e3fa86bda7d1e262a3debef80 
  src/examples/test_hook_module.cpp 0dc74d60576af6f88cbdc1c9a6f82348c5761d2f 
  src/hook/manager.hpp a517c05855b07831a9728c1191243d114781b70a 
  src/hook/manager.cpp 691976e94d5ac9fe4f8ba583214f24900d14248c 
  src/master/master.cpp 90ef8c663c90ffbdcb4aa2377bfba65ea5d3fda9 
  src/tests/hook_tests.cpp b23a587c683c391ca860b3b7d876902987f4d158 

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


Testing
---

make check with new masterSlaveLostHook test


Thanks,

Niklas Nielsen

Re: Review Request 38569: Made slave's work_dir a shared mount in LinuxFilesystemIsolator.

[Jiang Yan Xu]

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



src/slave/containerizer/isolators/filesystem/linux.cpp (lines 73 - 74)


After then sentense `using the bind backend)` add

`because cleanup operations within the work_dir can be propagted to all 
container namespaces.`?



src/slave/containerizer/isolators/filesystem/linux.cpp (line 84)


Is `workDirMount` better?



src/slave/containerizer/isolators/filesystem/linux.cpp (lines 97 - 98)


So this seems to work but oh my godness the the way `mount` command 
interacts with the syscall is very implicit.

I can confirm this:

```
# /home/jyx/tmp/ is created by mount command and /home/jyx/tmp/target is 
mounted by syscall.
[root tmp]# mount --make-shared target
mount: can't find /home/jyx/tmp/target in /etc/fstab or /etc/mtab
[root tmp]# mount --make-rshared .
# OK.
```

So the rule seemed to be **as long as the arguments provided to the `mount` 
command themselves are mounted by the the command, we are OK**. (I guess 
because whatever is recurively done by the command is using syscall directly so 
it's fine)

So I suspect that the `mount --make-rslave /` command inside the container 
**should work** because there is always a root `/` mount.

I think we should seek consistency: use fs::mount() as long as it doesn't 
break `mount` commands. Or stick with `mount` command whenever possible?


- Jiang Yan Xu


On Sept. 21, 2015, 2:23 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38569/
> ---
> 
> (Updated Sept. 21, 2015, 2:23 p.m.)
> 
> 
> Review request for mesos, Jojy Varghese, Timothy Chen, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-3483
> https://issues.apache.org/jira/browse/MESOS-3483
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Made slave's work_dir a shared mount in LinuxFilesystemIsolator.
> 
> See ticket for motivation.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/isolators/filesystem/linux.cpp 
> f1e6f7519bdeeff7790fff63e7a9cb3075001758 
>   src/tests/environment.cpp e40cde23daba25b0b61b567ee73682c67e7acbdc 
>   src/tests/utils.cpp 498c9aa1442c4e937a74364073b2267dde6ffcb2 
> 
> Diff: https://reviews.apache.org/r/38569/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 38573: Added changelog for 0.25.0 release

[Vinod Kone]

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

Ship it!


Ship It!

- Vinod Kone


On Sept. 21, 2015, 10:27 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38573/
> ---
> 
> (Updated Sept. 21, 2015, 10:27 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-3037
> https://issues.apache.org/jira/browse/MESOS-3037
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is a WIP CHANGLOG and only including one item of "add a
> suppress call to the scheduler", it should be updated in future
> to add more features in 0.25.0.
> 
> 
> Diffs
> -
> 
>   CHANGELOG bed2c1eb161dffd3dc20a2e145999db07c72fef8 
> 
> Diff: https://reviews.apache.org/r/38573/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 38573: Added changelog for 0.25.0 release

[Vinod Kone]

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


Can you also update docs/upgrades.md with the upgrade order dependency? (master 
--> libmesos for scheduler --> scheduler)

- Vinod Kone


On Sept. 21, 2015, 10:27 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38573/
> ---
> 
> (Updated Sept. 21, 2015, 10:27 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-3037
> https://issues.apache.org/jira/browse/MESOS-3037
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is a WIP CHANGLOG and only including one item of "add a
> suppress call to the scheduler", it should be updated in future
> to add more features in 0.25.0.
> 
> 
> Diffs
> -
> 
>   CHANGELOG bed2c1eb161dffd3dc20a2e145999db07c72fef8 
> 
> Diff: https://reviews.apache.org/r/38573/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 38574: Fixed race in hook self-message loop and reenabled VerifySlaveLaunchExecutorHook test

[Niklas Nielsen]

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

(Updated Sept. 21, 2015, 3:38 p.m.)


Review request for mesos, Joris Van Remoortere and Kapil Arya.


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


Repository: mesos


Description (updated)
---

Coordinating events across the library border is hard as we want to avoid 
exporting additional symbols between the test and the module code. To migitate 
this, the VerifySlaveLaunchExecutorHook used a technique where it creates a 
libprocess actors in-place and sends a message to itself. This can be caught by 
a message filter in the shared libprocess instance and the test code can 
synchronize over this, to make sure certain module code was executed.

However, the in-place actor could (potentially) shutdown before the message was 
received (and thus, didn't execute the filter).

This patch installs a message handler in the in-place actor and only shuts down 
the actors when the message has been received.


Diffs
-

  src/examples/test_hook_module.cpp 0dc74d60576af6f88cbdc1c9a6f82348c5761d2f 
  src/tests/hook_tests.cpp b23a587c683c391ca860b3b7d876902987f4d158 

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


Testing
---

make check (gtest_repeat=100 and gtest_shuffle=1)


Thanks,

Niklas Nielsen

Re: Review Request 38575: Added masterSlaveLostHook

[Niklas Nielsen]

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

(Updated Sept. 21, 2015, 3:40 p.m.)


Review request for mesos, Joris Van Remoortere and Kapil Arya.


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


Repository: mesos


Description (updated)
---

This patch adds a new masterSlaveLost hook to enable modules to clean up after 
lost slaves events (as in networking modules, where we want to avoid leaking 
IPs).


Diffs
-

  include/mesos/hook.hpp 2353602c8ac8b89e3fa86bda7d1e262a3debef80 
  src/examples/test_hook_module.cpp 0dc74d60576af6f88cbdc1c9a6f82348c5761d2f 
  src/hook/manager.hpp a517c05855b07831a9728c1191243d114781b70a 
  src/hook/manager.cpp 691976e94d5ac9fe4f8ba583214f24900d14248c 
  src/master/master.cpp 90ef8c663c90ffbdcb4aa2377bfba65ea5d3fda9 
  src/tests/hook_tests.cpp b23a587c683c391ca860b3b7d876902987f4d158 

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


Testing
---

make check with new masterSlaveLostHook test


Thanks,

Niklas Nielsen

Re: Review Request 38569: Made slave's work_dir a shared mount in LinuxFilesystemIsolator.

[Jie Yu]

> On Sept. 21, 2015, 10:35 p.m., Jiang Yan Xu wrote:
> > src/slave/containerizer/isolators/filesystem/linux.cpp, lines 97-98
> > 
> >
> > So this seems to work but oh my godness the the way `mount` command 
> > interacts with the syscall is very implicit.
> > 
> > I can confirm this:
> > 
> > ```
> > # /home/jyx/tmp/ is created by mount command and /home/jyx/tmp/target 
> > is mounted by syscall.
> > [root tmp]# mount --make-shared target
> > mount: can't find /home/jyx/tmp/target in /etc/fstab or /etc/mtab
> > [root tmp]# mount --make-rshared .
> > # OK.
> > ```
> > 
> > So the rule seemed to be **as long as the arguments provided to the 
> > `mount` command themselves are mounted by the the command, we are OK**. (I 
> > guess because whatever is recurively done by the command is using syscall 
> > directly so it's fine)
> > 
> > So I suspect that the `mount --make-rslave /` command inside the 
> > container **should work** because there is always a root `/` mount.
> > 
> > I think we should seek consistency: use fs::mount() as long as it 
> > doesn't break `mount` commands. Or stick with `mount` command whenever 
> > possible?

OK, I think the comments above do not capture my intention. It's copied from 
the port mapping isolator. The motivation for using the command 'mount' to 
mount the work_dir is because: the mount will still be there after all 
containers and slave stopped. It's better to show this mount when operator 
types command 'mount' (so that it's not quite invisible). We did the same thing 
for /var/run/netns self bind mount.


- Jie


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


On Sept. 21, 2015, 9:23 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38569/
> ---
> 
> (Updated Sept. 21, 2015, 9:23 p.m.)
> 
> 
> Review request for mesos, Jojy Varghese, Timothy Chen, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-3483
> https://issues.apache.org/jira/browse/MESOS-3483
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Made slave's work_dir a shared mount in LinuxFilesystemIsolator.
> 
> See ticket for motivation.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/isolators/filesystem/linux.cpp 
> f1e6f7519bdeeff7790fff63e7a9cb3075001758 
>   src/tests/environment.cpp e40cde23daba25b0b61b567ee73682c67e7acbdc 
>   src/tests/utils.cpp 498c9aa1442c4e937a74364073b2267dde6ffcb2 
> 
> Diff: https://reviews.apache.org/r/38569/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 38569: Made slave's work_dir a shared mount in LinuxFilesystemIsolator.

[Jiang Yan Xu]

> On Sept. 21, 2015, 3:35 p.m., Jiang Yan Xu wrote:
> > src/slave/containerizer/isolators/filesystem/linux.cpp, lines 97-98
> > 
> >
> > So this seems to work but oh my godness the the way `mount` command 
> > interacts with the syscall is very implicit.
> > 
> > I can confirm this:
> > 
> > ```
> > # /home/jyx/tmp/ is created by mount command and /home/jyx/tmp/target 
> > is mounted by syscall.
> > [root tmp]# mount --make-shared target
> > mount: can't find /home/jyx/tmp/target in /etc/fstab or /etc/mtab
> > [root tmp]# mount --make-rshared .
> > # OK.
> > ```
> > 
> > So the rule seemed to be **as long as the arguments provided to the 
> > `mount` command themselves are mounted by the the command, we are OK**. (I 
> > guess because whatever is recurively done by the command is using syscall 
> > directly so it's fine)
> > 
> > So I suspect that the `mount --make-rslave /` command inside the 
> > container **should work** because there is always a root `/` mount.
> > 
> > I think we should seek consistency: use fs::mount() as long as it 
> > doesn't break `mount` commands. Or stick with `mount` command whenever 
> > possible?
> 
> Jie Yu wrote:
> OK, I think the comments above do not capture my intention. It's copied 
> from the port mapping isolator. The motivation for using the command 'mount' 
> to mount the work_dir is because: the mount will still be there after all 
> containers and slave stopped. It's better to show this mount when operator 
> types command 'mount' (so that it's not quite invisible). We did the same 
> thing for /var/run/netns self bind mount.

This SGTM!


- Jiang Yan


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


On Sept. 21, 2015, 2:23 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38569/
> ---
> 
> (Updated Sept. 21, 2015, 2:23 p.m.)
> 
> 
> Review request for mesos, Jojy Varghese, Timothy Chen, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-3483
> https://issues.apache.org/jira/browse/MESOS-3483
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Made slave's work_dir a shared mount in LinuxFilesystemIsolator.
> 
> See ticket for motivation.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/isolators/filesystem/linux.cpp 
> f1e6f7519bdeeff7790fff63e7a9cb3075001758 
>   src/tests/environment.cpp e40cde23daba25b0b61b567ee73682c67e7acbdc 
>   src/tests/utils.cpp 498c9aa1442c4e937a74364073b2267dde6ffcb2 
> 
> Diff: https://reviews.apache.org/r/38569/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 37914: Updated the ReviewBot to flag reviews that do not contain reviewers.

[Ben Mahler]

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

Ship it!



support/verify_reviews.py (line 141)


Maybe this should say bad review instead of bad patch?



support/verify_reviews.py (line 142)


Is this line necessary for review related errors?


- Ben Mahler


On Aug. 29, 2015, 12:08 a.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37914/
> ---
> 
> (Updated Aug. 29, 2015, 12:08 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> I'm seeing more and more reviews being sent without tagging reviewers. This 
> should flag such reviews to guide new contributors.
> 
> 
> Diffs
> -
> 
>   support/verify_reviews.py b85a3245b2f248dbdb8e2783f2384092e3d53031 
> 
> Diff: https://reviews.apache.org/r/37914/diff/
> 
> 
> Testing
> ---
> 
> Tested locally
> 
> ./support/verify_reviews.py vinod kone 10 "?from-user=neilc"
> git rev-parse HEAD
> Checking if review: 37445 needs verification
> Latest diff timestamp: 2015-08-13 21:01:08
> Verifying review 37445
> Posting review: Bad patch!
> 
> Reviews applied: []
> 
> Error:
>  No reviewers specified. Please find a reviewer by asking on JIRA or the 
> mailing list.
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>

Re: Review Request 38570: Change documentation image links to absolute paths.

[Adam B]

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


Looks good. Do the images still show up when the markdown is rendered on the 
github website?

- Adam B


On Sept. 21, 2015, 2:49 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38570/
> ---
> 
> (Updated Sept. 21, 2015, 2:49 p.m.)
> 
> 
> Review request for mesos, Adam B, Artem Harutyunyan, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Also removes extraneous `?raw=true` from the links.
> 
> 
> Diffs
> -
> 
>   docs/external-containerizer.md 96932189b74dce1ae28e0bd73d5543d1afaffb0b 
>   docs/fetcher-cache-internals.md d17b41e7113ce72679734fc0bed16614011b6917 
>   docs/maintenance.md 465dcfb72777611c2a4be2aa38c8cd7e869a2baa 
>   docs/oversubscription.md b1a6c9979025ce33190792a07b8e9618c518430e 
> 
> Diff: https://reviews.apache.org/r/38570/diff/
> 
> 
> Testing
> ---
> 
> Rendered with: https://github.com/mesosphere/mesos-website-container
> 
> Confirmed that images show up on the modified docs.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 38569: Made slave's work_dir a shared mount in LinuxFilesystemIsolator.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [38569]

All tests passed.

- Mesos ReviewBot


On Sept. 21, 2015, 9:23 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38569/
> ---
> 
> (Updated Sept. 21, 2015, 9:23 p.m.)
> 
> 
> Review request for mesos, Jojy Varghese, Timothy Chen, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-3483
> https://issues.apache.org/jira/browse/MESOS-3483
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Made slave's work_dir a shared mount in LinuxFilesystemIsolator.
> 
> See ticket for motivation.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/isolators/filesystem/linux.cpp 
> f1e6f7519bdeeff7790fff63e7a9cb3075001758 
>   src/tests/environment.cpp e40cde23daba25b0b61b567ee73682c67e7acbdc 
>   src/tests/utils.cpp 498c9aa1442c4e937a74364073b2267dde6ffcb2 
> 
> Diff: https://reviews.apache.org/r/38569/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 38570: Change documentation image links to absolute paths.

[Joseph Wu]

> On Sept. 21, 2015, 4:02 p.m., Adam B wrote:
> > Looks good. Do the images still show up when the markdown is rendered on 
> > the github website?

Unfortunately, no.  This change will break the links when you look at them on 
Github.
We can make it work both ways if we remove that trailing `/` on all the 
documentation URLs.  But that means, if someone happens to put a slash on the 
end, they won't see the images.

We should probably document (separately?) that Artem's website container is how 
we should be previewing the docs now.


- Joseph


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


On Sept. 21, 2015, 2:49 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38570/
> ---
> 
> (Updated Sept. 21, 2015, 2:49 p.m.)
> 
> 
> Review request for mesos, Adam B, Artem Harutyunyan, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Also removes extraneous `?raw=true` from the links.
> 
> 
> Diffs
> -
> 
>   docs/external-containerizer.md 96932189b74dce1ae28e0bd73d5543d1afaffb0b 
>   docs/fetcher-cache-internals.md d17b41e7113ce72679734fc0bed16614011b6917 
>   docs/maintenance.md 465dcfb72777611c2a4be2aa38c8cd7e869a2baa 
>   docs/oversubscription.md b1a6c9979025ce33190792a07b8e9618c518430e 
> 
> Diff: https://reviews.apache.org/r/38570/diff/
> 
> 
> Testing
> ---
> 
> Rendered with: https://github.com/mesosphere/mesos-website-container
> 
> Confirmed that images show up on the modified docs.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 38570: Change documentation image links to absolute paths.

[Joseph Wu]

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

(Updated Sept. 21, 2015, 4:12 p.m.)


Review request for mesos, Adam B, Artem Harutyunyan, and Vinod Kone.


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


Repository: mesos


Description
---

Also removes extraneous `?raw=true` from the links.


Diffs
-

  docs/external-containerizer.md 96932189b74dce1ae28e0bd73d5543d1afaffb0b 
  docs/fetcher-cache-internals.md d17b41e7113ce72679734fc0bed16614011b6917 
  docs/maintenance.md 465dcfb72777611c2a4be2aa38c8cd7e869a2baa 
  docs/oversubscription.md b1a6c9979025ce33190792a07b8e9618c518430e 

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


Testing (updated)
---

Patched `Rakefile` to include images in the website (See JIRA for the patch).
Then rendered with: https://github.com/mesosphere/mesos-website-container

Confirmed that images show up on the modified docs.


Thanks,

Joseph Wu

Re: Review Request 38564: Add a new callback enabling custom attribute discovery logic

[Felix Abecassis]

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

(Updated Sept. 21, 2015, 11:22 p.m.)


Review request for mesos.


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


Repository: mesos


Description
---

Add a new callback enabling custom attribute discovery logic


Diffs
-

  include/mesos/hook.hpp 2353602c8ac8b89e3fa86bda7d1e262a3debef80 
  src/examples/test_hook_module.cpp 0dc74d60576af6f88cbdc1c9a6f82348c5761d2f 
  src/hook/manager.hpp a517c05855b07831a9728c1191243d114781b70a 
  src/hook/manager.cpp 691976e94d5ac9fe4f8ba583214f24900d14248c 
  src/slave/slave.cpp ad710d7b930a2f115d503ceb8f8fd7421ad30287 
  src/tests/hook_tests.cpp b23a587c683c391ca860b3b7d876902987f4d158 

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


Testing
---


Thanks,

Felix Abecassis

Re: Review Request 38137: Added Docker provisioner, store and local puller

[Jie Yu]

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


Overall, looks good. The main issue is that 'Store' should not have 
dependencies on 'Local' puller (see my detailed comments below).


src/Makefile.am (line 256)


Kill one blank line here.



src/Makefile.am (line 491)


Similar to master/resgistry.proto, we should put this proto file under

`slave/containerizer/provisioner/docker.proto`
or
`slave/containerizer/provisioner/docker/message.proto`
?



src/slave/containerizer/provisioner.cpp (lines 49 - 51)


You need to do a rebase. This file has been moved to 
src/containerizer/provisioner/provisioner.cpp.

Or you just forgot to remove this file from the patch?



src/slave/containerizer/provisioner/docker/local_puller.cpp (line 61)


I don't see the implementation of this method?



src/slave/containerizer/provisioner/docker/local_puller.cpp (line 122)


Insert a blank line above.



src/slave/containerizer/provisioner/docker/local_puller.cpp (lines 146 - 151)


Do you want to kill the 'tar' process when slave exits? You may want to 
take a look at how we setup death signal for 'perf' (src/linuc/perf.cpp).

It's ok you don't want to address that in this patch. Please add a TODO 
somewhere.



src/slave/containerizer/provisioner/docker/local_puller.cpp (line 158)


Can you just capture 'tarPath' here to be more explicit?



src/slave/containerizer/provisioner/docker/local_puller.cpp (line 293)


Insert a blank line above.



src/slave/containerizer/provisioner/docker/local_puller.cpp (line 321)


Insert a blank line above.



src/slave/containerizer/provisioner/docker/local_puller.cpp (line 326)


Why do you need to capture '='?



src/slave/containerizer/provisioner/docker/local_puller.cpp (line 338)


Kill a blank line here.



src/slave/containerizer/provisioner/docker/message.proto (line 21)


Remove .docker?



src/slave/containerizer/provisioner/docker/message.proto (line 40)


2 lines apart.



src/slave/containerizer/provisioner/docker/metadata_manager.hpp (lines 86 - 89)


Please move 'recover()' above 'put'.



src/slave/containerizer/provisioner/docker/metadata_manager.cpp (line 51)


Kill a blank line here.



src/slave/containerizer/provisioner/docker/metadata_manager.cpp (lines 57 - 58)


As we discussed before, please kill this method. Instead, expose the 
constructor.



src/slave/containerizer/provisioner/docker/metadata_manager.cpp (line 89)


Insert a blank line above.



src/slave/containerizer/provisioner/docker/metadata_manager.cpp (line 92)


Insert a blank line above.



src/slave/containerizer/provisioner/docker/metadata_manager.cpp (lines 120 - 
121)


Mind adjust the arguments like the following. It's less jagged:)

```
return dispatch(
process.get(),
&MetadataManagerProcess::put,
name,
layerIds);
```



src/slave/containerizer/provisioner/docker/metadata_manager.cpp (line 203)


Why do you need this?



src/slave/containerizer/provisioner/docker/metadata_manager.cpp (line 206)


Please quote the path (what if path contains spaces?)

Also, remove the period in the end (please make sure to fix all occurances 
in this patch).



src/slave/containerizer/provisioner/docker/metadata_manager.cpp (line 207)


Insert a blank line above.



src/slave/containerizer/provisioner/docker/paths.hpp (line 40)


What's this?



src/slave/containerizer/provisioner/docker/store.hpp (lines 22 - 37)


Please cleanup the header includes he

Review Request 38577: Added synchronous validation for Call in Agent

[Isabel Jimenez]

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

Review request for mesos, Anand Mazumdar, Ben Mahler, and Vinod Kone.


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


Repository: mesos


Description
---

Added validation for Call protobuf message in Agent /api/v1/executor endpoint.


Diffs
-

  src/Makefile.am e224060 
  src/slave/http.cpp 12a4d39 
  src/slave/validation.hpp PRE-CREATION 
  src/slave/validation.cpp PRE-CREATION 

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


Testing
---

make check


Thanks,

Isabel Jimenez