Review Request 29329: Add executor for docker containerizer

[Timothy Chen]

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

Review request for mesos and Benjamin Hindman.


Repository: mesos-git


Description
---

Add executor for docker containerizer, replaces the usage of command executor


Diffs
-

  src/Makefile.am 6f132b5a66e117c2a907763217bfafe1fce1b7a0 
  src/docker/executor.cpp PRE-CREATION 

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


Testing
---

make check


Thanks,

Timothy Chen

Re: Review Request 29329: Add executor for docker containerizer

[Vinod Kone]

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


Hey Tim. Made some comments on the doc. Mainly, I'm interested in understanding 
how the shortcomings you discussed in the doc will be addressed for custom 
executor scenarios. Also, can you create a JIRA ticket and attach?

- Vinod Kone


On Jan. 17, 2015, 1:26 a.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29329/
> ---
> 
> (Updated Jan. 17, 2015, 1:26 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Bernd Mathiske.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add executor for docker containerizer, replaces the usage of command executor
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 07bea1f 
>   src/docker/executor.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/29329/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 29329: Add executor for docker containerizer

[Timothy Chen]

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

(Updated April 7, 2015, 12:34 a.m.)


Review request for mesos, Benjamin Hindman and Bernd Mathiske.


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


Repository: mesos


Description
---

Add executor for docker containerizer, replaces the usage of command executor


Diffs (updated)
-

  src/Makefile.am 9c01f5d6c692f835100e7cade928748cc4763cc8 
  src/docker/executor.cpp PRE-CREATION 

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


Testing
---

make check


Thanks,

Timothy Chen

Re: Review Request 29329: Add executor for docker containerizer

[Timothy Chen]

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

(Updated April 7, 2015, 12:39 a.m.)


Review request for mesos, Benjamin Hindman and Bernd Mathiske.


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


Repository: mesos


Description
---

Add executor for docker containerizer, replaces the usage of command executor


Diffs
-

  src/Makefile.am 9c01f5d6c692f835100e7cade928748cc4763cc8 
  src/docker/executor.cpp PRE-CREATION 

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


Testing
---

make check


Thanks,

Timothy Chen

Re: Review Request 29329: Add executor for docker containerizer

[Timothy Chen]

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

(Updated Dec. 22, 2014, 11:47 p.m.)


Review request for mesos and Benjamin Hindman.


Repository: mesos-git


Description
---

Add executor for docker containerizer, replaces the usage of command executor


Diffs
-

  src/Makefile.am 6f132b5a66e117c2a907763217bfafe1fce1b7a0 
  src/docker/executor.cpp PRE-CREATION 

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


Testing
---

make check


Thanks,

Timothy Chen

Re: Review Request 29329: Add executor for docker containerizer

[Timothy Chen]

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

(Updated Jan. 6, 2015, 10:16 p.m.)


Review request for mesos, Benjamin Hindman and Bernd Mathiske.


Repository: mesos-git


Description
---

Add executor for docker containerizer, replaces the usage of command executor


Diffs
-

  src/Makefile.am 6f132b5a66e117c2a907763217bfafe1fce1b7a0 
  src/docker/executor.cpp PRE-CREATION 

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


Testing
---

make check


Thanks,

Timothy Chen

Re: Review Request 29329: Add executor for docker containerizer

[Bernd Mathiske]

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


What bug ID does this refer to?

Any tests? Does this work already?


src/docker/executor.cpp


The "_" is unnecessary.



src/docker/executor.cpp


This is a major constraint that should be mentioned at the top of the file.



src/docker/executor.cpp


An important constraint/assumption like this should be mentioned at the top 
of the file.



src/docker/executor.cpp


This cryptic statement is worth a comment (maybe mention "man waitpid"? Or 
mention higher up in this code that the status code is based on waitpid, which 
is not immediately evident in this scope.). 

Also maybe add some more output to increase human parsing ease. For 
example: CHECK(WIFEXITED(s) || WIFSIGNALED(s)) << "status code: " << s;

(I know you just copied this particular stretch of code (from 
executor.cpp). Is there a general rule that code can or should remain 
suboptimal if copied? Please feel free to drop this issue if this is the case.)



src/docker/executor.cpp


This stems from copied code. Any ideas how to do this properly?


- Bernd Mathiske


On Jan. 6, 2015, 2:16 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29329/
> ---
> 
> (Updated Jan. 6, 2015, 2:16 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Bernd Mathiske.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> Add executor for docker containerizer, replaces the usage of command executor
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 6f132b5a66e117c2a907763217bfafe1fce1b7a0 
>   src/docker/executor.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/29329/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 29329: Add executor for docker containerizer

[Timothy Chen]

> On Jan. 8, 2015, 11:27 a.m., Bernd Mathiske wrote:
> > What bug ID does this refer to?
> > 
> > Any tests? Does this work already?

This is a refactor and able to easier recover when the slave is running under a 
docker container.
All the docker tests inheriently test this executor since it's part of 
launching a docker containter.


- Timothy


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


On Jan. 6, 2015, 10:16 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29329/
> ---
> 
> (Updated Jan. 6, 2015, 10:16 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Bernd Mathiske.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> Add executor for docker containerizer, replaces the usage of command executor
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 6f132b5a66e117c2a907763217bfafe1fce1b7a0 
>   src/docker/executor.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/29329/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 29329: Add executor for docker containerizer

[Timothy Chen]

> On Jan. 8, 2015, 11:27 a.m., Bernd Mathiske wrote:
> > src/docker/executor.cpp, line 189
> > 
> >
> > This stems from copied code. Any ideas how to do this properly?

We probably need to add some visibility under libprocess, or perhaps a send 
that returns a future that we want wait on.
Either way, it's a TODO that should be fixed altogether with other executors in 
another review.


- Timothy


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


On Jan. 6, 2015, 10:16 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29329/
> ---
> 
> (Updated Jan. 6, 2015, 10:16 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Bernd Mathiske.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> Add executor for docker containerizer, replaces the usage of command executor
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 6f132b5a66e117c2a907763217bfafe1fce1b7a0 
>   src/docker/executor.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/29329/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 29329: Add executor for docker containerizer

[Timothy Chen]

> On Jan. 8, 2015, 11:27 a.m., Bernd Mathiske wrote:
> > src/docker/executor.cpp, line 165
> > 
> >
> > This cryptic statement is worth a comment (maybe mention "man waitpid"? 
> > Or mention higher up in this code that the status code is based on waitpid, 
> > which is not immediately evident in this scope.). 
> > 
> > Also maybe add some more output to increase human parsing ease. For 
> > example: CHECK(WIFEXITED(s) || WIFSIGNALED(s)) << "status code: " << s;
> > 
> > (I know you just copied this particular stretch of code (from 
> > executor.cpp). Is there a general rule that code can or should remain 
> > suboptimal if copied? Please feel free to drop this issue if this is the 
> > case.)

It is a general assumption across everywhere using subprocess, I can put it 
here again.


- Timothy


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


On Jan. 6, 2015, 10:16 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29329/
> ---
> 
> (Updated Jan. 6, 2015, 10:16 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Bernd Mathiske.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> Add executor for docker containerizer, replaces the usage of command executor
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 6f132b5a66e117c2a907763217bfafe1fce1b7a0 
>   src/docker/executor.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/29329/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 29329: Add executor for docker containerizer

[Timothy Chen]

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

(Updated Jan. 9, 2015, 1:20 a.m.)


Review request for mesos, Benjamin Hindman and Bernd Mathiske.


Repository: mesos-git


Description
---

Add executor for docker containerizer, replaces the usage of command executor


Diffs (updated)
-

  src/Makefile.am fc0c322 
  src/docker/executor.cpp PRE-CREATION 

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


Testing
---

make check


Thanks,

Timothy Chen

Re: Review Request 29329: Add executor for docker containerizer

[Bernd Mathiske]

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

Ship it!


Ship It!

- Bernd Mathiske


On Jan. 8, 2015, 5:20 p.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29329/
> ---
> 
> (Updated Jan. 8, 2015, 5:20 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Bernd Mathiske.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> Add executor for docker containerizer, replaces the usage of command executor
> 
> 
> Diffs
> -
> 
>   src/Makefile.am fc0c322 
>   src/docker/executor.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/29329/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 29329: Add executor for docker containerizer

[Timothy Chen]

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

(Updated Jan. 17, 2015, 1:26 a.m.)


Review request for mesos, Benjamin Hindman and Bernd Mathiske.


Changes
---

rebased


Repository: mesos-git


Description
---

Add executor for docker containerizer, replaces the usage of command executor


Diffs (updated)
-

  src/Makefile.am 07bea1f 
  src/docker/executor.cpp PRE-CREATION 

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


Testing
---

make check


Thanks,

Timothy Chen

Re: Review Request 29329: Add executor for docker containerizer

[Ben Mahler]

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


How are we going to manage the duplication across the command executor and the 
docker executor?


src/docker/executor.cpp


Just passing by, this says the executor launches the container, but below 
it just looks like it waits for it to terminate via `docker wait`, or am I 
missing something?



src/docker/executor.cpp


Probably warrants a comment, what does `"exit `...`" do?`


- Ben Mahler


On Jan. 17, 2015, 1:26 a.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29329/
> ---
> 
> (Updated Jan. 17, 2015, 1:26 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Bernd Mathiske.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> Add executor for docker containerizer, replaces the usage of command executor
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 07bea1f 
>   src/docker/executor.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/29329/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 29329: Add executor for docker containerizer

[Timothy Chen]

> On Jan. 17, 2015, 1:39 a.m., Ben Mahler wrote:
> > How are we going to manage the duplication across the command executor and 
> > the docker executor?

I think I'm going to leave them seperate as they most likely will grow 
independent in tangent.
The docker executor is solely responsible for launching docker containers, and 
have different assumptions.


> On Jan. 17, 2015, 1:39 a.m., Ben Mahler wrote:
> > src/docker/executor.cpp, lines 50-53
> > 
> >
> > Just passing by, this says the executor launches the container, but 
> > below it just looks like it waits for it to terminate via `docker wait`, or 
> > am I missing something?

Ah sorry, I think it's confusing that I later changed the executor to actually 
launch containers. Somehow I fixed the comment in this commit.


> On Jan. 17, 2015, 1:39 a.m., Ben Mahler wrote:
> > src/docker/executor.cpp, line 103
> > 
> >
> > Probably warrants a comment, what does `"exit `...`" do?`

This is actually going to removed in a later commit. I posted my whole commit 
chain.


- Timothy


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


On Jan. 17, 2015, 1:26 a.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29329/
> ---
> 
> (Updated Jan. 17, 2015, 1:26 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Bernd Mathiske.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> Add executor for docker containerizer, replaces the usage of command executor
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 07bea1f 
>   src/docker/executor.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/29329/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 29329: Add executor for docker containerizer

[Bill Farner]

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


Is there a shortcoming in the executor API that prevents this from being an 
independent binary?

- Bill Farner


On Jan. 17, 2015, 1:26 a.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29329/
> ---
> 
> (Updated Jan. 17, 2015, 1:26 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Bernd Mathiske.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add executor for docker containerizer, replaces the usage of command executor
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 07bea1f 
>   src/docker/executor.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/29329/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 29329: Add executor for docker containerizer

[Timothy Chen]

> On Feb. 3, 2015, 11:23 p.m., Bill Farner wrote:
> > Is there a shortcoming in the executor API that prevents this from being an 
> > independent binary?

Hi Bill, this is going to be a binary on itself just like command exexutor. Do 
you have something else in mind?


- Timothy


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


On Jan. 17, 2015, 1:26 a.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29329/
> ---
> 
> (Updated Jan. 17, 2015, 1:26 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Bernd Mathiske.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add executor for docker containerizer, replaces the usage of command executor
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 07bea1f 
>   src/docker/executor.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/29329/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 29329: Add executor for docker containerizer

[Ben Mahler]

> On Jan. 17, 2015, 1:39 a.m., Ben Mahler wrote:
> > How are we going to manage the duplication across the command executor and 
> > the docker executor?
> 
> Timothy Chen wrote:
> I think I'm going to leave them seperate as they most likely will grow 
> independent in tangent.
> The docker executor is solely responsible for launching docker 
> containers, and have different assumptions.

Is there a design doc about this docker architecture decision? It's a bit hard 
to see the context for this, and how this will play out for other executors 
that frameworks want to run inside docker, are you planning to support that, or 
do users have to bake-in their executors..?


- Ben


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


On Jan. 17, 2015, 1:26 a.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29329/
> ---
> 
> (Updated Jan. 17, 2015, 1:26 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Bernd Mathiske.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add executor for docker containerizer, replaces the usage of command executor
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 07bea1f 
>   src/docker/executor.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/29329/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 29329: Add executor for docker containerizer

[Steve Niemitz]

> On Jan. 17, 2015, 1:39 a.m., Ben Mahler wrote:
> > How are we going to manage the duplication across the command executor and 
> > the docker executor?
> 
> Timothy Chen wrote:
> I think I'm going to leave them seperate as they most likely will grow 
> independent in tangent.
> The docker executor is solely responsible for launching docker 
> containers, and have different assumptions.
> 
> Ben Mahler wrote:
> Is there a design doc about this docker architecture decision? It's a bit 
> hard to see the context for this, and how this will play out for other 
> executors that frameworks want to run inside docker, are you planning to 
> support that, or do users have to bake-in their executors..?

I'm curious about this too.  I'm having trouble understanding how other 
executors will run inside containers as they did previously after this change.  
I'd love to see a design doc about how this proposed system would work.


- Steve


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


On Jan. 17, 2015, 1:26 a.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29329/
> ---
> 
> (Updated Jan. 17, 2015, 1:26 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Bernd Mathiske.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add executor for docker containerizer, replaces the usage of command executor
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 07bea1f 
>   src/docker/executor.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/29329/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 29329: Add executor for docker containerizer

[Bill Farner]

> On Feb. 3, 2015, 11:23 p.m., Bill Farner wrote:
> > Is there a shortcoming in the executor API that prevents this from being an 
> > independent binary?
> 
> Timothy Chen wrote:
> Hi Bill, this is going to be a binary on itself just like command 
> exexutor. Do you have something else in mind?

Sorry for my late reply, gmail filter mishap on my end.

It's my understanding that this will not be a standalone binary, but a part of 
the slave binary (which, IIUC, is true of the command executor as well).  If 
this is incorrect, please excuse my ignorance and read no further.

The distinction worrying me is that this is creating a first-class executor 
implementation within mesos itself.  I agree that in some respects this is not 
dissimilar from the command executor, though i think coupling to a shell is not 
as extreme as a third-party container system.  This sets a risky precedent for 
mesos, as it allows the project to make short-sighted decisions coupling to the 
first-class executor, and fail to generalize its APIs in ways that limits other 
executors when compared to those built in to the slave.


- Bill


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


On Jan. 17, 2015, 1:26 a.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29329/
> ---
> 
> (Updated Jan. 17, 2015, 1:26 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Bernd Mathiske.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add executor for docker containerizer, replaces the usage of command executor
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 07bea1f 
>   src/docker/executor.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/29329/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 29329: Add executor for docker containerizer

[Timothy Chen]

> On Feb. 3, 2015, 11:23 p.m., Bill Farner wrote:
> > Is there a shortcoming in the executor API that prevents this from being an 
> > independent binary?
> 
> Timothy Chen wrote:
> Hi Bill, this is going to be a binary on itself just like command 
> exexutor. Do you have something else in mind?
> 
> Bill Farner wrote:
> Sorry for my late reply, gmail filter mishap on my end.
> 
> It's my understanding that this will not be a standalone binary, but a 
> part of the slave binary (which, IIUC, is true of the command executor as 
> well).  If this is incorrect, please excuse my ignorance and read no further.
> 
> The distinction worrying me is that this is creating a first-class 
> executor implementation within mesos itself.  I agree that in some respects 
> this is not dissimilar from the command executor, though i think coupling to 
> a shell is not as extreme as a third-party container system.  This sets a 
> risky precedent for mesos, as it allows the project to make short-sighted 
> decisions coupling to the first-class executor, and fail to generalize its 
> APIs in ways that limits other executors when compared to those built in to 
> the slave.

Hi Bill, in my mind this isn't changing the executor API, it's implementing the 
API that works the containerizer system we're integrating with. Unfortunately a 
container system (or any) has limitations that we will have to overcome, and 
Mesos being a pluggable containerizer system will have to integrate with the 
limitations in mind.

This isn't really a short-sighted decision as we're not changing the API to 
cope with Docker, we need to implement a specific implementation same as the 
Docker containerizer itself which is a implementation of the containerizer 
interface.

And really thinking about the nature of the executor in Mesos, we're only 
creating the necessary ones that we manage ourselves, which is command executor 
for the Mesos containerizer, and a docker executor for the Docker 
containerizer, and we still allow custom executors via the API.


- Timothy


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


On Jan. 17, 2015, 1:26 a.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29329/
> ---
> 
> (Updated Jan. 17, 2015, 1:26 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Bernd Mathiske.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add executor for docker containerizer, replaces the usage of command executor
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 07bea1f 
>   src/docker/executor.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/29329/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 29329: Add executor for docker containerizer

[Timothy Chen]

> On Jan. 17, 2015, 1:39 a.m., Ben Mahler wrote:
> > How are we going to manage the duplication across the command executor and 
> > the docker executor?
> 
> Timothy Chen wrote:
> I think I'm going to leave them seperate as they most likely will grow 
> independent in tangent.
> The docker executor is solely responsible for launching docker 
> containers, and have different assumptions.
> 
> Ben Mahler wrote:
> Is there a design doc about this docker architecture decision? It's a bit 
> hard to see the context for this, and how this will play out for other 
> executors that frameworks want to run inside docker, are you planning to 
> support that, or do users have to bake-in their executors..?
> 
> Steve Niemitz wrote:
> I'm curious about this too.  I'm having trouble understanding how other 
> executors will run inside containers as they did previously after this 
> change.  I'd love to see a design doc about how this proposed system would 
> work.

I've updated the Docker containerizer update google doc to hold a high level 
diagram with the new Docker executor.
https://docs.google.com/a/mesosphere.io/document/d/1_1oLHXg_aHj_fYCzsjYwox9xvIYNAKIeVjO5BFxsUGI/edit


- Timothy


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


On Jan. 17, 2015, 1:26 a.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29329/
> ---
> 
> (Updated Jan. 17, 2015, 1:26 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Bernd Mathiske.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add executor for docker containerizer, replaces the usage of command executor
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 07bea1f 
>   src/docker/executor.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/29329/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 29329: Add executor for docker containerizer

[Vinod Kone]

Thanks Tim for the doc. Can you open it up for comments (it's currently
view only)?

On Thu, Feb 19, 2015 at 5:15 PM, Timothy Chen  wrote:

>
>
> > On Jan. 17, 2015, 1:39 a.m., Ben Mahler wrote:
> > > How are we going to manage the duplication across the command executor
> and the docker executor?
> >
> > Timothy Chen wrote:
> > I think I'm going to leave them seperate as they most likely will
> grow independent in tangent.
> > The docker executor is solely responsible for launching docker
> containers, and have different assumptions.
> >
> > Ben Mahler wrote:
> > Is there a design doc about this docker architecture decision? It's
> a bit hard to see the context for this, and how this will play out for
> other executors that frameworks want to run inside docker, are you planning
> to support that, or do users have to bake-in their executors..?
> >
> > Steve Niemitz wrote:
> > I'm curious about this too.  I'm having trouble understanding how
> other executors will run inside containers as they did previously after
> this change.  I'd love to see a design doc about how this proposed system
> would work.
>
> I've updated the Docker containerizer update google doc to hold a high
> level diagram with the new Docker executor.
>
> https://docs.google.com/a/mesosphere.io/document/d/1_1oLHXg_aHj_fYCzsjYwox9xvIYNAKIeVjO5BFxsUGI/edit
>
>
> - Timothy
>
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29329/#review68533
> ---
>
>
> On Jan. 17, 2015, 1:26 a.m., Timothy Chen wrote:
> >
> > ---
> > This is an automatically generated e-mail. To reply, visit:
> > https://reviews.apache.org/r/29329/
> > ---
> >
> > (Updated Jan. 17, 2015, 1:26 a.m.)
> >
> >
> > Review request for mesos, Benjamin Hindman and Bernd Mathiske.
> >
> >
> > Repository: mesos
> >
> >
> > Description
> > ---
> >
> > Add executor for docker containerizer, replaces the usage of command
> executor
> >
> >
> > Diffs
> > -
> >
> >   src/Makefile.am 07bea1f
> >   src/docker/executor.cpp PRE-CREATION
> >
> > Diff: https://reviews.apache.org/r/29329/diff/
> >
> >
> > Testing
> > ---
> >
> > make check
> >
> >
> > Thanks,
> >
> > Timothy Chen
> >
> >
>
>

Re: Review Request 29329: Add executor for docker containerizer

[Timothy Chen]

Sorry about that! I just relaxed the perms so can anyone can edit/comment.

Tim

On Thu, Feb 19, 2015 at 7:58 PM, Vinod Kone  wrote:
> Thanks Tim for the doc. Can you open it up for comments (it's currently view
> only)?
>
> On Thu, Feb 19, 2015 at 5:15 PM, Timothy Chen  wrote:
>>
>>
>>
>> > On Jan. 17, 2015, 1:39 a.m., Ben Mahler wrote:
>> > > How are we going to manage the duplication across the command executor
>> > > and the docker executor?
>> >
>> > Timothy Chen wrote:
>> > I think I'm going to leave them seperate as they most likely will
>> > grow independent in tangent.
>> > The docker executor is solely responsible for launching docker
>> > containers, and have different assumptions.
>> >
>> > Ben Mahler wrote:
>> > Is there a design doc about this docker architecture decision? It's
>> > a bit hard to see the context for this, and how this will play out for 
>> > other
>> > executors that frameworks want to run inside docker, are you planning to
>> > support that, or do users have to bake-in their executors..?
>> >
>> > Steve Niemitz wrote:
>> > I'm curious about this too.  I'm having trouble understanding how
>> > other executors will run inside containers as they did previously after 
>> > this
>> > change.  I'd love to see a design doc about how this proposed system would
>> > work.
>>
>> I've updated the Docker containerizer update google doc to hold a high
>> level diagram with the new Docker executor.
>>
>> https://docs.google.com/a/mesosphere.io/document/d/1_1oLHXg_aHj_fYCzsjYwox9xvIYNAKIeVjO5BFxsUGI/edit
>>
>>
>> - Timothy
>>
>>
>> ---
>> This is an automatically generated e-mail. To reply, visit:
>> https://reviews.apache.org/r/29329/#review68533
>> ---
>>
>>
>> On Jan. 17, 2015, 1:26 a.m., Timothy Chen wrote:
>> >
>> > ---
>> > This is an automatically generated e-mail. To reply, visit:
>> > https://reviews.apache.org/r/29329/
>> > ---
>> >
>> > (Updated Jan. 17, 2015, 1:26 a.m.)
>> >
>> >
>> > Review request for mesos, Benjamin Hindman and Bernd Mathiske.
>> >
>> >
>> > Repository: mesos
>> >
>> >
>> > Description
>> > ---
>> >
>> > Add executor for docker containerizer, replaces the usage of command
>> > executor
>> >
>> >
>> > Diffs
>> > -
>> >
>> >   src/Makefile.am 07bea1f
>> >   src/docker/executor.cpp PRE-CREATION
>> >
>> > Diff: https://reviews.apache.org/r/29329/diff/
>> >
>> >
>> > Testing
>> > ---
>> >
>> > make check
>> >
>> >
>> > Thanks,
>> >
>> > Timothy Chen
>> >
>> >
>>
>