Re: Review Request 34137: Add support for container image provisioners.
--- 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.
On July 17, 2015, 1:36 a.m., Jie Yu wrote: src/slave/containerizer/mesos/containerizer.cpp, lines 637-643 https://reviews.apache.org/r/34137/diff/3/?file=1009143#file1009143line637 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.
On July 17, 2015, 1:36 a.m., Jie Yu wrote: src/slave/containerizer/mesos/containerizer.cpp, line 630 https://reviews.apache.org/r/34137/diff/3/?file=1009143#file1009143line630 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.
On July 17, 2015, 1:36 a.m., Jie Yu wrote: src/slave/containerizer/provisioner.cpp, line 24 https://reviews.apache.org/r/34137/diff/3/?file=1009145#file1009145line24 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.
--- 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) https://reviews.apache.org/r/34137/#comment146447 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.
On July 16, 2015, 6:36 p.m., Jie Yu wrote: src/slave/containerizer/mesos/containerizer.cpp, line 630 https://reviews.apache.org/r/34137/diff/3/?file=1009143#file1009143line630 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.
On July 8, 2015, 11:34 p.m., Vinod Kone wrote: src/slave/containerizer/mesos/containerizer.cpp, line 418 https://reviews.apache.org/r/34137/diff/2/?file=989756#file989756line418 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 https://reviews.apache.org/r/34137/diff/2/?file=989753#file989753line70 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 https://reviews.apache.org/r/34137/diff/2/?file=989756#file989756line1249 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 https://reviews.apache.org/r/34137/diff/2/?file=989756#file989756line1280 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 https://reviews.apache.org/r/34137/diff/2/?file=989756#file989756line818 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 idow...@twitter.com 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.
On July 14, 2015, 7:41 p.m., Vinod Kone wrote: src/slave/containerizer/provisioner.cpp, lines 43-56 https://reviews.apache.org/r/34137/diff/2-3/?file=989758#file989758line43 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.
On May 28, 2015, 9:49 p.m., Paul Brett wrote: src/slave/containerizer/mesos/containerizer.hpp, line 245 https://reviews.apache.org/r/34137/diff/1/?file=957263#file957263line245 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.
--- 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) https://reviews.apache.org/r/34137/#comment145846 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) https://reviews.apache.org/r/34137/#comment145843 No test for checkpointing??? src/slave/containerizer/mesos/containerizer.cpp (line 1236) https://reviews.apache.org/r/34137/#comment145848 `_destroy` is a void function. We should do: ``` _destroy(...); return; ``` src/slave/containerizer/mesos/containerizer.cpp (line 1247) https://reviews.apache.org/r/34137/#comment145849 Ditto. src/slave/containerizer/mesos/containerizer.cpp (line 1268) https://reviews.apache.org/r/34137/#comment145850 s/cleanup/future/ src/slave/containerizer/provisioner.hpp (lines 37 - 38) https://reviews.apache.org/r/34137/#comment145836 This should be #include mesos/slave/isolator.hpp src/slave/containerizer/provisioner.hpp (lines 52 - 54) https://reviews.apache.org/r/34137/#comment145837 Recover containers? What container? This is a little misleading. How about ``` Recover root filesystems for containers... src/slave/containerizer/provisioner.cpp (line 24) https://reviews.apache.org/r/34137/#comment145840 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.
On July 14, 2015, 7:41 p.m., Vinod Kone wrote: src/slave/containerizer/provisioner.cpp, lines 43-56 https://reviews.apache.org/r/34137/diff/2-3/?file=989758#file989758line43 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.
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.
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.
--- 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) https://reviews.apache.org/r/34137/#comment145206 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.
--- 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.
On June 29, 2015, 7:41 p.m., Jiang Yan Xu wrote: src/slave/containerizer/provisioner.cpp, lines 46-47 https://reviews.apache.org/r/34137/diff/2/?file=989758#file989758line46 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.
--- 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.
--- 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) https://reviews.apache.org/r/34137/#comment142882 I think we should include the type in the failure message too. src/slave/containerizer/mesos/containerizer.cpp (line 1247) https://reviews.apache.org/r/34137/#comment142883 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.
--- 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) https://reviews.apache.org/r/34137/#comment142551 Why not store ContainerInfo directly? src/slave/containerizer/mesos/containerizer.cpp (lines 642 - 646) https://reviews.apache.org/r/34137/#comment142560 Indent two more spaces. src/slave/containerizer/mesos/containerizer.cpp (lines 789 - 790) https://reviews.apache.org/r/34137/#comment142225 ``` mapstring, string 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) https://reviews.apache.org/r/34137/#comment142224 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) https://reviews.apache.org/r/34137/#comment142213 Shift one more space to the right for alignment. src/slave/containerizer/mesos/containerizer.cpp (lines 1278 - 1280) https://reviews.apache.org/r/34137/#comment142214 Four space indentation. src/slave/containerizer/mesos/containerizer.cpp (line 1279) https://reviews.apache.org/r/34137/#comment142223 s/ :/: / src/slave/containerizer/provisioner.cpp (lines 46 - 47) https://reviews.apache.org/r/34137/#comment142194 Seems like this belongs to a later patch. AppcProvisioner is not introduced yet. src/slave/paths.cpp (lines 243 - 247) https://reviews.apache.org/r/34137/#comment141883 Indent two more spaces. src/tests/containerizer_tests.cpp (line 344) https://reviews.apache.org/r/34137/#comment142215 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.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34137/#review89227 --- include/mesos/slave/isolator.hpp (line 71) https://reviews.apache.org/r/34137/#comment141822 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.
--- 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.
On May 14, 2015, 12:41 p.m., Timothy Chen wrote: src/slave/containerizer/mesos/containerizer.cpp, line 193 https://reviews.apache.org/r/34137/diff/1/?file=957264#file957264line193 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 https://reviews.apache.org/r/34137/diff/1/?file=957265#file957265line35 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
Review Request 34137: Add support for container image provisioners.
--- 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
