Re: Review Request 39492: Added status endpoint for quota master endpoint.

[Joerg Schad]

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

(Updated Dec. 21, 2015, 7:02 p.m.)


Review request for mesos, Alexander Rukletsov, Bernd Mathiske, and Joris Van 
Remoortere.


Changes
---

Adressed comments.


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


Repository: mesos


Description
---

Added status handling for quota master endpoint.


Diffs (updated)
-

  include/mesos/quota/quota.proto 03e816dcd4dead8326731ac221df7354c0610fed 
  src/master/master.hpp 8af82a0bbc2038e18180136c82cbaeeacc7b3526 
  src/master/quota_handler.cpp 0217149a865ede751b3a03fe40b2d91b487b7b10 

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


Testing
---

Tests are in next Review.


Thanks,

Joerg Schad

Re: Review Request 41514: Accepted a single JSON object for quota set request.

[Anand Mazumdar]

> On Dec. 18, 2015, 12:04 a.m., Anand Mazumdar wrote:
> > src/master/quota_handler.cpp, line 353
> > 
> >
> > hmmm .. Should we return a `BadRequest` for all other non-allowed 
> > values of `force` other then `true` or `false` ?
> 
> Alexander Rukletsov wrote:
> I was thinking about the [Postel's 
> law](https://en.wikipedia.org/wiki/Robustness_principle), but maybe you are 
> right and we should not interpret "force:t", "force:1", "force:god-damn-yes" 
> as `false`, I'll fix that.

I was wondering if we should accept only `true/false` as valid values. 
Currently the `/scheduler` endpoint also specifies a semantically similar field 
`force` in `Call::Subscribe` message that just takes these 2 possible values. 
Should we strive for consistency across all the endpoints and reject all other 
values ?


> On Dec. 18, 2015, 12:04 a.m., Anand Mazumdar wrote:
> > src/master/quota_handler.cpp, line 338
> > 
> >
> > I like the less jagged version more here. It's also more consistent 
> > with other similar strings in the function. What do you think ?
> 
> Alexander Rukletsov wrote:
> It's a known fact, that jaggedness is very subjective. I would argue, 
> that wrapping error message after `BadRequest` is less jagged and more 
> readable, because more there is more space for the message itself. 
> Consistency is important, hence the following review: 
> https://reviews.apache.org/r/41515/

sgtm, I dropped the issue.


- Anand


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


On Dec. 21, 2015, 1:27 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41514/
> ---
> 
> (Updated Dec. 21, 2015, 1:27 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Bernd Mathiske, Joerg Schad, and 
> Joris Van Remoortere.
> 
> 
> Bugs: MESOS-3960
> https://issues.apache.org/jira/browse/MESOS-3960
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> POST request to "/quota" requires a single JSON object as opposed to 
> key-value pairs encoded in a string.
> 
> 
> Diffs
> -
> 
>   src/master/quota_handler.cpp 0217149a865ede751b3a03fe40b2d91b487b7b10 
>   src/tests/master_quota_tests.cpp 89130ce9b09afe7c6dd332c8b5278abe0d2674f1 
>   src/tests/role_tests.cpp 2c5f68ccaac7e9a37345e2f331d1bc35cae77736 
> 
> Diff: https://reviews.apache.org/r/41514/diff/
> 
> 
> Testing
> ---
> 
> make check on Mac OS 10.10.4
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Review Request 41616: Proto buf definition for GPUInfo. This is defined to introduce GPU as first class resource within Mesos.

[Vikrama Ditya]

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

Review request for mesos and Ben Mahler.


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


Repository: mesos


Description
---

Proto buf definition for GPUInfo. This is defined to introduce GPU as first 
class resource within Mesos.


Diffs
-

  include/mesos/mesos.proto 2431fdd6b84625c6140a2b3913736bffada4e7f6 
  include/mesos/v1/mesos.proto 4aed0980b28dc1000aa2821f35303b736bc5bff8 

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


Testing
---

make and make check succceed.


Thanks,

Vikrama Ditya

Re: Review Request 41294: Logger Module: Adds the ContainerLogger into the DockerContainerizer.

[Joseph Wu]

> On Dec. 14, 2015, 8:06 p.m., Timothy Chen wrote:
> > src/docker/docker.cpp, line 603
> > 
> >
> > Just a general question, what is the reasoning behind allowing the 
> > logger to modify the path and argv of the original subprocess?
> 
> Joseph Wu wrote:
> A couple of reasons:
> 
> * We need the thing-performing-the-loggging to be run as a process 
> separate from the logging module.  `Subprocess::IO::PIPE` will not be 
> sufficient since we can't read from the pipe when the agent process falls 
> over.
> * The logging process might need to do something more complicated than 
> redirecting stdout/stderr to a FD.
> * Some logging daemons (wrappers) are launched by prepending the to the 
> argc/argv array.  i.e. `logger-daemon   
> `

We're going to change this in the next iteration.  The logger will only have 
access to `out` and `err`.


- Joseph


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


On Dec. 15, 2015, 12:41 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41294/
> ---
> 
> (Updated Dec. 15, 2015, 12:41 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, haosdent huang, and Artem 
> Harutyunyan.
> 
> 
> Bugs: MESOS-4137
> https://issues.apache.org/jira/browse/MESOS-4137
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The `DockerContainerizer` creates and initializes its own `ContainerLogger` 
> and passes it into the `Docker` wrapper.
> 
> The `ContainerLogger` is used before launching executors, in two two call 
> sites depending on whether the agent is running in a container or not.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 33d6fb3e82ff7328ad093648a45546a18ec7b8cb 
>   src/docker/docker.cpp 5dc4667d93b143b54841d85606affe3e4926757a 
>   src/slave/containerizer/docker.hpp 35712f599395b5f5fbc311a37c6e29b33bac0c8e 
>   src/slave/containerizer/docker.cpp 2d2dd4e0df36207c5f3cbb4fe2c50df51c0f3e9e 
> 
> Diff: https://reviews.apache.org/r/41294/diff/
> 
> 
> Testing
> ---
> 
> Tests will be modified and run later in the review chain.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Review Request 41613: Added `defaultRole` constant to persistent volume tests.

[Greg Mann]

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

Review request for mesos, Alexander Rukletsov, Jie Yu, and Michael Park.


Repository: mesos


Description
---

Added `defaultRole` constant to persistent volume tests.


Diffs
-

  src/tests/persistent_volume_tests.cpp 
2fb57814b2805bc76981d1877603a1a033f29289 

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


Testing
---

`GTEST_FILTER="PersistentVolumeTest*" bin/mesos-tests.sh`


Thanks,

Greg Mann

Re: Review Request 41169: Logger Module: Update tests that use the MesosContainerizer to pass in an ContainerLogger

[Joseph Wu]

> On Dec. 20, 2015, 3:01 p.m., Benjamin Hindman wrote:
> > This looks fine, assuming that we actually need `CreateContainerLogger` 
> > (see below).
> 
> Benjamin Hindman wrote:
> I looked ahead in the review chain and it doesn't look like you ever 
> extend `CreateContainerLogger`, so why was it necessary? Why couldn't you 
> just call `ContainerLogger::create`?

Removed the test helper from the review chain.


- Joseph


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


On Dec. 21, 2015, 11:37 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41169/
> ---
> 
> (Updated Dec. 21, 2015, 11:37 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Artem Harutyunyan.
> 
> 
> Bugs: MESOS-4088
> https://issues.apache.org/jira/browse/MESOS-4088
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/filesystem_isolator_tests.cpp 
> f1dd81bbf4bb42aebd6295e15973b7438bc9f9e7 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> fe679354d04d68b68e168cd8c4eab23898f6532f 
> 
> Diff: https://reviews.apache.org/r/41169/diff/
> 
> 
> Testing
> ---
> 
> make (OSX, Centos7, Ubuntu 14, Debian 8)
> make check (OSX, Centos7, Ubuntu 14, Debian 8)
> sudo bin/mesos-tests.sh (Centos7, Ubuntu 14, Debian 8)
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 40544: Added quota remove handling.

[Joerg Schad]

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

(Updated Dec. 21, 2015, 6:54 p.m.)


Review request for mesos, Alexander Rukletsov and Joris Van Remoortere.


Changes
---

Removed Dependency


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


Repository: mesos


Description
---

Added quota remove handling.


Diffs
-

  src/master/master.hpp 5e5a575dc7dd49324f3c837028df8a7f75cd1f80 
  src/master/quota_handler.cpp 03cef4117c52da7599a2800060f65483ca33bc3f 

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


Testing
---

Test are in the next review.


Thanks,

Joerg Schad

Re: Review Request 41169: Logger Module: Update tests that use the MesosContainerizer to pass in an ContainerLogger

[Joseph Wu]

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

(Updated Dec. 21, 2015, 11:37 a.m.)


Review request for mesos, Benjamin Hindman and Artem Harutyunyan.


Changes
---

Removed `CreateContainerLogger` helper and replaced with 
`ContainerLogger::create(...)` directly.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/tests/containerizer/filesystem_isolator_tests.cpp 
f1dd81bbf4bb42aebd6295e15973b7438bc9f9e7 
  src/tests/containerizer/mesos_containerizer_tests.cpp 
fe679354d04d68b68e168cd8c4eab23898f6532f 

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


Testing
---

make (OSX, Centos7, Ubuntu 14, Debian 8)
make check (OSX, Centos7, Ubuntu 14, Debian 8)
sudo bin/mesos-tests.sh (Centos7, Ubuntu 14, Debian 8)


Thanks,

Joseph Wu

Re: Review Request 41167: Logger Module: Add support for the ContainerLogger to the Mesos Containerizer.

[Joseph Wu]

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

(Updated Dec. 21, 2015, 11:37 a.m.)


Review request for mesos, Benjamin Hindman and Artem Harutyunyan.


Changes
---

Rename local variables from `containerLogger` to `logger`.  Update usage of 
`prepare`.


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


Repository: mesos


Description
---

Changes the `MesosContainerizer` to create and initialize the 
`ContainerLogger`.  

The `MesosContainerizer` modifies the arguments to `launcher->fork()` (in 
`::_launch`) by calling the `ContainerLogger` beforehand.


Diffs (updated)
-

  src/slave/containerizer/mesos/containerizer.hpp 
ab87cbcc843b471c7931aa38a590896f97be9865 
  src/slave/containerizer/mesos/containerizer.cpp 
8242190dffa4d011ee2728a9f0a04d3857767b69 

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


Testing
---

make

This is tested later in the review chain.


Thanks,

Joseph Wu

Re: Review Request 41166: Logger Module: Explicitly disallow use of the ContainerLogger with the External Containerizer.

[Joseph Wu]

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

(Updated Dec. 21, 2015, 11:37 a.m.)


Review request for mesos, Benjamin Hindman and Artem Harutyunyan.


Changes
---

Reworded error message.


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


Repository: mesos


Description
---

Using the `ContainerLogger` with the `ExternalContainerizer` is not supported 
at the moment.  We explicitly disallowed the combination, so that there are no 
incorrect assumptions.


Diffs (updated)
-

  src/slave/containerizer/containerizer.cpp 
dcdf98fea4ca6f96658886db5d09c99f3bff501d 

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


Testing
---

This is tested later in the review chain.


Thanks,

Joseph Wu

Re: Review Request 41603: Cleaned up quota HTTP handling code and tests.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [41514, 41515, 41603]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export 
COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Dec. 21, 2015, 2:17 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41603/
> ---
> 
> (Updated Dec. 21, 2015, 2:17 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, and Joris Van 
> Remoortere.
> 
> 
> Bugs: MESOS-3960
> https://issues.apache.org/jira/browse/MESOS-3960
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Define a variable earlier in the handler for clarity; add consts and
> using directive where appropriate.
> 
> 
> Diffs
> -
> 
>   src/master/quota_handler.cpp 0217149a865ede751b3a03fe40b2d91b487b7b10 
>   src/tests/master_quota_tests.cpp 89130ce9b09afe7c6dd332c8b5278abe0d2674f1 
> 
> Diff: https://reviews.apache.org/r/41603/diff/
> 
> 
> Testing
> ---
> 
> make check on Mac OS 10.10.4
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 41444: Cleaned up Authorizer interface.

[Greg Mann]

> On Dec. 21, 2015, 11:20 p.m., Greg Mann wrote:
> > Thanks, this is a big improvement!

Just a few small nits below:


- Greg


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


On Dec. 21, 2015, 4:04 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41444/
> ---
> 
> (Updated Dec. 21, 2015, 4:04 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, Greg Mann, Jan Schlicht, 
> and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Extract a repetitive part of the function comments into a class comment. 
> Added backticks, quotes when necessary, formatted comments to avoid 
> jaggedness.
> 
> 
> Diffs
> -
> 
>   include/mesos/authorizer/authorizer.hpp 
> 19f6e1a2d025bf6ff07f515b10d41e8a48d7d0b4 
>   src/master/main.cpp e00f878770f3e0bddae5a137b50a00822d154e2c 
> 
> Diff: https://reviews.apache.org/r/41444/diff/
> 
> 
> Testing
> ---
> 
> None: not a functional change.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 41514: Accepted a single JSON object for quota set request.

[Joris Van Remoortere]

> On Dec. 21, 2015, 10:03 p.m., Guangya Liu wrote:
> > src/master/quota_handler.cpp, lines 358-359
> > 
> >
> > What will happen if end user input TRUE, True, FALSE, False? Shall we 
> > document those keyword in quota documentation?

We lower-case the input before checking with this allowed set.


- Joris


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


On Dec. 21, 2015, 1:27 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41514/
> ---
> 
> (Updated Dec. 21, 2015, 1:27 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Bernd Mathiske, Joerg Schad, and 
> Joris Van Remoortere.
> 
> 
> Bugs: MESOS-3960
> https://issues.apache.org/jira/browse/MESOS-3960
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> POST request to "/quota" requires a single JSON object as opposed to 
> key-value pairs encoded in a string.
> 
> 
> Diffs
> -
> 
>   src/master/quota_handler.cpp 0217149a865ede751b3a03fe40b2d91b487b7b10 
>   src/tests/master_quota_tests.cpp 89130ce9b09afe7c6dd332c8b5278abe0d2674f1 
>   src/tests/role_tests.cpp 2c5f68ccaac7e9a37345e2f331d1bc35cae77736 
> 
> Diff: https://reviews.apache.org/r/41514/diff/
> 
> 
> Testing
> ---
> 
> make check on Mac OS 10.10.4
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 41613: Added `defaultRole` constant to persistent volume tests.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [41613]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export 
COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Dec. 21, 2015, 7:12 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41613/
> ---
> 
> (Updated Dec. 21, 2015, 7:12 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Jie Yu, and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added `defaultRole` constant to persistent volume tests.
> 
> 
> Diffs
> -
> 
>   src/tests/persistent_volume_tests.cpp 
> 2fb57814b2805bc76981d1877603a1a033f29289 
> 
> Diff: https://reviews.apache.org/r/41613/diff/
> 
> 
> Testing
> ---
> 
> `GTEST_FILTER="PersistentVolumeTest*" bin/mesos-tests.sh`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 41616: Proto buf definition for GPUInfo. This is defined to introduce GPU as first class resource within Mesos.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [41616]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export 
COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Dec. 21, 2015, 9:27 p.m., Vikrama Ditya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41616/
> ---
> 
> (Updated Dec. 21, 2015, 9:27 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-4232
> https://issues.apache.org/jira/browse/MESOS-4232
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Proto buf definition for GPUInfo. This is defined to introduce GPU as first 
> class resource within Mesos.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 2431fdd6b84625c6140a2b3913736bffada4e7f6 
>   include/mesos/v1/mesos.proto 4aed0980b28dc1000aa2821f35303b736bc5bff8 
> 
> Diff: https://reviews.apache.org/r/41616/diff/
> 
> 
> Testing
> ---
> 
> make and make check succceed.
> 
> 
> Thanks,
> 
> Vikrama Ditya
> 
>

Re: Review Request 41560: Change Docker::run to take Subprocess::IO instead of Option

[Timothy Chen]

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

Ship it!


Ship It!

- Timothy Chen


On Dec. 21, 2015, 11:49 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41560/
> ---
> 
> (Updated Dec. 21, 2015, 11:49 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, Joris 
> Van Remoortere, and Timothy Chen.
> 
> 
> Bugs: MESOS-4137
> https://issues.apache.org/jira/browse/MESOS-4137
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This changes the FDs used by the `mesos-docker-executor` to inherit rather 
> than open anew.
> 
> In the `mesos-docker-executor`, we originally passed the log file path (i.e. 
> `path::join(sandboxDirectory, "stdout")`) as an argument to `Docker::run`.  
> In the executor's context, the log file is already open as `STDOUT_FILENO`.
> 
> By inheriting the FD, the docker containerizer's logging-code will path 
> mirrors that of the mesos containerizer.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 33d6fb3e82ff7328ad093648a45546a18ec7b8cb 
>   src/docker/docker.cpp 5dc4667d93b143b54841d85606affe3e4926757a 
>   src/docker/executor.cpp 4042cec0acbe03d937ea3c53ffde745cbba552d2 
>   src/slave/containerizer/docker.cpp 2d2dd4e0df36207c5f3cbb4fe2c50df51c0f3e9e 
>   src/tests/mesos.hpp 1c6acabab9d189a6d3cc8d63359053fd230377b8 
> 
> Diff: https://reviews.apache.org/r/41560/diff/
> 
> 
> Testing
> ---
> 
> make check (Centos7)
> sudo bin/mesos-tests.sh --gtest_filter="*ROOT_DOCKER_Logs*"
> 
> Also confirmed that the changed pipes will still output to the sandbox.  See 
> below for repro steps.
> 
> ---
> 
> # Manual test for launching an executor from an agent launched with 
> `--docker_mesos_image=`
> 
> ---
> 
> Dockerfile:
> ```
> FROM centos:7
> MAINTAINER Joseph Wu 
> 
> RUN yum -y update systemd
> 
> RUN yum install -y tar wget
> RUN wget 
> http://repos.fedorapeople.org/repos/dchen/apache-maven/epel-apache-maven.repo 
> -O /etc/yum.repos.d/epel-apache-maven.repo
> 
> RUN yum groupinstall -y "Development Tools"
> RUN yum install -y apache-maven python-devel java-1.7.0-openjdk-devel 
> zlib-devel libcurl-devel openssl-devel cyrus-sasl-devel cyrus-sasl-md5 
> apr-devel subversion-devel apr-util-devel libevent-devel
> 
> RUN yum install -y perf nmap-ncat
> 
> RUN yum install -y git
> 
> RUN yum install -y docker
> 
> # Export environment.
> ENV JAVA_HOME /usr/lib/jvm/java-1.7.0-openjdk/
> 
> # Include libmesos on library path.
> ENV LD_LIBRARY_PATH /usr/local/lib
> 
> # Copy local checkout into /opt
> ADD . /opt
> 
> WORKDIR /opt
> 
> # Configure!
> RUN ./bootstrap
> RUN mkdir -p build && cd build && ../configure --enable-ssl --enable-libevent
> 
> WORKDIR /opt/build
> 
> # Build and cleanup in a single layer.
> RUN make -j4 install && cd /
> ```
> 
> ---
> 
> On Centos7, prepare the docker image:
> ```
> sudo docker build -t mesos/mesos:git-docker-pipe .
> ```
> 
> ---
> 
> Start the master:
> ```
> bin/mesos-master.sh --work_dir=/tmp/master --ip=127.0.0.1
> ```
> 
> Start the agent inside a Docker container:
> ```
> sudo docker run -t  -v /var/run/docker.sock:/var/run/docker.sock -v 
> /sys/fs/cgroup:/sys/fs/cgroup -v /tmp/mesos:/tmp/mesos --pid=host --net=host 
> -i mesos/mesos:git-docker-pipe \
> mesos-slave --master=127.0.0.1:5050 --containerizers=docker 
> --docker_mesos_image="mesos/mesos:git-docker-pipe" --switch_user=false
> ```
> 
> Run one of the following (they should both work):
> ```
> src/docker-no-executor-framework 127.0.0.1:5050
> 
> src/mesos-execute --master=127.0.0.1:5050 --containerizer=docker 
> --docker_image=busybox --name="DockerInDocker" --command="echo hello"
> ```
> 
> Then check to see if anything was printed in 
> `/tmp/mesos/slaves//frameworks//executors//runs/latest/`.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 41444: Cleaned up Authorizer interface.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [41444]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export 
COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Dec. 21, 2015, 4:04 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41444/
> ---
> 
> (Updated Dec. 21, 2015, 4:04 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, Greg Mann, Jan Schlicht, 
> and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Extract a repetitive part of the function comments into a class comment. 
> Added backticks, quotes when necessary, formatted comments to avoid 
> jaggedness.
> 
> 
> Diffs
> -
> 
>   include/mesos/authorizer/authorizer.hpp 
> 19f6e1a2d025bf6ff07f515b10d41e8a48d7d0b4 
>   src/master/main.cpp e00f878770f3e0bddae5a137b50a00822d154e2c 
> 
> Diff: https://reviews.apache.org/r/41444/diff/
> 
> 
> Testing
> ---
> 
> None: not a functional change.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 41403: Removed include tuple from values.cpp.

[Joris Van Remoortere]

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

Ship it!


Ship It!

- Joris Van Remoortere


On Dec. 17, 2015, 7:53 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41403/
> ---
> 
> (Updated Dec. 17, 2015, 7:53 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van 
> Remoortere, and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed include tuple from values.cpp.
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp 3af23f2f9233312ceda27470f44b39e1b4d741c3 
>   src/v1/values.cpp fcbf63fc94f776a77f6949fab8365d3f62fb44ef 
> 
> Diff: https://reviews.apache.org/r/41403/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 41539: Enabled agent do not kill exeucutor for some error cases.

[Joris Van Remoortere]

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

Ship it!


Ship It!

- Joris Van Remoortere


On Dec. 18, 2015, 5:38 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41539/
> ---
> 
> (Updated Dec. 18, 2015, 5:38 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van 
> Remoortere, Joseph Wu, Klaus Ma, and Jian Qiu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enabled agent do not kill exeucutor for some error cases.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp ef869695ffeb2e6d9ef0a78ddb676b1b7cd19afe 
> 
> Diff: https://reviews.apache.org/r/41539/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 41560: Change Docker::run to take Subprocess::IO instead of Option

[Joseph Wu]

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

(Updated Dec. 21, 2015, 3:49 p.m.)


Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, Joris 
Van Remoortere, and Timothy Chen.


Changes
---

Changed to a Dockerfile-based approach.  Managed a few successful runs.  (Note: 
https://issues.apache.org/jira/browse/MESOS-4234)


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


Repository: mesos


Description
---

This changes the FDs used by the `mesos-docker-executor` to inherit rather than 
open anew.

In the `mesos-docker-executor`, we originally passed the log file path (i.e. 
`path::join(sandboxDirectory, "stdout")`) as an argument to `Docker::run`.  In 
the executor's context, the log file is already open as `STDOUT_FILENO`.

By inheriting the FD, the docker containerizer's logging-code will path mirrors 
that of the mesos containerizer.


Diffs
-

  src/docker/docker.hpp 33d6fb3e82ff7328ad093648a45546a18ec7b8cb 
  src/docker/docker.cpp 5dc4667d93b143b54841d85606affe3e4926757a 
  src/docker/executor.cpp 4042cec0acbe03d937ea3c53ffde745cbba552d2 
  src/slave/containerizer/docker.cpp 2d2dd4e0df36207c5f3cbb4fe2c50df51c0f3e9e 
  src/tests/mesos.hpp 1c6acabab9d189a6d3cc8d63359053fd230377b8 

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


Testing (updated)
---

make check (Centos7)
sudo bin/mesos-tests.sh --gtest_filter="*ROOT_DOCKER_Logs*"

Also confirmed that the changed pipes will still output to the sandbox.  See 
below for repro steps.

---

# Manual test for launching an executor from an agent launched with 
`--docker_mesos_image=`

---

Dockerfile:
```
FROM centos:7
MAINTAINER Joseph Wu 

RUN yum -y update systemd

RUN yum install -y tar wget
RUN wget 
http://repos.fedorapeople.org/repos/dchen/apache-maven/epel-apache-maven.repo 
-O /etc/yum.repos.d/epel-apache-maven.repo

RUN yum groupinstall -y "Development Tools"
RUN yum install -y apache-maven python-devel java-1.7.0-openjdk-devel 
zlib-devel libcurl-devel openssl-devel cyrus-sasl-devel cyrus-sasl-md5 
apr-devel subversion-devel apr-util-devel libevent-devel

RUN yum install -y perf nmap-ncat

RUN yum install -y git

RUN yum install -y docker

# Export environment.
ENV JAVA_HOME /usr/lib/jvm/java-1.7.0-openjdk/

# Include libmesos on library path.
ENV LD_LIBRARY_PATH /usr/local/lib

# Copy local checkout into /opt
ADD . /opt

WORKDIR /opt

# Configure!
RUN ./bootstrap
RUN mkdir -p build && cd build && ../configure --enable-ssl --enable-libevent

WORKDIR /opt/build

# Build and cleanup in a single layer.
RUN make -j4 install && cd /
```

---

On Centos7, prepare the docker image:
```
sudo docker build -t mesos/mesos:git-docker-pipe .
```

---

Start the master:
```
bin/mesos-master.sh --work_dir=/tmp/master --ip=127.0.0.1
```

Start the agent inside a Docker container:
```
sudo docker run -t  -v /var/run/docker.sock:/var/run/docker.sock -v 
/sys/fs/cgroup:/sys/fs/cgroup -v /tmp/mesos:/tmp/mesos --pid=host --net=host -i 
mesos/mesos:git-docker-pipe \
mesos-slave --master=127.0.0.1:5050 --containerizers=docker 
--docker_mesos_image="mesos/mesos:git-docker-pipe" --switch_user=false
```

Run one of the following (they should both work):
```
src/docker-no-executor-framework 127.0.0.1:5050

src/mesos-execute --master=127.0.0.1:5050 --containerizer=docker 
--docker_image=busybox --name="DockerInDocker" --command="echo hello"
```

Then check to see if anything was printed in 
`/tmp/mesos/slaves//frameworks//executors//runs/latest/`.


Thanks,

Joseph Wu

Re: Review Request 41002: Logger Module: Introduce the ContainerLogger interface for logging the stdout/stderr of executors and tasks.

[Joseph Wu]

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

(Updated Dec. 21, 2015, 4:49 p.m.)


Review request for mesos, Benjamin Hindman, Cody Maloney, Artem Harutyunyan, 
and Thomas Rampelberg.


Changes
---

Change the restriction on `IO::PIPE` into a compile-time restriction.


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


Repository: mesos


Description
---

The container logger is an agent module whose job is to take an executor or 
task's stdout/stderr, and pipe it to some sink.

Existing executor and task stdout/stderr are piped into files ("stdout" and 
"stderr") located in the executor's sandbox directory.
This module aims to encompass this default behavior as well as allow more 
sophisticated logging solutions.


Diffs (updated)
-

  include/mesos/slave/container_logger.hpp PRE-CREATION 

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


Testing
---

This is added to the Makefile later in the review chain.


Thanks,

Joseph Wu

Re: Review Request 41003: Logger Module: Add the SandboxContainerLogger, the default ContainerLogger implementation.

[Joseph Wu]

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

(Updated Dec. 21, 2015, 4:49 p.m.)


Review request for mesos, Benjamin Hindman, Cody Maloney, and Artem Harutyunyan.


Changes
---

Tweak `prepare` based on change to `SubprocessInfo`.


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


Repository: mesos


Description
---

This implementation mirrors how executor/task stdout/stderr is currently saved 
to plain files.


Diffs (updated)
-

  src/slave/container_loggers/sandbox.hpp PRE-CREATION 
  src/slave/container_loggers/sandbox.cpp PRE-CREATION 

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


Testing
---

This is added to the Makefile later in the review chain.


Thanks,

Joseph Wu

Re: Review Request 41004: Logger Module: Introduce the ContainerLogger module.

[Joseph Wu]

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

(Updated Dec. 21, 2015, 4:49 p.m.)


Review request for mesos, Benjamin Hindman, Cody Maloney, and Artem Harutyunyan.


Changes
---

Remove runtime check `validate`.


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


Repository: mesos


Description
---

Modularizes the `ContainerLogger` interface and adds the 
`SandboxContainerLogger` as the default.


Diffs (updated)
-

  include/mesos/module/container_logger.hpp PRE-CREATION 
  src/Makefile.am cf7f950c0c320a6c8e956409a35a7f47d905f71d 
  src/examples/test_container_logger_module.cpp PRE-CREATION 
  src/module/manager.cpp 1f04790510a2ab9ccd6907fd01be192f52ee90c6 
  src/slave/container_logger.cpp PRE-CREATION 
  src/tests/module.hpp e46ed12c80707bf44ceef3ed1a4eb2f321ce10f6 
  src/tests/module.cpp e272bf0eccb61ae54440ec79adac8efad804c828 

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


Testing
---

make

Tests are modified and run later in the review chain.


Thanks,

Joseph Wu

Re: Review Request 41514: Accepted a single JSON object for quota set request.

[Guangya Liu]

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



src/master/quota_handler.cpp (lines 333 - 334)


What will happen if end user input TRUE, True, FALSE, False? Shall we 
document those keyword in quota documentation?


- Guangya Liu


On 十二月 21, 2015, 1:27 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41514/
> ---
> 
> (Updated 十二月 21, 2015, 1:27 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Bernd Mathiske, Joerg Schad, and 
> Joris Van Remoortere.
> 
> 
> Bugs: MESOS-3960
> https://issues.apache.org/jira/browse/MESOS-3960
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> POST request to "/quota" requires a single JSON object as opposed to 
> key-value pairs encoded in a string.
> 
> 
> Diffs
> -
> 
>   src/master/quota_handler.cpp 0217149a865ede751b3a03fe40b2d91b487b7b10 
>   src/tests/master_quota_tests.cpp 89130ce9b09afe7c6dd332c8b5278abe0d2674f1 
>   src/tests/role_tests.cpp 2c5f68ccaac7e9a37345e2f331d1bc35cae77736 
> 
> Diff: https://reviews.apache.org/r/41514/diff/
> 
> 
> Testing
> ---
> 
> make check on Mac OS 10.10.4
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 41603: Cleaned up quota HTTP handling code and tests.

[Guangya Liu]

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



src/master/quota_handler.cpp (lines 57 - 60)


Not yours but it would be greate to update it as following:

using mesos::quota::QuotaInfo;

using process::Future;
using process::Owned;


- Guangya Liu


On 十二月 21, 2015, 2:17 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41603/
> ---
> 
> (Updated 十二月 21, 2015, 2:17 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, and Joris Van 
> Remoortere.
> 
> 
> Bugs: MESOS-3960
> https://issues.apache.org/jira/browse/MESOS-3960
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Define a variable earlier in the handler for clarity; add consts and
> using directive where appropriate.
> 
> 
> Diffs
> -
> 
>   src/master/quota_handler.cpp 0217149a865ede751b3a03fe40b2d91b487b7b10 
>   src/tests/master_quota_tests.cpp 89130ce9b09afe7c6dd332c8b5278abe0d2674f1 
> 
> Diff: https://reviews.apache.org/r/41603/diff/
> 
> 
> Testing
> ---
> 
> make check on Mac OS 10.10.4
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 41444: Cleaned up Authorizer interface.

[Greg Mann]

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


Thanks, this is a big improvement!


include/mesos/authorizer/authorizer.hpp (line 35)


s/authorization for/authorization of/



include/mesos/authorizer/authorizer.hpp (lines 73 - 74)


s/module only/module-only/

s/module specific/module-specific/



include/mesos/authorizer/authorizer.hpp (lines 111 - 113)


Though I know what you're saying here, this one sounds a bit confusing to 
me. I wonder if something more concise like "shut down a framework registered 
by another framework principal." might be clearer?



include/mesos/authorizer/authorizer.hpp (line 182)


s/quota with the/quota for the/


- Greg Mann


On Dec. 21, 2015, 4:04 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41444/
> ---
> 
> (Updated Dec. 21, 2015, 4:04 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, Greg Mann, Jan Schlicht, 
> and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Extract a repetitive part of the function comments into a class comment. 
> Added backticks, quotes when necessary, formatted comments to avoid 
> jaggedness.
> 
> 
> Diffs
> -
> 
>   include/mesos/authorizer/authorizer.hpp 
> 19f6e1a2d025bf6ff07f515b10d41e8a48d7d0b4 
>   src/master/main.cpp e00f878770f3e0bddae5a137b50a00822d154e2c 
> 
> Diff: https://reviews.apache.org/r/41444/diff/
> 
> 
> Testing
> ---
> 
> None: not a functional change.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 41612: Fixed handling of failed authorization for (un)reserve operations.

[Guangya Liu]

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

Ship it!


Ship It!

- Guangya Liu


On 十二月 21, 2015, 7:12 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41612/
> ---
> 
> (Updated 十二月 21, 2015, 7:12 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Jie Yu, and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed handling of failed authorization for (un)reserve operations.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 40ce3e17fca88da689128bcf5d35fdddc396c011 
> 
> Diff: https://reviews.apache.org/r/41612/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 39492: Added status endpoint for quota master endpoint.

[Guangya Liu]

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

Ship it!


I assume that you will have another RR to cover the unit test, right?

- Guangya Liu


On 十二月 21, 2015, 7:02 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39492/
> ---
> 
> (Updated 十二月 21, 2015, 7:02 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Bernd Mathiske, and Joris Van 
> Remoortere.
> 
> 
> Bugs: MESOS-4013
> https://issues.apache.org/jira/browse/MESOS-4013
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added status handling for quota master endpoint.
> 
> 
> Diffs
> -
> 
>   include/mesos/quota/quota.proto 03e816dcd4dead8326731ac221df7354c0610fed 
>   src/master/master.hpp 8af82a0bbc2038e18180136c82cbaeeacc7b3526 
>   src/master/quota_handler.cpp 0217149a865ede751b3a03fe40b2d91b487b7b10 
> 
> Diff: https://reviews.apache.org/r/39492/diff/
> 
> 
> Testing
> ---
> 
> Tests are in next Review.
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Review Request 41618: Edited defer documentation in libprocess.

[Greg Mann]

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

Review request for mesos, Benjamin Hindman and Neil Conway.


Repository: mesos


Description
---

Edited defer documentation in libprocess.


Diffs
-

  3rdparty/libprocess/README.md 6a47f6772bb7a74af368ed970af0f1c154a070e3 

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


Testing
---

Viewed on github: 
https://github.com/mesosphere/mesos/tree/defer_doc_update/3rdparty/libprocess


Thanks,

Greg Mann

Re: Review Request 41002: Logger Module: Introduce the ContainerLogger interface for logging the stdout/stderr of executors and tasks.

[Joseph Wu]

> On Dec. 20, 2015, 6:21 p.m., Benjamin Hindman wrote:
> > include/mesos/slave/container_logger.hpp, lines 85-93
> > 
> >
> > I'm a little confused. Who executes this? The comment makes it read 
> > like there is some particular code that executes this, which makes me 
> > wonder why it needs to be a static function on `ContainerLogger`?
> > 
> > And why don't we want `SubprocessInfo::out/err` to use pipes? I thought 
> > that was the whole point?
> > 
> > Finally, killing the agent seems a little bit harsh. Couldn't the 
> > container just get killed?
> 
> Joseph Wu wrote:
> This is meant to enforce the `NOTE` in `SubprocessInfo` about how the 
> module may not specify `Subprocess::IO::PIPE()` as part of the return value.  
> 
> I talked with Joris about either making this a compile-time or run-time 
> check, and we decided that a compile-time check would be somewhat distracting 
> compared to a `validate` function like this.  (We would need to add a new 
> class that provides just `Subprocess::FD` and `Subprocess::PATH`.)
> 
> This function will exit on failure because it indicates that the chosen 
> `ContainerLogger` breaks that assumption.

Changed to a compile-time restriction, as discussed.


- Joseph


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


On Dec. 21, 2015, 4:49 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41002/
> ---
> 
> (Updated Dec. 21, 2015, 4:49 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Cody Maloney, Artem Harutyunyan, 
> and Thomas Rampelberg.
> 
> 
> Bugs: MESOS-4087
> https://issues.apache.org/jira/browse/MESOS-4087
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The container logger is an agent module whose job is to take an executor or 
> task's stdout/stderr, and pipe it to some sink.
> 
> Existing executor and task stdout/stderr are piped into files ("stdout" and 
> "stderr") located in the executor's sandbox directory.
> This module aims to encompass this default behavior as well as allow more 
> sophisticated logging solutions.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/container_logger.hpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/41002/diff/
> 
> 
> Testing
> ---
> 
> This is added to the Makefile later in the review chain.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 41597: Extending allocator interface to support dynamic weights

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [41597]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export 
COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Dec. 21, 2015, 6:47 a.m., Yongqiao Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41597/
> ---
> 
> (Updated Dec. 21, 2015, 6:47 a.m.)
> 
> 
> Review request for mesos, Adam B, Neil Conway, and Qian Zhang.
> 
> 
> Bugs: MESOS-3943
> https://issues.apache.org/jira/browse/MESOS-3943
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add the interface in allocator to support updating weight
> at runtime, and the allocator is invoked to allocate the
> resources based on the updated weights later.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/allocator.hpp f7ada68d7111486d264284990996413bb3d6 
>   src/master/allocator/mesos/allocator.hpp 
> 50ef3b20f34bc6d87cbeccabcebec9a5031a6554 
>   src/master/allocator/mesos/hierarchical.hpp 
> 86ea5a402ed67f8f22f11d5730147cd907d66a08 
>   src/master/allocator/mesos/hierarchical.cpp 
> 775182515dcb52bd873ecdf98c827320251a59c8 
>   src/master/allocator/sorter/drf/sorter.hpp 
> 050896e8b12cd4097ccd137d5284d6b39b0f06ab 
>   src/master/allocator/sorter/drf/sorter.cpp 
> 3a442f121f3a1505513877a5c78458a4b8d0a824 
>   src/master/allocator/sorter/sorter.hpp 
> 7be6b44a762fd62c2cd7f28b4dc4865a4587ed26 
>   src/tests/allocator.hpp 9bdfaecf1a148f113ad52956b50ed7cabe0902ef 
> 
> Diff: https://reviews.apache.org/r/41597/diff/
> 
> 
> Testing
> ---
> 
> Make & Make check successfully!
> 
> (TODO): Add a test for dynamic weights + allocation behavior later.
> 
> 
> Thanks,
> 
> Yongqiao Wang
> 
>

Re: Review Request 41598: FreeBSD: use BSD cp in copy provisioner backend.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [41598]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export 
COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Dec. 21, 2015, 7:37 a.m., David Forsythe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41598/
> ---
> 
> (Updated Dec. 21, 2015, 7:37 a.m.)
> 
> 
> Review request for mesos and Ian Downes.
> 
> 
> Bugs: MESOS-4211
> https://issues.apache.org/jira/browse/MESOS-4211
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> FreeBSD: use BSD cp in copy provisioner backend.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/backends/copy.cpp 
> 86bbe719fe8e268e19eb9cf5fc90a6e41f0e96f5 
> 
> Diff: https://reviews.apache.org/r/41598/diff/
> 
> 
> Testing
> ---
> 
> gmake check on FreeBSD 10.2-R
> 
> 
> Thanks,
> 
> David Forsythe
> 
>

Re: Review Request 41604: CMake: Added missing protobuf files to CMake build.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [41604]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export 
COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Dec. 21, 2015, 12:27 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41604/
> ---
> 
> (Updated Dec. 21, 2015, 12:27 p.m.)
> 
> 
> Review request for mesos, Diana Arroyo, Artem Harutyunyan, Joris Van 
> Remoortere, and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> CMake: Added missing protobuf files to CMake build.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt bdc45ae604c940dadc27ab6e8b8a3327bd00642b 
> 
> Diff: https://reviews.apache.org/r/41604/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 41461: stout: Added SFINAE-friendly `result_of`.

[Michael Park]

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

(Updated Dec. 21, 2015, 6:10 p.m.)


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


Repository: mesos


Description
---

VS 2015 won't support C++14 `std::result_of` SFINAE until Update 2, so 
`result_of` must be replaced with `decltype(invoke)`.

Here, we implement SFINAE `result_of` in `stout`.

Follow-up from [r40114](https://reviews.apache.org/r/40114/).


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
b2dea9b93adfa3ea0415a0b5c81a369dd29b6cfe 
  3rdparty/libprocess/3rdparty/stout/include/stout/result_of.hpp PRE-CREATION 

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


Testing
---

`make check` on OS X, compiled on Windows.


Thanks,

Michael Park

Re: Review Request 41514: Accepted a single JSON object for quota set request.

[Alexander Rukletsov]

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

(Updated Dec. 21, 2015, 1:27 p.m.)


Review request for mesos, Anand Mazumdar, Bernd Mathiske, Joerg Schad, and 
Joris Van Remoortere.


Changes
---

Addressed comments and rebased.


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


Repository: mesos


Description
---

POST request to "/quota" requires a single JSON object as opposed to key-value 
pairs encoded in a string.


Diffs (updated)
-

  src/master/quota_handler.cpp 0217149a865ede751b3a03fe40b2d91b487b7b10 
  src/tests/master_quota_tests.cpp 89130ce9b09afe7c6dd332c8b5278abe0d2674f1 
  src/tests/role_tests.cpp 2c5f68ccaac7e9a37345e2f331d1bc35cae77736 

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


Testing
---

make check on Mac OS 10.10.4


Thanks,

Alexander Rukletsov

Re: Review Request 41603: Cleaned up quota HTTP handling code and tests.

[Joerg Schad]

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



src/master/quota_handler.cpp (line 60)


Could you add a blank line between using and namespace? THX


- Joerg Schad


On Dec. 21, 2015, 1:30 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41603/
> ---
> 
> (Updated Dec. 21, 2015, 1:30 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, and Joris Van 
> Remoortere.
> 
> 
> Bugs: MESOS-3960
> https://issues.apache.org/jira/browse/MESOS-3960
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Define a variable earlier in the handler for clarity; add consts and
> using directive where appropriate.
> 
> 
> Diffs
> -
> 
>   src/master/quota_handler.cpp 0217149a865ede751b3a03fe40b2d91b487b7b10 
>   src/tests/master_quota_tests.cpp 89130ce9b09afe7c6dd332c8b5278abe0d2674f1 
> 
> Diff: https://reviews.apache.org/r/41603/diff/
> 
> 
> Testing
> ---
> 
> make check on Mac OS 10.10.4
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 41438: Added documentation on using network proxy for mesos fetcher

[Bernd Mathiske]

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

Ship it!



docs/fetcher.md (line 243)


(s/goes/go/, but actually, I'd prefer:)

s/also goes over/with a proxy also make use of an/


- Bernd Mathiske


On Dec. 18, 2015, 5:45 a.m., Shuai Lin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41438/
> ---
> 
> (Updated Dec. 18, 2015, 5:45 a.m.)
> 
> 
> Review request for mesos and Bernd Mathiske.
> 
> 
> Bugs: MESOS-4130
> https://issues.apache.org/jira/browse/MESOS-4130
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added documentation on using network proxy for mesos fetcher
> 
> 
> Diffs
> -
> 
>   docs/fetcher.md ec73722 
> 
> Diff: https://reviews.apache.org/r/41438/diff/
> 
> 
> Testing
> ---
> 
> - Set `https_proxy=http://localhost:3128` (a local squid server) in 
> `/etc/default/mesos-slave`, restart mesos-slave, launch a new marathon task 
> with a `https://` uri, and through the following sysdig command I can see 
> there is data exchanging between mesos-fetcher process and port 3128.
> 
> ```
> sudo sysdig -A -c echo_fds proc.name=mesos-fetcher and fd.port=3128
> ```
> 
> - Stop the local squid server, try to restart the marathon task, the task 
> would fail repeatly, from slave logs there are error messages that fetcher 
> failed to fetch the uri.
> ```
> Dec 16 15:16:35 lin-E400 mesos-slave[24247]: E1216 15:16:35.678032 24283 
> slave.cpp:3342] Container '45c14132-c56a-4cff-a6b5-f57ba2670643' for executor 
> 'testapp_web.f0ef72d2-a3c4-11e5-af60-56847afe9799' of framework 
> 'b52179fd-8968-4
> bf8-baf0-dddc8a38c903-' failed to start: Failed to fetch all URIs for 
> container '45c14132-c56a-4cff-a6b5-f57ba2670643' with exit status: 2
> ```
> 
> - Restart the squid server, the task would start without a problem.
> 
> 
> Thanks,
> 
> Shuai Lin
> 
>

Re: Review Request 39614: Quota: Added Status Validation Tests.

[Joerg Schad]

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

(Updated Dec. 21, 2015, 2:49 p.m.)


Review request for mesos, Alexander Rukletsov and Bernd Mathiske.


Changes
---

rebased


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


Repository: mesos


Description
---

Quota: Added Status Validation Tests.


Diffs (updated)
-

  src/tests/master_quota_tests.cpp 89130ce9b09afe7c6dd332c8b5278abe0d2674f1 

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


Testing
---

make check


Thanks,

Joerg Schad

Re: Review Request 41515: Refactored error messages to reduce jaggedness.

[Alexander Rukletsov]

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

(Updated Dec. 21, 2015, 1:30 p.m.)


Review request for mesos, Bernd Mathiske, Joerg Schad, Joris Van Remoortere, 
and Neil Conway.


Changes
---

Rebased.


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


Repository: mesos


Description
---

Includes minor updates to wording and punctuation.


Diffs (updated)
-

  src/master/quota_handler.cpp 0217149a865ede751b3a03fe40b2d91b487b7b10 

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


Testing
---

make check on Mac OS 10.10.4


Thanks,

Alexander Rukletsov

Re: Review Request 41514: Accepted a single JSON object for quota set request.

[Alexander Rukletsov]

> On Dec. 18, 2015, 12:04 a.m., Anand Mazumdar wrote:
> > src/master/quota_handler.cpp, line 299
> > 
> >
> > hmm,  why don't we do a `using google::protobuf::RepeatedPtrField` and 
> > get rid of all this jaggedness ?
> 
> Alexander Rukletsov wrote:
> Yeah, and also `#include` it properly : )

Fixed in https://reviews.apache.org/r/41603/


> On Dec. 18, 2015, 12:04 a.m., Anand Mazumdar wrote:
> > src/tests/master_quota_tests.cpp, line 199
> > 
> >
> > Not related to this review : Why don't we make all these `badRequest` 
> > expected strings `const` ?
> 
> Alexander Rukletsov wrote:
> Yeah, let's fix it. Thanks!

Cleaned up in https://reviews.apache.org/r/41603/


- Alexander


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


On Dec. 21, 2015, 1:27 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41514/
> ---
> 
> (Updated Dec. 21, 2015, 1:27 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Bernd Mathiske, Joerg Schad, and 
> Joris Van Remoortere.
> 
> 
> Bugs: MESOS-3960
> https://issues.apache.org/jira/browse/MESOS-3960
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> POST request to "/quota" requires a single JSON object as opposed to 
> key-value pairs encoded in a string.
> 
> 
> Diffs
> -
> 
>   src/master/quota_handler.cpp 0217149a865ede751b3a03fe40b2d91b487b7b10 
>   src/tests/master_quota_tests.cpp 89130ce9b09afe7c6dd332c8b5278abe0d2674f1 
>   src/tests/role_tests.cpp 2c5f68ccaac7e9a37345e2f331d1bc35cae77736 
> 
> Diff: https://reviews.apache.org/r/41514/diff/
> 
> 
> Testing
> ---
> 
> make check on Mac OS 10.10.4
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 41603: Cleaned up quota HTTP handling code and tests.

[Joerg Schad]

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

Ship it!


Ship It!

- Joerg Schad


On Dec. 21, 2015, 1:30 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41603/
> ---
> 
> (Updated Dec. 21, 2015, 1:30 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, and Joris Van 
> Remoortere.
> 
> 
> Bugs: MESOS-3960
> https://issues.apache.org/jira/browse/MESOS-3960
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Define a variable earlier in the handler for clarity; add consts and
> using directive where appropriate.
> 
> 
> Diffs
> -
> 
>   src/master/quota_handler.cpp 0217149a865ede751b3a03fe40b2d91b487b7b10 
>   src/tests/master_quota_tests.cpp 89130ce9b09afe7c6dd332c8b5278abe0d2674f1 
> 
> Diff: https://reviews.apache.org/r/41603/diff/
> 
> 
> Testing
> ---
> 
> make check on Mac OS 10.10.4
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 41294: Logger Module: Adds the ContainerLogger into the DockerContainerizer.

[Joseph Wu]

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

(Updated Dec. 21, 2015, 5:32 p.m.)


Review request for mesos, Benjamin Hindman, haosdent huang, and Artem 
Harutyunyan.


Changes
---

Remove `ContainerLogger` injection from the `Docker` class.


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


Repository: mesos


Description
---

The `DockerContainerizer` creates and initializes its own `ContainerLogger` and 
passes it into the `Docker` wrapper.

The `ContainerLogger` is used before launching executors, in two two call sites 
depending on whether the agent is running in a container or not.


Diffs (updated)
-

  src/docker/docker.hpp c769372090513b702daa2def265aedf536565f96 
  src/slave/containerizer/docker.hpp 35712f599395b5f5fbc311a37c6e29b33bac0c8e 
  src/slave/containerizer/docker.cpp 2d2dd4e0df36207c5f3cbb4fe2c50df51c0f3e9e 

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


Testing
---

Tests will be modified and run later in the review chain.


Thanks,

Joseph Wu

Re: Review Request 41560: Change Docker::run to take Subprocess::IO instead of Option

[Joseph Wu]

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

(Updated Dec. 21, 2015, 5:32 p.m.)


Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, Joris 
Van Remoortere, and Timothy Chen.


Changes
---

Add to Logger Module review chain.


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


Repository: mesos


Description
---

This changes the FDs used by the `mesos-docker-executor` to inherit rather than 
open anew.

In the `mesos-docker-executor`, we originally passed the log file path (i.e. 
`path::join(sandboxDirectory, "stdout")`) as an argument to `Docker::run`.  In 
the executor's context, the log file is already open as `STDOUT_FILENO`.

By inheriting the FD, the docker containerizer's logging-code will path mirrors 
that of the mesos containerizer.


Diffs
-

  src/docker/docker.hpp 33d6fb3e82ff7328ad093648a45546a18ec7b8cb 
  src/docker/docker.cpp 5dc4667d93b143b54841d85606affe3e4926757a 
  src/docker/executor.cpp 4042cec0acbe03d937ea3c53ffde745cbba552d2 
  src/slave/containerizer/docker.cpp 2d2dd4e0df36207c5f3cbb4fe2c50df51c0f3e9e 
  src/tests/mesos.hpp 1c6acabab9d189a6d3cc8d63359053fd230377b8 

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


Testing
---

make check (Centos7)
sudo bin/mesos-tests.sh --gtest_filter="*ROOT_DOCKER_Logs*"

Also confirmed that the changed pipes will still output to the sandbox.  See 
below for repro steps.

---

# Manual test for launching an executor from an agent launched with 
`--docker_mesos_image=`

---

Dockerfile:
```
FROM centos:7
MAINTAINER Joseph Wu 

RUN yum -y update systemd

RUN yum install -y tar wget
RUN wget 
http://repos.fedorapeople.org/repos/dchen/apache-maven/epel-apache-maven.repo 
-O /etc/yum.repos.d/epel-apache-maven.repo

RUN yum groupinstall -y "Development Tools"
RUN yum install -y apache-maven python-devel java-1.7.0-openjdk-devel 
zlib-devel libcurl-devel openssl-devel cyrus-sasl-devel cyrus-sasl-md5 
apr-devel subversion-devel apr-util-devel libevent-devel

RUN yum install -y perf nmap-ncat

RUN yum install -y git

RUN yum install -y docker

# Export environment.
ENV JAVA_HOME /usr/lib/jvm/java-1.7.0-openjdk/

# Include libmesos on library path.
ENV LD_LIBRARY_PATH /usr/local/lib

# Copy local checkout into /opt
ADD . /opt

WORKDIR /opt

# Configure!
RUN ./bootstrap
RUN mkdir -p build && cd build && ../configure --enable-ssl --enable-libevent

WORKDIR /opt/build

# Build and cleanup in a single layer.
RUN make -j4 install && cd /
```

---

On Centos7, prepare the docker image:
```
sudo docker build -t mesos/mesos:git-docker-pipe .
```

---

Start the master:
```
bin/mesos-master.sh --work_dir=/tmp/master --ip=127.0.0.1
```

Start the agent inside a Docker container:
```
sudo docker run -t  -v /var/run/docker.sock:/var/run/docker.sock -v 
/sys/fs/cgroup:/sys/fs/cgroup -v /tmp/mesos:/tmp/mesos --pid=host --net=host -i 
mesos/mesos:git-docker-pipe \
mesos-slave --master=127.0.0.1:5050 --containerizers=docker 
--docker_mesos_image="mesos/mesos:git-docker-pipe" --switch_user=false
```

Run one of the following (they should both work):
```
src/docker-no-executor-framework 127.0.0.1:5050

src/mesos-execute --master=127.0.0.1:5050 --containerizer=docker 
--docker_image=busybox --name="DockerInDocker" --command="echo hello"
```

Then check to see if anything was printed in 
`/tmp/mesos/slaves//frameworks//executors//runs/latest/`.


Thanks,

Joseph Wu

Re: Review Request 41378: Logger Module: Update tests that use the DockerContainerizer to pass in a ContainerLogger.

[Mesos ReviewBot]

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


Bad patch!

Reviews applied: [41002, 41003, 41004, 41061, 4]

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

Error:
 2015-12-22 04:23:32 URL:https://reviews.apache.org/r/4/diff/raw/ 
[5617/5617] -> "4.patch" [1]
error: patch failed: src/Makefile.am:1731
error: src/Makefile.am: patch does not apply

- Mesos ReviewBot


On Dec. 22, 2015, 1:35 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41378/
> ---
> 
> (Updated Dec. 22, 2015, 1:35 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Artem Harutyunyan.
> 
> 
> Bugs: MESOS-4137
> https://issues.apache.org/jira/browse/MESOS-4137
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> a43e355405b37443601c0993d36821c50056c24f 
>   src/tests/hook_tests.cpp e034534295fe6ddd280e25f1d9ee5182cddae187 
> 
> Diff: https://reviews.apache.org/r/41378/diff/
> 
> 
> Testing
> ---
> 
> make (OSX, Centos7)
> make check (OSX, Centos7)
> sudo bin/mesos-tests.sh (Centos7)
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 41167: Logger Module: Add support for the ContainerLogger to the Mesos Containerizer.

[Joseph Wu]

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

(Updated Dec. 21, 2015, 5:21 p.m.)


Review request for mesos, Benjamin Hindman and Artem Harutyunyan.


Changes
---

Fix ambiguity in ternary conditional.


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


Repository: mesos


Description
---

Changes the `MesosContainerizer` to create and initialize the 
`ContainerLogger`.  

The `MesosContainerizer` modifies the arguments to `launcher->fork()` (in 
`::_launch`) by calling the `ContainerLogger` beforehand.


Diffs (updated)
-

  src/slave/containerizer/mesos/containerizer.hpp 
ab87cbcc843b471c7931aa38a590896f97be9865 
  src/slave/containerizer/mesos/containerizer.cpp 
8242190dffa4d011ee2728a9f0a04d3857767b69 

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


Testing
---

make

This is tested later in the review chain.


Thanks,

Joseph Wu

Re: Review Request 41378: Logger Module: Update tests that use the DockerContainerizer to pass in a ContainerLogger.

[Joseph Wu]

> On Dec. 14, 2015, 8:07 p.m., Timothy Chen wrote:
> > src/tests/environment.cpp, line 226
> > 
> >
> > If we expect NULL to be a valid option we should make it an Option.
> 
> Joseph Wu wrote:
> NULL is not a valid argument.  But for some of these tests, I thought it 
> would unnecessarily bloat the test code to create an unused `ContainerLogger`.

Note: the latest diff removed this change.


- Joseph


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


On Dec. 21, 2015, 5:35 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41378/
> ---
> 
> (Updated Dec. 21, 2015, 5:35 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Artem Harutyunyan.
> 
> 
> Bugs: MESOS-4137
> https://issues.apache.org/jira/browse/MESOS-4137
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> a43e355405b37443601c0993d36821c50056c24f 
>   src/tests/hook_tests.cpp e034534295fe6ddd280e25f1d9ee5182cddae187 
> 
> Diff: https://reviews.apache.org/r/41378/diff/
> 
> 
> Testing
> ---
> 
> make (OSX, Centos7)
> make check (OSX, Centos7)
> sudo bin/mesos-tests.sh (Centos7)
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 41378: Logger Module: Update tests that use the DockerContainerizer to pass in a ContainerLogger.

[Joseph Wu]

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

(Updated Dec. 21, 2015, 5:35 p.m.)


Review request for mesos, Benjamin Hindman and Artem Harutyunyan.


Changes
---

Remove `ContainerLogger` injection from the `Docker` class.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/tests/containerizer/docker_containerizer_tests.cpp 
a43e355405b37443601c0993d36821c50056c24f 
  src/tests/hook_tests.cpp e034534295fe6ddd280e25f1d9ee5182cddae187 

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


Testing
---

make (OSX, Centos7)
make check (OSX, Centos7)
sudo bin/mesos-tests.sh (Centos7)


Thanks,

Joseph Wu

Re: Review Request 41370: Logger Module: Update DockerContainerizer test helpers

[Joseph Wu]

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

(Updated Dec. 21, 2015, 5:35 p.m.)


Review request for mesos, Benjamin Hindman and Artem Harutyunyan.


Changes
---

Remove `ContainerLogger` injection from the `Docker` class.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/tests/mesos.hpp 582b018ea0e41cb2a9cdbf8cd1cf46870179757b 
  src/tests/mesos.cpp 11ca0519ad0dad44328b6ebdf52c5356fda818b7 

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


Testing
---

Tests will be modified and run later in the review chain.


Thanks,

Joseph Wu

Re: Review Request 41378: Logger Module: Update tests that use the DockerContainerizer to pass in a ContainerLogger.

[Joseph Wu]

> On Dec. 20, 2015, 3:24 p.m., Benjamin Hindman wrote:
> > So, are you sure we need `CreateContainerLogger` and not just 
> > `ContainerLogger::create`? If we're not extending it than I don't know that 
> > we need it for now. Also, this looks nice and mechanical given 
> > https://reviews.apache.org/r/41294 but please see comment there. Thanks!

Yup, the test helper was removed.


- Joseph


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


On Dec. 21, 2015, 5:35 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41378/
> ---
> 
> (Updated Dec. 21, 2015, 5:35 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Artem Harutyunyan.
> 
> 
> Bugs: MESOS-4137
> https://issues.apache.org/jira/browse/MESOS-4137
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> a43e355405b37443601c0993d36821c50056c24f 
>   src/tests/hook_tests.cpp e034534295fe6ddd280e25f1d9ee5182cddae187 
> 
> Diff: https://reviews.apache.org/r/41378/diff/
> 
> 
> Testing
> ---
> 
> make (OSX, Centos7)
> make check (OSX, Centos7)
> sudo bin/mesos-tests.sh (Centos7)
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 40936: Windows: Unified POSIX and Windows implementation of shell.hpp

[Daniel Pravat]

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

(Updated Dec. 22, 2015, 2:32 a.m.)


Review request for mesos, Alex Naparu, Alex Clemmer, and M Lawindi.


Repository: mesos


Description
---

Windows: Unified POSIX and Windows implementation of shell.hpp


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/shell.hpp 
b18cae1118302e18d2cfb7ce4089ab5079a01d1a 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/shell.hpp 
4e8d56bf778556341637487b7e6dcecedc8dcfbd 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/shell.hpp 
bf737c87b8a337cc46e6c16d6fec2eef61e6ea05 
  3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 
42733d429814bf0512540188264830aeaabcabbe 

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


Testing
---


Thanks,

Daniel Pravat

Re: Review Request 39492: Added status endpoint for quota master endpoint.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [39211, 39018, 39102, 36913, 38059, 39285, 38110, 40342, 
40351, 40396, 39223, 39492]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export 
COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Dec. 21, 2015, 7:02 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39492/
> ---
> 
> (Updated Dec. 21, 2015, 7:02 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Bernd Mathiske, and Joris Van 
> Remoortere.
> 
> 
> Bugs: MESOS-4013
> https://issues.apache.org/jira/browse/MESOS-4013
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added status handling for quota master endpoint.
> 
> 
> Diffs
> -
> 
>   include/mesos/quota/quota.proto 03e816dcd4dead8326731ac221df7354c0610fed 
>   src/master/master.hpp 8af82a0bbc2038e18180136c82cbaeeacc7b3526 
>   src/master/quota_handler.cpp 0217149a865ede751b3a03fe40b2d91b487b7b10 
> 
> Diff: https://reviews.apache.org/r/39492/diff/
> 
> 
> Testing
> ---
> 
> Tests are in next Review.
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 41618: Edited defer documentation in libprocess.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [41618]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export 
COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Dec. 22, 2015, midnight, Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41618/
> ---
> 
> (Updated Dec. 22, 2015, midnight)
> 
> 
> Review request for mesos, Benjamin Hindman and Neil Conway.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Edited defer documentation in libprocess.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/README.md 6a47f6772bb7a74af368ed970af0f1c154a070e3 
> 
> Diff: https://reviews.apache.org/r/41618/diff/
> 
> 
> Testing
> ---
> 
> Viewed on github: 
> https://github.com/mesosphere/mesos/tree/defer_doc_update/3rdparty/libprocess
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 41612: Fixed handling of failed authorization for (un)reserve operations.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [41612]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export 
COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Dec. 21, 2015, 7:12 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41612/
> ---
> 
> (Updated Dec. 21, 2015, 7:12 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Jie Yu, and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed handling of failed authorization for (un)reserve operations.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 40ce3e17fca88da689128bcf5d35fdddc396c011 
> 
> Diff: https://reviews.apache.org/r/41612/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 40115: Windows: Added support for `slave/gc.cpp'.

[Daniel Pravat]

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

(Updated Dec. 22, 2015, 7:32 a.m.)


Review request for Alex Naparu and Alex Clemmer.


Repository: mesos


Description
---

Windows: Added support for `slave/gc.cpp'.


Diffs (updated)
-

  src/CMakeLists.txt 0d46043dd06007f2cba56626c82e8edd251cb8fb 
  src/slave/gc.cpp 7a8c69b4410df46ca8fd6ac009cc14e8fe5ff6d3 

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


Testing
---

Windows 10: make.bat
OSX: make check
Ubuntu: 15.1 make check


Thanks,

Daniel Pravat

Re: Review Request 41560: Change Docker::run to take Subprocess::IO instead of Option

[Joseph Wu]

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

(Updated Dec. 21, 2015, 5:34 p.m.)


Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Jie Yu, Joris 
Van Remoortere, and Timothy Chen.


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


Repository: mesos


Description
---

This changes the FDs used by the `mesos-docker-executor` to inherit rather than 
open anew.

In the `mesos-docker-executor`, we originally passed the log file path (i.e. 
`path::join(sandboxDirectory, "stdout")`) as an argument to `Docker::run`.  In 
the executor's context, the log file is already open as `STDOUT_FILENO`.

By inheriting the FD, the docker containerizer's logging-code will path mirrors 
that of the mesos containerizer.


Diffs
-

  src/docker/docker.hpp 33d6fb3e82ff7328ad093648a45546a18ec7b8cb 
  src/docker/docker.cpp 5dc4667d93b143b54841d85606affe3e4926757a 
  src/docker/executor.cpp 4042cec0acbe03d937ea3c53ffde745cbba552d2 
  src/slave/containerizer/docker.cpp 2d2dd4e0df36207c5f3cbb4fe2c50df51c0f3e9e 
  src/tests/mesos.hpp 1c6acabab9d189a6d3cc8d63359053fd230377b8 

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


Testing
---

make check (Centos7)
sudo bin/mesos-tests.sh --gtest_filter="*ROOT_DOCKER_Logs*"

Also confirmed that the changed pipes will still output to the sandbox.  See 
below for repro steps.

---

# Manual test for launching an executor from an agent launched with 
`--docker_mesos_image=`

---

Dockerfile:
```
FROM centos:7
MAINTAINER Joseph Wu 

RUN yum -y update systemd

RUN yum install -y tar wget
RUN wget 
http://repos.fedorapeople.org/repos/dchen/apache-maven/epel-apache-maven.repo 
-O /etc/yum.repos.d/epel-apache-maven.repo

RUN yum groupinstall -y "Development Tools"
RUN yum install -y apache-maven python-devel java-1.7.0-openjdk-devel 
zlib-devel libcurl-devel openssl-devel cyrus-sasl-devel cyrus-sasl-md5 
apr-devel subversion-devel apr-util-devel libevent-devel

RUN yum install -y perf nmap-ncat

RUN yum install -y git

RUN yum install -y docker

# Export environment.
ENV JAVA_HOME /usr/lib/jvm/java-1.7.0-openjdk/

# Include libmesos on library path.
ENV LD_LIBRARY_PATH /usr/local/lib

# Copy local checkout into /opt
ADD . /opt

WORKDIR /opt

# Configure!
RUN ./bootstrap
RUN mkdir -p build && cd build && ../configure --enable-ssl --enable-libevent

WORKDIR /opt/build

# Build and cleanup in a single layer.
RUN make -j4 install && cd /
```

---

On Centos7, prepare the docker image:
```
sudo docker build -t mesos/mesos:git-docker-pipe .
```

---

Start the master:
```
bin/mesos-master.sh --work_dir=/tmp/master --ip=127.0.0.1
```

Start the agent inside a Docker container:
```
sudo docker run -t  -v /var/run/docker.sock:/var/run/docker.sock -v 
/sys/fs/cgroup:/sys/fs/cgroup -v /tmp/mesos:/tmp/mesos --pid=host --net=host -i 
mesos/mesos:git-docker-pipe \
mesos-slave --master=127.0.0.1:5050 --containerizers=docker 
--docker_mesos_image="mesos/mesos:git-docker-pipe" --switch_user=false
```

Run one of the following (they should both work):
```
src/docker-no-executor-framework 127.0.0.1:5050

src/mesos-execute --master=127.0.0.1:5050 --containerizer=docker 
--docker_image=busybox --name="DockerInDocker" --command="echo hello"
```

Then check to see if anything was printed in 
`/tmp/mesos/slaves//frameworks//executors//runs/latest/`.


Thanks,

Joseph Wu

Re: Review Request 41294: Logger Module: Adds the ContainerLogger into the DockerContainerizer.

[Joseph Wu]

> On Dec. 20, 2015, 3:17 p.m., Benjamin Hindman wrote:
> > We shouldn't pass `ContainerLogger` into `Docker` while also keeping it 
> > `Owned` inside `DockerContainerizer`. It's confusing to people, we're 
> > explicitly breaking the contract of `Owned`.
> > 
> > Instead, why not change the `Docker::run` to take `Subprocess::IO` instead 
> > of the stdout/stderr paths? Could we make that work?

Note: The updated diff no longer passes the `ContainerLogger` into the `Docker` 
class.


- Joseph


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


On Dec. 21, 2015, 5:32 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41294/
> ---
> 
> (Updated Dec. 21, 2015, 5:32 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, haosdent huang, and Artem 
> Harutyunyan.
> 
> 
> Bugs: MESOS-4137
> https://issues.apache.org/jira/browse/MESOS-4137
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The `DockerContainerizer` creates and initializes its own `ContainerLogger` 
> and passes it into the `Docker` wrapper.
> 
> The `ContainerLogger` is used before launching executors, in two two call 
> sites depending on whether the agent is running in a container or not.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp c769372090513b702daa2def265aedf536565f96 
>   src/slave/containerizer/docker.hpp 35712f599395b5f5fbc311a37c6e29b33bac0c8e 
>   src/slave/containerizer/docker.cpp 2d2dd4e0df36207c5f3cbb4fe2c50df51c0f3e9e 
> 
> Diff: https://reviews.apache.org/r/41294/diff/
> 
> 
> Testing
> ---
> 
> Tests will be modified and run later in the review chain.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Review Request 41604: CMake: Added missing protobuf files to CMake build.

[Alex Clemmer]

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

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


Repository: mesos


Description
---

CMake: Added missing protobuf files to CMake build.


Diffs
-

  src/CMakeLists.txt bdc45ae604c940dadc27ab6e8b8a3327bd00642b 

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


Testing
---


Thanks,

Alex Clemmer

Re: Review Request 41514: Accepted a single JSON object for quota set request.

[Alexander Rukletsov]

> On Dec. 18, 2015, 12:04 a.m., Anand Mazumdar wrote:
> > LGTM.. Just some nits around:
> > 
> > - using `const` for test strings.
> > - reducing jaggedness for some blocks.
> > 
> > Also, a query regarding just accepting `true/false` as `force` field values.

Good points, thanks for the review, Anand!


> On Dec. 18, 2015, 12:04 a.m., Anand Mazumdar wrote:
> > src/master/quota_handler.cpp, line 273
> > 
> >
> > nit: use backticks (`force`)
> > s/flag/field

I'm not sure we backtick flags, do we? AFAIK, we do that for object, variables, 
function names, types.


> On Dec. 18, 2015, 12:04 a.m., Anand Mazumdar wrote:
> > src/master/quota_handler.cpp, line 299
> > 
> >
> > hmm,  why don't we do a `using google::protobuf::RepeatedPtrField` and 
> > get rid of all this jaggedness ?

Yeah, and also `#include` it properly : )


> On Dec. 18, 2015, 12:04 a.m., Anand Mazumdar wrote:
> > src/master/quota_handler.cpp, line 338
> > 
> >
> > I like the less jagged version more here. It's also more consistent 
> > with other similar strings in the function. What do you think ?

It's a known fact, that jaggedness is very subjective. I would argue, that 
wrapping error message after `BadRequest` is less jagged and more readable, 
because more there is more space for the message itself. Consistency is 
important, hence the following review: https://reviews.apache.org/r/41515/


> On Dec. 18, 2015, 12:04 a.m., Anand Mazumdar wrote:
> > src/master/quota_handler.cpp, line 353
> > 
> >
> > hmmm .. Should we return a `BadRequest` for all other non-allowed 
> > values of `force` other then `true` or `false` ?

I was thinking about the [Postel's 
law](https://en.wikipedia.org/wiki/Robustness_principle), but maybe you are 
right and we should not interpret "force:t", "force:1", "force:god-damn-yes" as 
`false`, I'll fix that.


> On Dec. 18, 2015, 12:04 a.m., Anand Mazumdar wrote:
> > src/tests/master_quota_tests.cpp, line 199
> > 
> >
> > Not related to this review : Why don't we make all these `badRequest` 
> > expected strings `const` ?

Yeah, let's fix it. Thanks!


- Alexander


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


On Dec. 17, 2015, 2:52 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41514/
> ---
> 
> (Updated Dec. 17, 2015, 2:52 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Bernd Mathiske, Joerg Schad, and 
> Joris Van Remoortere.
> 
> 
> Bugs: MESOS-3960
> https://issues.apache.org/jira/browse/MESOS-3960
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> POST request to "/quota" requires a single JSON object as opposed to 
> key-value pairs encoded in a string.
> 
> 
> Diffs
> -
> 
>   src/master/quota_handler.cpp 11167879b2480d9c8dd6398ca39c479089ec2272 
>   src/tests/master_quota_tests.cpp 0473869783a714766ed26fff61d7f8c56342df74 
> 
> Diff: https://reviews.apache.org/r/41514/diff/
> 
> 
> Testing
> ---
> 
> make check on Mac OS 10.10.4
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 41609: Cleaned up formatting for multiple inheritance.

[Alexander Rukletsov]

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

(Updated Dec. 21, 2015, 4:56 p.m.)


Review request for mesos, Bernd Mathiske, Joerg Schad, and Neil Conway.


Repository: mesos


Description
---

Cleaned up formatting for multiple inheritance.


Diffs
-

  src/tests/containerizer/provisioner_docker_tests.cpp 
7c1197ed03df8b5d77d26307560535f9ff5bf350 
  src/tests/hierarchical_allocator_tests.cpp 
88bb7e987c471587d46e15082fe1149fd94db5d6 
  src/tests/registrar_tests.cpp c5022233fa88c4d8578055bc4812055b4cb9060d 

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


Testing (updated)
---

None: not a functional change.

I've grepped the code base to find out what is the common wrapping here. 
"tests/executor_http_api_tests.cpp:74" uses the proposed wrapping:
```
class ExecutorHttpApiTest
  : public MesosTest,
public WithParamInterface {};
```

It also seems to be consistent another similar syntax pattern (c-tor 
initialization list), for example see
"libprocess/include/process/http.hpp"
"src/master/master.cpp"


Thanks,

Alexander Rukletsov

Review Request 41608: Removed extra '; ' after inline class function definitions.

[Alexander Rukletsov]

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

Review request for mesos, Bernd Mathiske, Joerg Schad, and Neil Conway.


Repository: mesos


Description
---

Removed extra ';' after inline class function definitions.


Diffs
-

  include/mesos/hook.hpp 9b20ac752ff5517213c6ea3e56c98046c24295b6 
  src/docker/docker.hpp 4cb7a4e43c01f490643486d3b8797018b9787179 
  src/slave/containerizer/mesos/isolators/network/port_mapping.hpp 
cbb94077d46d7b87ffc09b72e02269bc16f25f92 
  src/tests/containerizer/memory_test_helper.hpp 
2d91e33662a8a0959254d6acb08d09f3ba70e30f 

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


Testing
---

None: not a functional change.


Thanks,

Alexander Rukletsov

Review Request 41609: Cleaned up formatting for multiple inheritance.

[Alexander Rukletsov]

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

Review request for mesos, Bernd Mathiske, Joerg Schad, and Neil Conway.


Repository: mesos


Description
---

Cleaned up formatting for multiple inheritance.


Diffs
-

  src/tests/containerizer/provisioner_docker_tests.cpp 
7c1197ed03df8b5d77d26307560535f9ff5bf350 
  src/tests/hierarchical_allocator_tests.cpp 
88bb7e987c471587d46e15082fe1149fd94db5d6 
  src/tests/registrar_tests.cpp c5022233fa88c4d8578055bc4812055b4cb9060d 

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


Testing
---

None: not a functional change.


Thanks,

Alexander Rukletsov

Re: Review Request 41347: Added a TODO to remove a `friend class` declaration.

[Joris Van Remoortere]

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

Ship it!


Ship It!

- Joris Van Remoortere


On Dec. 14, 2015, 2:28 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41347/
> ---
> 
> (Updated Dec. 14, 2015, 2:28 p.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp 9aa548aa6e159046c94e4ec96f631ea8b3bfd5d8 
> 
> Diff: https://reviews.apache.org/r/41347/diff/
> 
> 
> Testing
> ---
> 
> Not a functional change.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 39492: Added status endpoint for quota master endpoint.

[Joerg Schad]

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

(Updated Dec. 21, 2015, 2:56 p.m.)


Review request for mesos, Alexander Rukletsov, Bernd Mathiske, and Joris Van 
Remoortere.


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


Repository: mesos


Description
---

Added status handling for quota master endpoint.


Diffs (updated)
-

  include/mesos/quota/quota.proto 03e816dcd4dead8326731ac221df7354c0610fed 
  src/master/master.hpp 8af82a0bbc2038e18180136c82cbaeeacc7b3526 
  src/master/quota_handler.cpp 0217149a865ede751b3a03fe40b2d91b487b7b10 

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


Testing
---

Tests are in next Review.


Thanks,

Joerg Schad

Re: Review Request 41444: Cleaned up Authorizer interface.

[Alexander Rukletsov]

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

(Updated Dec. 21, 2015, 4:04 p.m.)


Review request for mesos, Adam B, Alexander Rojas, Greg Mann, Jan Schlicht, and 
Till Toenshoff.


Changes
---

More comments; s/is allowed to/can.


Repository: mesos


Description
---

Extract a repetitive part of the function comments into a class comment. Added 
backticks, quotes when necessary, formatted comments to avoid jaggedness.


Diffs (updated)
-

  include/mesos/authorizer/authorizer.hpp 
19f6e1a2d025bf6ff07f515b10d41e8a48d7d0b4 
  src/master/main.cpp e00f878770f3e0bddae5a137b50a00822d154e2c 

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


Testing
---

None: not a functional change.


Thanks,

Alexander Rukletsov

Re: Review Request 41609: Cleaned up formatting for multiple inheritance.

[Michael Park]

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

Ship it!


Ship It!

- Michael Park


On Dec. 21, 2015, 4:58 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41609/
> ---
> 
> (Updated Dec. 21, 2015, 4:58 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, and Neil Conway.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Cleaned up formatting for multiple inheritance.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> 7c1197ed03df8b5d77d26307560535f9ff5bf350 
>   src/tests/hierarchical_allocator_tests.cpp 
> 88bb7e987c471587d46e15082fe1149fd94db5d6 
>   src/tests/registrar_tests.cpp c5022233fa88c4d8578055bc4812055b4cb9060d 
> 
> Diff: https://reviews.apache.org/r/41609/diff/
> 
> 
> Testing
> ---
> 
> None: not a functional change.
> 
> I've grepped the code base to find out what is the common wrapping here. 
> "tests/executor_http_api_tests.cpp:74" uses the proposed wrapping:
> ```
> class ExecutorHttpApiTest
>   : public MesosTest,
> public WithParamInterface {};
> ```
> 
> It also seems to be consistent another similar syntax pattern (c-tor 
> initialization list), for example see
> "libprocess/include/process/http.hpp"
> "src/slave/master.cpp:118"
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 41472: Disallowed dynamic reservations for roles not on the role whitelist.

[Michael Park]

> On Dec. 17, 2015, 1:17 a.m., Michael Park wrote:
> > src/master/http.cpp, lines 1000-1006
> > 
> >
> > Hm, should this be added to `validation::operation::validate` instead? 
> > That way if/when we have frameworks reserving for specific roles, it'll 
> > just work?
> 
> Neil Conway wrote:
> I thought about that, but it would require passing either the role 
> whitelist or a pointer to the master itself into the 
> `validation::operation::validate()` -- both of which seemed like they would 
> be regrettable changes to make. What do you think?
> 
> Adam B wrote:
> I'm fine with it as is, but I'll wait for @mcypark to make the call.
> 
> Michael Park wrote:
> To me, it seems natural to have to pass whatever state is necessary to 
> perform validation properly.
> The `offer` validation for example takes `Master*` since it needs some 
> information from the master.
> 
> ```cpp
> // Validates the given offers.
> Option validate(
> const google::protobuf::RepeatedPtrField& offerIds,
> Master* master,
> Framework* framework);
> ```
> 
> Ideally, we would pass along something more constrained than "all of 
> master", similar to `CreateVolume` validation.
> 
> ```cpp
> // Validates the CREATE operation. We need slave's checkpointed
> // resources so that we can validate persistence ID uniqueness.
> Option validate(
> const Offer::Operation::Create& create,
> const Resources& checkpointedResources);
> ```
> 
> Here, `checkpointedResources` is a part of the master state. Passing 
> along the role whitelist would be similar to this case.
> 
> What do you think?
> 
> Neil Conway wrote:
> I started implementing this: 
> https://gist.github.com/neilconway/b6dc7a3b6eb84f187923
> 
> However, it also requires changes to the tests in 
> `ReserveOperationValidationTest`, which will be non-trivial. Personally, 
> given that (a) making the check in `http.cpp` is functionally fine (b) we're 
> going to remove the role whitelist in ~six months anyway, I'd vote for 
> keeping the current approach and not moving the checking of the role 
> whitelist into `::validate`. I'll add a TODO, although I suspect we'll remove 
> all this code before anyone gets around to implementing it :)

Just so I understand your points correctly, do you mean that the changes to the 
tests in `ReserveOperationValidationTest` will be non-trivial because we would 
have to start up a master, and potentially other components like allocator? If 
that's the case, it seems to me like that would only be true if we take 
`Master*` (as you have in the implementation you started). If we were to pass 
the role whitelist instead, we could simply construct an instance of 
`hashmap` within the test and pass that to `validate`, right? That 
actually seems quite clean to me.


- Michael


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


On Dec. 16, 2015, 10:48 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41472/
> ---
> 
> (Updated Dec. 16, 2015, 10:48 p.m.)
> 
> 
> Review request for mesos, Adam B and Michael Park.
> 
> 
> Bugs: MESOS-4143
> https://issues.apache.org/jira/browse/MESOS-4143
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Also added a test that dynamic reservations via the "/reserve" endpoint are
> allowed when using implicit roles.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 8dfd67f7d51b8395953d6beb77b2d71bc538eacd 
>   src/tests/reservation_endpoints_tests.cpp 
> b8edd6fafedd4c2221a8d19c1ebc71254071a8c7 
> 
> Diff: https://reviews.apache.org/r/41472/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 41444: Cleaned up Authorizer interface.

[Alexander Rukletsov]

> On Dec. 18, 2015, 8:42 a.m., Adam B wrote:
> > include/mesos/authorizer/authorizer.hpp, line 66
> > 
> >
> > s/on/of/

killed the entire phrase.


- Alexander


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


On Dec. 21, 2015, 3:42 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41444/
> ---
> 
> (Updated Dec. 21, 2015, 3:42 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, Greg Mann, Jan Schlicht, 
> and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Extract a repetitive part of the function comments into a class comment. 
> Added backticks, quotes when necessary, formatted comments to avoid 
> jaggedness.
> 
> 
> Diffs
> -
> 
>   include/mesos/authorizer/authorizer.hpp 
> 19f6e1a2d025bf6ff07f515b10d41e8a48d7d0b4 
>   src/master/main.cpp e00f878770f3e0bddae5a137b50a00822d154e2c 
> 
> Diff: https://reviews.apache.org/r/41444/diff/
> 
> 
> Testing
> ---
> 
> None: not a functional change.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 41444: Cleaned up Authorizer interface.

[Alexander Rukletsov]

> On Dec. 18, 2015, 8:42 a.m., Adam B wrote:
> > Thank you for cleaning this up. It looked like an overwhelming amount of 
> > documentation for what is not really that complex of an API. It still looks 
> > a bit verbose/repetitive, so I've made some suggestions of what else to cut 
> > out.
> > I guess we're still waiting on the ACLs for create/remove persistent 
> > volumes, in MESOS-4179

... and set/remove quota, which are all committed now, except remove quota.


> On Dec. 18, 2015, 8:42 a.m., Adam B wrote:
> > include/mesos/authorizer/authorizer.hpp, lines 64-65
> > 
> >
> > How formal. I would think you could get away with
> > s/the "authorizer.proto" file/"authorizor.proto"/
> > s|the "docs/authorization.md"|"docs/authorization.md"|

Not sure what the second substitution means, but being inspired by the first 
sentence ("How formal"), I killed most of the comment : ).


> On Dec. 18, 2015, 8:42 a.m., Adam B wrote:
> > include/mesos/authorizer/authorizer.hpp, line 75
> > 
> >
> > What is "it"? Are we removing the initialize function, the acls 
> > parameter, or what?
> > 
> > This seems very related to the first paragraph "Only relevant..." which 
> > should not be the first paragraph in the doxygen, since it is in no way a 
> > summary of the method.

Good point! I moved the first paragraph here and refactored the comment.


> On Dec. 18, 2015, 8:42 a.m., Adam B wrote:
> > include/mesos/authorizer/authorizer.hpp, lines 114-116
> > 
> >
> > You can probably shorten this here and everywhere by just saying "A 
> > failed future indicates a problem processing the request; the request can 
> > be retried."

Till suggested the following in [one of the 
reviews](https://reviews.apache.org/r/40346/): "A failed future however 
indicates a problem processing the request and the request can be retried." 
Almost identical to your proposal.


> On Dec. 18, 2015, 8:42 a.m., Adam B wrote:
> > include/mesos/authorizer/authorizer.hpp, line 126
> > 
> >
> > s/RunTask/ShutdownFramework/

Good catch!


> On Dec. 18, 2015, 8:42 a.m., Adam B wrote:
> > include/mesos/authorizer/authorizer.hpp, line 144
> > 
> >
> > s/reserve particular resources/reserve resources/ since the only values 
> > currently allowed for `resources` are ANY or NONE.

I'd say, that's an implementation detail of `LocalAuthorizer`; AFAIK, we do not 
enforce it anywhere. I'm fine with leaving a `NOTE` though. What do you think?


> On Dec. 18, 2015, 8:42 a.m., Adam B wrote:
> > include/mesos/authorizer/authorizer.hpp, line 153
> > 
> >
> > s/reserve one or more types of resources/reserve resources/

See above.


> On Dec. 18, 2015, 8:42 a.m., Adam B wrote:
> > include/mesos/authorizer/authorizer.hpp, lines 155-156
> > 
> >
> > s/reserve the types of resources contained in the request/reserve 
> > resources/

See above.


- Alexander


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


On Dec. 21, 2015, 3:42 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41444/
> ---
> 
> (Updated Dec. 21, 2015, 3:42 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, Greg Mann, Jan Schlicht, 
> and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Extract a repetitive part of the function comments into a class comment. 
> Added backticks, quotes when necessary, formatted comments to avoid 
> jaggedness.
> 
> 
> Diffs
> -
> 
>   include/mesos/authorizer/authorizer.hpp 
> 19f6e1a2d025bf6ff07f515b10d41e8a48d7d0b4 
>   src/master/main.cpp e00f878770f3e0bddae5a137b50a00822d154e2c 
> 
> Diff: https://reviews.apache.org/r/41444/diff/
> 
> 
> Testing
> ---
> 
> None: not a functional change.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 41444: Cleaned up Authorizer interface.

[Alexander Rukletsov]

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

(Updated Dec. 21, 2015, 3:42 p.m.)


Review request for mesos, Adam B, Alexander Rojas, Greg Mann, Jan Schlicht, and 
Till Toenshoff.


Repository: mesos


Description
---

Extract a repetitive part of the function comments into a class comment. Added 
backticks, quotes when necessary, formatted comments to avoid jaggedness.


Diffs (updated)
-

  include/mesos/authorizer/authorizer.hpp 
19f6e1a2d025bf6ff07f515b10d41e8a48d7d0b4 
  src/master/main.cpp e00f878770f3e0bddae5a137b50a00822d154e2c 

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


Testing
---

None: not a functional change.


Thanks,

Alexander Rukletsov

Re: Review Request 41472: Disallowed dynamic reservations for roles not on the role whitelist.

[Michael Park]

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



src/master/http.cpp (line 1001)


We actually don't need this `if` statement. Although the `role` field is 
marked `optional`, we "fill it in" with the default value of `"*"` if 
unspecified by simply calling `role()`. `roleWhitelist` always has `"*"`.

https://github.com/apache/mesos/blob/master/src/master/master.cpp#L564-L565


- Michael Park


On Dec. 16, 2015, 10:48 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41472/
> ---
> 
> (Updated Dec. 16, 2015, 10:48 p.m.)
> 
> 
> Review request for mesos, Adam B and Michael Park.
> 
> 
> Bugs: MESOS-4143
> https://issues.apache.org/jira/browse/MESOS-4143
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Also added a test that dynamic reservations via the "/reserve" endpoint are
> allowed when using implicit roles.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 8dfd67f7d51b8395953d6beb77b2d71bc538eacd 
>   src/tests/reservation_endpoints_tests.cpp 
> b8edd6fafedd4c2221a8d19c1ebc71254071a8c7 
> 
> Diff: https://reviews.apache.org/r/41472/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 41608: Removed extra '; ' after inline class function definitions.

[Michael Park]

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

Ship it!


Ship It!

- Michael Park


On Dec. 21, 2015, 4:48 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41608/
> ---
> 
> (Updated Dec. 21, 2015, 4:48 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, and Neil Conway.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed extra ';' after inline class function definitions.
> 
> 
> Diffs
> -
> 
>   include/mesos/hook.hpp 9b20ac752ff5517213c6ea3e56c98046c24295b6 
>   src/docker/docker.hpp 4cb7a4e43c01f490643486d3b8797018b9787179 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.hpp 
> cbb94077d46d7b87ffc09b72e02269bc16f25f92 
>   src/tests/containerizer/memory_test_helper.hpp 
> 2d91e33662a8a0959254d6acb08d09f3ba70e30f 
> 
> Diff: https://reviews.apache.org/r/41608/diff/
> 
> 
> Testing
> ---
> 
> None: not a functional change.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 39614: Quota: Added Status Validation Tests.

[Joerg Schad]

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

(Updated Dec. 21, 2015, 5:13 p.m.)


Review request for mesos, Alexander Rukletsov and Bernd Mathiske.


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


Repository: mesos


Description
---

Quota: Added Status Validation Tests.


Diffs
-

  src/tests/master_quota_tests.cpp 89130ce9b09afe7c6dd332c8b5278abe0d2674f1 

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


Testing
---

make check


Thanks,

Joerg Schad

Re: Review Request 41002: Logger Module: Introduce the ContainerLogger interface for logging the stdout/stderr of executors and tasks.

[Joseph Wu]

> On Dec. 20, 2015, 6:21 p.m., Benjamin Hindman wrote:
> > include/mesos/slave/container_logger.hpp, lines 85-93
> > 
> >
> > I'm a little confused. Who executes this? The comment makes it read 
> > like there is some particular code that executes this, which makes me 
> > wonder why it needs to be a static function on `ContainerLogger`?
> > 
> > And why don't we want `SubprocessInfo::out/err` to use pipes? I thought 
> > that was the whole point?
> > 
> > Finally, killing the agent seems a little bit harsh. Couldn't the 
> > container just get killed?

This is meant to enforce the `NOTE` in `SubprocessInfo` about how the module 
may not specify `Subprocess::IO::PIPE()` as part of the return value.  

I talked with Joris about either making this a compile-time or run-time check, 
and we decided that a compile-time check would be somewhat distracting compared 
to a `validate` function like this.  (We would need to add a new class that 
provides just `Subprocess::FD` and `Subprocess::PATH`.)

This function will exit on failure because it indicates that the chosen 
`ContainerLogger` breaks that assumption.


- Joseph


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


On Dec. 20, 2015, 4:39 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41002/
> ---
> 
> (Updated Dec. 20, 2015, 4:39 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Cody Maloney, Artem Harutyunyan, 
> and Thomas Rampelberg.
> 
> 
> Bugs: MESOS-4087
> https://issues.apache.org/jira/browse/MESOS-4087
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The container logger is an agent module whose job is to take an executor or 
> task's stdout/stderr, and pipe it to some sink.
> 
> Existing executor and task stdout/stderr are piped into files ("stdout" and 
> "stderr") located in the executor's sandbox directory.
> This module aims to encompass this default behavior as well as allow more 
> sophisticated logging solutions.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/container_logger.hpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/41002/diff/
> 
> 
> Testing
> ---
> 
> This is added to the Makefile later in the review chain.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 39492: Added status endpoint for quota master endpoint.

[Alexander Rukletsov]

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

Ship it!



src/master/master.hpp (line 945)


"quota settings" sounds like a new term we haven't used before. Let's be 
repetitive, but predictable. How about "Returns a list of set quotas".

If you have something in mind how the response will be extended, feel free 
to add a `TODO`.



src/master/quota_handler.cpp (line 474)


s/entry/an entry?


- Alexander Rukletsov


On Dec. 21, 2015, 2:56 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39492/
> ---
> 
> (Updated Dec. 21, 2015, 2:56 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Bernd Mathiske, and Joris Van 
> Remoortere.
> 
> 
> Bugs: MESOS-4013
> https://issues.apache.org/jira/browse/MESOS-4013
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added status handling for quota master endpoint.
> 
> 
> Diffs
> -
> 
>   include/mesos/quota/quota.proto 03e816dcd4dead8326731ac221df7354c0610fed 
>   src/master/master.hpp 8af82a0bbc2038e18180136c82cbaeeacc7b3526 
>   src/master/quota_handler.cpp 0217149a865ede751b3a03fe40b2d91b487b7b10 
> 
> Diff: https://reviews.apache.org/r/39492/diff/
> 
> 
> Testing
> ---
> 
> Tests are in next Review.
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>