Re: Review Request 37722: Added definitions of container rootfs directories.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37722/#review97094 --- Ship it! Ship It! - Timothy Chen On Aug. 25, 2015, 6:41 p.m., Jiang Yan Xu wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/37722/ > --- > > (Updated Aug. 25, 2015, 6:41 p.m.) > > > Review request for mesos, Lily Chen, Chi Zhang, Jie Yu, and Timothy Chen. > > > Bugs: MESOS-3308 > https://issues.apache.org/jira/browse/MESOS-3308 > > > Repository: mesos > > > Description > --- > > More context in /r/37382/ and MESOS-3308. > > I will add the appc backend + provisioner implementation shortly but would > like to seek feedback about this design early. > > > Diffs > - > > include/mesos/type_utils.hpp dafe1df0cb5d0b83ca0579068916fe7fda848f02 > src/slave/containerizer/provisioners/appc/paths.hpp > e35805179e67770c6eff7406668caecabefe4fea > src/slave/containerizer/provisioners/appc/paths.cpp > a244d9a40e7143134b7bf883514bfcd04d6a6af5 > src/slave/paths.hpp dbb1f68d8a789303b595d9e455e2f6f1f0de5c5b > src/slave/paths.cpp 0741616b656e947cb460dd6ee6a9a4852be001c2 > src/tests/paths_tests.cpp 9b7179691bb8c146e5d5cca7437ec64a456a38ad > > Diff: https://reviews.apache.org/r/37722/diff/ > > > Testing > --- > > make check. > > > Thanks, > > Jiang Yan Xu > >
Re: Review Request 37722: Added definitions of container rootfs directories.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37722/#review96372 --- Ship it! src/slave/containerizer/provisioners/appc/paths.hpp (line 52) https://reviews.apache.org/r/37722/#comment151710 s/appc/APPC/ src/slave/containerizer/provisioners/appc/paths.hpp (line 53) https://reviews.apache.org/r/37722/#comment151712 How about the following layout: `APPC/containers/container_id` IN that way, it's more easy to add any metadata that is not associated with any container. src/slave/containerizer/provisioners/appc/paths.hpp (line 54) https://reviews.apache.org/r/37722/#comment151716 Similarly, can you use the following layout: `container_id/backends/backend/...` src/slave/containerizer/provisioners/appc/paths.hpp (line 59) https://reviews.apache.org/r/37722/#comment151717 s/docker/DOCKER/ src/slave/paths.cpp (line 457) https://reviews.apache.org/r/37722/#comment151702 You can use stringify(imageType) directly. You might need to add a operator for Image::Type - Jie Yu On Aug. 24, 2015, 6:26 p.m., Jiang Yan Xu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37722/ --- (Updated Aug. 24, 2015, 6:26 p.m.) Review request for mesos, Lily Chen, Chi Zhang, Jie Yu, and Timothy Chen. Bugs: MESOS-3308 https://issues.apache.org/jira/browse/MESOS-3308 Repository: mesos Description --- More context in /r/37382/ and MESOS-3308. I will add the appc backend + provisioner implementation shortly but would like to seek feedback about this design early. Diffs - src/slave/containerizer/provisioners/appc/paths.hpp e35805179e67770c6eff7406668caecabefe4fea src/slave/containerizer/provisioners/appc/paths.cpp a244d9a40e7143134b7bf883514bfcd04d6a6af5 src/slave/paths.hpp dbb1f68d8a789303b595d9e455e2f6f1f0de5c5b src/slave/paths.cpp 0741616b656e947cb460dd6ee6a9a4852be001c2 Diff: https://reviews.apache.org/r/37722/diff/ Testing --- make check. Thanks, Jiang Yan Xu
Re: Review Request 37722: Added definitions of container rootfs directories.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37722/ --- (Updated Aug. 25, 2015, 11:41 a.m.) Review request for mesos, Lily Chen, Chi Zhang, Jie Yu, and Timothy Chen. Changes --- Comments. NNFR. Bugs: MESOS-3308 https://issues.apache.org/jira/browse/MESOS-3308 Repository: mesos Description --- More context in /r/37382/ and MESOS-3308. I will add the appc backend + provisioner implementation shortly but would like to seek feedback about this design early. Diffs (updated) - include/mesos/type_utils.hpp dafe1df0cb5d0b83ca0579068916fe7fda848f02 src/slave/containerizer/provisioners/appc/paths.hpp e35805179e67770c6eff7406668caecabefe4fea src/slave/containerizer/provisioners/appc/paths.cpp a244d9a40e7143134b7bf883514bfcd04d6a6af5 src/slave/paths.hpp dbb1f68d8a789303b595d9e455e2f6f1f0de5c5b src/slave/paths.cpp 0741616b656e947cb460dd6ee6a9a4852be001c2 src/tests/paths_tests.cpp 9b7179691bb8c146e5d5cca7437ec64a456a38ad Diff: https://reviews.apache.org/r/37722/diff/ Testing --- make check. Thanks, Jiang Yan Xu
Re: Review Request 37722: Added definitions of container rootfs directories.
On Aug. 24, 2015, 5:20 p.m., Timothy Chen wrote: src/slave/containerizer/provisioners/appc/paths.hpp, line 55 https://reviews.apache.org/r/37722/diff/2/?file=1048535#file1048535line55 Suggestion: It might make sense to nest one more directory rootfs so you can add metadata about that rootfs besides it, also allow more flexibility in the long run. Thanks Tim! I chose to use a parent rootfses dir to address this concern. This is more inline with current practices: Create a parent dir and use a sibling file to store the metadata. e.g.: for ``` |-- backend (copy, bind, etc.) |-- rootfses |-- XYZ (the rootfs) ``` The meta file could be ``` |-- backend (copy, bind, etc.) |-- rootfses |-- XYZ (the meta file) ``` **(Under the meta dir!)**, but of course we'll sort this out when there is an actual use case. - Jiang Yan --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37722/#review96252 --- On Aug. 24, 2015, 11:26 a.m., Jiang Yan Xu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37722/ --- (Updated Aug. 24, 2015, 11:26 a.m.) Review request for mesos, Lily Chen, Chi Zhang, Jie Yu, and Timothy Chen. Bugs: MESOS-3308 https://issues.apache.org/jira/browse/MESOS-3308 Repository: mesos Description --- More context in /r/37382/ and MESOS-3308. I will add the appc backend + provisioner implementation shortly but would like to seek feedback about this design early. Diffs - src/slave/containerizer/provisioners/appc/paths.hpp e35805179e67770c6eff7406668caecabefe4fea src/slave/containerizer/provisioners/appc/paths.cpp a244d9a40e7143134b7bf883514bfcd04d6a6af5 src/slave/paths.hpp dbb1f68d8a789303b595d9e455e2f6f1f0de5c5b src/slave/paths.cpp 0741616b656e947cb460dd6ee6a9a4852be001c2 Diff: https://reviews.apache.org/r/37722/diff/ Testing --- make check. Thanks, Jiang Yan Xu
Re: Review Request 37722: Added definitions of container rootfs directories.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37722/ --- (Updated Aug. 24, 2015, 11:26 a.m.) Review request for mesos, Lily Chen, Chi Zhang, Jie Yu, and Timothy Chen. Changes --- Minor fix. Bugs: MESOS-3308 https://issues.apache.org/jira/browse/MESOS-3308 Repository: mesos Description --- More context in /r/37382/ and MESOS-3308. I will add the appc backend + provisioner implementation shortly but would like to seek feedback about this design early. Diffs (updated) - src/slave/containerizer/provisioners/appc/paths.hpp e35805179e67770c6eff7406668caecabefe4fea src/slave/containerizer/provisioners/appc/paths.cpp a244d9a40e7143134b7bf883514bfcd04d6a6af5 src/slave/paths.hpp dbb1f68d8a789303b595d9e455e2f6f1f0de5c5b src/slave/paths.cpp 0741616b656e947cb460dd6ee6a9a4852be001c2 Diff: https://reviews.apache.org/r/37722/diff/ Testing --- make check. Thanks, Jiang Yan Xu
Review Request 37722: Added definitions of container rootfs directories.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37722/ --- Review request for mesos, Lily Chen, Chi Zhang, Jie Yu, and Timothy Chen. Bugs: MESOS-3308 https://issues.apache.org/jira/browse/MESOS-3308 Repository: mesos Description --- More context in /r/37382/ and MESOS-3308. I will add the appc backend + provisioner implementation shortly but would like to seek feedback about this design early. Diffs - src/slave/containerizer/provisioners/appc/paths.hpp e35805179e67770c6eff7406668caecabefe4fea src/slave/containerizer/provisioners/appc/paths.cpp a244d9a40e7143134b7bf883514bfcd04d6a6af5 src/slave/paths.hpp dbb1f68d8a789303b595d9e455e2f6f1f0de5c5b src/slave/paths.cpp 0741616b656e947cb460dd6ee6a9a4852be001c2 Diff: https://reviews.apache.org/r/37722/diff/ Testing --- make check. Thanks, Jiang Yan Xu
Re: Review Request 37722: Added definitions of container rootfs directories.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37722/#review96252 --- The slave provisioners path LGTM! src/slave/containerizer/provisioners/appc/paths.hpp (line 55) https://reviews.apache.org/r/37722/#comment151586 Suggestion: It might make sense to nest one more directory rootfs so you can add metadata about that rootfs besides it, also allow more flexibility in the long run. - Timothy Chen On Aug. 24, 2015, 6:26 p.m., Jiang Yan Xu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37722/ --- (Updated Aug. 24, 2015, 6:26 p.m.) Review request for mesos, Lily Chen, Chi Zhang, Jie Yu, and Timothy Chen. Bugs: MESOS-3308 https://issues.apache.org/jira/browse/MESOS-3308 Repository: mesos Description --- More context in /r/37382/ and MESOS-3308. I will add the appc backend + provisioner implementation shortly but would like to seek feedback about this design early. Diffs - src/slave/containerizer/provisioners/appc/paths.hpp e35805179e67770c6eff7406668caecabefe4fea src/slave/containerizer/provisioners/appc/paths.cpp a244d9a40e7143134b7bf883514bfcd04d6a6af5 src/slave/paths.hpp dbb1f68d8a789303b595d9e455e2f6f1f0de5c5b src/slave/paths.cpp 0741616b656e947cb460dd6ee6a9a4852be001c2 Diff: https://reviews.apache.org/r/37722/diff/ Testing --- make check. Thanks, Jiang Yan Xu
Re: Review Request 37722: Added definitions of container rootfs directories.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37722/#review96246 --- Patch looks great! Reviews applied: [37722] All tests passed. - Mesos ReviewBot On Aug. 24, 2015, 6:26 p.m., Jiang Yan Xu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37722/ --- (Updated Aug. 24, 2015, 6:26 p.m.) Review request for mesos, Lily Chen, Chi Zhang, Jie Yu, and Timothy Chen. Bugs: MESOS-3308 https://issues.apache.org/jira/browse/MESOS-3308 Repository: mesos Description --- More context in /r/37382/ and MESOS-3308. I will add the appc backend + provisioner implementation shortly but would like to seek feedback about this design early. Diffs - src/slave/containerizer/provisioners/appc/paths.hpp e35805179e67770c6eff7406668caecabefe4fea src/slave/containerizer/provisioners/appc/paths.cpp a244d9a40e7143134b7bf883514bfcd04d6a6af5 src/slave/paths.hpp dbb1f68d8a789303b595d9e455e2f6f1f0de5c5b src/slave/paths.cpp 0741616b656e947cb460dd6ee6a9a4852be001c2 Diff: https://reviews.apache.org/r/37722/diff/ Testing --- make check. Thanks, Jiang Yan Xu
