Review Request 36422: Fix incorrect glog link.

2015-07-11 Thread haosdent huang

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

Review request for mesos and Adam B.


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


Repository: mesos


Description
---

Fix incorrect glog link.


Diffs
-

  docs/logging-and-debugging.md 1f037d6cdbc9e76976a796961c300d8842a16601 

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


Testing
---


Thanks,

haosdent huang



Re: Review Request 36185: Create pre-launch hook before a docker container launches in slave.

2015-07-11 Thread haosdent huang

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

(Updated July 11, 2015, 8:45 a.m.)


Review request for mesos and Timothy Chen.


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


Repository: mesos


Description
---

Create pre-launch hook before a docker container launches in slave.


Diffs (updated)
-

  include/mesos/hook.hpp 0995c249e9f07c6c4a26d1c5c369d48bb8056f1f 
  src/examples/test_hook_module.cpp d61cd557d8e44e5089f324edf97b0335a4ededab 
  src/hook/manager.hpp 47e8eb7d54d55049d054cf9b1225e67333f22adc 
  src/hook/manager.cpp 0108534c1fc527a0c66d201d7a5232e80b9928bf 
  src/slave/containerizer/docker.hpp 9a7a951a72390b38f5dda3a3d50c4b35a6c784fd 
  src/slave/containerizer/docker.cpp cfb60177fe48ec0eeab12ff392c6c9f89634b92f 
  src/tests/docker_containerizer_tests.cpp 
a3da786c1b733e9dd4abf1d02d5dae051393d921 
  src/tests/hook_tests.cpp 09205fb89925c22b1157294a756db87d911a63db 
  src/tests/mesos.hpp 9157ac079808d2686592e54ea26a26e6a0825ed3 

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


Testing
---

# Add a new unit test 
DockerContainerizerTest.ROOT_DOCKER_VerifySlavePreLaunchDockerHook
make check -j8 GTEST_FILTER=-*
sudo ./bin/mesos-tests.sh --verbose 
--gtest_filter=DockerContainerizerTest.ROOT_DOCKER_VerifySlavePreLaunchDockerHook


Thanks,

haosdent huang



Re: Review Request 36185: Create pre-launch hook before a docker container launches in slave.

2015-07-11 Thread haosdent huang


 On July 10, 2015, 11:20 p.m., Timothy Chen wrote:
  src/tests/docker_containerizer_tests.cpp, line 3114
  https://reviews.apache.org/r/36185/diff/3/?file=1002346#file1002346line3114
 
  What you suggested sounds fine.
  I think we need to comment on the top of the test what this tests and 
  what is it expecting to see. It wasn't too obvious when I read this.

Sorry for the not clear test case before, I updated it now.


- haosdent


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


On July 11, 2015, 8:45 a.m., haosdent huang wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/36185/
 ---
 
 (Updated July 11, 2015, 8:45 a.m.)
 
 
 Review request for mesos and Timothy Chen.
 
 
 Bugs: MESOS-2588
 https://issues.apache.org/jira/browse/MESOS-2588
 
 
 Repository: mesos
 
 
 Description
 ---
 
 Create pre-launch hook before a docker container launches in slave.
 
 
 Diffs
 -
 
   include/mesos/hook.hpp 0995c249e9f07c6c4a26d1c5c369d48bb8056f1f 
   src/examples/test_hook_module.cpp d61cd557d8e44e5089f324edf97b0335a4ededab 
   src/hook/manager.hpp 47e8eb7d54d55049d054cf9b1225e67333f22adc 
   src/hook/manager.cpp 0108534c1fc527a0c66d201d7a5232e80b9928bf 
   src/slave/containerizer/docker.hpp 9a7a951a72390b38f5dda3a3d50c4b35a6c784fd 
   src/slave/containerizer/docker.cpp cfb60177fe48ec0eeab12ff392c6c9f89634b92f 
   src/tests/docker_containerizer_tests.cpp 
 a3da786c1b733e9dd4abf1d02d5dae051393d921 
   src/tests/hook_tests.cpp 09205fb89925c22b1157294a756db87d911a63db 
   src/tests/mesos.hpp 9157ac079808d2686592e54ea26a26e6a0825ed3 
 
 Diff: https://reviews.apache.org/r/36185/diff/
 
 
 Testing
 ---
 
 # Add a new unit test 
 DockerContainerizerTest.ROOT_DOCKER_VerifySlavePreLaunchDockerHook
 make check -j8 GTEST_FILTER=-*
 sudo ./bin/mesos-tests.sh --verbose 
 --gtest_filter=DockerContainerizerTest.ROOT_DOCKER_VerifySlavePreLaunchDockerHook
 
 
 Thanks,
 
 haosdent huang
 




Re: Review Request 36185: Create pre-launch hook before a docker container launches in slave.

2015-07-11 Thread Mesos ReviewBot

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


Patch looks great!

Reviews applied: [36185]

All tests passed.

- Mesos ReviewBot


On July 11, 2015, 8:46 a.m., haosdent huang wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/36185/
 ---
 
 (Updated July 11, 2015, 8:46 a.m.)
 
 
 Review request for mesos and Timothy Chen.
 
 
 Bugs: MESOS-2588
 https://issues.apache.org/jira/browse/MESOS-2588
 
 
 Repository: mesos
 
 
 Description
 ---
 
 Create pre-launch hook before a docker container launches in slave.
 
 
 Diffs
 -
 
   include/mesos/hook.hpp 0995c249e9f07c6c4a26d1c5c369d48bb8056f1f 
   src/examples/test_hook_module.cpp d61cd557d8e44e5089f324edf97b0335a4ededab 
   src/hook/manager.hpp 47e8eb7d54d55049d054cf9b1225e67333f22adc 
   src/hook/manager.cpp 0108534c1fc527a0c66d201d7a5232e80b9928bf 
   src/slave/containerizer/docker.hpp 9a7a951a72390b38f5dda3a3d50c4b35a6c784fd 
   src/slave/containerizer/docker.cpp cfb60177fe48ec0eeab12ff392c6c9f89634b92f 
   src/tests/docker_containerizer_tests.cpp 
 a3da786c1b733e9dd4abf1d02d5dae051393d921 
   src/tests/hook_tests.cpp 09205fb89925c22b1157294a756db87d911a63db 
   src/tests/mesos.hpp 9157ac079808d2686592e54ea26a26e6a0825ed3 
 
 Diff: https://reviews.apache.org/r/36185/diff/
 
 
 Testing
 ---
 
 # Add a new unit test HookTest.ROOT_DOCKER_VerifySlavePreLaunchDockerHook
 make check -j8 GTEST_FILTER=-*
 sudo ./bin/mesos-tests.sh --verbose 
 --gtest_filter=HookTest.ROOT_DOCKER_VerifySlavePreLaunchDockerHook
 
 
 Thanks,
 
 haosdent huang
 




Re: Review Request 36181: Port CFS support to Docker Containerizer

2015-07-11 Thread Mesos ReviewBot

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


Patch looks great!

Reviews applied: [36181]

All tests passed.

- Mesos ReviewBot


On July 11, 2015, 8:59 a.m., haosdent huang wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/36181/
 ---
 
 (Updated July 11, 2015, 8:59 a.m.)
 
 
 Review request for mesos and Timothy Chen.
 
 
 Bugs: MESOS-2154
 https://issues.apache.org/jira/browse/MESOS-2154
 
 
 Repository: mesos
 
 
 Description
 ---
 
 Port CFS support to Docker Containerizer
 
 
 Diffs
 -
 
   src/slave/containerizer/docker.cpp cfb60177fe48ec0eeab12ff392c6c9f89634b92f 
 
 Diff: https://reviews.apache.org/r/36181/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 haosdent huang
 




Re: Review Request 35687: Added capabilities to state.json

2015-07-11 Thread Aditi Dixit


 On July 9, 2015, 4:26 p.m., Vinod Kone wrote:
  src/master/http.cpp, lines 124-125
  https://reviews.apache.org/r/35687/diff/2/?file=1003411#file1003411line124
 
  were you not able to use 'foreach' here?
  
  foreach(FrameworkInfo::Capability capability, 
  framework.info.capabilities()) {
  
  }

No. I think the problem is occurring because capabilities is a repeated field.


- Aditi


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


On July 9, 2015, 4:04 p.m., Aditi Dixit wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/35687/
 ---
 
 (Updated July 9, 2015, 4:04 p.m.)
 
 
 Review request for mesos, Marco Massenzio and Vinod Kone.
 
 
 Bugs: MESOS-2900
 https://issues.apache.org/jira/browse/MESOS-2900
 
 
 Repository: mesos
 
 
 Description
 ---
 
 Added capabilities to state.json and test for the same
 
 
 Diffs
 -
 
   src/master/http.cpp 350383362311cfbc830965e1155a8515f0dfb332 
   src/tests/master_tests.cpp 962455cc368c6e5405599d6565660d4c3fd0fc22 
 
 Diff: https://reviews.apache.org/r/35687/diff/
 
 
 Testing
 ---
 
 make check
 
 
 Thanks,
 
 Aditi Dixit
 




Re: Review Request 34136: Add ContainerImage protobuf.

2015-07-11 Thread Ian Downes


 On June 25, 2015, 3:32 p.m., Paul Brett wrote:
  include/mesos/mesos.proto, line 1221
  https://reviews.apache.org/r/34136/diff/2/?file=989752#file989752line1221
 
  Mesos info is optional, but if present can optionally contain an image? 
   So what does a mesos present with a MesosInfo message with no image mean?
 
 Vinod Kone wrote:
 yea. why optional?
 
 Timothy Chen wrote:
 I think image can be optional which bypasses the image provisioning. 
 MesosInfo potentially will contain other information like what DockerInfo 
 does that is MesosContainerizer specific, so I think this API makes sense to 
 me.

Yep, Tim's comments reflect what I'm thinking.


 On June 25, 2015, 3:32 p.m., Paul Brett wrote:
  include/mesos/mesos.proto, line 1202
  https://reviews.apache.org/r/34136/diff/2/?file=989752#file989752line1202
 
  Not really an id, more of a signature for the image.  Would we be 
  better making this optional string sha512hash to allow for the possibility 
  that a different signature might be used in the future?
 
 Vinod Kone wrote:
 i'm assuming this is because appc calls it an image id.

Correct, this reflects naming in the appc spec.


- Ian


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


On June 22, 2015, 9:42 a.m., Ian Downes wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/34136/
 ---
 
 (Updated June 22, 2015, 9:42 a.m.)
 
 
 Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone.
 
 
 Repository: mesos
 
 
 Description
 ---
 
 Add ContainerImage protobuf.
 
 
 Diffs
 -
 
   include/mesos/mesos.proto 8df1211165169c9595e0e6e85b5ddc404345ff70 
 
 Diff: https://reviews.apache.org/r/34136/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Ian Downes
 




Re: Review Request 34136: Add ContainerImage protobuf.

2015-07-11 Thread Ian Downes


 On June 26, 2015, 2:57 p.m., Jiang Yan Xu wrote:
  include/mesos/mesos.proto, lines 1212-1214
  https://reviews.apache.org/r/34136/diff/2/?file=989752#file989752line1212
 
  Is it the intention that Image type is **defined** outside MesosInfo 
  because DockerInfo can later reference it?
  
  Otherwise it feels more natual to define Image within MesosInfo.
 
 Vinod Kone wrote:
 +1 to put it inside MesosInfo.

I wanted it to be outside MesosInfo so it could be used by other container 
types, i.e., I don't see it as specific to MesosInfo, e.g., we could also 
support an AppcInfo which contained an Image::APPC.


- Ian


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


On June 22, 2015, 9:42 a.m., Ian Downes wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/34136/
 ---
 
 (Updated June 22, 2015, 9:42 a.m.)
 
 
 Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone.
 
 
 Repository: mesos
 
 
 Description
 ---
 
 Add ContainerImage protobuf.
 
 
 Diffs
 -
 
   include/mesos/mesos.proto 8df1211165169c9595e0e6e85b5ddc404345ff70 
 
 Diff: https://reviews.apache.org/r/34136/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Ian Downes
 




Re: Review Request 34135: Add filesystem/ isolators for persistent volumes.

2015-07-11 Thread Ian Downes

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

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


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


Changes
---

Moved filesystem/linux to separate review. Addressed comments.


Repository: mesos


Description
---

Moved code from Mesos Containerizer to filesystem isolators
 - filesystem/posix (symlinks, doesn't support container rootfs)
 - filesystem/linux (bind mounts, does support container rootfs)

The filesystem/posix isolator will be automatically included if no filesystem/ 
isolator is specified.


Diffs (updated)
-

  src/Makefile.am e5b5d36f0ac160e5a3a9fdc50b31c060a413ce2c 
  src/slave/containerizer/isolators/filesystem/posix.hpp PRE-CREATION 
  src/slave/containerizer/isolators/filesystem/posix.cpp PRE-CREATION 
  src/slave/containerizer/mesos/containerizer.cpp 
47d146125dfd4ea909e7ec9d94f41cfa11d035e5 

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


Testing
---

existing persistent volumes tests.


Thanks,

Ian Downes



Review Request 36428: Remove erroneous code for isolator modules.

2015-07-11 Thread Ian Downes

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

Review request for mesos, Ben Mahler, Kapil Arya, and Timothy Chen.


Repository: mesos


Description
---

Kapil: this appears to have been introduced in your commit 3a179ce2. Can you 
please verify this should be removed.


Diffs
-

  src/slave/containerizer/mesos/containerizer.cpp 
47d146125dfd4ea909e7ec9d94f41cfa11d035e5 

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


Testing
---


Thanks,

Ian Downes



Review Request 36429: Add filesystem/linux isolator for persistent volumes.

2015-07-11 Thread Ian Downes

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

Review request for mesos, Jie Yu, Timothy Chen, and Vinod Kone.


Repository: mesos


Description
---

Moved filesystem/linux from review https://reviews.apache.org/r/34135/


Diffs
-

  src/Makefile.am e5b5d36f0ac160e5a3a9fdc50b31c060a413ce2c 
  src/slave/containerizer/isolators/filesystem/linux.hpp PRE-CREATION 
  src/slave/containerizer/isolators/filesystem/linux.cpp PRE-CREATION 
  src/slave/containerizer/mesos/containerizer.cpp 
47d146125dfd4ea909e7ec9d94f41cfa11d035e5 

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


Testing
---


Thanks,

Ian Downes



Re: Review Request 34135: Add filesystem/ isolators for persistent volumes.

2015-07-11 Thread Mesos ReviewBot

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


Bad patch!

Reviews applied: [34908, 34136, 34137]

Failed command: ./support/apply-review.sh -n -r 34137

Error:
 2015-07-12 04:46:07 URL:https://reviews.apache.org/r/34137/diff/raw/ 
[27315/27315] - 34137.patch [1]
error: patch failed: include/mesos/slave/isolator.hpp:30
error: include/mesos/slave/isolator.hpp: patch does not apply
error: patch failed: src/Makefile.am:525
error: src/Makefile.am: patch does not apply
error: patch failed: src/slave/containerizer/mesos/containerizer.cpp:548
error: src/slave/containerizer/mesos/containerizer.cpp: patch does not apply
Failed to apply patch

- Mesos ReviewBot


On July 12, 2015, 4:42 a.m., Ian Downes wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/34135/
 ---
 
 (Updated July 12, 2015, 4:42 a.m.)
 
 
 Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone.
 
 
 Repository: mesos
 
 
 Description
 ---
 
 Moved code from Mesos Containerizer to filesystem isolators
  - filesystem/posix (symlinks, doesn't support container rootfs)
  - filesystem/linux (bind mounts, does support container rootfs)
 
 The filesystem/posix isolator will be automatically included if no 
 filesystem/ isolator is specified.
 
 
 Diffs
 -
 
   src/Makefile.am e5b5d36f0ac160e5a3a9fdc50b31c060a413ce2c 
   src/slave/containerizer/isolators/filesystem/posix.hpp PRE-CREATION 
   src/slave/containerizer/isolators/filesystem/posix.cpp PRE-CREATION 
   src/slave/containerizer/mesos/containerizer.cpp 
 47d146125dfd4ea909e7ec9d94f41cfa11d035e5 
 
 Diff: https://reviews.apache.org/r/34135/diff/
 
 
 Testing
 ---
 
 existing persistent volumes tests.
 
 
 Thanks,
 
 Ian Downes
 




Re: Review Request 36428: Remove erroneous code for isolator modules.

2015-07-11 Thread Kapil Arya

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


It looks like it was introduced in f511395e instead (unless I am missing 
something here):

https://github.com/apache/mesos/commit/f511395e#diff-c8ca6e064a8bf7b1b3c70e6525eabeceR129

- Kapil Arya


On July 12, 2015, 12:44 a.m., Ian Downes wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/36428/
 ---
 
 (Updated July 12, 2015, 12:44 a.m.)
 
 
 Review request for mesos, Ben Mahler, Kapil Arya, and Timothy Chen.
 
 
 Repository: mesos
 
 
 Description
 ---
 
 Kapil: this appears to have been introduced in your commit 3a179ce2. Can you 
 please verify this should be removed.
 
 
 Diffs
 -
 
   src/slave/containerizer/mesos/containerizer.cpp 
 47d146125dfd4ea909e7ec9d94f41cfa11d035e5 
 
 Diff: https://reviews.apache.org/r/36428/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Ian Downes
 




Re: Review Request 36428: Remove erroneous code for isolator modules.

2015-07-11 Thread Mesos ReviewBot

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


Patch looks great!

Reviews applied: [36428]

All tests passed.

- Mesos ReviewBot


On July 12, 2015, 4:44 a.m., Ian Downes wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/36428/
 ---
 
 (Updated July 12, 2015, 4:44 a.m.)
 
 
 Review request for mesos, Ben Mahler, Kapil Arya, and Timothy Chen.
 
 
 Repository: mesos
 
 
 Description
 ---
 
 Kapil: this appears to have been introduced in your commit 3a179ce2. Can you 
 please verify this should be removed.
 
 
 Diffs
 -
 
   src/slave/containerizer/mesos/containerizer.cpp 
 47d146125dfd4ea909e7ec9d94f41cfa11d035e5 
 
 Diff: https://reviews.apache.org/r/36428/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Ian Downes