subject:"Review Request 25549\: Basic filesystem isolator for Linux."

Re: Review Request 25549: Basic filesystem isolator for Linux.

2015-03-06 Thread Ben Mahler


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



src/slave/slave.cpp
https://reviews.apache.org/r/25549/#comment122774

This has the same issue as 
https://issues.apache.org/jira/browse/MESOS-2463, we can't mutate the 
ExecutorInfo stored by the slave.


- Ben Mahler


On Oct. 20, 2014, 6:49 p.m., Ian Downes wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/25549/
 ---
 
 (Updated Oct. 20, 2014, 6:49 p.m.)
 
 
 Review request for mesos, Ben Mahler, Jie Yu, and Vinod Kone.
 
 
 Bugs: MESOS-1586
 https://issues.apache.org/jira/browse/MESOS-1586
 
 
 Repository: mesos
 
 
 Description
 ---
 
 Does not report usage or enforce quota but can create 'private' directories 
 for each container which mask parts of the shared host filesystem.
 
 This review replaces https://reviews.apache.org/r/24178/ because of some file 
 renaming. I addressed all comments from earlier reviews.
 
 
 Diffs
 -
 
   include/mesos/mesos.proto 6b93e9000761857c4f335f2a8c8088e155078f54 
   src/Makefile.am c44a9ad47d6e1262949b9049f4ae25b049440d99 
   src/common/parse.hpp c9ca30fde5580d8e388f3c616e78df2b032ac0ad 
   src/common/type_utils.hpp f10b2d31f2781bd056898f93eb8e8b6d9809e80b 
   src/slave/containerizer/isolators/filesystem/shared.hpp PRE-CREATION 
   src/slave/containerizer/isolators/filesystem/shared.cpp PRE-CREATION 
   src/slave/containerizer/linux_launcher.cpp 
 f7bc894830a7ca3f55465dacc7b653cdc2d7758b 
   src/slave/containerizer/mesos/containerizer.cpp 
 9d083294caa5c5a47ba3ceaa1b57346144cb795c 
   src/slave/flags.hpp 159d4ef63643d17be353533992049a38aec72437 
   src/slave/slave.cpp 0e342ed35e3db3b68f9f32b6cf4ace23e4a4db38 
   src/tests/isolator_tests.cpp c38f87632cb6984543cb3767dbd656cde7459610 
   src/tests/mesos.hpp 957e2233cc11c438fd80d3b6d1907a1223093104 
 
 Diff: https://reviews.apache.org/r/25549/diff/
 
 
 Testing
 ---
 
 make check # added a test
 
 
 Thanks,
 
 Ian Downes

Re: Review Request 25549: Basic filesystem isolator for Linux.

2014-10-20 Thread Jie Yu


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

Ship it!



src/slave/containerizer/isolators/filesystem/shared.cpp
https://reviews.apache.org/r/25549/#comment98018

Please make sure to wrap the comments in 70 char width. Here and everywhere 
else.

If you are using vim, set tw=70 :)



src/slave/containerizer/mesos/containerizer.cpp
https://reviews.apache.org/r/25549/#comment98024

I still don't get it. Is this absolute necessary? Why volumes need to be 
mounted first?



src/slave/containerizer/mesos/containerizer.cpp
https://reviews.apache.org/r/25549/#comment98016

Why removing this check?


- Jie Yu


On Oct. 15, 2014, 7:25 p.m., Ian Downes wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/25549/
 ---
 
 (Updated Oct. 15, 2014, 7:25 p.m.)
 
 
 Review request for mesos, Ben Mahler, Jie Yu, and Vinod Kone.
 
 
 Bugs: MESOS-1586
 https://issues.apache.org/jira/browse/MESOS-1586
 
 
 Repository: mesos-git
 
 
 Description
 ---
 
 Does not report usage or enforce quota but can create 'private' directories 
 for each container which mask parts of the shared host filesystem.
 
 This review replaces https://reviews.apache.org/r/24178/ because of some file 
 renaming. I addressed all comments from earlier reviews.
 
 
 Diffs
 -
 
   include/mesos/mesos.proto 6b93e9000761857c4f335f2a8c8088e155078f54 
   src/Makefile.am d503c8df73cda15a9d59254e8265e4a5d0e003a4 
   src/common/parse.hpp c9ca30fde5580d8e388f3c616e78df2b032ac0ad 
   src/common/type_utils.hpp f10b2d31f2781bd056898f93eb8e8b6d9809e80b 
   src/slave/containerizer/isolators/filesystem/shared.hpp PRE-CREATION 
   src/slave/containerizer/isolators/filesystem/shared.cpp PRE-CREATION 
   src/slave/containerizer/linux_launcher.cpp 
 f7bc894830a7ca3f55465dacc7b653cdc2d7758b 
   src/slave/containerizer/mesos/containerizer.cpp 
 9d083294caa5c5a47ba3ceaa1b57346144cb795c 
   src/slave/flags.hpp 16f0cc2ab5ba16a39499608174278b3082e0585d 
   src/slave/slave.cpp 0e342ed35e3db3b68f9f32b6cf4ace23e4a4db38 
   src/tests/isolator_tests.cpp c38f87632cb6984543cb3767dbd656cde7459610 
   src/tests/mesos.hpp 957e2233cc11c438fd80d3b6d1907a1223093104 
 
 Diff: https://reviews.apache.org/r/25549/diff/
 
 
 Testing
 ---
 
 make check # added a test
 
 
 Thanks,
 
 Ian Downes

Re: Review Request 25549: Basic filesystem isolator for Linux.

2014-10-20 Thread Ian Downes



 On Oct. 20, 2014, 10:41 a.m., Jie Yu wrote:
  src/slave/containerizer/mesos/containerizer.cpp, line 118
  https://reviews.apache.org/r/25549/diff/7/?file=722505#file722505line118
 
  I still don't get it. Is this absolute necessary? Why volumes need to 
  be mounted first?

In the current code this is not required, but I'd like to keep it here because 
it will be confusing if any isolator writes to a path only to have it masked by 
a volume.


- Ian


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


On Oct. 15, 2014, 12:25 p.m., Ian Downes wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/25549/
 ---
 
 (Updated Oct. 15, 2014, 12:25 p.m.)
 
 
 Review request for mesos, Ben Mahler, Jie Yu, and Vinod Kone.
 
 
 Bugs: MESOS-1586
 https://issues.apache.org/jira/browse/MESOS-1586
 
 
 Repository: mesos-git
 
 
 Description
 ---
 
 Does not report usage or enforce quota but can create 'private' directories 
 for each container which mask parts of the shared host filesystem.
 
 This review replaces https://reviews.apache.org/r/24178/ because of some file 
 renaming. I addressed all comments from earlier reviews.
 
 
 Diffs
 -
 
   include/mesos/mesos.proto 6b93e9000761857c4f335f2a8c8088e155078f54 
   src/Makefile.am d503c8df73cda15a9d59254e8265e4a5d0e003a4 
   src/common/parse.hpp c9ca30fde5580d8e388f3c616e78df2b032ac0ad 
   src/common/type_utils.hpp f10b2d31f2781bd056898f93eb8e8b6d9809e80b 
   src/slave/containerizer/isolators/filesystem/shared.hpp PRE-CREATION 
   src/slave/containerizer/isolators/filesystem/shared.cpp PRE-CREATION 
   src/slave/containerizer/linux_launcher.cpp 
 f7bc894830a7ca3f55465dacc7b653cdc2d7758b 
   src/slave/containerizer/mesos/containerizer.cpp 
 9d083294caa5c5a47ba3ceaa1b57346144cb795c 
   src/slave/flags.hpp 16f0cc2ab5ba16a39499608174278b3082e0585d 
   src/slave/slave.cpp 0e342ed35e3db3b68f9f32b6cf4ace23e4a4db38 
   src/tests/isolator_tests.cpp c38f87632cb6984543cb3767dbd656cde7459610 
   src/tests/mesos.hpp 957e2233cc11c438fd80d3b6d1907a1223093104 
 
 Diff: https://reviews.apache.org/r/25549/diff/
 
 
 Testing
 ---
 
 make check # added a test
 
 
 Thanks,
 
 Ian Downes

Re: Review Request 25549: Basic filesystem isolator for Linux.

2014-10-20 Thread Ian Downes


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

(Updated Oct. 20, 2014, 11:49 a.m.)


Review request for mesos, Ben Mahler, Jie Yu, and Vinod Kone.


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


Repository: mesos-git


Description
---

Does not report usage or enforce quota but can create 'private' directories for 
each container which mask parts of the shared host filesystem.

This review replaces https://reviews.apache.org/r/24178/ because of some file 
renaming. I addressed all comments from earlier reviews.


Diffs (updated)
-

  include/mesos/mesos.proto 6b93e9000761857c4f335f2a8c8088e155078f54 
  src/Makefile.am c44a9ad47d6e1262949b9049f4ae25b049440d99 
  src/common/parse.hpp c9ca30fde5580d8e388f3c616e78df2b032ac0ad 
  src/common/type_utils.hpp f10b2d31f2781bd056898f93eb8e8b6d9809e80b 
  src/slave/containerizer/isolators/filesystem/shared.hpp PRE-CREATION 
  src/slave/containerizer/isolators/filesystem/shared.cpp PRE-CREATION 
  src/slave/containerizer/linux_launcher.cpp 
f7bc894830a7ca3f55465dacc7b653cdc2d7758b 
  src/slave/containerizer/mesos/containerizer.cpp 
9d083294caa5c5a47ba3ceaa1b57346144cb795c 
  src/slave/flags.hpp 159d4ef63643d17be353533992049a38aec72437 
  src/slave/slave.cpp 0e342ed35e3db3b68f9f32b6cf4ace23e4a4db38 
  src/tests/isolator_tests.cpp c38f87632cb6984543cb3767dbd656cde7459610 
  src/tests/mesos.hpp 957e2233cc11c438fd80d3b6d1907a1223093104 

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


Testing
---

make check # added a test


Thanks,

Ian Downes

Re: Review Request 25549: Basic filesystem isolator for Linux.

2014-10-20 Thread Mesos ReviewBot


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


Patch looks great!

Reviews applied: [26273, 25861, 24177, 25655, 25549]

All tests passed.

- Mesos ReviewBot


On Oct. 20, 2014, 6:49 p.m., Ian Downes wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/25549/
 ---
 
 (Updated Oct. 20, 2014, 6:49 p.m.)
 
 
 Review request for mesos, Ben Mahler, Jie Yu, and Vinod Kone.
 
 
 Bugs: MESOS-1586
 https://issues.apache.org/jira/browse/MESOS-1586
 
 
 Repository: mesos-git
 
 
 Description
 ---
 
 Does not report usage or enforce quota but can create 'private' directories 
 for each container which mask parts of the shared host filesystem.
 
 This review replaces https://reviews.apache.org/r/24178/ because of some file 
 renaming. I addressed all comments from earlier reviews.
 
 
 Diffs
 -
 
   include/mesos/mesos.proto 6b93e9000761857c4f335f2a8c8088e155078f54 
   src/Makefile.am c44a9ad47d6e1262949b9049f4ae25b049440d99 
   src/common/parse.hpp c9ca30fde5580d8e388f3c616e78df2b032ac0ad 
   src/common/type_utils.hpp f10b2d31f2781bd056898f93eb8e8b6d9809e80b 
   src/slave/containerizer/isolators/filesystem/shared.hpp PRE-CREATION 
   src/slave/containerizer/isolators/filesystem/shared.cpp PRE-CREATION 
   src/slave/containerizer/linux_launcher.cpp 
 f7bc894830a7ca3f55465dacc7b653cdc2d7758b 
   src/slave/containerizer/mesos/containerizer.cpp 
 9d083294caa5c5a47ba3ceaa1b57346144cb795c 
   src/slave/flags.hpp 159d4ef63643d17be353533992049a38aec72437 
   src/slave/slave.cpp 0e342ed35e3db3b68f9f32b6cf4ace23e4a4db38 
   src/tests/isolator_tests.cpp c38f87632cb6984543cb3767dbd656cde7459610 
   src/tests/mesos.hpp 957e2233cc11c438fd80d3b6d1907a1223093104 
 
 Diff: https://reviews.apache.org/r/25549/diff/
 
 
 Testing
 ---
 
 make check # added a test
 
 
 Thanks,
 
 Ian Downes

Re: Review Request 25549: Basic filesystem isolator for Linux.

2014-10-15 Thread Ian Downes



 On Oct. 14, 2014, 1:43 p.m., Timothy Chen wrote:
  src/slave/containerizer/linux_launcher.cpp, line 355
  https://reviews.apache.org/r/25549/diff/6/?file=721010#file721010line355
 
  print the contianer id for easier tracing in the logs.

Added to caller's log message.


- Ian


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


On Oct. 14, 2014, 1:31 p.m., Ian Downes wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/25549/
 ---
 
 (Updated Oct. 14, 2014, 1:31 p.m.)
 
 
 Review request for mesos, Ben Mahler, Jie Yu, and Vinod Kone.
 
 
 Bugs: MESOS-1586
 https://issues.apache.org/jira/browse/MESOS-1586
 
 
 Repository: mesos-git
 
 
 Description
 ---
 
 Does not report usage or enforce quota but can create 'private' directories 
 for each container which mask parts of the shared host filesystem.
 
 This review replaces https://reviews.apache.org/r/24178/ because of some file 
 renaming. I addressed all comments from earlier reviews.
 
 
 Diffs
 -
 
   include/mesos/mesos.proto 6b93e9000761857c4f335f2a8c8088e155078f54 
   src/Makefile.am d503c8df73cda15a9d59254e8265e4a5d0e003a4 
   src/common/parse.hpp c9ca30fde5580d8e388f3c616e78df2b032ac0ad 
   src/common/type_utils.hpp f10b2d31f2781bd056898f93eb8e8b6d9809e80b 
   src/slave/containerizer/isolators/filesystem/shared.hpp PRE-CREATION 
   src/slave/containerizer/isolators/filesystem/shared.cpp PRE-CREATION 
   src/slave/containerizer/linux_launcher.cpp 
 f7bc894830a7ca3f55465dacc7b653cdc2d7758b 
   src/slave/containerizer/mesos/containerizer.cpp 
 9d083294caa5c5a47ba3ceaa1b57346144cb795c 
   src/slave/flags.hpp 16f0cc2ab5ba16a39499608174278b3082e0585d 
   src/slave/slave.cpp 0e342ed35e3db3b68f9f32b6cf4ace23e4a4db38 
   src/tests/isolator_tests.cpp c38f87632cb6984543cb3767dbd656cde7459610 
   src/tests/mesos.hpp 957e2233cc11c438fd80d3b6d1907a1223093104 
 
 Diff: https://reviews.apache.org/r/25549/diff/
 
 
 Testing
 ---
 
 make check # added a test
 
 
 Thanks,
 
 Ian Downes

Re: Review Request 25549: Basic filesystem isolator for Linux.

2014-10-15 Thread Ian Downes


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

(Updated Oct. 15, 2014, 12:25 p.m.)


Review request for mesos, Ben Mahler, Jie Yu, and Vinod Kone.


Changes
---

Improved log message.


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


Repository: mesos-git


Description
---

Does not report usage or enforce quota but can create 'private' directories for 
each container which mask parts of the shared host filesystem.

This review replaces https://reviews.apache.org/r/24178/ because of some file 
renaming. I addressed all comments from earlier reviews.


Diffs (updated)
-

  include/mesos/mesos.proto 6b93e9000761857c4f335f2a8c8088e155078f54 
  src/Makefile.am d503c8df73cda15a9d59254e8265e4a5d0e003a4 
  src/common/parse.hpp c9ca30fde5580d8e388f3c616e78df2b032ac0ad 
  src/common/type_utils.hpp f10b2d31f2781bd056898f93eb8e8b6d9809e80b 
  src/slave/containerizer/isolators/filesystem/shared.hpp PRE-CREATION 
  src/slave/containerizer/isolators/filesystem/shared.cpp PRE-CREATION 
  src/slave/containerizer/linux_launcher.cpp 
f7bc894830a7ca3f55465dacc7b653cdc2d7758b 
  src/slave/containerizer/mesos/containerizer.cpp 
9d083294caa5c5a47ba3ceaa1b57346144cb795c 
  src/slave/flags.hpp 16f0cc2ab5ba16a39499608174278b3082e0585d 
  src/slave/slave.cpp 0e342ed35e3db3b68f9f32b6cf4ace23e4a4db38 
  src/tests/isolator_tests.cpp c38f87632cb6984543cb3767dbd656cde7459610 
  src/tests/mesos.hpp 957e2233cc11c438fd80d3b6d1907a1223093104 

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


Testing
---

make check # added a test


Thanks,

Ian Downes

Re: Review Request 25549: Basic filesystem isolator for Linux.

2014-10-15 Thread Mesos ReviewBot


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


Patch looks great!

Reviews applied: [26273, 25861, 24177, 25655, 25549]

All tests passed.

- Mesos ReviewBot


On Oct. 15, 2014, 7:25 p.m., Ian Downes wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/25549/
 ---
 
 (Updated Oct. 15, 2014, 7:25 p.m.)
 
 
 Review request for mesos, Ben Mahler, Jie Yu, and Vinod Kone.
 
 
 Bugs: MESOS-1586
 https://issues.apache.org/jira/browse/MESOS-1586
 
 
 Repository: mesos-git
 
 
 Description
 ---
 
 Does not report usage or enforce quota but can create 'private' directories 
 for each container which mask parts of the shared host filesystem.
 
 This review replaces https://reviews.apache.org/r/24178/ because of some file 
 renaming. I addressed all comments from earlier reviews.
 
 
 Diffs
 -
 
   include/mesos/mesos.proto 6b93e9000761857c4f335f2a8c8088e155078f54 
   src/Makefile.am d503c8df73cda15a9d59254e8265e4a5d0e003a4 
   src/common/parse.hpp c9ca30fde5580d8e388f3c616e78df2b032ac0ad 
   src/common/type_utils.hpp f10b2d31f2781bd056898f93eb8e8b6d9809e80b 
   src/slave/containerizer/isolators/filesystem/shared.hpp PRE-CREATION 
   src/slave/containerizer/isolators/filesystem/shared.cpp PRE-CREATION 
   src/slave/containerizer/linux_launcher.cpp 
 f7bc894830a7ca3f55465dacc7b653cdc2d7758b 
   src/slave/containerizer/mesos/containerizer.cpp 
 9d083294caa5c5a47ba3ceaa1b57346144cb795c 
   src/slave/flags.hpp 16f0cc2ab5ba16a39499608174278b3082e0585d 
   src/slave/slave.cpp 0e342ed35e3db3b68f9f32b6cf4ace23e4a4db38 
   src/tests/isolator_tests.cpp c38f87632cb6984543cb3767dbd656cde7459610 
   src/tests/mesos.hpp 957e2233cc11c438fd80d3b6d1907a1223093104 
 
 Diff: https://reviews.apache.org/r/25549/diff/
 
 
 Testing
 ---
 
 make check # added a test
 
 
 Thanks,
 
 Ian Downes

Re: Review Request 25549: Basic filesystem isolator for Linux.

2014-10-15 Thread Timothy Chen


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

Ship it!


Ship It!

- Timothy Chen


On Oct. 15, 2014, 7:25 p.m., Ian Downes wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/25549/
 ---
 
 (Updated Oct. 15, 2014, 7:25 p.m.)
 
 
 Review request for mesos, Ben Mahler, Jie Yu, and Vinod Kone.
 
 
 Bugs: MESOS-1586
 https://issues.apache.org/jira/browse/MESOS-1586
 
 
 Repository: mesos-git
 
 
 Description
 ---
 
 Does not report usage or enforce quota but can create 'private' directories 
 for each container which mask parts of the shared host filesystem.
 
 This review replaces https://reviews.apache.org/r/24178/ because of some file 
 renaming. I addressed all comments from earlier reviews.
 
 
 Diffs
 -
 
   include/mesos/mesos.proto 6b93e9000761857c4f335f2a8c8088e155078f54 
   src/Makefile.am d503c8df73cda15a9d59254e8265e4a5d0e003a4 
   src/common/parse.hpp c9ca30fde5580d8e388f3c616e78df2b032ac0ad 
   src/common/type_utils.hpp f10b2d31f2781bd056898f93eb8e8b6d9809e80b 
   src/slave/containerizer/isolators/filesystem/shared.hpp PRE-CREATION 
   src/slave/containerizer/isolators/filesystem/shared.cpp PRE-CREATION 
   src/slave/containerizer/linux_launcher.cpp 
 f7bc894830a7ca3f55465dacc7b653cdc2d7758b 
   src/slave/containerizer/mesos/containerizer.cpp 
 9d083294caa5c5a47ba3ceaa1b57346144cb795c 
   src/slave/flags.hpp 16f0cc2ab5ba16a39499608174278b3082e0585d 
   src/slave/slave.cpp 0e342ed35e3db3b68f9f32b6cf4ace23e4a4db38 
   src/tests/isolator_tests.cpp c38f87632cb6984543cb3767dbd656cde7459610 
   src/tests/mesos.hpp 957e2233cc11c438fd80d3b6d1907a1223093104 
 
 Diff: https://reviews.apache.org/r/25549/diff/
 
 
 Testing
 ---
 
 make check # added a test
 
 
 Thanks,
 
 Ian Downes

Re: Review Request 25549: Basic filesystem isolator for Linux.

2014-10-15 Thread Cody Maloney


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


Bad patch!

Reviews applied: [26273]

Failed command: ['./appy-review.sh', '-r', '26273']

Error:
 -r: ./appy-review.sh: No such file or directory

- Cody Maloney


On Oct. 15, 2014, 7:25 p.m., Ian Downes wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/25549/
 ---
 
 (Updated Oct. 15, 2014, 7:25 p.m.)
 
 
 Review request for mesos, Ben Mahler, Jie Yu, and Vinod Kone.
 
 
 Bugs: MESOS-1586
 https://issues.apache.org/jira/browse/MESOS-1586
 
 
 Repository: mesos-git
 
 
 Description
 ---
 
 Does not report usage or enforce quota but can create 'private' directories 
 for each container which mask parts of the shared host filesystem.
 
 This review replaces https://reviews.apache.org/r/24178/ because of some file 
 renaming. I addressed all comments from earlier reviews.
 
 
 Diffs
 -
 
   include/mesos/mesos.proto 6b93e9000761857c4f335f2a8c8088e155078f54 
   src/Makefile.am d503c8df73cda15a9d59254e8265e4a5d0e003a4 
   src/common/parse.hpp c9ca30fde5580d8e388f3c616e78df2b032ac0ad 
   src/common/type_utils.hpp f10b2d31f2781bd056898f93eb8e8b6d9809e80b 
   src/slave/containerizer/isolators/filesystem/shared.hpp PRE-CREATION 
   src/slave/containerizer/isolators/filesystem/shared.cpp PRE-CREATION 
   src/slave/containerizer/linux_launcher.cpp 
 f7bc894830a7ca3f55465dacc7b653cdc2d7758b 
   src/slave/containerizer/mesos/containerizer.cpp 
 9d083294caa5c5a47ba3ceaa1b57346144cb795c 
   src/slave/flags.hpp 16f0cc2ab5ba16a39499608174278b3082e0585d 
   src/slave/slave.cpp 0e342ed35e3db3b68f9f32b6cf4ace23e4a4db38 
   src/tests/isolator_tests.cpp c38f87632cb6984543cb3767dbd656cde7459610 
   src/tests/mesos.hpp 957e2233cc11c438fd80d3b6d1907a1223093104 
 
 Diff: https://reviews.apache.org/r/25549/diff/
 
 
 Testing
 ---
 
 make check # added a test
 
 
 Thanks,
 
 Ian Downes

Re: Review Request 25549: Basic filesystem isolator for Linux.

2014-10-15 Thread Cody Maloney



 On Oct. 16, 2014, 2:48 a.m., Cody Maloney wrote:
  Bad patch!
  
  Reviews applied: [26273]
  
  Failed command: ['./appy-review.sh', '-r', '26273']
  
  Error:
   -r: ./appy-review.sh: No such file or directory

Ignore this. I was testing out some changes to the review checking script, 
broke things.


- Cody


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


On Oct. 15, 2014, 7:25 p.m., Ian Downes wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/25549/
 ---
 
 (Updated Oct. 15, 2014, 7:25 p.m.)
 
 
 Review request for mesos, Ben Mahler, Jie Yu, and Vinod Kone.
 
 
 Bugs: MESOS-1586
 https://issues.apache.org/jira/browse/MESOS-1586
 
 
 Repository: mesos-git
 
 
 Description
 ---
 
 Does not report usage or enforce quota but can create 'private' directories 
 for each container which mask parts of the shared host filesystem.
 
 This review replaces https://reviews.apache.org/r/24178/ because of some file 
 renaming. I addressed all comments from earlier reviews.
 
 
 Diffs
 -
 
   include/mesos/mesos.proto 6b93e9000761857c4f335f2a8c8088e155078f54 
   src/Makefile.am d503c8df73cda15a9d59254e8265e4a5d0e003a4 
   src/common/parse.hpp c9ca30fde5580d8e388f3c616e78df2b032ac0ad 
   src/common/type_utils.hpp f10b2d31f2781bd056898f93eb8e8b6d9809e80b 
   src/slave/containerizer/isolators/filesystem/shared.hpp PRE-CREATION 
   src/slave/containerizer/isolators/filesystem/shared.cpp PRE-CREATION 
   src/slave/containerizer/linux_launcher.cpp 
 f7bc894830a7ca3f55465dacc7b653cdc2d7758b 
   src/slave/containerizer/mesos/containerizer.cpp 
 9d083294caa5c5a47ba3ceaa1b57346144cb795c 
   src/slave/flags.hpp 16f0cc2ab5ba16a39499608174278b3082e0585d 
   src/slave/slave.cpp 0e342ed35e3db3b68f9f32b6cf4ace23e4a4db38 
   src/tests/isolator_tests.cpp c38f87632cb6984543cb3767dbd656cde7459610 
   src/tests/mesos.hpp 957e2233cc11c438fd80d3b6d1907a1223093104 
 
 Diff: https://reviews.apache.org/r/25549/diff/
 
 
 Testing
 ---
 
 make check # added a test
 
 
 Thanks,
 
 Ian Downes

Re: Review Request 25549: Basic filesystem isolator for Linux.

2014-10-14 Thread Ian Downes


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

(Updated Oct. 14, 2014, 1:31 p.m.)


Review request for mesos, Ben Mahler, Jie Yu, and Vinod Kone.


Changes
---

Addressed some clean up comments.


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


Repository: mesos-git


Description
---

Does not report usage or enforce quota but can create 'private' directories for 
each container which mask parts of the shared host filesystem.

This review replaces https://reviews.apache.org/r/24178/ because of some file 
renaming. I addressed all comments from earlier reviews.


Diffs (updated)
-

  include/mesos/mesos.proto 6b93e9000761857c4f335f2a8c8088e155078f54 
  src/Makefile.am d503c8df73cda15a9d59254e8265e4a5d0e003a4 
  src/common/parse.hpp c9ca30fde5580d8e388f3c616e78df2b032ac0ad 
  src/common/type_utils.hpp f10b2d31f2781bd056898f93eb8e8b6d9809e80b 
  src/slave/containerizer/isolators/filesystem/shared.hpp PRE-CREATION 
  src/slave/containerizer/isolators/filesystem/shared.cpp PRE-CREATION 
  src/slave/containerizer/linux_launcher.cpp 
f7bc894830a7ca3f55465dacc7b653cdc2d7758b 
  src/slave/containerizer/mesos/containerizer.cpp 
9d083294caa5c5a47ba3ceaa1b57346144cb795c 
  src/slave/flags.hpp 16f0cc2ab5ba16a39499608174278b3082e0585d 
  src/slave/slave.cpp 0e342ed35e3db3b68f9f32b6cf4ace23e4a4db38 
  src/tests/isolator_tests.cpp c38f87632cb6984543cb3767dbd656cde7459610 
  src/tests/mesos.hpp 957e2233cc11c438fd80d3b6d1907a1223093104 

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


Testing
---

make check # added a test


Thanks,

Ian Downes

Re: Review Request 25549: Basic filesystem isolator for Linux.

2014-10-14 Thread Timothy Chen


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



src/slave/containerizer/linux_launcher.cpp
https://reviews.apache.org/r/25549/#comment96915

print the contianer id for easier tracing in the logs.


- Timothy Chen


On Oct. 14, 2014, 8:31 p.m., Ian Downes wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/25549/
 ---
 
 (Updated Oct. 14, 2014, 8:31 p.m.)
 
 
 Review request for mesos, Ben Mahler, Jie Yu, and Vinod Kone.
 
 
 Bugs: MESOS-1586
 https://issues.apache.org/jira/browse/MESOS-1586
 
 
 Repository: mesos-git
 
 
 Description
 ---
 
 Does not report usage or enforce quota but can create 'private' directories 
 for each container which mask parts of the shared host filesystem.
 
 This review replaces https://reviews.apache.org/r/24178/ because of some file 
 renaming. I addressed all comments from earlier reviews.
 
 
 Diffs
 -
 
   include/mesos/mesos.proto 6b93e9000761857c4f335f2a8c8088e155078f54 
   src/Makefile.am d503c8df73cda15a9d59254e8265e4a5d0e003a4 
   src/common/parse.hpp c9ca30fde5580d8e388f3c616e78df2b032ac0ad 
   src/common/type_utils.hpp f10b2d31f2781bd056898f93eb8e8b6d9809e80b 
   src/slave/containerizer/isolators/filesystem/shared.hpp PRE-CREATION 
   src/slave/containerizer/isolators/filesystem/shared.cpp PRE-CREATION 
   src/slave/containerizer/linux_launcher.cpp 
 f7bc894830a7ca3f55465dacc7b653cdc2d7758b 
   src/slave/containerizer/mesos/containerizer.cpp 
 9d083294caa5c5a47ba3ceaa1b57346144cb795c 
   src/slave/flags.hpp 16f0cc2ab5ba16a39499608174278b3082e0585d 
   src/slave/slave.cpp 0e342ed35e3db3b68f9f32b6cf4ace23e4a4db38 
   src/tests/isolator_tests.cpp c38f87632cb6984543cb3767dbd656cde7459610 
   src/tests/mesos.hpp 957e2233cc11c438fd80d3b6d1907a1223093104 
 
 Diff: https://reviews.apache.org/r/25549/diff/
 
 
 Testing
 ---
 
 make check # added a test
 
 
 Thanks,
 
 Ian Downes

Re: Review Request 25549: Basic filesystem isolator for Linux.

2014-10-14 Thread Mesos ReviewBot


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


Patch looks great!

Reviews applied: [26273, 25861, 24177, 25655, 25549]

All tests passed.

- Mesos ReviewBot


On Oct. 14, 2014, 8:31 p.m., Ian Downes wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/25549/
 ---
 
 (Updated Oct. 14, 2014, 8:31 p.m.)
 
 
 Review request for mesos, Ben Mahler, Jie Yu, and Vinod Kone.
 
 
 Bugs: MESOS-1586
 https://issues.apache.org/jira/browse/MESOS-1586
 
 
 Repository: mesos-git
 
 
 Description
 ---
 
 Does not report usage or enforce quota but can create 'private' directories 
 for each container which mask parts of the shared host filesystem.
 
 This review replaces https://reviews.apache.org/r/24178/ because of some file 
 renaming. I addressed all comments from earlier reviews.
 
 
 Diffs
 -
 
   include/mesos/mesos.proto 6b93e9000761857c4f335f2a8c8088e155078f54 
   src/Makefile.am d503c8df73cda15a9d59254e8265e4a5d0e003a4 
   src/common/parse.hpp c9ca30fde5580d8e388f3c616e78df2b032ac0ad 
   src/common/type_utils.hpp f10b2d31f2781bd056898f93eb8e8b6d9809e80b 
   src/slave/containerizer/isolators/filesystem/shared.hpp PRE-CREATION 
   src/slave/containerizer/isolators/filesystem/shared.cpp PRE-CREATION 
   src/slave/containerizer/linux_launcher.cpp 
 f7bc894830a7ca3f55465dacc7b653cdc2d7758b 
   src/slave/containerizer/mesos/containerizer.cpp 
 9d083294caa5c5a47ba3ceaa1b57346144cb795c 
   src/slave/flags.hpp 16f0cc2ab5ba16a39499608174278b3082e0585d 
   src/slave/slave.cpp 0e342ed35e3db3b68f9f32b6cf4ace23e4a4db38 
   src/tests/isolator_tests.cpp c38f87632cb6984543cb3767dbd656cde7459610 
   src/tests/mesos.hpp 957e2233cc11c438fd80d3b6d1907a1223093104 
 
 Diff: https://reviews.apache.org/r/25549/diff/
 
 
 Testing
 ---
 
 make check # added a test
 
 
 Thanks,
 
 Ian Downes

Re: Review Request 25549: Basic filesystem isolator for Linux.

2014-10-09 Thread Ian Downes



 On Oct. 7, 2014, 2:44 p.m., Jie Yu wrote:
  src/slave/containerizer/isolators/filesystem/shared.cpp, lines 74-77
  https://reviews.apache.org/r/25549/diff/5/?file=711209#file711209line74
 
  Should we check other error conditions as well?
  
  For example, `!executorInfo.has_container()`?

This is checked a few lines below and will return None().


 On Oct. 7, 2014, 2:44 p.m., Jie Yu wrote:
  src/slave/containerizer/mesos/containerizer.cpp, lines 434-440
  https://reviews.apache.org/r/25549/diff/5/?file=711211#file711211line434
 
  This check is redundent, right?

Redundant with what? This is returns false if launching a container with a 
ContainerInfo != MESOS. Do you mean the check in filesystem/shared.cpp is 
redundant?


- Ian


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


On Oct. 2, 2014, 11:21 a.m., Ian Downes wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/25549/
 ---
 
 (Updated Oct. 2, 2014, 11:21 a.m.)
 
 
 Review request for mesos, Ben Mahler, Jie Yu, and Vinod Kone.
 
 
 Bugs: MESOS-1586
 https://issues.apache.org/jira/browse/MESOS-1586
 
 
 Repository: mesos-git
 
 
 Description
 ---
 
 Does not report usage or enforce quota but can create 'private' directories 
 for each container which mask parts of the shared host filesystem.
 
 This review replaces https://reviews.apache.org/r/24178/ because of some file 
 renaming. I addressed all comments from earlier reviews.
 
 
 Diffs
 -
 
   include/mesos/mesos.proto 735da535f136d1188d3c6cf47b2e11153dab6fc3 
   src/Makefile.am 27c42dfde45a449750132e416b4eaf776f8c5e3b 
   src/common/parse.hpp e6153d8a1f25bc9ddbe1e391306beeacfc8d5ff6 
   src/common/type_utils.hpp 480c0883fe6ed7f6a9daf77d83ebb077da2e66ee 
   src/slave/containerizer/isolators/filesystem/shared.hpp PRE-CREATION 
   src/slave/containerizer/isolators/filesystem/shared.cpp PRE-CREATION 
   src/slave/containerizer/linux_launcher.cpp 
 f7bc894830a7ca3f55465dacc7b653cdc2d7758b 
   src/slave/containerizer/mesos/containerizer.cpp 
 9d083294caa5c5a47ba3ceaa1b57346144cb795c 
   src/slave/flags.hpp 16f0cc2ab5ba16a39499608174278b3082e0585d 
   src/slave/slave.cpp e56dcbd80114730949a0d4b553470802a4d38281 
   src/tests/isolator_tests.cpp c38f87632cb6984543cb3767dbd656cde7459610 
   src/tests/mesos.hpp 957e2233cc11c438fd80d3b6d1907a1223093104 
 
 Diff: https://reviews.apache.org/r/25549/diff/
 
 
 Testing
 ---
 
 make check # added a test
 
 
 Thanks,
 
 Ian Downes

Re: Review Request 25549: Basic filesystem isolator for Linux.

2014-10-07 Thread Jie Yu


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


LGTM overall. I'll give a shipit once the following comments are addressed. 
Thanks!


src/slave/containerizer/isolators/filesystem/shared.cpp
https://reviews.apache.org/r/25549/#comment96059

```
if (!user.isSome()) {
}
```



src/slave/containerizer/isolators/filesystem/shared.cpp
https://reviews.apache.org/r/25549/#comment96067

Should we check other error conditions as well?

For example, `!executorInfo.has_container()`?



src/slave/containerizer/isolators/filesystem/shared.cpp
https://reviews.apache.org/r/25549/#comment96091

Could you please expand a little bit in the comments explaining why this is 
necessary. One would ask why not just create the path if it does not exist.

I guess the reason is because this is a shared filesystem isolator, you 
don't want to create an arbitratry directory outside a sandbox.



src/slave/containerizer/isolators/filesystem/shared.cpp
https://reviews.apache.org/r/25549/#comment96096

Could you please add a NOTE here saying that you are assuming 
container_path is an absolute path in case we change the semantics in the 
future.



src/slave/containerizer/isolators/filesystem/shared.cpp
https://reviews.apache.org/r/25549/#comment96097

Add a comment about why -n is used.



src/slave/containerizer/mesos/containerizer.cpp
https://reviews.apache.org/r/25549/#comment96098

Could you please expand why filesystem isolation must be the first one to 
use prepare()?



src/slave/containerizer/mesos/containerizer.cpp
https://reviews.apache.org/r/25549/#comment96100

This check is redundent, right?


- Jie Yu


On Oct. 2, 2014, 6:21 p.m., Ian Downes wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/25549/
 ---
 
 (Updated Oct. 2, 2014, 6:21 p.m.)
 
 
 Review request for mesos, Ben Mahler, Jie Yu, and Vinod Kone.
 
 
 Bugs: MESOS-1586
 https://issues.apache.org/jira/browse/MESOS-1586
 
 
 Repository: mesos-git
 
 
 Description
 ---
 
 Does not report usage or enforce quota but can create 'private' directories 
 for each container which mask parts of the shared host filesystem.
 
 This review replaces https://reviews.apache.org/r/24178/ because of some file 
 renaming. I addressed all comments from earlier reviews.
 
 
 Diffs
 -
 
   include/mesos/mesos.proto 735da535f136d1188d3c6cf47b2e11153dab6fc3 
   src/Makefile.am 27c42dfde45a449750132e416b4eaf776f8c5e3b 
   src/common/parse.hpp e6153d8a1f25bc9ddbe1e391306beeacfc8d5ff6 
   src/common/type_utils.hpp 480c0883fe6ed7f6a9daf77d83ebb077da2e66ee 
   src/slave/containerizer/isolators/filesystem/shared.hpp PRE-CREATION 
   src/slave/containerizer/isolators/filesystem/shared.cpp PRE-CREATION 
   src/slave/containerizer/linux_launcher.cpp 
 f7bc894830a7ca3f55465dacc7b653cdc2d7758b 
   src/slave/containerizer/mesos/containerizer.cpp 
 9d083294caa5c5a47ba3ceaa1b57346144cb795c 
   src/slave/flags.hpp 16f0cc2ab5ba16a39499608174278b3082e0585d 
   src/slave/slave.cpp e56dcbd80114730949a0d4b553470802a4d38281 
   src/tests/isolator_tests.cpp c38f87632cb6984543cb3767dbd656cde7459610 
   src/tests/mesos.hpp 957e2233cc11c438fd80d3b6d1907a1223093104 
 
 Diff: https://reviews.apache.org/r/25549/diff/
 
 
 Testing
 ---
 
 make check # added a test
 
 
 Thanks,
 
 Ian Downes

Re: Review Request 25549: Basic filesystem isolator for Linux.

2014-09-25 Thread Vinod Kone


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



src/slave/containerizer/isolators/filesystem/shared.cpp
https://reviews.apache.org/r/25549/#comment94861

why are the remount commands different for proc and sys? also, why do we 
need to remount /sys and /proc in the fs isolator? it's not clear to me from 
the reflect namespace changes for the container processes. maybe expand on 
the comment?



src/slave/containerizer/isolators/network/port_mapping.cpp
https://reviews.apache.org/r/25549/#comment94864

Hmm. Instead of returning an error here, how about just adding 
filesystem/shared isolator to the list of isolators in mesos containerizer, 
if either of filesystem/shared or network/portmapping is specified in flags?


- Vinod Kone


On Sept. 24, 2014, 6:09 p.m., Ian Downes wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/25549/
 ---
 
 (Updated Sept. 24, 2014, 6:09 p.m.)
 
 
 Review request for mesos, Ben Mahler, Jie Yu, and Vinod Kone.
 
 
 Bugs: MESOS-1586
 https://issues.apache.org/jira/browse/MESOS-1586
 
 
 Repository: mesos-git
 
 
 Description
 ---
 
 Does not report usage or enforce quota but can create 'private' directories 
 for each container which mask parts of the shared host filesystem.
 
 This review replaces https://reviews.apache.org/r/24178/ because of some file 
 renaming. I addressed all comments from earlier reviews.
 
 
 Diffs
 -
 
   include/mesos/mesos.proto be45494b2c2f5c1295409889b70004462c6eba49 
   src/Makefile.am 9b973e5503e30180045e270220987ba647da8038 
   src/common/parse.hpp e6153d8a1f25bc9ddbe1e391306beeacfc8d5ff6 
   src/common/type_utils.hpp 480c0883fe6ed7f6a9daf77d83ebb077da2e66ee 
   src/slave/containerizer/isolators/filesystem/shared.hpp PRE-CREATION 
   src/slave/containerizer/isolators/filesystem/shared.cpp PRE-CREATION 
   src/slave/containerizer/isolators/network/port_mapping.cpp 
 2766a00ff81dc550c21387f920666f81705db4f0 
   src/slave/containerizer/linux_launcher.cpp 
 f7bc894830a7ca3f55465dacc7b653cdc2d7758b 
   src/slave/containerizer/mesos/containerizer.cpp 
 9d083294caa5c5a47ba3ceaa1b57346144cb795c 
   src/slave/flags.hpp 32e51d214b0dbbb2f106236c6fa42ddec9774585 
   src/slave/slave.cpp 9a6646f0249fd43ae5d13bd9ee3b5da08412 
   src/tests/isolator_tests.cpp c38f87632cb6984543cb3767dbd656cde7459610 
   src/tests/mesos.hpp 957e2233cc11c438fd80d3b6d1907a1223093104 
 
 Diff: https://reviews.apache.org/r/25549/diff/
 
 
 Testing
 ---
 
 make check # added a test
 
 
 Thanks,
 
 Ian Downes

Re: Review Request 25549: Basic filesystem isolator for Linux.

2014-09-25 Thread Vinod Kone



 On Sept. 25, 2014, 11:56 p.m., Vinod Kone wrote:
 

Mind adding documentation for this?


- Vinod


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


On Sept. 24, 2014, 6:09 p.m., Ian Downes wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/25549/
 ---
 
 (Updated Sept. 24, 2014, 6:09 p.m.)
 
 
 Review request for mesos, Ben Mahler, Jie Yu, and Vinod Kone.
 
 
 Bugs: MESOS-1586
 https://issues.apache.org/jira/browse/MESOS-1586
 
 
 Repository: mesos-git
 
 
 Description
 ---
 
 Does not report usage or enforce quota but can create 'private' directories 
 for each container which mask parts of the shared host filesystem.
 
 This review replaces https://reviews.apache.org/r/24178/ because of some file 
 renaming. I addressed all comments from earlier reviews.
 
 
 Diffs
 -
 
   include/mesos/mesos.proto be45494b2c2f5c1295409889b70004462c6eba49 
   src/Makefile.am 9b973e5503e30180045e270220987ba647da8038 
   src/common/parse.hpp e6153d8a1f25bc9ddbe1e391306beeacfc8d5ff6 
   src/common/type_utils.hpp 480c0883fe6ed7f6a9daf77d83ebb077da2e66ee 
   src/slave/containerizer/isolators/filesystem/shared.hpp PRE-CREATION 
   src/slave/containerizer/isolators/filesystem/shared.cpp PRE-CREATION 
   src/slave/containerizer/isolators/network/port_mapping.cpp 
 2766a00ff81dc550c21387f920666f81705db4f0 
   src/slave/containerizer/linux_launcher.cpp 
 f7bc894830a7ca3f55465dacc7b653cdc2d7758b 
   src/slave/containerizer/mesos/containerizer.cpp 
 9d083294caa5c5a47ba3ceaa1b57346144cb795c 
   src/slave/flags.hpp 32e51d214b0dbbb2f106236c6fa42ddec9774585 
   src/slave/slave.cpp 9a6646f0249fd43ae5d13bd9ee3b5da08412 
   src/tests/isolator_tests.cpp c38f87632cb6984543cb3767dbd656cde7459610 
   src/tests/mesos.hpp 957e2233cc11c438fd80d3b6d1907a1223093104 
 
 Diff: https://reviews.apache.org/r/25549/diff/
 
 
 Testing
 ---
 
 make check # added a test
 
 
 Thanks,
 
 Ian Downes

Re: Review Request 25549: Basic filesystem isolator for Linux.

2014-09-24 Thread Ian Downes


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

(Updated Sept. 24, 2014, 11:09 a.m.)


Review request for mesos, Ben Mahler, Jie Yu, and Vinod Kone.


Changes
---

Added dependant review.


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


Repository: mesos-git


Description
---

Does not report usage or enforce quota but can create 'private' directories for 
each container which mask parts of the shared host filesystem.

This review replaces https://reviews.apache.org/r/24178/ because of some file 
renaming. I addressed all comments from earlier reviews.


Diffs
-

  include/mesos/mesos.proto be45494b2c2f5c1295409889b70004462c6eba49 
  src/Makefile.am 9b973e5503e30180045e270220987ba647da8038 
  src/common/parse.hpp e6153d8a1f25bc9ddbe1e391306beeacfc8d5ff6 
  src/common/type_utils.hpp 480c0883fe6ed7f6a9daf77d83ebb077da2e66ee 
  src/slave/containerizer/isolators/filesystem/shared.hpp PRE-CREATION 
  src/slave/containerizer/isolators/filesystem/shared.cpp PRE-CREATION 
  src/slave/containerizer/isolators/network/port_mapping.cpp 
2766a00ff81dc550c21387f920666f81705db4f0 
  src/slave/containerizer/linux_launcher.cpp 
f7bc894830a7ca3f55465dacc7b653cdc2d7758b 
  src/slave/containerizer/mesos/containerizer.cpp 
9d083294caa5c5a47ba3ceaa1b57346144cb795c 
  src/slave/flags.hpp 32e51d214b0dbbb2f106236c6fa42ddec9774585 
  src/slave/slave.cpp 9a6646f0249fd43ae5d13bd9ee3b5da08412 
  src/tests/isolator_tests.cpp c38f87632cb6984543cb3767dbd656cde7459610 
  src/tests/mesos.hpp 957e2233cc11c438fd80d3b6d1907a1223093104 

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


Testing
---

make check # added a test


Thanks,

Ian Downes

Re: Review Request 25549: Basic filesystem isolator for Linux.

2014-09-24 Thread Mesos ReviewBot


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


Bad patch!

Reviews applied: [25655, 24177, 25861]

Failed command: git apply --index 25861.patch

Error:
 error: patch failed: src/slave/containerizer/mesos/containerizer.cpp:455
error: src/slave/containerizer/mesos/containerizer.cpp: patch does not apply

- Mesos ReviewBot


On Sept. 24, 2014, 6:09 p.m., Ian Downes wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/25549/
 ---
 
 (Updated Sept. 24, 2014, 6:09 p.m.)
 
 
 Review request for mesos, Ben Mahler, Jie Yu, and Vinod Kone.
 
 
 Bugs: MESOS-1586
 https://issues.apache.org/jira/browse/MESOS-1586
 
 
 Repository: mesos-git
 
 
 Description
 ---
 
 Does not report usage or enforce quota but can create 'private' directories 
 for each container which mask parts of the shared host filesystem.
 
 This review replaces https://reviews.apache.org/r/24178/ because of some file 
 renaming. I addressed all comments from earlier reviews.
 
 
 Diffs
 -
 
   include/mesos/mesos.proto be45494b2c2f5c1295409889b70004462c6eba49 
   src/Makefile.am 9b973e5503e30180045e270220987ba647da8038 
   src/common/parse.hpp e6153d8a1f25bc9ddbe1e391306beeacfc8d5ff6 
   src/common/type_utils.hpp 480c0883fe6ed7f6a9daf77d83ebb077da2e66ee 
   src/slave/containerizer/isolators/filesystem/shared.hpp PRE-CREATION 
   src/slave/containerizer/isolators/filesystem/shared.cpp PRE-CREATION 
   src/slave/containerizer/isolators/network/port_mapping.cpp 
 2766a00ff81dc550c21387f920666f81705db4f0 
   src/slave/containerizer/linux_launcher.cpp 
 f7bc894830a7ca3f55465dacc7b653cdc2d7758b 
   src/slave/containerizer/mesos/containerizer.cpp 
 9d083294caa5c5a47ba3ceaa1b57346144cb795c 
   src/slave/flags.hpp 32e51d214b0dbbb2f106236c6fa42ddec9774585 
   src/slave/slave.cpp 9a6646f0249fd43ae5d13bd9ee3b5da08412 
   src/tests/isolator_tests.cpp c38f87632cb6984543cb3767dbd656cde7459610 
   src/tests/mesos.hpp 957e2233cc11c438fd80d3b6d1907a1223093104 
 
 Diff: https://reviews.apache.org/r/25549/diff/
 
 
 Testing
 ---
 
 make check # added a test
 
 
 Thanks,
 
 Ian Downes

Re: Review Request 25549: Basic filesystem isolator for Linux.

2014-09-23 Thread Vinod Kone


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



src/slave/containerizer/isolators/filesystem/shared.cpp
https://reviews.apache.org/r/25549/#comment94405

Should namespaces/pid check be added as part of the pid namespace patch? 
Did you add it here because the pid patch is independent from this and either 
could land first?



src/slave/containerizer/isolators/filesystem/shared.cpp
https://reviews.apache.org/r/25549/#comment94406

Can you add some comments on why we need to mount /proc and /sys?



src/slave/containerizer/isolators/filesystem/shared.cpp
https://reviews.apache.org/r/25549/#comment94407

Hmm. How is this possible given you return None() on line #75?



src/slave/containerizer/isolators/filesystem/shared.cpp
https://reviews.apache.org/r/25549/#comment94408

s/container/Container/



src/slave/containerizer/isolators/filesystem/shared.cpp
https://reviews.apache.org/r/25549/#comment94409

s/host/Host/



src/slave/containerizer/isolators/filesystem/shared.cpp
https://reviews.apache.org/r/25549/#comment94410

s/host_ ath/host_path/



src/slave/containerizer/isolators/filesystem/shared.cpp
https://reviews.apache.org/r/25549/#comment94411

s/container_path/container path/



src/slave/containerizer/isolators/filesystem/shared.cpp
https://reviews.apache.org/r/25549/#comment94412

include error?



src/slave/containerizer/isolators/filesystem/shared.cpp
https://reviews.apache.org/r/25549/#comment94413

include error?



src/slave/containerizer/isolators/filesystem/shared.cpp
https://reviews.apache.org/r/25549/#comment94414

Where is set -x being added to commands? Should this if block be killed 
altogether?



src/slave/containerizer/isolators/filesystem/shared.cpp
https://reviews.apache.org/r/25549/#comment94415

s/mounts done/mounts is/



src/slave/slave.cpp
https://reviews.apache.org/r/25549/#comment94418

Is there any particular reason the container was added to ExecutorInfo 
instead of TaskInfo?


- Vinod Kone


On Sept. 22, 2014, 6:45 p.m., Ian Downes wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/25549/
 ---
 
 (Updated Sept. 22, 2014, 6:45 p.m.)
 
 
 Review request for mesos, Ben Mahler, Jie Yu, and Vinod Kone.
 
 
 Bugs: MESOS-1586
 https://issues.apache.org/jira/browse/MESOS-1586
 
 
 Repository: mesos-git
 
 
 Description
 ---
 
 Does not report usage or enforce quota but can create 'private' directories 
 for each container which mask parts of the shared host filesystem.
 
 This review replaces https://reviews.apache.org/r/24178/ because of some file 
 renaming. I addressed all comments from earlier reviews.
 
 
 Diffs
 -
 
   include/mesos/mesos.proto dea51f94d130c131421c43e7fd774ceb8941f501 
   src/Makefile.am 9b973e5503e30180045e270220987ba647da8038 
   src/common/parse.hpp e6153d8a1f25bc9ddbe1e391306beeacfc8d5ff6 
   src/common/type_utils.hpp 480c0883fe6ed7f6a9daf77d83ebb077da2e66ee 
   src/slave/containerizer/isolators/filesystem/shared.hpp PRE-CREATION 
   src/slave/containerizer/isolators/filesystem/shared.cpp PRE-CREATION 
   src/slave/containerizer/linux_launcher.cpp 
 d5ef1d6aa762cf81a3e8384552d97fe95b9cbd95 
   src/slave/containerizer/mesos/containerizer.cpp 
 9d083294caa5c5a47ba3ceaa1b57346144cb795c 
   src/slave/flags.hpp 21e00212bc402674eaea73b44b3f91df477a7213 
   src/slave/slave.cpp 1b3dc7370a2441e4159aa5ee552b64ca5e511e96 
   src/tests/isolator_tests.cpp c38f87632cb6984543cb3767dbd656cde7459610 
   src/tests/mesos.hpp 957e2233cc11c438fd80d3b6d1907a1223093104 
 
 Diff: https://reviews.apache.org/r/25549/diff/
 
 
 Testing
 ---
 
 make check # added a test
 
 
 Thanks,
 
 Ian Downes

Re: Review Request 25549: Basic filesystem isolator for Linux.

2014-09-23 Thread Ian Downes



 On Sept. 23, 2014, 1 p.m., Vinod Kone wrote:
  src/slave/containerizer/isolators/filesystem/shared.cpp, lines 92-99
  https://reviews.apache.org/r/25549/diff/3/?file=699763#file699763line92
 
  Hmm. How is this possible given you return None() on line #75?

Sorry, changes split across two reviews. Will correct.


 On Sept. 23, 2014, 1 p.m., Vinod Kone wrote:
  src/slave/containerizer/isolators/filesystem/shared.cpp, line 200
  https://reviews.apache.org/r/25549/diff/3/?file=699763#file699763line200
 
  include error?

The error is simply that the directory doesn't exist, as detailed in the 
message.


 On Sept. 23, 2014, 1 p.m., Vinod Kone wrote:
  src/slave/containerizer/isolators/filesystem/shared.cpp, line 211
  https://reviews.apache.org/r/25549/diff/3/?file=699763#file699763line211
 
  Where is set -x being added to commands? Should this if block be 
  killed altogether?

Removed in subsequent commit (which will be merged back).


 On Sept. 23, 2014, 1 p.m., Vinod Kone wrote:
  src/slave/slave.cpp, lines 2486-2490
  https://reviews.apache.org/r/25549/diff/3/?file=699767#file699767line2486
 
  Is there any particular reason the container was added to ExecutorInfo 
  instead of TaskInfo?

Where would this be done? At the moment both Containerizer::launch() methods 
take an ExecutorInfo so I'd suggest we defer adding it to the TaskInfo until 
this behavior is changed.


- Ian


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


On Sept. 22, 2014, 11:45 a.m., Ian Downes wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/25549/
 ---
 
 (Updated Sept. 22, 2014, 11:45 a.m.)
 
 
 Review request for mesos, Ben Mahler, Jie Yu, and Vinod Kone.
 
 
 Bugs: MESOS-1586
 https://issues.apache.org/jira/browse/MESOS-1586
 
 
 Repository: mesos-git
 
 
 Description
 ---
 
 Does not report usage or enforce quota but can create 'private' directories 
 for each container which mask parts of the shared host filesystem.
 
 This review replaces https://reviews.apache.org/r/24178/ because of some file 
 renaming. I addressed all comments from earlier reviews.
 
 
 Diffs
 -
 
   include/mesos/mesos.proto dea51f94d130c131421c43e7fd774ceb8941f501 
   src/Makefile.am 9b973e5503e30180045e270220987ba647da8038 
   src/common/parse.hpp e6153d8a1f25bc9ddbe1e391306beeacfc8d5ff6 
   src/common/type_utils.hpp 480c0883fe6ed7f6a9daf77d83ebb077da2e66ee 
   src/slave/containerizer/isolators/filesystem/shared.hpp PRE-CREATION 
   src/slave/containerizer/isolators/filesystem/shared.cpp PRE-CREATION 
   src/slave/containerizer/linux_launcher.cpp 
 d5ef1d6aa762cf81a3e8384552d97fe95b9cbd95 
   src/slave/containerizer/mesos/containerizer.cpp 
 9d083294caa5c5a47ba3ceaa1b57346144cb795c 
   src/slave/flags.hpp 21e00212bc402674eaea73b44b3f91df477a7213 
   src/slave/slave.cpp 1b3dc7370a2441e4159aa5ee552b64ca5e511e96 
   src/tests/isolator_tests.cpp c38f87632cb6984543cb3767dbd656cde7459610 
   src/tests/mesos.hpp 957e2233cc11c438fd80d3b6d1907a1223093104 
 
 Diff: https://reviews.apache.org/r/25549/diff/
 
 
 Testing
 ---
 
 make check # added a test
 
 
 Thanks,
 
 Ian Downes

Re: Review Request 25549: Basic filesystem isolator for Linux.

2014-09-23 Thread Ian Downes


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

(Updated Sept. 23, 2014, 4:42 p.m.)


Review request for mesos, Ben Mahler, Jie Yu, and Vinod Kone.


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


Repository: mesos-git


Description
---

Does not report usage or enforce quota but can create 'private' directories for 
each container which mask parts of the shared host filesystem.

This review replaces https://reviews.apache.org/r/24178/ because of some file 
renaming. I addressed all comments from earlier reviews.


Diffs (updated)
-

  include/mesos/mesos.proto be45494b2c2f5c1295409889b70004462c6eba49 
  src/Makefile.am 9b973e5503e30180045e270220987ba647da8038 
  src/common/parse.hpp e6153d8a1f25bc9ddbe1e391306beeacfc8d5ff6 
  src/common/type_utils.hpp 480c0883fe6ed7f6a9daf77d83ebb077da2e66ee 
  src/slave/containerizer/isolators/filesystem/shared.hpp PRE-CREATION 
  src/slave/containerizer/isolators/filesystem/shared.cpp PRE-CREATION 
  src/slave/containerizer/isolators/network/port_mapping.cpp 
2766a00ff81dc550c21387f920666f81705db4f0 
  src/slave/containerizer/linux_launcher.cpp 
f7bc894830a7ca3f55465dacc7b653cdc2d7758b 
  src/slave/containerizer/mesos/containerizer.cpp 
9d083294caa5c5a47ba3ceaa1b57346144cb795c 
  src/slave/flags.hpp 32e51d214b0dbbb2f106236c6fa42ddec9774585 
  src/slave/slave.cpp 9a6646f0249fd43ae5d13bd9ee3b5da08412 
  src/tests/isolator_tests.cpp c38f87632cb6984543cb3767dbd656cde7459610 
  src/tests/mesos.hpp 957e2233cc11c438fd80d3b6d1907a1223093104 

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


Testing
---

make check # added a test


Thanks,

Ian Downes

Re: Review Request 25549: Basic filesystem isolator for Linux.

2014-09-22 Thread Ian Downes


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

(Updated Sept. 22, 2014, 11:45 a.m.)


Review request for mesos, Ben Mahler, Jie Yu, and Vinod Kone.


Changes
---

Addressed comments. 


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


Repository: mesos-git


Description
---

Does not report usage or enforce quota but can create 'private' directories for 
each container which mask parts of the shared host filesystem.

This review replaces https://reviews.apache.org/r/24178/ because of some file 
renaming. I addressed all comments from earlier reviews.


Diffs (updated)
-

  include/mesos/mesos.proto dea51f94d130c131421c43e7fd774ceb8941f501 
  src/Makefile.am 9b973e5503e30180045e270220987ba647da8038 
  src/common/parse.hpp e6153d8a1f25bc9ddbe1e391306beeacfc8d5ff6 
  src/common/type_utils.hpp 480c0883fe6ed7f6a9daf77d83ebb077da2e66ee 
  src/slave/containerizer/isolators/filesystem/shared.hpp PRE-CREATION 
  src/slave/containerizer/isolators/filesystem/shared.cpp PRE-CREATION 
  src/slave/containerizer/linux_launcher.cpp 
d5ef1d6aa762cf81a3e8384552d97fe95b9cbd95 
  src/slave/containerizer/mesos/containerizer.cpp 
9d083294caa5c5a47ba3ceaa1b57346144cb795c 
  src/slave/flags.hpp 21e00212bc402674eaea73b44b3f91df477a7213 
  src/slave/slave.cpp 1b3dc7370a2441e4159aa5ee552b64ca5e511e96 
  src/tests/isolator_tests.cpp c38f87632cb6984543cb3767dbd656cde7459610 
  src/tests/mesos.hpp 957e2233cc11c438fd80d3b6d1907a1223093104 

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


Testing
---

make check # added a test


Thanks,

Ian Downes

Re: Review Request 25549: Basic filesystem isolator for Linux.

2014-09-22 Thread Mesos ReviewBot


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


Patch looks great!

Reviews applied: [25655, 24177, 25549]

All tests passed.

- Mesos ReviewBot


On Sept. 22, 2014, 6:45 p.m., Ian Downes wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/25549/
 ---
 
 (Updated Sept. 22, 2014, 6:45 p.m.)
 
 
 Review request for mesos, Ben Mahler, Jie Yu, and Vinod Kone.
 
 
 Bugs: MESOS-1586
 https://issues.apache.org/jira/browse/MESOS-1586
 
 
 Repository: mesos-git
 
 
 Description
 ---
 
 Does not report usage or enforce quota but can create 'private' directories 
 for each container which mask parts of the shared host filesystem.
 
 This review replaces https://reviews.apache.org/r/24178/ because of some file 
 renaming. I addressed all comments from earlier reviews.
 
 
 Diffs
 -
 
   include/mesos/mesos.proto dea51f94d130c131421c43e7fd774ceb8941f501 
   src/Makefile.am 9b973e5503e30180045e270220987ba647da8038 
   src/common/parse.hpp e6153d8a1f25bc9ddbe1e391306beeacfc8d5ff6 
   src/common/type_utils.hpp 480c0883fe6ed7f6a9daf77d83ebb077da2e66ee 
   src/slave/containerizer/isolators/filesystem/shared.hpp PRE-CREATION 
   src/slave/containerizer/isolators/filesystem/shared.cpp PRE-CREATION 
   src/slave/containerizer/linux_launcher.cpp 
 d5ef1d6aa762cf81a3e8384552d97fe95b9cbd95 
   src/slave/containerizer/mesos/containerizer.cpp 
 9d083294caa5c5a47ba3ceaa1b57346144cb795c 
   src/slave/flags.hpp 21e00212bc402674eaea73b44b3f91df477a7213 
   src/slave/slave.cpp 1b3dc7370a2441e4159aa5ee552b64ca5e511e96 
   src/tests/isolator_tests.cpp c38f87632cb6984543cb3767dbd656cde7459610 
   src/tests/mesos.hpp 957e2233cc11c438fd80d3b6d1907a1223093104 
 
 Diff: https://reviews.apache.org/r/25549/diff/
 
 
 Testing
 ---
 
 make check # added a test
 
 
 Thanks,
 
 Ian Downes

Re: Review Request 25549: Basic filesystem isolator for Linux.

2014-09-22 Thread Timothy Chen


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



src/slave/slave.cpp
https://reviews.apache.org/r/25549/#comment94177

Thanks for adding this, I was thinking about more about the TaskInfo's 
container info instead of the CommandExecutor executor info since I'm not sure 
how the command executor is going to use it yet.
I don't want to block your rb though, I can add it in another rb myself :)


- Timothy Chen


On Sept. 22, 2014, 6:45 p.m., Ian Downes wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/25549/
 ---
 
 (Updated Sept. 22, 2014, 6:45 p.m.)
 
 
 Review request for mesos, Ben Mahler, Jie Yu, and Vinod Kone.
 
 
 Bugs: MESOS-1586
 https://issues.apache.org/jira/browse/MESOS-1586
 
 
 Repository: mesos-git
 
 
 Description
 ---
 
 Does not report usage or enforce quota but can create 'private' directories 
 for each container which mask parts of the shared host filesystem.
 
 This review replaces https://reviews.apache.org/r/24178/ because of some file 
 renaming. I addressed all comments from earlier reviews.
 
 
 Diffs
 -
 
   include/mesos/mesos.proto dea51f94d130c131421c43e7fd774ceb8941f501 
   src/Makefile.am 9b973e5503e30180045e270220987ba647da8038 
   src/common/parse.hpp e6153d8a1f25bc9ddbe1e391306beeacfc8d5ff6 
   src/common/type_utils.hpp 480c0883fe6ed7f6a9daf77d83ebb077da2e66ee 
   src/slave/containerizer/isolators/filesystem/shared.hpp PRE-CREATION 
   src/slave/containerizer/isolators/filesystem/shared.cpp PRE-CREATION 
   src/slave/containerizer/linux_launcher.cpp 
 d5ef1d6aa762cf81a3e8384552d97fe95b9cbd95 
   src/slave/containerizer/mesos/containerizer.cpp 
 9d083294caa5c5a47ba3ceaa1b57346144cb795c 
   src/slave/flags.hpp 21e00212bc402674eaea73b44b3f91df477a7213 
   src/slave/slave.cpp 1b3dc7370a2441e4159aa5ee552b64ca5e511e96 
   src/tests/isolator_tests.cpp c38f87632cb6984543cb3767dbd656cde7459610 
   src/tests/mesos.hpp 957e2233cc11c438fd80d3b6d1907a1223093104 
 
 Diff: https://reviews.apache.org/r/25549/diff/
 
 
 Testing
 ---
 
 make check # added a test
 
 
 Thanks,
 
 Ian Downes

Re: Review Request 25549: Basic filesystem isolator for Linux.

2014-09-17 Thread Vinod Kone


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


I'll take a look once the outstanding issues are addressed.

- Vinod Kone


On Sept. 15, 2014, 8:01 p.m., Ian Downes wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/25549/
 ---
 
 (Updated Sept. 15, 2014, 8:01 p.m.)
 
 
 Review request for mesos, Ben Mahler, Jie Yu, and Vinod Kone.
 
 
 Bugs: MESOS-1586
 https://issues.apache.org/jira/browse/MESOS-1586
 
 
 Repository: mesos-git
 
 
 Description
 ---
 
 Does not report usage or enforce quota but can create 'private' directories 
 for each container which mask parts of the shared host filesystem.
 
 This review replaces https://reviews.apache.org/r/24178/ because of some file 
 renaming. I addressed all comments from earlier reviews.
 
 
 Diffs
 -
 
   include/mesos/mesos.proto dea51f94d130c131421c43e7fd774ceb8941f501 
   src/Makefile.am 9b973e5503e30180045e270220987ba647da8038 
   src/common/parse.hpp e6153d8a1f25bc9ddbe1e391306beeacfc8d5ff6 
   src/common/type_utils.hpp 480c0883fe6ed7f6a9daf77d83ebb077da2e66ee 
   src/slave/containerizer/isolators/filesystem/shared.hpp PRE-CREATION 
   src/slave/containerizer/isolators/filesystem/shared.cpp PRE-CREATION 
   src/slave/containerizer/linux_launcher.cpp 
 d5ef1d6aa762cf81a3e8384552d97fe95b9cbd95 
   src/slave/containerizer/mesos/containerizer.cpp 
 9d083294caa5c5a47ba3ceaa1b57346144cb795c 
   src/slave/flags.hpp 21e00212bc402674eaea73b44b3f91df477a7213 
   src/slave/slave.cpp 1b3dc7370a2441e4159aa5ee552b64ca5e511e96 
   src/tests/isolator_tests.cpp c38f87632cb6984543cb3767dbd656cde7459610 
   src/tests/mesos.hpp 957e2233cc11c438fd80d3b6d1907a1223093104 
 
 Diff: https://reviews.apache.org/r/25549/diff/
 
 
 Testing
 ---
 
 make check # added a test
 
 
 Thanks,
 
 Ian Downes

Re: Review Request 25549: Basic filesystem isolator for Linux.

2014-09-16 Thread Jie Yu


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


A few style issues. I'll let Vinod to give a final ship it.


src/slave/containerizer/isolators/filesystem/shared.hpp
https://reviews.apache.org/r/25549/#comment93052

Style: comments should have 70 char limit each line.



src/slave/containerizer/isolators/filesystem/shared.cpp
https://reviews.apache.org/r/25549/#comment93056

Do not use snake case:)



src/slave/containerizer/isolators/filesystem/shared.cpp
https://reviews.apache.org/r/25549/#comment93053

This shoudl fit in one line



src/slave/containerizer/isolators/filesystem/shared.cpp
https://reviews.apache.org/r/25549/#comment93054

Captialize the comment.



src/slave/containerizer/isolators/filesystem/shared.cpp
https://reviews.apache.org/r/25549/#comment93055

Ditto.



src/slave/containerizer/isolators/filesystem/shared.cpp
https://reviews.apache.org/r/25549/#comment93057

Ditto, snake_case



src/slave/containerizer/isolators/filesystem/shared.cpp
https://reviews.apache.org/r/25549/#comment93058

Add a comment about why -n is used here.



src/slave/slave.cpp
https://reviews.apache.org/r/25549/#comment93050

Style: move { to the above line.



src/slave/slave.cpp
https://reviews.apache.org/r/25549/#comment93051

Ditto.



src/tests/isolator_tests.cpp
https://reviews.apache.org/r/25549/#comment93060

Add a blank line


- Jie Yu


On Sept. 15, 2014, 8:01 p.m., Ian Downes wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/25549/
 ---
 
 (Updated Sept. 15, 2014, 8:01 p.m.)
 
 
 Review request for mesos, Ben Mahler, Jie Yu, and Vinod Kone.
 
 
 Bugs: MESOS-1586
 https://issues.apache.org/jira/browse/MESOS-1586
 
 
 Repository: mesos-git
 
 
 Description
 ---
 
 Does not report usage or enforce quota but can create 'private' directories 
 for each container which mask parts of the shared host filesystem.
 
 This review replaces https://reviews.apache.org/r/24178/ because of some file 
 renaming. I addressed all comments from earlier reviews.
 
 
 Diffs
 -
 
   include/mesos/mesos.proto dea51f94d130c131421c43e7fd774ceb8941f501 
   src/Makefile.am 9b973e5503e30180045e270220987ba647da8038 
   src/common/parse.hpp e6153d8a1f25bc9ddbe1e391306beeacfc8d5ff6 
   src/common/type_utils.hpp 480c0883fe6ed7f6a9daf77d83ebb077da2e66ee 
   src/slave/containerizer/isolators/filesystem/shared.hpp PRE-CREATION 
   src/slave/containerizer/isolators/filesystem/shared.cpp PRE-CREATION 
   src/slave/containerizer/linux_launcher.cpp 
 d5ef1d6aa762cf81a3e8384552d97fe95b9cbd95 
   src/slave/containerizer/mesos/containerizer.cpp 
 9d083294caa5c5a47ba3ceaa1b57346144cb795c 
   src/slave/flags.hpp 21e00212bc402674eaea73b44b3f91df477a7213 
   src/slave/slave.cpp 1b3dc7370a2441e4159aa5ee552b64ca5e511e96 
   src/tests/isolator_tests.cpp c38f87632cb6984543cb3767dbd656cde7459610 
   src/tests/mesos.hpp 957e2233cc11c438fd80d3b6d1907a1223093104 
 
 Diff: https://reviews.apache.org/r/25549/diff/
 
 
 Testing
 ---
 
 make check # added a test
 
 
 Thanks,
 
 Ian Downes

Re: Review Request 25549: Basic filesystem isolator for Linux.

2014-09-15 Thread Ian Downes



 On Sept. 11, 2014, 4:56 p.m., Vinod Kone wrote:
  src/slave/containerizer/isolators/filesystem/shared.cpp, line 124
  https://reviews.apache.org/r/25549/diff/1/?file=686147#file686147line124
 
  s/host_path/hostPath/

is this style strictly enforced? It matches the snaked cased Volume.host_path.


 On Sept. 11, 2014, 4:56 p.m., Vinod Kone wrote:
  src/slave/flags.hpp, line 28
  https://reviews.apache.org/r/25549/diff/1/?file=686150#file686150line28
 
  I don't think you need this header in this file?

Believe necessary for parsing the OptionContainerInfo flag.


 On Sept. 11, 2014, 4:56 p.m., Vinod Kone wrote:
  src/tests/isolator_tests.cpp, line 757
  https://reviews.apache.org/r/25549/diff/1/?file=686152#file686152line757
 
  Is this guaranteed to exist on all systems?

(practically) yes. It's part of FHS and followed by OSX, Linux, *BSD.

http://www.tldp.org/LDP/sag/html/var-fs.html
http://www.pathname.com/fhs/2.2/fhs-5.15.html


 On Sept. 11, 2014, 4:56 p.m., Vinod Kone wrote:
  src/slave/flags.hpp, lines 303-304
  https://reviews.apache.org/r/25549/diff/1/?file=686150#file686150line303
 
  What about TaskInfo that doesn't use a ExecutorInfo. Is the default 
  injected into that as well?

both forms of Containerizer::launch take an ExecutorInfo which will have any 
default ContainerInfo added to it.


- Ian


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


On Sept. 11, 2014, 11:46 a.m., Ian Downes wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/25549/
 ---
 
 (Updated Sept. 11, 2014, 11:46 a.m.)
 
 
 Review request for mesos, Ben Mahler, Jie Yu, and Vinod Kone.
 
 
 Bugs: MESOS-1586
 https://issues.apache.org/jira/browse/MESOS-1586
 
 
 Repository: mesos-git
 
 
 Description
 ---
 
 Does not report usage or enforce quota but can create 'private' directories 
 for each container which mask parts of the shared host filesystem.
 
 This review replaces https://reviews.apache.org/r/24178/ because of some file 
 renaming. I addressed all comments from earlier reviews.
 
 
 Diffs
 -
 
   include/mesos/mesos.proto dea51f94d130c131421c43e7fd774ceb8941f501 
   src/Makefile.am 9b973e5503e30180045e270220987ba647da8038 
   src/common/parse.hpp e6153d8a1f25bc9ddbe1e391306beeacfc8d5ff6 
   src/common/type_utils.hpp 480c0883fe6ed7f6a9daf77d83ebb077da2e66ee 
   src/slave/containerizer/isolators/filesystem/shared.hpp PRE-CREATION 
   src/slave/containerizer/isolators/filesystem/shared.cpp PRE-CREATION 
   src/slave/containerizer/linux_launcher.cpp 
 d5ef1d6aa762cf81a3e8384552d97fe95b9cbd95 
   src/slave/containerizer/mesos/containerizer.cpp 
 9d083294caa5c5a47ba3ceaa1b57346144cb795c 
   src/slave/flags.hpp 21e00212bc402674eaea73b44b3f91df477a7213 
   src/slave/slave.cpp 1b3dc7370a2441e4159aa5ee552b64ca5e511e96 
   src/tests/isolator_tests.cpp c38f87632cb6984543cb3767dbd656cde7459610 
 
 Diff: https://reviews.apache.org/r/25549/diff/
 
 
 Testing
 ---
 
 make check # added a test
 
 
 Thanks,
 
 Ian Downes

Re: Review Request 25549: Basic filesystem isolator for Linux.

2014-09-15 Thread Ian Downes


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

(Updated Sept. 15, 2014, 1:01 p.m.)


Review request for mesos, Ben Mahler, Jie Yu, and Vinod Kone.


Changes
---

Addressed all comments, including refactoring os::chown (see other review) and 
adding additional test for absolute volume.


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


Repository: mesos-git


Description
---

Does not report usage or enforce quota but can create 'private' directories for 
each container which mask parts of the shared host filesystem.

This review replaces https://reviews.apache.org/r/24178/ because of some file 
renaming. I addressed all comments from earlier reviews.


Diffs (updated)
-

  include/mesos/mesos.proto dea51f94d130c131421c43e7fd774ceb8941f501 
  src/Makefile.am 9b973e5503e30180045e270220987ba647da8038 
  src/common/parse.hpp e6153d8a1f25bc9ddbe1e391306beeacfc8d5ff6 
  src/common/type_utils.hpp 480c0883fe6ed7f6a9daf77d83ebb077da2e66ee 
  src/slave/containerizer/isolators/filesystem/shared.hpp PRE-CREATION 
  src/slave/containerizer/isolators/filesystem/shared.cpp PRE-CREATION 
  src/slave/containerizer/linux_launcher.cpp 
d5ef1d6aa762cf81a3e8384552d97fe95b9cbd95 
  src/slave/containerizer/mesos/containerizer.cpp 
9d083294caa5c5a47ba3ceaa1b57346144cb795c 
  src/slave/flags.hpp 21e00212bc402674eaea73b44b3f91df477a7213 
  src/slave/slave.cpp 1b3dc7370a2441e4159aa5ee552b64ca5e511e96 
  src/tests/isolator_tests.cpp c38f87632cb6984543cb3767dbd656cde7459610 
  src/tests/mesos.hpp 957e2233cc11c438fd80d3b6d1907a1223093104 

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


Testing
---

make check # added a test


Thanks,

Ian Downes

Re: Review Request 25549: Basic filesystem isolator for Linux.

2014-09-15 Thread Mesos ReviewBot


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


Patch looks great!

Reviews applied: [25655, 24177, 25549]

All tests passed.

- Mesos ReviewBot


On Sept. 15, 2014, 8:01 p.m., Ian Downes wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/25549/
 ---
 
 (Updated Sept. 15, 2014, 8:01 p.m.)
 
 
 Review request for mesos, Ben Mahler, Jie Yu, and Vinod Kone.
 
 
 Bugs: MESOS-1586
 https://issues.apache.org/jira/browse/MESOS-1586
 
 
 Repository: mesos-git
 
 
 Description
 ---
 
 Does not report usage or enforce quota but can create 'private' directories 
 for each container which mask parts of the shared host filesystem.
 
 This review replaces https://reviews.apache.org/r/24178/ because of some file 
 renaming. I addressed all comments from earlier reviews.
 
 
 Diffs
 -
 
   include/mesos/mesos.proto dea51f94d130c131421c43e7fd774ceb8941f501 
   src/Makefile.am 9b973e5503e30180045e270220987ba647da8038 
   src/common/parse.hpp e6153d8a1f25bc9ddbe1e391306beeacfc8d5ff6 
   src/common/type_utils.hpp 480c0883fe6ed7f6a9daf77d83ebb077da2e66ee 
   src/slave/containerizer/isolators/filesystem/shared.hpp PRE-CREATION 
   src/slave/containerizer/isolators/filesystem/shared.cpp PRE-CREATION 
   src/slave/containerizer/linux_launcher.cpp 
 d5ef1d6aa762cf81a3e8384552d97fe95b9cbd95 
   src/slave/containerizer/mesos/containerizer.cpp 
 9d083294caa5c5a47ba3ceaa1b57346144cb795c 
   src/slave/flags.hpp 21e00212bc402674eaea73b44b3f91df477a7213 
   src/slave/slave.cpp 1b3dc7370a2441e4159aa5ee552b64ca5e511e96 
   src/tests/isolator_tests.cpp c38f87632cb6984543cb3767dbd656cde7459610 
   src/tests/mesos.hpp 957e2233cc11c438fd80d3b6d1907a1223093104 
 
 Diff: https://reviews.apache.org/r/25549/diff/
 
 
 Testing
 ---
 
 make check # added a test
 
 
 Thanks,
 
 Ian Downes

Re: Review Request 25549: Basic filesystem isolator for Linux.

2014-09-15 Thread Timothy Chen


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



src/slave/slave.cpp
https://reviews.apache.org/r/25549/#comment93061

This only adds the default container Info for the executor, but I think it 
makes sense to add it on the TaskInfo when just CommandInfo is defined as well. 
It's not related to this patch but since you added the default containerInfo 
already I think it will be great to wire that as well.



src/tests/isolator_tests.cpp
https://reviews.apache.org/r/25549/#comment93073

One more space between tests



src/tests/isolator_tests.cpp
https://reviews.apache.org/r/25549/#comment93072

end with period.


- Timothy Chen


On Sept. 15, 2014, 8:01 p.m., Ian Downes wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/25549/
 ---
 
 (Updated Sept. 15, 2014, 8:01 p.m.)
 
 
 Review request for mesos, Ben Mahler, Jie Yu, and Vinod Kone.
 
 
 Bugs: MESOS-1586
 https://issues.apache.org/jira/browse/MESOS-1586
 
 
 Repository: mesos-git
 
 
 Description
 ---
 
 Does not report usage or enforce quota but can create 'private' directories 
 for each container which mask parts of the shared host filesystem.
 
 This review replaces https://reviews.apache.org/r/24178/ because of some file 
 renaming. I addressed all comments from earlier reviews.
 
 
 Diffs
 -
 
   include/mesos/mesos.proto dea51f94d130c131421c43e7fd774ceb8941f501 
   src/Makefile.am 9b973e5503e30180045e270220987ba647da8038 
   src/common/parse.hpp e6153d8a1f25bc9ddbe1e391306beeacfc8d5ff6 
   src/common/type_utils.hpp 480c0883fe6ed7f6a9daf77d83ebb077da2e66ee 
   src/slave/containerizer/isolators/filesystem/shared.hpp PRE-CREATION 
   src/slave/containerizer/isolators/filesystem/shared.cpp PRE-CREATION 
   src/slave/containerizer/linux_launcher.cpp 
 d5ef1d6aa762cf81a3e8384552d97fe95b9cbd95 
   src/slave/containerizer/mesos/containerizer.cpp 
 9d083294caa5c5a47ba3ceaa1b57346144cb795c 
   src/slave/flags.hpp 21e00212bc402674eaea73b44b3f91df477a7213 
   src/slave/slave.cpp 1b3dc7370a2441e4159aa5ee552b64ca5e511e96 
   src/tests/isolator_tests.cpp c38f87632cb6984543cb3767dbd656cde7459610 
   src/tests/mesos.hpp 957e2233cc11c438fd80d3b6d1907a1223093104 
 
 Diff: https://reviews.apache.org/r/25549/diff/
 
 
 Testing
 ---
 
 make check # added a test
 
 
 Thanks,
 
 Ian Downes

Re: Review Request 25549: Basic filesystem isolator for Linux.

2014-09-12 Thread Jie Yu


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



src/slave/flags.hpp
https://reviews.apache.org/r/25549/#comment92802

I was thinking about creating a new indirection for the working directory 
here:

Say the current working directory for containers are 
WORK_DIR=/var/lib/mesos/slaves/slave_id/frameworks/framework_id/executors/executor_id/runs/container_id

Since you create $WORK_DIR/.private, user will see this directory if ls -al 
is used.

I am wondering can we create a new workdir in $WORK_DIR and use that as the 
working directory for the executor. THe layout is something like the following:

```
WORK_DIR/
   |--- .private
   |--- tmp
   |--- workdir
```

We will 'cd' into $WORK_DIR/workdir before executing executor command.

That will also simplify persistent resources in the near future because it 
may require persisting the entire working directory.


- Jie Yu


On Sept. 11, 2014, 6:46 p.m., Ian Downes wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/25549/
 ---
 
 (Updated Sept. 11, 2014, 6:46 p.m.)
 
 
 Review request for mesos, Ben Mahler, Jie Yu, and Vinod Kone.
 
 
 Bugs: MESOS-1586
 https://issues.apache.org/jira/browse/MESOS-1586
 
 
 Repository: mesos-git
 
 
 Description
 ---
 
 Does not report usage or enforce quota but can create 'private' directories 
 for each container which mask parts of the shared host filesystem.
 
 This review replaces https://reviews.apache.org/r/24178/ because of some file 
 renaming. I addressed all comments from earlier reviews.
 
 
 Diffs
 -
 
   include/mesos/mesos.proto dea51f94d130c131421c43e7fd774ceb8941f501 
   src/Makefile.am 9b973e5503e30180045e270220987ba647da8038 
   src/common/parse.hpp e6153d8a1f25bc9ddbe1e391306beeacfc8d5ff6 
   src/common/type_utils.hpp 480c0883fe6ed7f6a9daf77d83ebb077da2e66ee 
   src/slave/containerizer/isolators/filesystem/shared.hpp PRE-CREATION 
   src/slave/containerizer/isolators/filesystem/shared.cpp PRE-CREATION 
   src/slave/containerizer/linux_launcher.cpp 
 d5ef1d6aa762cf81a3e8384552d97fe95b9cbd95 
   src/slave/containerizer/mesos/containerizer.cpp 
 9d083294caa5c5a47ba3ceaa1b57346144cb795c 
   src/slave/flags.hpp 21e00212bc402674eaea73b44b3f91df477a7213 
   src/slave/slave.cpp 1b3dc7370a2441e4159aa5ee552b64ca5e511e96 
   src/tests/isolator_tests.cpp c38f87632cb6984543cb3767dbd656cde7459610 
 
 Diff: https://reviews.apache.org/r/25549/diff/
 
 
 Testing
 ---
 
 make check # added a test
 
 
 Thanks,
 
 Ian Downes

Review Request 25549: Basic filesystem isolator for Linux.

2014-09-11 Thread Ian Downes


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

Review request for mesos, Ben Mahler, Jie Yu, and Vinod Kone.


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


Repository: mesos-git


Description
---

Does not report usage or enforce quota but can create 'private' directories for 
each container which mask parts of the shared host filesystem.

This review replaces https://reviews.apache.org/r/24178/ because of some file 
renaming. I addressed all comments from earlier reviews.


Diffs
-

  include/mesos/mesos.proto dea51f94d130c131421c43e7fd774ceb8941f501 
  src/Makefile.am 9b973e5503e30180045e270220987ba647da8038 
  src/common/parse.hpp e6153d8a1f25bc9ddbe1e391306beeacfc8d5ff6 
  src/common/type_utils.hpp 480c0883fe6ed7f6a9daf77d83ebb077da2e66ee 
  src/slave/containerizer/isolators/filesystem/shared.hpp PRE-CREATION 
  src/slave/containerizer/isolators/filesystem/shared.cpp PRE-CREATION 
  src/slave/containerizer/linux_launcher.cpp 
d5ef1d6aa762cf81a3e8384552d97fe95b9cbd95 
  src/slave/containerizer/mesos/containerizer.cpp 
9d083294caa5c5a47ba3ceaa1b57346144cb795c 
  src/slave/flags.hpp 21e00212bc402674eaea73b44b3f91df477a7213 
  src/slave/slave.cpp 1b3dc7370a2441e4159aa5ee552b64ca5e511e96 
  src/tests/isolator_tests.cpp c38f87632cb6984543cb3767dbd656cde7459610 

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


Testing
---

make check # added a test


Thanks,

Ian Downes

Re: Review Request 25549: Basic filesystem isolator for Linux.

2014-09-11 Thread Timothy Chen


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



src/slave/containerizer/isolators/filesystem/shared.cpp
https://reviews.apache.org/r/25549/#comment92487

how about when the host path doesn't start with /?



src/slave/containerizer/isolators/filesystem/shared.cpp
https://reviews.apache.org/r/25549/#comment92489

I don't see how commands can be empty with line 78. I think you should be 
checking == 1?



src/slave/containerizer/isolators/filesystem/shared.cpp
https://reviews.apache.org/r/25549/#comment92490

Extra line before last return.



src/slave/flags.hpp
https://reviews.apache.org/r/25549/#comment92485

This is going to be very useful for all Containers!



src/tests/isolator_tests.cpp
https://reviews.apache.org/r/25549/#comment92491

How about a test with absolute path?


- Timothy Chen


On Sept. 11, 2014, 6:46 p.m., Ian Downes wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/25549/
 ---
 
 (Updated Sept. 11, 2014, 6:46 p.m.)
 
 
 Review request for mesos, Ben Mahler, Jie Yu, and Vinod Kone.
 
 
 Bugs: MESOS-1586
 https://issues.apache.org/jira/browse/MESOS-1586
 
 
 Repository: mesos-git
 
 
 Description
 ---
 
 Does not report usage or enforce quota but can create 'private' directories 
 for each container which mask parts of the shared host filesystem.
 
 This review replaces https://reviews.apache.org/r/24178/ because of some file 
 renaming. I addressed all comments from earlier reviews.
 
 
 Diffs
 -
 
   include/mesos/mesos.proto dea51f94d130c131421c43e7fd774ceb8941f501 
   src/Makefile.am 9b973e5503e30180045e270220987ba647da8038 
   src/common/parse.hpp e6153d8a1f25bc9ddbe1e391306beeacfc8d5ff6 
   src/common/type_utils.hpp 480c0883fe6ed7f6a9daf77d83ebb077da2e66ee 
   src/slave/containerizer/isolators/filesystem/shared.hpp PRE-CREATION 
   src/slave/containerizer/isolators/filesystem/shared.cpp PRE-CREATION 
   src/slave/containerizer/linux_launcher.cpp 
 d5ef1d6aa762cf81a3e8384552d97fe95b9cbd95 
   src/slave/containerizer/mesos/containerizer.cpp 
 9d083294caa5c5a47ba3ceaa1b57346144cb795c 
   src/slave/flags.hpp 21e00212bc402674eaea73b44b3f91df477a7213 
   src/slave/slave.cpp 1b3dc7370a2441e4159aa5ee552b64ca5e511e96 
   src/tests/isolator_tests.cpp c38f87632cb6984543cb3767dbd656cde7459610 
 
 Diff: https://reviews.apache.org/r/25549/diff/
 
 
 Testing
 ---
 
 make check # added a test
 
 
 Thanks,
 
 Ian Downes

Re: Review Request 25549: Basic filesystem isolator for Linux.

2014-09-11 Thread Mesos ReviewBot


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


Bad patch!

Reviews applied: [24177, 25549]

Failed command: git apply --index 25549.patch

Error:
 error: patch failed: src/slave/containerizer/mesos/containerizer.cpp:449
error: src/slave/containerizer/mesos/containerizer.cpp: patch does not apply

- Mesos ReviewBot


On Sept. 11, 2014, 6:46 p.m., Ian Downes wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/25549/
 ---
 
 (Updated Sept. 11, 2014, 6:46 p.m.)
 
 
 Review request for mesos, Ben Mahler, Jie Yu, and Vinod Kone.
 
 
 Bugs: MESOS-1586
 https://issues.apache.org/jira/browse/MESOS-1586
 
 
 Repository: mesos-git
 
 
 Description
 ---
 
 Does not report usage or enforce quota but can create 'private' directories 
 for each container which mask parts of the shared host filesystem.
 
 This review replaces https://reviews.apache.org/r/24178/ because of some file 
 renaming. I addressed all comments from earlier reviews.
 
 
 Diffs
 -
 
   include/mesos/mesos.proto dea51f94d130c131421c43e7fd774ceb8941f501 
   src/Makefile.am 9b973e5503e30180045e270220987ba647da8038 
   src/common/parse.hpp e6153d8a1f25bc9ddbe1e391306beeacfc8d5ff6 
   src/common/type_utils.hpp 480c0883fe6ed7f6a9daf77d83ebb077da2e66ee 
   src/slave/containerizer/isolators/filesystem/shared.hpp PRE-CREATION 
   src/slave/containerizer/isolators/filesystem/shared.cpp PRE-CREATION 
   src/slave/containerizer/linux_launcher.cpp 
 d5ef1d6aa762cf81a3e8384552d97fe95b9cbd95 
   src/slave/containerizer/mesos/containerizer.cpp 
 9d083294caa5c5a47ba3ceaa1b57346144cb795c 
   src/slave/flags.hpp 21e00212bc402674eaea73b44b3f91df477a7213 
   src/slave/slave.cpp 1b3dc7370a2441e4159aa5ee552b64ca5e511e96 
   src/tests/isolator_tests.cpp c38f87632cb6984543cb3767dbd656cde7459610 
 
 Diff: https://reviews.apache.org/r/25549/diff/
 
 
 Testing
 ---
 
 make check # added a test
 
 
 Thanks,
 
 Ian Downes

Re: Review Request 25549: Basic filesystem isolator for Linux.

2014-09-11 Thread Vinod Kone


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



src/common/parse.hpp
https://reviews.apache.org/r/25549/#comment92541

s/string/string or file/



src/slave/containerizer/isolators/filesystem/shared.hpp
https://reviews.apache.org/r/25549/#comment92549

Can you add a high level comment on what this isolator does?



src/slave/containerizer/isolators/filesystem/shared.cpp
https://reviews.apache.org/r/25549/#comment92550

You should print user.Error().



src/slave/containerizer/isolators/filesystem/shared.cpp
https://reviews.apache.org/r/25549/#comment92552

s/container_path/container path/
s/host_path/host path/

here and everywhere else.

either that or you need to say 'Volume.container_path' or 
'Volume.host_path' which I'm assuming you don't want to do :)



src/slave/containerizer/isolators/filesystem/shared.cpp
https://reviews.apache.org/r/25549/#comment92551

s/shared/host/ ?



src/slave/containerizer/isolators/filesystem/shared.cpp
https://reviews.apache.org/r/25549/#comment92576

consider creating it if it's a relative path.



src/slave/containerizer/isolators/filesystem/shared.cpp
https://reviews.apache.org/r/25549/#comment92554

quote the paths here and everywhere else.



src/slave/containerizer/isolators/filesystem/shared.cpp
https://reviews.apache.org/r/25549/#comment92555

s/other/container_path/ ?



src/slave/containerizer/isolators/filesystem/shared.cpp
https://reviews.apache.org/r/25549/#comment92556

s/host_path/hostPath/



src/slave/containerizer/isolators/filesystem/shared.cpp
https://reviews.apache.org/r/25549/#comment92559

hostPath = path::join(
directory,
strings::remove(...));



src/slave/containerizer/isolators/filesystem/shared.cpp
https://reviews.apache.org/r/25549/#comment92560

include the error.



src/slave/containerizer/isolators/filesystem/shared.cpp
https://reviews.apache.org/r/25549/#comment92561

include chmod.error()



src/slave/containerizer/isolators/filesystem/shared.cpp
https://reviews.apache.org/r/25549/#comment92565

mind refactoring os::chown() to take uid and gid?



src/slave/containerizer/isolators/filesystem/shared.cpp
https://reviews.apache.org/r/25549/#comment92557

s/automatically/automatically by the kernel/ ?

also, how is the mount namespace destroyed?



src/slave/containerizer/linux_launcher.cpp
https://reviews.apache.org/r/25549/#comment92548

Kill this TODO?



src/slave/containerizer/mesos/containerizer.cpp
https://reviews.apache.org/r/25549/#comment92577

I'm assuming we want to support TaskInfo.ContainerInfo.Type() to be MESOS? 
If yes, this check should be fixed too.



src/slave/flags.hpp
https://reviews.apache.org/r/25549/#comment92543

I don't think you need this header in this file?



src/slave/flags.hpp
https://reviews.apache.org/r/25549/#comment92546

What about TaskInfo that doesn't use a ExecutorInfo. Is the default 
injected into that as well?



src/slave/flags.hpp
https://reviews.apache.org/r/25549/#comment92545

s/RO/RW/ ? :)



src/slave/slave.cpp
https://reviews.apache.org/r/25549/#comment92547

How about doing this in Slave::getExecutorInfo() instead? Thats the place 
we do all sorts of manipulations on ExecutorInfo.



src/tests/isolator_tests.cpp
https://reviews.apache.org/r/25549/#comment92584

Can you add a comment here on what this test is doing?



src/tests/isolator_tests.cpp
https://reviews.apache.org/r/25549/#comment92578

s/container_path/containerPath/



src/tests/isolator_tests.cpp
https://reviews.apache.org/r/25549/#comment92580

Is this guaranteed to exist on all systems?



src/tests/isolator_tests.cpp
https://reviews.apache.org/r/25549/#comment92579

s/host_path/hostPath/



src/tests/isolator_tests.cpp
https://reviews.apache.org/r/25549/#comment92581

const?



src/tests/isolator_tests.cpp
https://reviews.apache.org/r/25549/#comment92586

s/sHost/hostStat/



src/tests/isolator_tests.cpp
https://reviews.apache.org/r/25549/#comment92587

s/sContainer/containerStat/


- Vinod Kone


On Sept. 11, 2014, 6:46 p.m., Ian Downes wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/25549/
 ---
 
 (Updated Sept. 11, 2014, 6:46 p.m.)
 
 
 Review request for mesos, Ben Mahler, Jie Yu, and Vinod Kone.
 
 
 Bugs: MESOS-1586
 https://issues.apache.org/jira/browse/MESOS-1586
 
 
 Repository: mesos-git
 
 
 Description
 ---
 
 Does not report usage or enforce quota but can create 'private' directories 
 for each container which mask parts of the shared host filesystem.
 
 This review replaces

Re: Review Request 25549: Basic filesystem isolator for Linux.

2014-09-11 Thread Vinod Kone



 On Sept. 11, 2014, 7:13 p.m., Timothy Chen wrote:
  src/slave/containerizer/isolators/filesystem/shared.cpp, line 125
  https://reviews.apache.org/r/25549/diff/1/?file=686147#file686147line125
 
  how about when the host path doesn't start with /?

+1


- Vinod


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


On Sept. 11, 2014, 6:46 p.m., Ian Downes wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/25549/
 ---
 
 (Updated Sept. 11, 2014, 6:46 p.m.)
 
 
 Review request for mesos, Ben Mahler, Jie Yu, and Vinod Kone.
 
 
 Bugs: MESOS-1586
 https://issues.apache.org/jira/browse/MESOS-1586
 
 
 Repository: mesos-git
 
 
 Description
 ---
 
 Does not report usage or enforce quota but can create 'private' directories 
 for each container which mask parts of the shared host filesystem.
 
 This review replaces https://reviews.apache.org/r/24178/ because of some file 
 renaming. I addressed all comments from earlier reviews.
 
 
 Diffs
 -
 
   include/mesos/mesos.proto dea51f94d130c131421c43e7fd774ceb8941f501 
   src/Makefile.am 9b973e5503e30180045e270220987ba647da8038 
   src/common/parse.hpp e6153d8a1f25bc9ddbe1e391306beeacfc8d5ff6 
   src/common/type_utils.hpp 480c0883fe6ed7f6a9daf77d83ebb077da2e66ee 
   src/slave/containerizer/isolators/filesystem/shared.hpp PRE-CREATION 
   src/slave/containerizer/isolators/filesystem/shared.cpp PRE-CREATION 
   src/slave/containerizer/linux_launcher.cpp 
 d5ef1d6aa762cf81a3e8384552d97fe95b9cbd95 
   src/slave/containerizer/mesos/containerizer.cpp 
 9d083294caa5c5a47ba3ceaa1b57346144cb795c 
   src/slave/flags.hpp 21e00212bc402674eaea73b44b3f91df477a7213 
   src/slave/slave.cpp 1b3dc7370a2441e4159aa5ee552b64ca5e511e96 
   src/tests/isolator_tests.cpp c38f87632cb6984543cb3767dbd656cde7459610 
 
 Diff: https://reviews.apache.org/r/25549/diff/
 
 
 Testing
 ---
 
 make check # added a test
 
 
 Thanks,
 
 Ian Downes

38 matches

Mail list logo