Re: Review Request 34137: Add support for container image provisioners.

[Jie Yu]

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

Ship it!


Ship It!

- Jie Yu


On July 12, 2015, 4:47 a.m., Ian Downes wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34137/
> ---
> 
> (Updated July 12, 2015, 4:47 a.m.)
> 
> 
> Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The MesosContainerizer can optionally provision a root filesystem for the 
> containers.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/isolator.hpp ef2205dd7f4619e53e6cca7cac301f874d08c036 
>   src/Makefile.am e5b5d36f0ac160e5a3a9fdc50b31c060a413ce2c 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 3ac2387eabded61c897a5016655aae10cd1bca91 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 47d146125dfd4ea909e7ec9d94f41cfa11d035e5 
>   src/slave/containerizer/provisioner.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner.cpp PRE-CREATION 
>   src/slave/flags.hpp 26c778db2303167369af8675fe0441a00a1e9151 
>   src/slave/flags.cpp 8632677ebbdbfef8ffa45204b6f63a700baff7f3 
>   src/slave/paths.hpp 00476f107ed8233123a6eab0925c9f28aac0a86f 
>   src/slave/paths.cpp 0741616b656e947cb460dd6ee6a9a4852be001c2 
>   src/slave/state.hpp bb0eee78e118210ccd7fcbd909029412ff106128 
>   src/slave/state.cpp f8a9514f52bf9f886171c2a0e674e5a89f8dbea7 
>   src/tests/containerizer_tests.cpp 0cdb2d2a3f19a4835e85c6b040759019b03f051e 
> 
> Diff: https://reviews.apache.org/r/34137/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Ian Downes
> 
>

Re: Review Request 34137: Add support for container image provisioners.

[Jie Yu]

> On July 17, 2015, 1:36 a.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/containerizer.cpp, lines 637-643
> > 
> >
> > No test for checkpointing???

I decided to punt this in this review. Will follow up with a test shortly. The 
idea is to create a TestProvisioner which will prepare a chroot based on 
copying the host filesystem (similar to that in launch_tests.cpp).


- Jie


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


On July 12, 2015, 4:47 a.m., Ian Downes wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34137/
> ---
> 
> (Updated July 12, 2015, 4:47 a.m.)
> 
> 
> Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The MesosContainerizer can optionally provision a root filesystem for the 
> containers.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/isolator.hpp ef2205dd7f4619e53e6cca7cac301f874d08c036 
>   src/Makefile.am e5b5d36f0ac160e5a3a9fdc50b31c060a413ce2c 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 3ac2387eabded61c897a5016655aae10cd1bca91 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 47d146125dfd4ea909e7ec9d94f41cfa11d035e5 
>   src/slave/containerizer/provisioner.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner.cpp PRE-CREATION 
>   src/slave/flags.hpp 26c778db2303167369af8675fe0441a00a1e9151 
>   src/slave/flags.cpp 8632677ebbdbfef8ffa45204b6f63a700baff7f3 
>   src/slave/paths.hpp 00476f107ed8233123a6eab0925c9f28aac0a86f 
>   src/slave/paths.cpp 0741616b656e947cb460dd6ee6a9a4852be001c2 
>   src/slave/state.hpp bb0eee78e118210ccd7fcbd909029412ff106128 
>   src/slave/state.cpp f8a9514f52bf9f886171c2a0e674e5a89f8dbea7 
>   src/tests/containerizer_tests.cpp 0cdb2d2a3f19a4835e85c6b040759019b03f051e 
> 
> Diff: https://reviews.apache.org/r/34137/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Ian Downes
> 
>

Re: Review Request 34137: Add support for container image provisioners.

[Jie Yu]

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



src/slave/containerizer/mesos/containerizer.cpp (lines 185 - 186)


Hum, you put provisioner.cpp under OS_LINUX guard. There'll be a link error 
on OSX since MesosContainerizer supports all platforms.


- Jie Yu


On July 12, 2015, 4:47 a.m., Ian Downes wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34137/
> ---
> 
> (Updated July 12, 2015, 4:47 a.m.)
> 
> 
> Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The MesosContainerizer can optionally provision a root filesystem for the 
> containers.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/isolator.hpp ef2205dd7f4619e53e6cca7cac301f874d08c036 
>   src/Makefile.am e5b5d36f0ac160e5a3a9fdc50b31c060a413ce2c 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 3ac2387eabded61c897a5016655aae10cd1bca91 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 47d146125dfd4ea909e7ec9d94f41cfa11d035e5 
>   src/slave/containerizer/provisioner.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner.cpp PRE-CREATION 
>   src/slave/flags.hpp 26c778db2303167369af8675fe0441a00a1e9151 
>   src/slave/flags.cpp 8632677ebbdbfef8ffa45204b6f63a700baff7f3 
>   src/slave/paths.hpp 00476f107ed8233123a6eab0925c9f28aac0a86f 
>   src/slave/paths.cpp 0741616b656e947cb460dd6ee6a9a4852be001c2 
>   src/slave/state.hpp bb0eee78e118210ccd7fcbd909029412ff106128 
>   src/slave/state.cpp f8a9514f52bf9f886171c2a0e674e5a89f8dbea7 
>   src/tests/containerizer_tests.cpp 0cdb2d2a3f19a4835e85c6b040759019b03f051e 
> 
> Diff: https://reviews.apache.org/r/34137/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Ian Downes
> 
>

Re: Review Request 34137: Add support for container image provisioners.

[Jie Yu]

> On July 17, 2015, 1:36 a.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/containerizer.cpp, line 630
> > 
> >
> > Hum, looks like a bug since, for example, slaveId is a reference and 
> > will be invalid when the lambda is called. In general, I think we should 
> > avoid using [=] for lambdas because its dangeous!
> > 
> > I would prefer we resort to our old style defer style (e.g., introduce 
> > `_provision`).
> 
> Jiang Yan Xu wrote:
> [=] captures slaveId by value (copy) so it won't be invalid?
> 
> But after when to use lambdas, I think this is a good point and we should 
> establish some best practices. 
> Google style guide has these guidelines: 
> https://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Lambda_expressions
>  
> 
> Avoiding default captures is one of them; limiting the length of lambdas 
> is another.
> 
> We should document these, at least reference Google's.
> 
> And how about lambdas that simply call another method vs. bind? :)

Aha, good call. I checked the C++11 standard, $5.1.2/14 says that capturing a 
variable of reference type by copy will create a copy of the value referenced 
instead of creating a copy of the reference. Thus the lambda will have its own 
copy of the value that the reference was referencing when it was created.

However, looks like gcc has a bug related to capturing a reference type by 
using [=] so we should probably avoid that as much as possible
http://stackoverflow.com/questions/6529177/capturing-reference-variable-by-copy-in-c0x-lambda


- Jie


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


On July 12, 2015, 4:47 a.m., Ian Downes wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34137/
> ---
> 
> (Updated July 12, 2015, 4:47 a.m.)
> 
> 
> Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The MesosContainerizer can optionally provision a root filesystem for the 
> containers.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/isolator.hpp ef2205dd7f4619e53e6cca7cac301f874d08c036 
>   src/Makefile.am e5b5d36f0ac160e5a3a9fdc50b31c060a413ce2c 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 3ac2387eabded61c897a5016655aae10cd1bca91 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 47d146125dfd4ea909e7ec9d94f41cfa11d035e5 
>   src/slave/containerizer/provisioner.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner.cpp PRE-CREATION 
>   src/slave/flags.hpp 26c778db2303167369af8675fe0441a00a1e9151 
>   src/slave/flags.cpp 8632677ebbdbfef8ffa45204b6f63a700baff7f3 
>   src/slave/paths.hpp 00476f107ed8233123a6eab0925c9f28aac0a86f 
>   src/slave/paths.cpp 0741616b656e947cb460dd6ee6a9a4852be001c2 
>   src/slave/state.hpp bb0eee78e118210ccd7fcbd909029412ff106128 
>   src/slave/state.cpp f8a9514f52bf9f886171c2a0e674e5a89f8dbea7 
>   src/tests/containerizer_tests.cpp 0cdb2d2a3f19a4835e85c6b040759019b03f051e 
> 
> Diff: https://reviews.apache.org/r/34137/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Ian Downes
> 
>

Re: Review Request 34137: Add support for container image provisioners.

[Timothy Chen]

> On July 17, 2015, 1:36 a.m., Jie Yu wrote:
> > src/slave/containerizer/provisioner.cpp, line 24
> > 
> >
> > Where is this? This won't compile!

You're right it won't, wierd it compiled for me :( let's fix this!


- Timothy


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


On July 12, 2015, 4:47 a.m., Ian Downes wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34137/
> ---
> 
> (Updated July 12, 2015, 4:47 a.m.)
> 
> 
> Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The MesosContainerizer can optionally provision a root filesystem for the 
> containers.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/isolator.hpp ef2205dd7f4619e53e6cca7cac301f874d08c036 
>   src/Makefile.am e5b5d36f0ac160e5a3a9fdc50b31c060a413ce2c 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 3ac2387eabded61c897a5016655aae10cd1bca91 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 47d146125dfd4ea909e7ec9d94f41cfa11d035e5 
>   src/slave/containerizer/provisioner.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner.cpp PRE-CREATION 
>   src/slave/flags.hpp 26c778db2303167369af8675fe0441a00a1e9151 
>   src/slave/flags.cpp 8632677ebbdbfef8ffa45204b6f63a700baff7f3 
>   src/slave/paths.hpp 00476f107ed8233123a6eab0925c9f28aac0a86f 
>   src/slave/paths.cpp 0741616b656e947cb460dd6ee6a9a4852be001c2 
>   src/slave/state.hpp bb0eee78e118210ccd7fcbd909029412ff106128 
>   src/slave/state.cpp f8a9514f52bf9f886171c2a0e674e5a89f8dbea7 
>   src/tests/containerizer_tests.cpp 0cdb2d2a3f19a4835e85c6b040759019b03f051e 
> 
> Diff: https://reviews.apache.org/r/34137/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Ian Downes
> 
>

Re: Review Request 34137: Add support for container image provisioners.

[Jiang Yan Xu]

> On July 16, 2015, 6:36 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/containerizer.cpp, line 630
> > 
> >
> > Hum, looks like a bug since, for example, slaveId is a reference and 
> > will be invalid when the lambda is called. In general, I think we should 
> > avoid using [=] for lambdas because its dangeous!
> > 
> > I would prefer we resort to our old style defer style (e.g., introduce 
> > `_provision`).

[=] captures slaveId by value (copy) so it won't be invalid?

But after when to use lambdas, I think this is a good point and we should 
establish some best practices. 
Google style guide has these guidelines: 
https://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Lambda_expressions
 

Avoiding default captures is one of them; limiting the length of lambdas is 
another.

We should document these, at least reference Google's.

And how about lambdas that simply call another method vs. bind? :)


- Jiang Yan


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


On July 11, 2015, 9:47 p.m., Ian Downes wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34137/
> ---
> 
> (Updated July 11, 2015, 9:47 p.m.)
> 
> 
> Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The MesosContainerizer can optionally provision a root filesystem for the 
> containers.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/isolator.hpp ef2205dd7f4619e53e6cca7cac301f874d08c036 
>   src/Makefile.am e5b5d36f0ac160e5a3a9fdc50b31c060a413ce2c 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 3ac2387eabded61c897a5016655aae10cd1bca91 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 47d146125dfd4ea909e7ec9d94f41cfa11d035e5 
>   src/slave/containerizer/provisioner.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner.cpp PRE-CREATION 
>   src/slave/flags.hpp 26c778db2303167369af8675fe0441a00a1e9151 
>   src/slave/flags.cpp 8632677ebbdbfef8ffa45204b6f63a700baff7f3 
>   src/slave/paths.hpp 00476f107ed8233123a6eab0925c9f28aac0a86f 
>   src/slave/paths.cpp 0741616b656e947cb460dd6ee6a9a4852be001c2 
>   src/slave/state.hpp bb0eee78e118210ccd7fcbd909029412ff106128 
>   src/slave/state.cpp f8a9514f52bf9f886171c2a0e674e5a89f8dbea7 
>   src/tests/containerizer_tests.cpp 0cdb2d2a3f19a4835e85c6b040759019b03f051e 
> 
> Diff: https://reviews.apache.org/r/34137/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Ian Downes
> 
>

Re: Review Request 34137: Add support for container image provisioners.

[Jie Yu]

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


Tim, I probably wound wait for Vinod's shipit before committing this.

There seems to be a bug in the code as well (besides the obvious broken build 
issue).


src/slave/containerizer/mesos/containerizer.cpp (line 629)


Hum, looks like a bug since, for example, slaveId is a reference and will 
be invalid when the lambda is called. In general, I think we should avoid using 
[=] for lambdas because its dangeous!

I would prefer we resort to our old style defer style (e.g., introduce 
`_provision`).



src/slave/containerizer/mesos/containerizer.cpp (lines 636 - 642)


No test for checkpointing???



src/slave/containerizer/mesos/containerizer.cpp (line 1236)


`_destroy` is a void function. We should do:

```
_destroy(...);
return;
```



src/slave/containerizer/mesos/containerizer.cpp (line 1247)


Ditto.



src/slave/containerizer/mesos/containerizer.cpp (line 1268)


s/cleanup/future/



src/slave/containerizer/provisioner.hpp (lines 37 - 38)


This should be #include 



src/slave/containerizer/provisioner.hpp (lines 52 - 54)


Recover containers? What container? This is a little misleading. How about

```
Recover root filesystems for containers...




src/slave/containerizer/provisioner.cpp (line 24)


Where is this? This won't compile!


- Jie Yu


On July 12, 2015, 4:47 a.m., Ian Downes wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34137/
> ---
> 
> (Updated July 12, 2015, 4:47 a.m.)
> 
> 
> Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The MesosContainerizer can optionally provision a root filesystem for the 
> containers.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/isolator.hpp ef2205dd7f4619e53e6cca7cac301f874d08c036 
>   src/Makefile.am e5b5d36f0ac160e5a3a9fdc50b31c060a413ce2c 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 3ac2387eabded61c897a5016655aae10cd1bca91 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 47d146125dfd4ea909e7ec9d94f41cfa11d035e5 
>   src/slave/containerizer/provisioner.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner.cpp PRE-CREATION 
>   src/slave/flags.hpp 26c778db2303167369af8675fe0441a00a1e9151 
>   src/slave/flags.cpp 8632677ebbdbfef8ffa45204b6f63a700baff7f3 
>   src/slave/paths.hpp 00476f107ed8233123a6eab0925c9f28aac0a86f 
>   src/slave/paths.cpp 0741616b656e947cb460dd6ee6a9a4852be001c2 
>   src/slave/state.hpp bb0eee78e118210ccd7fcbd909029412ff106128 
>   src/slave/state.cpp f8a9514f52bf9f886171c2a0e674e5a89f8dbea7 
>   src/tests/containerizer_tests.cpp 0cdb2d2a3f19a4835e85c6b040759019b03f051e 
> 
> Diff: https://reviews.apache.org/r/34137/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Ian Downes
> 
>

Re: Review Request 34137: Add support for container image provisioners.

[Timothy Chen]

> On May 28, 2015, 9:49 p.m., Paul Brett wrote:
> > src/slave/containerizer/mesos/containerizer.hpp, line 245
> > 
> >
> > To many underscores - can we come up with a better name?

We can refactor later.


- Timothy


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


On July 12, 2015, 4:47 a.m., Ian Downes wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34137/
> ---
> 
> (Updated July 12, 2015, 4:47 a.m.)
> 
> 
> Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The MesosContainerizer can optionally provision a root filesystem for the 
> containers.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/isolator.hpp ef2205dd7f4619e53e6cca7cac301f874d08c036 
>   src/Makefile.am e5b5d36f0ac160e5a3a9fdc50b31c060a413ce2c 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 3ac2387eabded61c897a5016655aae10cd1bca91 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 47d146125dfd4ea909e7ec9d94f41cfa11d035e5 
>   src/slave/containerizer/provisioner.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner.cpp PRE-CREATION 
>   src/slave/flags.hpp 26c778db2303167369af8675fe0441a00a1e9151 
>   src/slave/flags.cpp 8632677ebbdbfef8ffa45204b6f63a700baff7f3 
>   src/slave/paths.hpp 00476f107ed8233123a6eab0925c9f28aac0a86f 
>   src/slave/paths.cpp 0741616b656e947cb460dd6ee6a9a4852be001c2 
>   src/slave/state.hpp bb0eee78e118210ccd7fcbd909029412ff106128 
>   src/slave/state.cpp f8a9514f52bf9f886171c2a0e674e5a89f8dbea7 
>   src/tests/containerizer_tests.cpp 0cdb2d2a3f19a4835e85c6b040759019b03f051e 
> 
> Diff: https://reviews.apache.org/r/34137/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Ian Downes
> 
>

Re: Review Request 34137: Add support for container image provisioners.

[Timothy Chen]

> On July 14, 2015, 7:41 p.m., Vinod Kone wrote:
> > src/slave/containerizer/provisioner.cpp, lines 43-56
> > 
> >
> > Why do you need foreach loop here if you were going to return error 
> > anyway?
> 
> Timothy Chen wrote:
> We need the foreach to go over all the provisioners though, as there 
> could be more than one although there is one listed for now.

I can remove the for loop and just return empty map, and also add a log that 
provisioners are not supported yet if something is specified.
Sounds good?


- Timothy


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


On July 12, 2015, 4:47 a.m., Ian Downes wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34137/
> ---
> 
> (Updated July 12, 2015, 4:47 a.m.)
> 
> 
> Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The MesosContainerizer can optionally provision a root filesystem for the 
> containers.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/isolator.hpp ef2205dd7f4619e53e6cca7cac301f874d08c036 
>   src/Makefile.am e5b5d36f0ac160e5a3a9fdc50b31c060a413ce2c 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 3ac2387eabded61c897a5016655aae10cd1bca91 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 47d146125dfd4ea909e7ec9d94f41cfa11d035e5 
>   src/slave/containerizer/provisioner.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner.cpp PRE-CREATION 
>   src/slave/flags.hpp 26c778db2303167369af8675fe0441a00a1e9151 
>   src/slave/flags.cpp 8632677ebbdbfef8ffa45204b6f63a700baff7f3 
>   src/slave/paths.hpp 00476f107ed8233123a6eab0925c9f28aac0a86f 
>   src/slave/paths.cpp 0741616b656e947cb460dd6ee6a9a4852be001c2 
>   src/slave/state.hpp bb0eee78e118210ccd7fcbd909029412ff106128 
>   src/slave/state.cpp f8a9514f52bf9f886171c2a0e674e5a89f8dbea7 
>   src/tests/containerizer_tests.cpp 0cdb2d2a3f19a4835e85c6b040759019b03f051e 
> 
> Diff: https://reviews.apache.org/r/34137/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Ian Downes
> 
>

Re: Review Request 34137: Add support for container image provisioners.

[Timothy Chen]

> On July 8, 2015, 11:34 p.m., Vinod Kone wrote:
> > src/slave/containerizer/mesos/containerizer.cpp, line 418
> > 
> >
> > Is it possible for rootfs to not exist when we are here? If not, there 
> > should be a CHECK and a comment on what guarantees its presence (the fact 
> > that there is a forked pid?).

the field rootfs is just a option, so I think that's why you can just pass that 
in.


> On July 8, 2015, 11:34 p.m., Vinod Kone wrote:
> > include/mesos/slave/isolator.hpp, lines 70-71
> > 
> >
> > We don't align arguments for constructors this way IIRC.
> > 
> > ExecutorRunState(arg1,
> >  arg2,
> >  );

Looks like the latest revision has the right format right?


> On July 8, 2015, 11:34 p.m., Vinod Kone wrote:
> > src/slave/containerizer/mesos/containerizer.cpp, line 1249
> > 
> >
> > why include the containerid? doesn't the caller of this method already 
> > know that?

The caller knows this, but since this shows up in the log it's easier to 
correlate the error with a container id.


> On July 8, 2015, 11:34 p.m., Vinod Kone wrote:
> > src/slave/containerizer/mesos/containerizer.cpp, line 1280
> > 
> >
> > ditto. why include the container id?

You're right I don't tihnk containerId here is needed.


> On July 8, 2015, 11:34 p.m., Vinod Kone wrote:
> > src/slave/containerizer/mesos/containerizer.cpp, line 818
> > 
> >
> > where is the update to launchFlags to add 'rootfs' ? which review?

Looks like this field is already added in master.
commit 15bf9f67e3d9489e49b2176e1e4221a1a47fd0c9
Author: Ian Downes 
Date:   Thu Dec 18 15:46:15 2014 -0800

Support chrooting in MesosContainerizer launch helper.

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


- Timothy


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


On July 12, 2015, 4:47 a.m., Ian Downes wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34137/
> ---
> 
> (Updated July 12, 2015, 4:47 a.m.)
> 
> 
> Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The MesosContainerizer can optionally provision a root filesystem for the 
> containers.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/isolator.hpp ef2205dd7f4619e53e6cca7cac301f874d08c036 
>   src/Makefile.am e5b5d36f0ac160e5a3a9fdc50b31c060a413ce2c 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 3ac2387eabded61c897a5016655aae10cd1bca91 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 47d146125dfd4ea909e7ec9d94f41cfa11d035e5 
>   src/slave/containerizer/provisioner.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner.cpp PRE-CREATION 
>   src/slave/flags.hpp 26c778db2303167369af8675fe0441a00a1e9151 
>   src/slave/flags.cpp 8632677ebbdbfef8ffa45204b6f63a700baff7f3 
>   src/slave/paths.hpp 00476f107ed8233123a6eab0925c9f28aac0a86f 
>   src/slave/paths.cpp 0741616b656e947cb460dd6ee6a9a4852be001c2 
>   src/slave/state.hpp bb0eee78e118210ccd7fcbd909029412ff106128 
>   src/slave/state.cpp f8a9514f52bf9f886171c2a0e674e5a89f8dbea7 
>   src/tests/containerizer_tests.cpp 0cdb2d2a3f19a4835e85c6b040759019b03f051e 
> 
> Diff: https://reviews.apache.org/r/34137/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Ian Downes
> 
>

Re: Review Request 34137: Add support for container image provisioners.

[Timothy Chen]

> On July 14, 2015, 7:41 p.m., Vinod Kone wrote:
> > can you fix the "depends on" please?

I think the two listed are the correct dependencies for this rb?


- Timothy


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


On July 12, 2015, 4:47 a.m., Ian Downes wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34137/
> ---
> 
> (Updated July 12, 2015, 4:47 a.m.)
> 
> 
> Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The MesosContainerizer can optionally provision a root filesystem for the 
> containers.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/isolator.hpp ef2205dd7f4619e53e6cca7cac301f874d08c036 
>   src/Makefile.am e5b5d36f0ac160e5a3a9fdc50b31c060a413ce2c 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 3ac2387eabded61c897a5016655aae10cd1bca91 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 47d146125dfd4ea909e7ec9d94f41cfa11d035e5 
>   src/slave/containerizer/provisioner.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner.cpp PRE-CREATION 
>   src/slave/flags.hpp 26c778db2303167369af8675fe0441a00a1e9151 
>   src/slave/flags.cpp 8632677ebbdbfef8ffa45204b6f63a700baff7f3 
>   src/slave/paths.hpp 00476f107ed8233123a6eab0925c9f28aac0a86f 
>   src/slave/paths.cpp 0741616b656e947cb460dd6ee6a9a4852be001c2 
>   src/slave/state.hpp bb0eee78e118210ccd7fcbd909029412ff106128 
>   src/slave/state.cpp f8a9514f52bf9f886171c2a0e674e5a89f8dbea7 
>   src/tests/containerizer_tests.cpp 0cdb2d2a3f19a4835e85c6b040759019b03f051e 
> 
> Diff: https://reviews.apache.org/r/34137/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Ian Downes
> 
>

Re: Review Request 34137: Add support for container image provisioners.

[Timothy Chen]

> On July 14, 2015, 7:41 p.m., Vinod Kone wrote:
> > src/slave/containerizer/provisioner.cpp, lines 43-56
> > 
> >
> > Why do you need foreach loop here if you were going to return error 
> > anyway?

We need the foreach to go over all the provisioners though, as there could be 
more than one although there is one listed for now.


- Timothy


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


On July 12, 2015, 4:47 a.m., Ian Downes wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34137/
> ---
> 
> (Updated July 12, 2015, 4:47 a.m.)
> 
> 
> Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The MesosContainerizer can optionally provision a root filesystem for the 
> containers.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/isolator.hpp ef2205dd7f4619e53e6cca7cac301f874d08c036 
>   src/Makefile.am e5b5d36f0ac160e5a3a9fdc50b31c060a413ce2c 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 3ac2387eabded61c897a5016655aae10cd1bca91 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 47d146125dfd4ea909e7ec9d94f41cfa11d035e5 
>   src/slave/containerizer/provisioner.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner.cpp PRE-CREATION 
>   src/slave/flags.hpp 26c778db2303167369af8675fe0441a00a1e9151 
>   src/slave/flags.cpp 8632677ebbdbfef8ffa45204b6f63a700baff7f3 
>   src/slave/paths.hpp 00476f107ed8233123a6eab0925c9f28aac0a86f 
>   src/slave/paths.cpp 0741616b656e947cb460dd6ee6a9a4852be001c2 
>   src/slave/state.hpp bb0eee78e118210ccd7fcbd909029412ff106128 
>   src/slave/state.cpp f8a9514f52bf9f886171c2a0e674e5a89f8dbea7 
>   src/tests/containerizer_tests.cpp 0cdb2d2a3f19a4835e85c6b040759019b03f051e 
> 
> Diff: https://reviews.apache.org/r/34137/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Ian Downes
> 
>

Re: Review Request 34137: Add support for container image provisioners.

[Vinod Kone]

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


can you fix the "depends on" please?


src/slave/containerizer/provisioner.cpp (lines 41 - 44)


Why do you need foreach loop here if you were going to return error anyway?


- Vinod Kone


On July 12, 2015, 4:47 a.m., Ian Downes wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34137/
> ---
> 
> (Updated July 12, 2015, 4:47 a.m.)
> 
> 
> Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The MesosContainerizer can optionally provision a root filesystem for the 
> containers.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/isolator.hpp ef2205dd7f4619e53e6cca7cac301f874d08c036 
>   src/Makefile.am e5b5d36f0ac160e5a3a9fdc50b31c060a413ce2c 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 3ac2387eabded61c897a5016655aae10cd1bca91 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 47d146125dfd4ea909e7ec9d94f41cfa11d035e5 
>   src/slave/containerizer/provisioner.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner.cpp PRE-CREATION 
>   src/slave/flags.hpp 26c778db2303167369af8675fe0441a00a1e9151 
>   src/slave/flags.cpp 8632677ebbdbfef8ffa45204b6f63a700baff7f3 
>   src/slave/paths.hpp 00476f107ed8233123a6eab0925c9f28aac0a86f 
>   src/slave/paths.cpp 0741616b656e947cb460dd6ee6a9a4852be001c2 
>   src/slave/state.hpp bb0eee78e118210ccd7fcbd909029412ff106128 
>   src/slave/state.cpp f8a9514f52bf9f886171c2a0e674e5a89f8dbea7 
>   src/tests/containerizer_tests.cpp 0cdb2d2a3f19a4835e85c6b040759019b03f051e 
> 
> Diff: https://reviews.apache.org/r/34137/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Ian Downes
> 
>

Re: Review Request 34137: Add support for container image provisioners.

[Vinod Kone]

> On July 8, 2015, 11:34 p.m., Vinod Kone wrote:
> > can you please set the dependency for this review correctly? it's hard to 
> > understand which reviews introduced the code that this review depends on.

none of the dropped issues have comments. did you forget to hit publish on your 
comments?


- Vinod


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


On July 12, 2015, 4:47 a.m., Ian Downes wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34137/
> ---
> 
> (Updated July 12, 2015, 4:47 a.m.)
> 
> 
> Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The MesosContainerizer can optionally provision a root filesystem for the 
> containers.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/isolator.hpp ef2205dd7f4619e53e6cca7cac301f874d08c036 
>   src/Makefile.am e5b5d36f0ac160e5a3a9fdc50b31c060a413ce2c 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 3ac2387eabded61c897a5016655aae10cd1bca91 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 47d146125dfd4ea909e7ec9d94f41cfa11d035e5 
>   src/slave/containerizer/provisioner.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner.cpp PRE-CREATION 
>   src/slave/flags.hpp 26c778db2303167369af8675fe0441a00a1e9151 
>   src/slave/flags.cpp 8632677ebbdbfef8ffa45204b6f63a700baff7f3 
>   src/slave/paths.hpp 00476f107ed8233123a6eab0925c9f28aac0a86f 
>   src/slave/paths.cpp 0741616b656e947cb460dd6ee6a9a4852be001c2 
>   src/slave/state.hpp bb0eee78e118210ccd7fcbd909029412ff106128 
>   src/slave/state.cpp f8a9514f52bf9f886171c2a0e674e5a89f8dbea7 
>   src/tests/containerizer_tests.cpp 0cdb2d2a3f19a4835e85c6b040759019b03f051e 
> 
> Diff: https://reviews.apache.org/r/34137/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Ian Downes
> 
>

Re: Review Request 34137: Add support for container image provisioners.

[Timothy Chen]

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

Ship it!


Ship It!

- Timothy Chen


On July 12, 2015, 4:47 a.m., Ian Downes wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34137/
> ---
> 
> (Updated July 12, 2015, 4:47 a.m.)
> 
> 
> Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The MesosContainerizer can optionally provision a root filesystem for the 
> containers.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/isolator.hpp ef2205dd7f4619e53e6cca7cac301f874d08c036 
>   src/Makefile.am e5b5d36f0ac160e5a3a9fdc50b31c060a413ce2c 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 3ac2387eabded61c897a5016655aae10cd1bca91 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 47d146125dfd4ea909e7ec9d94f41cfa11d035e5 
>   src/slave/containerizer/provisioner.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner.cpp PRE-CREATION 
>   src/slave/flags.hpp 26c778db2303167369af8675fe0441a00a1e9151 
>   src/slave/flags.cpp 8632677ebbdbfef8ffa45204b6f63a700baff7f3 
>   src/slave/paths.hpp 00476f107ed8233123a6eab0925c9f28aac0a86f 
>   src/slave/paths.cpp 0741616b656e947cb460dd6ee6a9a4852be001c2 
>   src/slave/state.hpp bb0eee78e118210ccd7fcbd909029412ff106128 
>   src/slave/state.cpp f8a9514f52bf9f886171c2a0e674e5a89f8dbea7 
>   src/tests/containerizer_tests.cpp 0cdb2d2a3f19a4835e85c6b040759019b03f051e 
> 
> Diff: https://reviews.apache.org/r/34137/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Ian Downes
> 
>

Re: Review Request 34137: Add support for container image provisioners.

[Ian Downes]

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

(Updated July 11, 2015, 9:47 p.m.)


Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone.


Changes
---

Addressed comments.


Repository: mesos


Description
---

The MesosContainerizer can optionally provision a root filesystem for the 
containers.


Diffs (updated)
-

  include/mesos/slave/isolator.hpp ef2205dd7f4619e53e6cca7cac301f874d08c036 
  src/Makefile.am e5b5d36f0ac160e5a3a9fdc50b31c060a413ce2c 
  src/slave/containerizer/mesos/containerizer.hpp 
3ac2387eabded61c897a5016655aae10cd1bca91 
  src/slave/containerizer/mesos/containerizer.cpp 
47d146125dfd4ea909e7ec9d94f41cfa11d035e5 
  src/slave/containerizer/provisioner.hpp PRE-CREATION 
  src/slave/containerizer/provisioner.cpp PRE-CREATION 
  src/slave/flags.hpp 26c778db2303167369af8675fe0441a00a1e9151 
  src/slave/flags.cpp 8632677ebbdbfef8ffa45204b6f63a700baff7f3 
  src/slave/paths.hpp 00476f107ed8233123a6eab0925c9f28aac0a86f 
  src/slave/paths.cpp 0741616b656e947cb460dd6ee6a9a4852be001c2 
  src/slave/state.hpp bb0eee78e118210ccd7fcbd909029412ff106128 
  src/slave/state.cpp f8a9514f52bf9f886171c2a0e674e5a89f8dbea7 
  src/tests/containerizer_tests.cpp 0cdb2d2a3f19a4835e85c6b040759019b03f051e 

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


Testing
---


Thanks,

Ian Downes

Re: Review Request 34137: Add support for container image provisioners.

[Ian Downes]

> On June 29, 2015, 12:41 p.m., Jiang Yan Xu wrote:
> > src/slave/containerizer/mesos/containerizer.hpp, lines 319-320
> > 
> >
> > Why not store ContainerInfo directly?

It's optional so I decided to store the required ExecutorInfo and use the 
container ContainerInfo when appropriate.


- Ian


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


On July 10, 2015, 5:05 p.m., Ian Downes wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34137/
> ---
> 
> (Updated July 10, 2015, 5:05 p.m.)
> 
> 
> Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The MesosContainerizer can optionally provision a root filesystem for the 
> containers.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/isolator.hpp 18edc030367e42240090f2f3dbc92aec7a4c6234 
>   src/Makefile.am e7de0f3d1a5efeaef47d5074defe3b40db94f573 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 3ac2387eabded61c897a5016655aae10cd1bca91 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 8c102fb7d1f79ee768cb06de3a976ea12f958712 
>   src/slave/containerizer/provisioner.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner.cpp PRE-CREATION 
>   src/slave/flags.hpp 7634e368c72e83932dcd992d78eaca146326606b 
>   src/slave/flags.cpp cbf431eb0627bdaf07241cc0fc4630df06fb20e2 
>   src/slave/paths.hpp 00476f107ed8233123a6eab0925c9f28aac0a86f 
>   src/slave/paths.cpp 0741616b656e947cb460dd6ee6a9a4852be001c2 
>   src/slave/state.hpp fed4b7ecf9572a8dbb1a99dbb1769d3e55ef7bc5 
>   src/slave/state.cpp 8eda22a550e5add0f84c46cc2ed762b006c0dcec 
>   src/tests/containerizer_tests.cpp 0cdb2d2a3f19a4835e85c6b040759019b03f051e 
> 
> Diff: https://reviews.apache.org/r/34137/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Ian Downes
> 
>

Re: Review Request 34137: Add support for container image provisioners.

[Ian Downes]

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

(Updated July 10, 2015, 5:05 p.m.)


Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone.


Changes
---

Addressed comments.


Repository: mesos


Description
---

The MesosContainerizer can optionally provision a root filesystem for the 
containers.


Diffs
-

  include/mesos/slave/isolator.hpp 18edc030367e42240090f2f3dbc92aec7a4c6234 
  src/Makefile.am e7de0f3d1a5efeaef47d5074defe3b40db94f573 
  src/slave/containerizer/mesos/containerizer.hpp 
3ac2387eabded61c897a5016655aae10cd1bca91 
  src/slave/containerizer/mesos/containerizer.cpp 
8c102fb7d1f79ee768cb06de3a976ea12f958712 
  src/slave/containerizer/provisioner.hpp PRE-CREATION 
  src/slave/containerizer/provisioner.cpp PRE-CREATION 
  src/slave/flags.hpp 7634e368c72e83932dcd992d78eaca146326606b 
  src/slave/flags.cpp cbf431eb0627bdaf07241cc0fc4630df06fb20e2 
  src/slave/paths.hpp 00476f107ed8233123a6eab0925c9f28aac0a86f 
  src/slave/paths.cpp 0741616b656e947cb460dd6ee6a9a4852be001c2 
  src/slave/state.hpp fed4b7ecf9572a8dbb1a99dbb1769d3e55ef7bc5 
  src/slave/state.cpp 8eda22a550e5add0f84c46cc2ed762b006c0dcec 
  src/tests/containerizer_tests.cpp 0cdb2d2a3f19a4835e85c6b040759019b03f051e 

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


Testing
---


Thanks,

Ian Downes

Re: Review Request 34137: Add support for container image provisioners.

[Vinod Kone]

> On June 29, 2015, 7:41 p.m., Jiang Yan Xu wrote:
> > src/slave/containerizer/provisioner.cpp, lines 46-47
> > 
> >
> > Seems like this belongs to a later patch. AppcProvisioner is not 
> > introduced yet.

+1


- Vinod


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


On July 7, 2015, 7:42 p.m., Ian Downes wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34137/
> ---
> 
> (Updated July 7, 2015, 7:42 p.m.)
> 
> 
> Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The MesosContainerizer can optionally provision a root filesystem for the 
> containers.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/isolator.hpp 18edc030367e42240090f2f3dbc92aec7a4c6234 
>   src/Makefile.am e7de0f3d1a5efeaef47d5074defe3b40db94f573 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 3ac2387eabded61c897a5016655aae10cd1bca91 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 8c102fb7d1f79ee768cb06de3a976ea12f958712 
>   src/slave/containerizer/provisioner.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner.cpp PRE-CREATION 
>   src/slave/flags.hpp 7634e368c72e83932dcd992d78eaca146326606b 
>   src/slave/flags.cpp cbf431eb0627bdaf07241cc0fc4630df06fb20e2 
>   src/slave/paths.hpp 00476f107ed8233123a6eab0925c9f28aac0a86f 
>   src/slave/paths.cpp 0741616b656e947cb460dd6ee6a9a4852be001c2 
>   src/slave/state.hpp fed4b7ecf9572a8dbb1a99dbb1769d3e55ef7bc5 
>   src/slave/state.cpp 8eda22a550e5add0f84c46cc2ed762b006c0dcec 
>   src/tests/containerizer_tests.cpp 0cdb2d2a3f19a4835e85c6b040759019b03f051e 
> 
> Diff: https://reviews.apache.org/r/34137/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Ian Downes
> 
>

Re: Review Request 34137: Add support for container image provisioners.

[Vinod Kone]

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


can you please set the dependency for this review correctly? it's hard to 
understand which reviews introduced the code that this review depends on.


include/mesos/slave/isolator.hpp (lines 70 - 71)


We don't align arguments for constructors this way IIRC.

ExecutorRunState(arg1,
 arg2,
 );



src/slave/containerizer/mesos/containerizer.cpp (line 400)


kill new line.



src/slave/containerizer/mesos/containerizer.cpp (line 418)


Is it possible for rootfs to not exist when we are here? If not, there 
should be a CHECK and a comment on what guarantees its presence (the fact that 
there is a forked pid?).



src/slave/containerizer/mesos/containerizer.cpp (lines 612 - 614)


Why do you want to consider that? Don't we want to support both type of 
containers on the same host?



src/slave/containerizer/mesos/containerizer.cpp (lines 622 - 623)


shouldn't there be a check to see if 'image' even exists in the protobuf?



src/slave/containerizer/mesos/containerizer.cpp (line 632)


If you've extracted 'image' into a temp variable instead of 'type' above in 
#622, you could fit 'provision()' in one line.



src/slave/containerizer/mesos/containerizer.cpp (line 651)


So what happens if the provisioner provisions the file system but the slave 
restarts before checkpointing 'rootfs'? Is that safe? We don't leak anything?



src/slave/containerizer/mesos/containerizer.cpp (line 790)


when was flags.sandbox_directory introduced? which review?



src/slave/containerizer/mesos/containerizer.cpp (lines 815 - 816)


+1



src/slave/containerizer/mesos/containerizer.cpp (line 817)


where is the update to launchFlags to add 'rootfs' ? which review?



src/slave/containerizer/mesos/containerizer.cpp (line 1248)


why include the containerid? doesn't the caller of this method already know 
that?



src/slave/containerizer/mesos/containerizer.cpp (line 1279)


ditto. why include the container id?



src/slave/containerizer/provisioner.hpp (lines 59 - 60)


What does this return?



src/slave/containerizer/provisioner.cpp (line 37)


if (flags.provisioners.isNone()) {

}



src/slave/flags.cpp (line 59)


s/./(e.g., appc, docker)./


- Vinod Kone


On July 7, 2015, 7:42 p.m., Ian Downes wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34137/
> ---
> 
> (Updated July 7, 2015, 7:42 p.m.)
> 
> 
> Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The MesosContainerizer can optionally provision a root filesystem for the 
> containers.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/isolator.hpp 18edc030367e42240090f2f3dbc92aec7a4c6234 
>   src/Makefile.am e7de0f3d1a5efeaef47d5074defe3b40db94f573 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 3ac2387eabded61c897a5016655aae10cd1bca91 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 8c102fb7d1f79ee768cb06de3a976ea12f958712 
>   src/slave/containerizer/provisioner.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner.cpp PRE-CREATION 
>   src/slave/flags.hpp 7634e368c72e83932dcd992d78eaca146326606b 
>   src/slave/flags.cpp cbf431eb0627bdaf07241cc0fc4630df06fb20e2 
>   src/slave/paths.hpp 00476f107ed8233123a6eab0925c9f28aac0a86f 
>   src/slave/paths.cpp 0741616b656e947cb460dd6ee6a9a4852be001c2 
>   src/slave/state.hpp fed4b7ecf9572a8dbb1a99dbb1769d3e55ef7bc5 
>   src/slave/state.cpp 8eda22a550e5add0f84c46cc2ed762b006c0dcec 
>   src/tests/containerizer_tests.cpp 0cdb2d2a3f19a4835e85c6b040759019b03f051e 
> 
> Diff: https://reviews.apache.org/r/34137/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Ian Downes
> 
>

Re: Review Request 34137: Add support for container image provisioners.

[Ian Downes]

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

(Updated July 7, 2015, 12:42 p.m.)


Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone.


Changes
---

Updated dependencies.


Repository: mesos


Description
---

The MesosContainerizer can optionally provision a root filesystem for the 
containers.


Diffs
-

  include/mesos/slave/isolator.hpp 18edc030367e42240090f2f3dbc92aec7a4c6234 
  src/Makefile.am e7de0f3d1a5efeaef47d5074defe3b40db94f573 
  src/slave/containerizer/mesos/containerizer.hpp 
3ac2387eabded61c897a5016655aae10cd1bca91 
  src/slave/containerizer/mesos/containerizer.cpp 
8c102fb7d1f79ee768cb06de3a976ea12f958712 
  src/slave/containerizer/provisioner.hpp PRE-CREATION 
  src/slave/containerizer/provisioner.cpp PRE-CREATION 
  src/slave/flags.hpp 7634e368c72e83932dcd992d78eaca146326606b 
  src/slave/flags.cpp cbf431eb0627bdaf07241cc0fc4630df06fb20e2 
  src/slave/paths.hpp 00476f107ed8233123a6eab0925c9f28aac0a86f 
  src/slave/paths.cpp 0741616b656e947cb460dd6ee6a9a4852be001c2 
  src/slave/state.hpp fed4b7ecf9572a8dbb1a99dbb1769d3e55ef7bc5 
  src/slave/state.cpp 8eda22a550e5add0f84c46cc2ed762b006c0dcec 
  src/tests/containerizer_tests.cpp 0cdb2d2a3f19a4835e85c6b040759019b03f051e 

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


Testing
---


Thanks,

Ian Downes

Re: Review Request 34137: Add support for container image provisioners.

[Timothy Chen]

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



src/slave/containerizer/mesos/containerizer.cpp (line 626)


I think we should include the type in the failure message too.



src/slave/containerizer/mesos/containerizer.cpp (line 1247)


Include the type in the warning message so it's easier to debug.


- Timothy Chen


On June 22, 2015, 4:44 p.m., Ian Downes wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34137/
> ---
> 
> (Updated June 22, 2015, 4:44 p.m.)
> 
> 
> Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The MesosContainerizer can optionally provision a root filesystem for the 
> containers.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/isolator.hpp 18edc030367e42240090f2f3dbc92aec7a4c6234 
>   src/Makefile.am e7de0f3d1a5efeaef47d5074defe3b40db94f573 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 3ac2387eabded61c897a5016655aae10cd1bca91 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 8c102fb7d1f79ee768cb06de3a976ea12f958712 
>   src/slave/containerizer/provisioner.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner.cpp PRE-CREATION 
>   src/slave/flags.hpp 7634e368c72e83932dcd992d78eaca146326606b 
>   src/slave/flags.cpp cbf431eb0627bdaf07241cc0fc4630df06fb20e2 
>   src/slave/paths.hpp 00476f107ed8233123a6eab0925c9f28aac0a86f 
>   src/slave/paths.cpp 0741616b656e947cb460dd6ee6a9a4852be001c2 
>   src/slave/state.hpp fed4b7ecf9572a8dbb1a99dbb1769d3e55ef7bc5 
>   src/slave/state.cpp 8eda22a550e5add0f84c46cc2ed762b006c0dcec 
>   src/tests/containerizer_tests.cpp 0cdb2d2a3f19a4835e85c6b040759019b03f051e 
> 
> Diff: https://reviews.apache.org/r/34137/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Ian Downes
> 
>

Re: Review Request 34137: Add support for container image provisioners.

[Jiang Yan Xu]

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



src/slave/containerizer/mesos/containerizer.hpp (lines 319 - 320)


Why not store ContainerInfo directly?



src/slave/containerizer/mesos/containerizer.cpp (lines 642 - 646)


Indent two more spaces.



src/slave/containerizer/mesos/containerizer.cpp (lines 789 - 790)


```
map env = executorEnvironment(
executorInfo,
containers_[containerId]->rootfs.isSome()
  ? flags.sandbox_directory
  : directory,
slaveId,
slavePid,
checkpoint,
flags.recovery_timeout);
```
?

Just a comment, not an issue.



src/slave/containerizer/mesos/containerizer.cpp (lines 815 - 816)


Does

```
containers_[containerId]->rootfs.isSome()
  ? flags.sandbox_directory
  : directory;
```

look better?
Just thought this boarders "too much jaggedness" in the sytle guilde.



src/slave/containerizer/mesos/containerizer.cpp (lines 1254 - 1259)


Shift one more space to the right for alignment.



src/slave/containerizer/mesos/containerizer.cpp (lines 1278 - 1280)


Four space indentation.



src/slave/containerizer/mesos/containerizer.cpp (line 1279)


s/" :"/": "/



src/slave/containerizer/provisioner.cpp (lines 46 - 47)


Seems like this belongs to a later patch. AppcProvisioner is not introduced 
yet.



src/slave/paths.cpp (lines 243 - 247)


Indent two more spaces.



src/tests/containerizer_tests.cpp (line 344)


Kill empty line.


- Jiang Yan Xu


On June 22, 2015, 9:44 a.m., Ian Downes wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34137/
> ---
> 
> (Updated June 22, 2015, 9:44 a.m.)
> 
> 
> Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The MesosContainerizer can optionally provision a root filesystem for the 
> containers.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/isolator.hpp 18edc030367e42240090f2f3dbc92aec7a4c6234 
>   src/Makefile.am e7de0f3d1a5efeaef47d5074defe3b40db94f573 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 3ac2387eabded61c897a5016655aae10cd1bca91 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 8c102fb7d1f79ee768cb06de3a976ea12f958712 
>   src/slave/containerizer/provisioner.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner.cpp PRE-CREATION 
>   src/slave/flags.hpp 7634e368c72e83932dcd992d78eaca146326606b 
>   src/slave/flags.cpp cbf431eb0627bdaf07241cc0fc4630df06fb20e2 
>   src/slave/paths.hpp 00476f107ed8233123a6eab0925c9f28aac0a86f 
>   src/slave/paths.cpp 0741616b656e947cb460dd6ee6a9a4852be001c2 
>   src/slave/state.hpp fed4b7ecf9572a8dbb1a99dbb1769d3e55ef7bc5 
>   src/slave/state.cpp 8eda22a550e5add0f84c46cc2ed762b006c0dcec 
>   src/tests/containerizer_tests.cpp 0cdb2d2a3f19a4835e85c6b040759019b03f051e 
> 
> Diff: https://reviews.apache.org/r/34137/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Ian Downes
> 
>

Re: Review Request 34137: Add support for container image provisioners.

[Timothy Chen]

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



include/mesos/slave/isolator.hpp (line 71)


We actually don't need the underscores anymore right? According to the 
updated style?


- Timothy Chen


On June 22, 2015, 4:44 p.m., Ian Downes wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34137/
> ---
> 
> (Updated June 22, 2015, 4:44 p.m.)
> 
> 
> Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The MesosContainerizer can optionally provision a root filesystem for the 
> containers.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/isolator.hpp 18edc030367e42240090f2f3dbc92aec7a4c6234 
>   src/Makefile.am e7de0f3d1a5efeaef47d5074defe3b40db94f573 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 3ac2387eabded61c897a5016655aae10cd1bca91 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 8c102fb7d1f79ee768cb06de3a976ea12f958712 
>   src/slave/containerizer/provisioner.hpp PRE-CREATION 
>   src/slave/containerizer/provisioner.cpp PRE-CREATION 
>   src/slave/flags.hpp 7634e368c72e83932dcd992d78eaca146326606b 
>   src/slave/flags.cpp cbf431eb0627bdaf07241cc0fc4630df06fb20e2 
>   src/slave/paths.hpp 00476f107ed8233123a6eab0925c9f28aac0a86f 
>   src/slave/paths.cpp 0741616b656e947cb460dd6ee6a9a4852be001c2 
>   src/slave/state.hpp fed4b7ecf9572a8dbb1a99dbb1769d3e55ef7bc5 
>   src/slave/state.cpp 8eda22a550e5add0f84c46cc2ed762b006c0dcec 
>   src/tests/containerizer_tests.cpp 0cdb2d2a3f19a4835e85c6b040759019b03f051e 
> 
> Diff: https://reviews.apache.org/r/34137/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Ian Downes
> 
>

Re: Review Request 34137: Add support for container image provisioners.

[Ian Downes]

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

(Updated June 22, 2015, 9:44 a.m.)


Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone.


Changes
---

- Support multiple provisioners
- Add recover() to provisioner interface
- Checkpoint and recover optional container rootfs


Repository: mesos


Description
---

The MesosContainerizer can optionally provision a root filesystem for the 
containers.


Diffs (updated)
-

  include/mesos/slave/isolator.hpp 18edc030367e42240090f2f3dbc92aec7a4c6234 
  src/Makefile.am e7de0f3d1a5efeaef47d5074defe3b40db94f573 
  src/slave/containerizer/mesos/containerizer.hpp 
3ac2387eabded61c897a5016655aae10cd1bca91 
  src/slave/containerizer/mesos/containerizer.cpp 
8c102fb7d1f79ee768cb06de3a976ea12f958712 
  src/slave/containerizer/provisioner.hpp PRE-CREATION 
  src/slave/containerizer/provisioner.cpp PRE-CREATION 
  src/slave/flags.hpp 7634e368c72e83932dcd992d78eaca146326606b 
  src/slave/flags.cpp cbf431eb0627bdaf07241cc0fc4630df06fb20e2 
  src/slave/paths.hpp 00476f107ed8233123a6eab0925c9f28aac0a86f 
  src/slave/paths.cpp 0741616b656e947cb460dd6ee6a9a4852be001c2 
  src/slave/state.hpp fed4b7ecf9572a8dbb1a99dbb1769d3e55ef7bc5 
  src/slave/state.cpp 8eda22a550e5add0f84c46cc2ed762b006c0dcec 
  src/tests/containerizer_tests.cpp 0cdb2d2a3f19a4835e85c6b040759019b03f051e 

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


Testing
---


Thanks,

Ian Downes

Re: Review Request 34137: Add support for container image provisioners.

[Paul Brett]

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



src/slave/containerizer/mesos/containerizer.hpp


To many underscores - can we come up with a better name?


- Paul Brett


On May 13, 2015, 12:47 a.m., Ian Downes wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34137/
> ---
> 
> (Updated May 13, 2015, 12:47 a.m.)
> 
> 
> Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The MesosContainerizer can optionally provision a root filesystem for the 
> containers.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 14bc976a7b6a656fb58085484d25c3de3cf0f693 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 3e18617b0dbac58176bfd41dc550898eb6a4a79e 
>   src/slave/containerizer/mesos/containerizer.cpp 
> b644b9c74bc23cf78c0a53284544be6cdaef2f8a 
>   src/slave/containerizer/provisioner.hpp PRE-CREATION 
>   src/slave/flags.hpp d3b1ce117fbb4e0b97852ef150b63f35cc991032 
>   src/slave/flags.cpp d0932b04e3825abb6173efe0d1aee199aa356932 
> 
> Diff: https://reviews.apache.org/r/34137/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Ian Downes
> 
>

Re: Review Request 34137: Add support for container image provisioners.

[Ian Downes]

> On May 14, 2015, 12:41 p.m., Timothy Chen wrote:
> > src/slave/containerizer/mesos/containerizer.cpp, line 193
> > 
> >
> > Not sure if I follow this logic or if I'm missing something, but the 
> > provisioners hashmap seems to be a local variable and I don't see anything 
> > populating it? And how will it contain anything?

This is to support the AppC provisioner in a later review, and subsequent 
provisioners.


> On May 14, 2015, 12:41 p.m., Timothy Chen wrote:
> > src/slave/containerizer/provisioner.hpp, line 35
> > 
> >
> > What would recover do?

Don't know actually, so I removed the TODO. If needed it'll be clear.


- Ian


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


On May 12, 2015, 5:47 p.m., Ian Downes wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34137/
> ---
> 
> (Updated May 12, 2015, 5:47 p.m.)
> 
> 
> Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The MesosContainerizer can optionally provision a root filesystem for the 
> containers.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 14bc976a7b6a656fb58085484d25c3de3cf0f693 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 3e18617b0dbac58176bfd41dc550898eb6a4a79e 
>   src/slave/containerizer/mesos/containerizer.cpp 
> b644b9c74bc23cf78c0a53284544be6cdaef2f8a 
>   src/slave/containerizer/provisioner.hpp PRE-CREATION 
>   src/slave/flags.hpp d3b1ce117fbb4e0b97852ef150b63f35cc991032 
>   src/slave/flags.cpp d0932b04e3825abb6173efe0d1aee199aa356932 
> 
> Diff: https://reviews.apache.org/r/34137/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Ian Downes
> 
>

Re: Review Request 34137: Add support for container image provisioners.

[Timothy Chen]

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



src/slave/containerizer/mesos/containerizer.cpp


Not sure if I follow this logic or if I'm missing something, but the 
provisioners hashmap seems to be a local variable and I don't see anything 
populating it? And how will it contain anything?



src/slave/containerizer/mesos/containerizer.cpp


What happens if they specify a image but no provisioner is there?



src/slave/containerizer/mesos/containerizer.cpp


This seems like a wierd style to me, how about:

 return provisioner.get()->provision(
   containerId,
   executorInfo.container().mesos().image())
   .then(defer(self(),
   &Self::_provision,
   containerId,
   executorInfo,
   directory,
   lambda::_1));
   
This seems more aligned at least.



src/slave/containerizer/mesos/containerizer.cpp


So currently looks like a containerizer is only able to support one 
provisioner then? 
I don't think it's a big deal, but I think we do want to be able to support 
multiple image provisioners in mesos containerizer in the future, so perhaps at 
least leave a comment that we need to change this in the future.



src/slave/containerizer/provisioner.hpp


What would recover do?


- Timothy Chen


On May 13, 2015, 12:47 a.m., Ian Downes wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34137/
> ---
> 
> (Updated May 13, 2015, 12:47 a.m.)
> 
> 
> Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The MesosContainerizer can optionally provision a root filesystem for the 
> containers.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 14bc976a7b6a656fb58085484d25c3de3cf0f693 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 3e18617b0dbac58176bfd41dc550898eb6a4a79e 
>   src/slave/containerizer/mesos/containerizer.cpp 
> b644b9c74bc23cf78c0a53284544be6cdaef2f8a 
>   src/slave/containerizer/provisioner.hpp PRE-CREATION 
>   src/slave/flags.hpp d3b1ce117fbb4e0b97852ef150b63f35cc991032 
>   src/slave/flags.cpp d0932b04e3825abb6173efe0d1aee199aa356932 
> 
> Diff: https://reviews.apache.org/r/34137/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Ian Downes
> 
>

Review Request 34137: Add support for container image provisioners.

[Ian Downes]

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

Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone.


Repository: mesos


Description
---

The MesosContainerizer can optionally provision a root filesystem for the 
containers.


Diffs
-

  src/Makefile.am 14bc976a7b6a656fb58085484d25c3de3cf0f693 
  src/slave/containerizer/mesos/containerizer.hpp 
3e18617b0dbac58176bfd41dc550898eb6a4a79e 
  src/slave/containerizer/mesos/containerizer.cpp 
b644b9c74bc23cf78c0a53284544be6cdaef2f8a 
  src/slave/containerizer/provisioner.hpp PRE-CREATION 
  src/slave/flags.hpp d3b1ce117fbb4e0b97852ef150b63f35cc991032 
  src/slave/flags.cpp d0932b04e3825abb6173efe0d1aee199aa356932 

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


Testing
---


Thanks,

Ian Downes