Review Request 36422: Fix incorrect glog link.
--- 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.
--- 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.
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.
--- 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
--- 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
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.
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.
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.
--- 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.
--- 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.
--- 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.
--- 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.
--- 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.
--- 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