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/#review95972 --- src/tests/mesos.hpp (line 965) https://reviews.apache.org/r/36185/#comment151164 Could you please move this to a separate header file like? ``` src/tests/containerizer/docker.hpp ``` - Jie Yu On Aug. 20, 2015, 2:23 a.m., haosdent huang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36185/ --- (Updated Aug. 20, 2015, 2:23 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 6c3861b1ca82f0b5488bd2e766b1c5abc4fd7582 src/examples/test_hook_module.cpp 2f0a1a04e26b6f8e250560befab5921c89149edd src/hook/manager.hpp df32484fdf1de0cec8022546ef63b81c29e53ce4 src/hook/manager.cpp 0b693d2564310d2a71f31e991672117d91983452 src/slave/containerizer/docker.hpp e7436d054a5b3f29cb7d2d455f00e7cb3b734d97 src/slave/containerizer/docker.cpp 8f5d302477b216df9ac2f59156304bbc4a96f24b src/tests/containerizer/docker_containerizer_tests.cpp c8c27a64c06cf37bdaa5b474ea25bd2e971c8fea src/tests/hook_tests.cpp e9f1c777153b94eebea0f2c4fc2f4afb0e382afb src/tests/mesos.hpp 64987f0cc1632eb8b0c2cccd8446a5324127910c 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 36185: Create pre-launch hook before a docker container launches in slave.
On Aug. 20, 2015, 5:02 p.m., Jie Yu wrote: src/tests/mesos.hpp, line 965 https://reviews.apache.org/r/36185/diff/10/?file=1044480#file1044480line965 Could you please move this to a separate header file like? ``` src/tests/containerizer/docker.hpp ``` Thank you. Let me move it out. - haosdent --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36185/#review95972 --- On Aug. 20, 2015, 2:23 a.m., haosdent huang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36185/ --- (Updated Aug. 20, 2015, 2:23 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 6c3861b1ca82f0b5488bd2e766b1c5abc4fd7582 src/examples/test_hook_module.cpp 2f0a1a04e26b6f8e250560befab5921c89149edd src/hook/manager.hpp df32484fdf1de0cec8022546ef63b81c29e53ce4 src/hook/manager.cpp 0b693d2564310d2a71f31e991672117d91983452 src/slave/containerizer/docker.hpp e7436d054a5b3f29cb7d2d455f00e7cb3b734d97 src/slave/containerizer/docker.cpp 8f5d302477b216df9ac2f59156304bbc4a96f24b src/tests/containerizer/docker_containerizer_tests.cpp c8c27a64c06cf37bdaa5b474ea25bd2e971c8fea src/tests/hook_tests.cpp e9f1c777153b94eebea0f2c4fc2f4afb0e382afb src/tests/mesos.hpp 64987f0cc1632eb8b0c2cccd8446a5324127910c 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 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/#review95932 --- Patch looks great! Reviews applied: [36185] All tests passed. - Mesos ReviewBot On Aug. 20, 2015, 2:23 a.m., haosdent huang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36185/ --- (Updated Aug. 20, 2015, 2:23 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 6c3861b1ca82f0b5488bd2e766b1c5abc4fd7582 src/examples/test_hook_module.cpp 2f0a1a04e26b6f8e250560befab5921c89149edd src/hook/manager.hpp df32484fdf1de0cec8022546ef63b81c29e53ce4 src/hook/manager.cpp 0b693d2564310d2a71f31e991672117d91983452 src/slave/containerizer/docker.hpp e7436d054a5b3f29cb7d2d455f00e7cb3b734d97 src/slave/containerizer/docker.cpp 8f5d302477b216df9ac2f59156304bbc4a96f24b src/tests/containerizer/docker_containerizer_tests.cpp c8c27a64c06cf37bdaa5b474ea25bd2e971c8fea src/tests/hook_tests.cpp e9f1c777153b94eebea0f2c4fc2f4afb0e382afb src/tests/mesos.hpp 64987f0cc1632eb8b0c2cccd8446a5324127910c 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 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/#review95904 --- src/tests/hook_tests.cpp (line 527) https://reviews.apache.org/r/36185/#comment151104 Btw we no longer need a extra space between as we upgraded our gcc version. src/tests/hook_tests.cpp (line 539) https://reviews.apache.org/r/36185/#comment151105 Same here - Timothy Chen On Aug. 16, 2015, 7:15 a.m., haosdent huang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36185/ --- (Updated Aug. 16, 2015, 7:15 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 6c3861b1ca82f0b5488bd2e766b1c5abc4fd7582 src/examples/test_hook_module.cpp 2f0a1a04e26b6f8e250560befab5921c89149edd src/hook/manager.hpp df32484fdf1de0cec8022546ef63b81c29e53ce4 src/hook/manager.cpp 0b693d2564310d2a71f31e991672117d91983452 src/slave/containerizer/docker.hpp e7436d054a5b3f29cb7d2d455f00e7cb3b734d97 src/slave/containerizer/docker.cpp 8f5d302477b216df9ac2f59156304bbc4a96f24b src/tests/containerizer/docker_containerizer_tests.cpp c8c27a64c06cf37bdaa5b474ea25bd2e971c8fea src/tests/hook_tests.cpp e9f1c777153b94eebea0f2c4fc2f4afb0e382afb src/tests/mesos.hpp 64987f0cc1632eb8b0c2cccd8446a5324127910c 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 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 Aug. 20, 2015, 2:23 a.m.) Review request for mesos and Timothy Chen. Changes --- Update according tnachen review opinions 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 6c3861b1ca82f0b5488bd2e766b1c5abc4fd7582 src/examples/test_hook_module.cpp 2f0a1a04e26b6f8e250560befab5921c89149edd src/hook/manager.hpp df32484fdf1de0cec8022546ef63b81c29e53ce4 src/hook/manager.cpp 0b693d2564310d2a71f31e991672117d91983452 src/slave/containerizer/docker.hpp e7436d054a5b3f29cb7d2d455f00e7cb3b734d97 src/slave/containerizer/docker.cpp 8f5d302477b216df9ac2f59156304bbc4a96f24b src/tests/containerizer/docker_containerizer_tests.cpp c8c27a64c06cf37bdaa5b474ea25bd2e971c8fea src/tests/hook_tests.cpp e9f1c777153b94eebea0f2c4fc2f4afb0e382afb src/tests/mesos.hpp 64987f0cc1632eb8b0c2cccd8446a5324127910c 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 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 Aug. 16, 2015, 6:05 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 6c3861b1ca82f0b5488bd2e766b1c5abc4fd7582 src/examples/test_hook_module.cpp 2f0a1a04e26b6f8e250560befab5921c89149edd src/hook/manager.hpp df32484fdf1de0cec8022546ef63b81c29e53ce4 src/hook/manager.cpp 0b693d2564310d2a71f31e991672117d91983452 src/slave/containerizer/docker.hpp e7436d054a5b3f29cb7d2d455f00e7cb3b734d97 src/slave/containerizer/docker.cpp 8f5d302477b216df9ac2f59156304bbc4a96f24b src/tests/containerizer/docker_containerizer_tests.cpp c8c27a64c06cf37bdaa5b474ea25bd2e971c8fea src/tests/hook_tests.cpp e9f1c777153b94eebea0f2c4fc2f4afb0e382afb src/tests/mesos.hpp 64987f0cc1632eb8b0c2cccd8446a5324127910c 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 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/#review95536 --- src/tests/hook_tests.cpp (line 573) https://reviews.apache.org/r/36185/#comment150563 path::join(flags.sandbox_directory, foo) src/tests/hook_tests.cpp (line 618) https://reviews.apache.org/r/36185/#comment150562 I don't think you need to create a temporary docker again, you can simply reuse the docker you created in the beginning of the test. - Timothy Chen On Aug. 16, 2015, 6:05 a.m., haosdent huang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36185/ --- (Updated Aug. 16, 2015, 6:05 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 6c3861b1ca82f0b5488bd2e766b1c5abc4fd7582 src/examples/test_hook_module.cpp 2f0a1a04e26b6f8e250560befab5921c89149edd src/hook/manager.hpp df32484fdf1de0cec8022546ef63b81c29e53ce4 src/hook/manager.cpp 0b693d2564310d2a71f31e991672117d91983452 src/slave/containerizer/docker.hpp e7436d054a5b3f29cb7d2d455f00e7cb3b734d97 src/slave/containerizer/docker.cpp 8f5d302477b216df9ac2f59156304bbc4a96f24b src/tests/containerizer/docker_containerizer_tests.cpp c8c27a64c06cf37bdaa5b474ea25bd2e971c8fea src/tests/hook_tests.cpp e9f1c777153b94eebea0f2c4fc2f4afb0e382afb src/tests/mesos.hpp 64987f0cc1632eb8b0c2cccd8446a5324127910c 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 36185: Create pre-launch hook before a docker container launches in slave.
On Aug. 16, 2015, 6:53 a.m., Timothy Chen wrote: I think other than the minor issues I see this seems to good to go, once you fix this I'll run this locally and this should be ready to go. The jenkins seems have problem. https://builds.apache.org/job/Mesos/705/ The job hang 2 days. I could pass make -j8 distcheck local, and not sure the reviewbot failure because by my patch or others. - haosdent --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36185/#review95537 --- On Aug. 16, 2015, 7:15 a.m., haosdent huang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36185/ --- (Updated Aug. 16, 2015, 7:15 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 6c3861b1ca82f0b5488bd2e766b1c5abc4fd7582 src/examples/test_hook_module.cpp 2f0a1a04e26b6f8e250560befab5921c89149edd src/hook/manager.hpp df32484fdf1de0cec8022546ef63b81c29e53ce4 src/hook/manager.cpp 0b693d2564310d2a71f31e991672117d91983452 src/slave/containerizer/docker.hpp e7436d054a5b3f29cb7d2d455f00e7cb3b734d97 src/slave/containerizer/docker.cpp 8f5d302477b216df9ac2f59156304bbc4a96f24b src/tests/containerizer/docker_containerizer_tests.cpp c8c27a64c06cf37bdaa5b474ea25bd2e971c8fea src/tests/hook_tests.cpp e9f1c777153b94eebea0f2c4fc2f4afb0e382afb src/tests/mesos.hpp 64987f0cc1632eb8b0c2cccd8446a5324127910c 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 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 Aug. 16, 2015, 7:15 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 6c3861b1ca82f0b5488bd2e766b1c5abc4fd7582 src/examples/test_hook_module.cpp 2f0a1a04e26b6f8e250560befab5921c89149edd src/hook/manager.hpp df32484fdf1de0cec8022546ef63b81c29e53ce4 src/hook/manager.cpp 0b693d2564310d2a71f31e991672117d91983452 src/slave/containerizer/docker.hpp e7436d054a5b3f29cb7d2d455f00e7cb3b734d97 src/slave/containerizer/docker.cpp 8f5d302477b216df9ac2f59156304bbc4a96f24b src/tests/containerizer/docker_containerizer_tests.cpp c8c27a64c06cf37bdaa5b474ea25bd2e971c8fea src/tests/hook_tests.cpp e9f1c777153b94eebea0f2c4fc2f4afb0e382afb src/tests/mesos.hpp 64987f0cc1632eb8b0c2cccd8446a5324127910c 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 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/#review95539 --- Patch looks great! Reviews applied: [36185] All tests passed. - Mesos ReviewBot On Aug. 16, 2015, 7:15 a.m., haosdent huang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36185/ --- (Updated Aug. 16, 2015, 7:15 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 6c3861b1ca82f0b5488bd2e766b1c5abc4fd7582 src/examples/test_hook_module.cpp 2f0a1a04e26b6f8e250560befab5921c89149edd src/hook/manager.hpp df32484fdf1de0cec8022546ef63b81c29e53ce4 src/hook/manager.cpp 0b693d2564310d2a71f31e991672117d91983452 src/slave/containerizer/docker.hpp e7436d054a5b3f29cb7d2d455f00e7cb3b734d97 src/slave/containerizer/docker.cpp 8f5d302477b216df9ac2f59156304bbc4a96f24b src/tests/containerizer/docker_containerizer_tests.cpp c8c27a64c06cf37bdaa5b474ea25bd2e971c8fea src/tests/hook_tests.cpp e9f1c777153b94eebea0f2c4fc2f4afb0e382afb src/tests/mesos.hpp 64987f0cc1632eb8b0c2cccd8446a5324127910c 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 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 Aug. 15, 2015, 2:47 p.m.) Review request for mesos and Timothy Chen. Changes --- Update according to tnachen reviews. 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 6c3861b1ca82f0b5488bd2e766b1c5abc4fd7582 src/examples/test_hook_module.cpp 2f0a1a04e26b6f8e250560befab5921c89149edd src/hook/manager.hpp df32484fdf1de0cec8022546ef63b81c29e53ce4 src/hook/manager.cpp 0b693d2564310d2a71f31e991672117d91983452 src/slave/containerizer/docker.hpp e7436d054a5b3f29cb7d2d455f00e7cb3b734d97 src/slave/containerizer/docker.cpp 8f5d302477b216df9ac2f59156304bbc4a96f24b src/tests/containerizer/docker_containerizer_tests.cpp c8c27a64c06cf37bdaa5b474ea25bd2e971c8fea src/tests/hook_tests.cpp e9f1c777153b94eebea0f2c4fc2f4afb0e382afb src/tests/mesos.hpp 64987f0cc1632eb8b0c2cccd8446a5324127910c 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 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 Aug. 15, 2015, 4:34 p.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 6c3861b1ca82f0b5488bd2e766b1c5abc4fd7582 src/examples/test_hook_module.cpp 2f0a1a04e26b6f8e250560befab5921c89149edd src/hook/manager.hpp df32484fdf1de0cec8022546ef63b81c29e53ce4 src/hook/manager.cpp 0b693d2564310d2a71f31e991672117d91983452 src/slave/containerizer/docker.hpp e7436d054a5b3f29cb7d2d455f00e7cb3b734d97 src/slave/containerizer/docker.cpp 8f5d302477b216df9ac2f59156304bbc4a96f24b src/tests/containerizer/docker_containerizer_tests.cpp c8c27a64c06cf37bdaa5b474ea25bd2e971c8fea src/tests/hook_tests.cpp e9f1c777153b94eebea0f2c4fc2f4afb0e382afb src/tests/mesos.hpp 64987f0cc1632eb8b0c2cccd8446a5324127910c 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 36185: Create pre-launch hook before a docker container launches in slave.
On Aug. 1, 2015, 5:07 p.m., Timothy Chen wrote: src/hook/manager.cpp, line 187 https://reviews.apache.org/r/36185/diff/4/?file=1008885#file1008885line187 From the hook signature it seems like we make not distinction between launching a custom executor or a task. I don't know what usage we need from a hook like this yet, what use cases do you have in mind? I think user could do some prepare works here which have to run outside the docker container. Or this issue [MESOS-2588](https://issues.apache.org/jira/browse/MESOS-2588) is not accepted now? I could discard this patch if it is not accepted. - haosdent --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36185/#review93844 --- 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 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/#review93844 --- src/hook/manager.cpp (line 187) https://reviews.apache.org/r/36185/#comment148256 From the hook signature it seems like we make not distinction between launching a custom executor or a task. I don't know what usage we need from a hook like this yet, what use cases do you have in mind? src/hook/manager.cpp (line 197) https://reviews.apache.org/r/36185/#comment148255 We don't hold on to references on temporaries anymore according to mesos style guide. const TryNothing/TryNothing - Timothy Chen 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 36185: Create pre-launch hook before a docker container launches in slave.
On Aug. 1, 2015, 5:07 p.m., Timothy Chen wrote: src/hook/manager.cpp, line 187 https://reviews.apache.org/r/36185/diff/4/?file=1008885#file1008885line187 From the hook signature it seems like we make not distinction between launching a custom executor or a task. I don't know what usage we need from a hook like this yet, what use cases do you have in mind? haosdent huang wrote: I think user could do some prepare works here which have to run outside the docker container. Or this issue [MESOS-2588](https://issues.apache.org/jira/browse/MESOS-2588) is not accepted now? I could discard this patch if it is not accepted. I think it's fine to add a hook but it's just not clear how would a user distinguish the difference between custom executor or task, as I would assume that's important here. Perhaps we should also pass in TaskInfo/ExecutorInfo so that teh user has more ifnromation to know what to do. - Timothy --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36185/#review93844 --- 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 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 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/#review91367 --- src/tests/docker_containerizer_tests.cpp (line 3114) https://reviews.apache.org/r/36185/#comment144736 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. - Timothy Chen On July 8, 2015, 5:06 p.m., haosdent huang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36185/ --- (Updated July 8, 2015, 5:06 p.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.cpp cfb60177fe48ec0eeab12ff392c6c9f89634b92f src/tests/docker_containerizer_tests.cpp a3da786c1b733e9dd4abf1d02d5dae051393d921 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/ --- (Updated July 8, 2015, 5:04 p.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 - 3rdparty/libprocess/src/libevent_ssl_socket.cpp cecd7af2016556ed90feac2ba8d886a1ea960a06 3rdparty/libprocess/src/tests/ssl_tests.cpp 97a3f2a576b6fbb2c4c38c24d39c76622ae91217 CHANGELOG e4686cbc7349a07f98e8e4d59dd51b75a50d0421 include/mesos/hook.hpp 0995c249e9f07c6c4a26d1c5c369d48bb8056f1f src/Makefile.am e5b5d36f0ac160e5a3a9fdc50b31c060a413ce2c src/examples/test_hook_module.cpp d61cd557d8e44e5089f324edf97b0335a4ededab src/hook/manager.hpp 47e8eb7d54d55049d054cf9b1225e67333f22adc src/hook/manager.cpp 0108534c1fc527a0c66d201d7a5232e80b9928bf src/linux/fs.hpp f3aa0c2a385527eab5895f5d10c29e3d68e30a49 src/linux/fs.cpp ea0891e320154b85a21ed2d138c192821efae9cd src/slave/containerizer/containerizer.hpp 52145540ff250890885a2d3f247fedcb8498b58b src/slave/containerizer/containerizer.cpp 25c87e9f948b7efe8b9a853c403bee69982d6c4c src/slave/containerizer/docker.hpp 9a7a951a72390b38f5dda3a3d50c4b35a6c784fd src/slave/containerizer/docker.cpp cfb60177fe48ec0eeab12ff392c6c9f89634b92f src/slave/containerizer/isolators/network/port_mapping.cpp a7757f2a51f04da27645074f048722c22a2be752 src/slave/containerizer/mesos/launch.hpp 7a7f3d7503962fe079e40b3b059d44459eedf3a5 src/slave/containerizer/mesos/launch.cpp 96ca47dff69d7706b570a96244549164918d2c2f src/tests/docker_containerizer_tests.cpp a3da786c1b733e9dd4abf1d02d5dae051393d921 src/tests/environment.cpp 525347090f38b61f2085a2b2a6002d28d11b222f src/tests/launch_tests.cpp 73c8c5fa17936b1bab4817e9f1e691144e87c35f src/tests/port_mapping_tests.cpp f01c3e1308755bb07da16b8bed576cdb958f0e0e src/tests/slave_tests.cpp ea0ddf38444915a1cda71cce6a8897803fa49198 Diff: https://reviews.apache.org/r/36185/diff/ Testing (updated) --- # 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/ --- (Updated July 8, 2015, 5:04 p.m.) Review request for mesos and Timothy Chen. Changes --- Add unit test 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) - 3rdparty/libprocess/src/libevent_ssl_socket.cpp cecd7af2016556ed90feac2ba8d886a1ea960a06 3rdparty/libprocess/src/tests/ssl_tests.cpp 97a3f2a576b6fbb2c4c38c24d39c76622ae91217 CHANGELOG e4686cbc7349a07f98e8e4d59dd51b75a50d0421 include/mesos/hook.hpp 0995c249e9f07c6c4a26d1c5c369d48bb8056f1f src/Makefile.am e5b5d36f0ac160e5a3a9fdc50b31c060a413ce2c src/examples/test_hook_module.cpp d61cd557d8e44e5089f324edf97b0335a4ededab src/hook/manager.hpp 47e8eb7d54d55049d054cf9b1225e67333f22adc src/hook/manager.cpp 0108534c1fc527a0c66d201d7a5232e80b9928bf src/linux/fs.hpp f3aa0c2a385527eab5895f5d10c29e3d68e30a49 src/linux/fs.cpp ea0891e320154b85a21ed2d138c192821efae9cd src/slave/containerizer/containerizer.hpp 52145540ff250890885a2d3f247fedcb8498b58b src/slave/containerizer/containerizer.cpp 25c87e9f948b7efe8b9a853c403bee69982d6c4c src/slave/containerizer/docker.hpp 9a7a951a72390b38f5dda3a3d50c4b35a6c784fd src/slave/containerizer/docker.cpp cfb60177fe48ec0eeab12ff392c6c9f89634b92f src/slave/containerizer/isolators/network/port_mapping.cpp a7757f2a51f04da27645074f048722c22a2be752 src/slave/containerizer/mesos/launch.hpp 7a7f3d7503962fe079e40b3b059d44459eedf3a5 src/slave/containerizer/mesos/launch.cpp 96ca47dff69d7706b570a96244549164918d2c2f src/tests/docker_containerizer_tests.cpp a3da786c1b733e9dd4abf1d02d5dae051393d921 src/tests/environment.cpp 525347090f38b61f2085a2b2a6002d28d11b222f src/tests/launch_tests.cpp 73c8c5fa17936b1bab4817e9f1e691144e87c35f src/tests/port_mapping_tests.cpp f01c3e1308755bb07da16b8bed576cdb958f0e0e src/tests/slave_tests.cpp ea0ddf38444915a1cda71cce6a8897803fa49198 Diff: https://reviews.apache.org/r/36185/diff/ Testing (updated) --- 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/ --- (Updated July 8, 2015, 5:04 p.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 - 3rdparty/libprocess/src/libevent_ssl_socket.cpp cecd7af2016556ed90feac2ba8d886a1ea960a06 3rdparty/libprocess/src/tests/ssl_tests.cpp 97a3f2a576b6fbb2c4c38c24d39c76622ae91217 CHANGELOG e4686cbc7349a07f98e8e4d59dd51b75a50d0421 include/mesos/hook.hpp 0995c249e9f07c6c4a26d1c5c369d48bb8056f1f src/Makefile.am e5b5d36f0ac160e5a3a9fdc50b31c060a413ce2c src/examples/test_hook_module.cpp d61cd557d8e44e5089f324edf97b0335a4ededab src/hook/manager.hpp 47e8eb7d54d55049d054cf9b1225e67333f22adc src/hook/manager.cpp 0108534c1fc527a0c66d201d7a5232e80b9928bf src/linux/fs.hpp f3aa0c2a385527eab5895f5d10c29e3d68e30a49 src/linux/fs.cpp ea0891e320154b85a21ed2d138c192821efae9cd src/slave/containerizer/containerizer.hpp 52145540ff250890885a2d3f247fedcb8498b58b src/slave/containerizer/containerizer.cpp 25c87e9f948b7efe8b9a853c403bee69982d6c4c src/slave/containerizer/docker.hpp 9a7a951a72390b38f5dda3a3d50c4b35a6c784fd src/slave/containerizer/docker.cpp cfb60177fe48ec0eeab12ff392c6c9f89634b92f src/slave/containerizer/isolators/network/port_mapping.cpp a7757f2a51f04da27645074f048722c22a2be752 src/slave/containerizer/mesos/launch.hpp 7a7f3d7503962fe079e40b3b059d44459eedf3a5 src/slave/containerizer/mesos/launch.cpp 96ca47dff69d7706b570a96244549164918d2c2f src/tests/docker_containerizer_tests.cpp a3da786c1b733e9dd4abf1d02d5dae051393d921 src/tests/environment.cpp 525347090f38b61f2085a2b2a6002d28d11b222f src/tests/launch_tests.cpp 73c8c5fa17936b1bab4817e9f1e691144e87c35f src/tests/port_mapping_tests.cpp f01c3e1308755bb07da16b8bed576cdb958f0e0e src/tests/slave_tests.cpp ea0ddf38444915a1cda71cce6a8897803fa49198 Diff: https://reviews.apache.org/r/36185/diff/ Testing (updated) --- # 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 7, 2015, 12:05 a.m., Timothy Chen wrote: Thanks for adding the hook! There is one more code path that a docker container can be launched, which is in launchExecutorProcess. Can you add the hook call in that path as well? Hi, @tnachen I update the patch. Could you review again, thank you very much. - haosdent --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36185/#review90610 --- On July 8, 2015, 5:06 p.m., haosdent huang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36185/ --- (Updated July 8, 2015, 5:06 p.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.cpp cfb60177fe48ec0eeab12ff392c6c9f89634b92f src/tests/docker_containerizer_tests.cpp a3da786c1b733e9dd4abf1d02d5dae051393d921 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/ --- (Updated July 8, 2015, 5:06 p.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.cpp cfb60177fe48ec0eeab12ff392c6c9f89634b92f src/tests/docker_containerizer_tests.cpp a3da786c1b733e9dd4abf1d02d5dae051393d921 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/#review90934 --- Patch looks great! Reviews applied: [36185] All tests passed. - Mesos ReviewBot On July 8, 2015, 5:06 p.m., haosdent huang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36185/ --- (Updated July 8, 2015, 5:06 p.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.cpp cfb60177fe48ec0eeab12ff392c6c9f89634b92f src/tests/docker_containerizer_tests.cpp a3da786c1b733e9dd4abf1d02d5dae051393d921 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 8, 2015, 9:07 p.m., Timothy Chen wrote: src/tests/docker_containerizer_tests.cpp, line 3011 https://reviews.apache.org/r/36185/diff/3/?file=1002346#file1002346line3011 nit: I think we should name this to reflect the test like org_apache_mesos_TestSlavePreLaunchDockerHook The name is from mesos/src/examples/test_hook_module.cpp, so could not change it here. On July 8, 2015, 9:07 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 exactly is this test trying to verify? I don't see any hook interactions? This test would create a file in sandbox ```cpp LOG(INFO) Executing 'slavePreLaunchDockerHook'; return os::touch(sandboxDirectory + /foo); ``` And when the directory mount to docker, verify the file in docker exist or not ```cpp command.set_value(test -f + flags.docker_sandbox_directory + /foo); ``` Currently, all hook tests are put in `./src/tests/hook_tests.cpp`. It should be better move this test case to that file, but this test case also depends on `MockDockerContainerizer` and `MockDocker`. How about move `MockDocker` and `MockDockerContainerizer` to `mesos/src/tests/mesos.hpp`? - haosdent --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36185/#review90992 --- On July 8, 2015, 5:06 p.m., haosdent huang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36185/ --- (Updated July 8, 2015, 5:06 p.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.cpp cfb60177fe48ec0eeab12ff392c6c9f89634b92f src/tests/docker_containerizer_tests.cpp a3da786c1b733e9dd4abf1d02d5dae051393d921 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/#review90992 --- src/tests/docker_containerizer_tests.cpp (line 3011) https://reviews.apache.org/r/36185/#comment144176 nit: I think we should name this to reflect the test like org_apache_mesos_TestSlavePreLaunchDockerHook src/tests/docker_containerizer_tests.cpp (line 3114) https://reviews.apache.org/r/36185/#comment144178 What exactly is this test trying to verify? I don't see any hook interactions? - Timothy Chen On July 8, 2015, 5:06 p.m., haosdent huang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36185/ --- (Updated July 8, 2015, 5:06 p.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.cpp cfb60177fe48ec0eeab12ff392c6c9f89634b92f src/tests/docker_containerizer_tests.cpp a3da786c1b733e9dd4abf1d02d5dae051393d921 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/#review90610 --- Thanks for adding the hook! There is one more code path that a docker container can be launched, which is in launchExecutorProcess. Can you add the hook call in that path as well? - Timothy Chen On July 5, 2015, 9:14 a.m., haosdent huang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36185/ --- (Updated July 5, 2015, 9:14 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/hook/manager.hpp 47e8eb7d54d55049d054cf9b1225e67333f22adc src/hook/manager.cpp 0108534c1fc527a0c66d201d7a5232e80b9928bf src/slave/containerizer/docker.cpp 6eb1c84b1a6ef17c3e2ea2028e2f2d75110176ba Diff: https://reviews.apache.org/r/36185/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/#review90405 --- Patch looks great! Reviews applied: [36185] All tests passed. - Mesos ReviewBot On July 5, 2015, 9:14 a.m., haosdent huang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36185/ --- (Updated July 5, 2015, 9:14 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/hook/manager.hpp 47e8eb7d54d55049d054cf9b1225e67333f22adc src/hook/manager.cpp 0108534c1fc527a0c66d201d7a5232e80b9928bf src/slave/containerizer/docker.cpp 6eb1c84b1a6ef17c3e2ea2028e2f2d75110176ba Diff: https://reviews.apache.org/r/36185/diff/ Testing --- Thanks, haosdent huang