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

2015-12-21 Thread 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 41560: Change Docker::run to take Subprocess::IO instead of Option

2015-12-21 Thread 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 41560: Change Docker::run to take Subprocess::IO instead of Option

2015-12-21 Thread 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 41560: Change Docker::run to take Subprocess::IO instead of Option

2015-12-21 Thread 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 41560: Change Docker::run to take Subprocess::IO instead of Option

2015-12-18 Thread Joseph Wu

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

(Updated Dec. 18, 2015, 3:57 p.m.)


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


Changes
---

Updated "Testing Done" to a partially complete, but currently incorrect manual 
test.


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

---

[WIP] Manual test for launching an executor from an agent launched with 
`--docker_mesos_image=`

On Centos7, prepare the docker image:
```
make

# Run the base Centos7 image with the build directory.
# Make sure you copy/remember the CONTAINER_ID from the shell.
sudo docker run -t -i centos:7 /bin/bash

# Install some shared libraries.
[root@ /]# yum install -y libevent zlib libcurl openssl 
cyrus-sasl cyrus-sasl-md5 apr subversion apr-util

[root@ /]# yum install -y docker

# We're done with edits.
[root@ /]# exit

sudo docker commit -m "Add Mesos runtime dependencies" -a "John Doe" 
 mesos/centos7:dev
```

---

Start the master:
```
# We don't really care where the master is.
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 /home/vagrant/mesos/build:/home/vagrant/mesos/build -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/centos7:dev /bin/bash
[root@centos /]# cd /home/vagrant/mesos/build/
[root@centos /]# bin/mesos-slave.sh --master=127.0.0.1:5050 
--containerizers=docker --docker_mesos_image="mesos/centos7:dev" 
--switch_user=false
```

Run our handy command-task spawning framework:
```
src/docker-no-executor-framework 127.0.0.1:5050
```

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

---

Currently, the above steps lead to the following output in "stderr":
```
/bin/sh: /home/vagrant/mesos/build/src/mesos-docker-executor: No such file or 
directory
```


Thanks,

Joseph Wu



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

2015-12-18 Thread Joseph Wu

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

(Updated Dec. 18, 2015, 2:55 p.m.)


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


Changes
---

Incomplete manual testing of the docker in docker case.


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

---

[WIP] Manual test for launching an executor from an agent launched with 
`--docker_mesos_image=`

On Centos7, prepare the docker image:
```
make

# Run the base Centos7 image with the build directory.
# Make sure you copy/remember the CONTAINER_ID from the shell.
sudo docker run -t -i centos:7 /bin/bash

# Install some shared libraries.
[root@ /]# yum install -y libevent zlib libcurl openssl 
cyrus-sasl cyrus-sasl-md5 apr subversion apr-util

[root@ /]# yum install -y docker

# We're done with edits.
[root@ /]# exit

sudo docker commit -m "Add Mesos runtime dependencies" -a "John Doe" 
 mesos/centos7:dev
```

---

Start the master:
```
# We don't really care where the master is.
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 /home/vagrant/mesos/build:/home/vagrant/mesos/build 
--net=host -i mesos/centos7:dev /bin/bash
[root@centos /]# cd /home/vagrant/mesos/build/
[root@centos /]# bin/mesos-slave.sh --master=127.0.0.1:5050 
--containerizers=docker --docker_mesos_image="mesos/centos7:dev" 
--switch_user=false
```

Run our handy command-task spawning framework:
```
src/docker-no-executor-framework 127.0.0.1:5050
curl localhost:5050/master/state | python -m json.tool
```


Thanks,

Joseph Wu



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

2015-12-18 Thread Mesos ReviewBot

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


Patch looks great!

Reviews applied: [41560]

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

- Mesos ReviewBot


On Dec. 18, 2015, 7:06 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41560/
> ---
> 
> (Updated Dec. 18, 2015, 7:06 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*"
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>



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

2015-12-18 Thread Joseph Wu

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

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


Thanks,

Joseph Wu