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 37655: MESOS-3299: Add protobuf to represent time with integer precision.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37655/ --- (Updated Aug. 20, 2015, 10:03 a.m.) Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Joris Van Remoortere, and Vinod Kone. Bugs: MESOS-3299 https://issues.apache.org/jira/browse/MESOS-3299 Repository: mesos Description (updated) --- Instead of using doubles, seconds and nanoseconds can be represented like `struct timespec`, with one field for seconds and one for nanoseconds. This will be important if frameworks need to compare times to make decisions (such as for maintenance primitives). Note about the naming: * Time will conflict with the Time class. * Most denominations of time (Seconds, Minutes, Hours, etc) will conflict with Duration. Diffs - include/mesos/mesos.proto 33e1b28f1ccbe227657a14395f81df20e0a9e193 Diff: https://reviews.apache.org/r/37655/diff/ Testing --- `make check` Thanks, Joseph Wu
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 37617: Added an error message to MesosContainerizerLaunch command.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37617/#review95930 --- Patch looks great! Reviews applied: [37617] All tests passed. - Mesos ReviewBot On Aug. 19, 2015, 6:26 p.m., Jie Yu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37617/ --- (Updated Aug. 19, 2015, 6:26 p.m.) Review request for mesos, Marco Massenzio and Vinod Kone. Bugs: MESOS-3296 https://issues.apache.org/jira/browse/MESOS-3296 Repository: mesos Description --- Added an error message to MesosContainerizerLaunch command. So that we can triage MESOS-3296. Diffs - src/slave/containerizer/mesos/launch.cpp bd594d3f3d729aa000a1a7fbf9038f6cfcbb169b Diff: https://reviews.apache.org/r/37617/diff/ Testing --- make check Thanks, Jie Yu
Re: Review Request 37336: [WIP] Added `wait()` method to process::Subprocess
On Aug. 17, 2015, 11:01 p.m., Ben Mahler wrote: In the same vein as os::shell, we should probably introduce an 'os' namespace in libprocess for asynchronous os utilities. In this case, process::os::shell which returns a Future of the output (although, ideally status, output, error). Sounds good. If I understand this correctly, the `process::os::shell` would take advantage of this refactoring, correct? (also, how about returning a `FutureCommandResult`: this has all of status, output, error and a bit more). Could you please take a look at the next revision I'm about to post: it now all works and passes the tests, but I'm somewhat confused as to why the `onFailed` never got called and another nit - they are marked as FIXME(marco) and will be obviously removed - I needed a way to mark those areas visibly: they are not TODOs). Thanks! - Marco --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37336/#review95658 --- On Aug. 15, 2015, 2:02 a.m., Marco Massenzio wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37336/ --- (Updated Aug. 15, 2015, 2:02 a.m.) Review request for mesos and Joris Van Remoortere. Bugs: MESOS-3035 https://issues.apache.org/jira/browse/MESOS-3035 Repository: mesos Description --- Jira: MESOS-3035 The original API for `process::Subprocess` still left a lot of legwork to do for the caller; we have now added a `wait(Duration timeout)` method that returns a `CommandResult` (also introduced with this patch) which contains useful information about the command invocation; the exit code; stdout and, optionally, stderr too. The `wait()` method will wait for both the process to terminate, and stdout/stderr to be available to read from; it also unpacks them into ready-to-use `string`s. This is still WIP as I'm seeing some unusual behavior and I'd like to discuss with someone more expert on libprocess. Diffs - 3rdparty/libprocess/include/process/subprocess.hpp 310cb4f8e4e2faa5545dffd196d7490c868bc5d6 3rdparty/libprocess/src/subprocess.cpp d6ea62ed1c914d34e0e189395831c86fff8aac22 3rdparty/libprocess/src/tests/subprocess_tests.cpp ab7515325e5db0a4fd222bb982f51243d7b7e39d Diff: https://reviews.apache.org/r/37336/diff/ Testing --- make check Thanks, Marco Massenzio
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 37466: Update perf tests to including testing the supported perf output formats.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37466/ --- (Updated Aug. 21, 2015, 4:06 a.m.) Review request for mesos and Ben Mahler. Changes --- Fix broken patch. Bugs: MESOS-2834 https://issues.apache.org/jira/browse/MESOS-2834 Repository: mesos Description --- Update perf tests to including testing the supported perf output formats. Diffs (updated) - src/linux/perf.cpp cdc5f8314a875ababf2b17a32873775d808e1c78 src/tests/containerizer/perf_tests.cpp 6b3d70f3e7ea8f59f94e6961491d4e9a730e3334 Diff: https://reviews.apache.org/r/37466/diff/ Testing --- sudo make check Thanks, Paul Brett
Re: Review Request 37462: Add support for version detection and parsing.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37462/ --- (Updated Aug. 21, 2015, 12:48 a.m.) Review request for mesos and Ben Mahler. Changes --- Rebase. Bugs: MESOS-2834 https://issues.apache.org/jira/browse/MESOS-2834 Repository: mesos Description --- Add support for version detection and parsing. Diffs (updated) - src/linux/perf.cpp cdc5f8314a875ababf2b17a32873775d808e1c78 Diff: https://reviews.apache.org/r/37462/diff/ Testing --- sudo make check Perf tests may fail on many machines because the tests are currently using the version of perf installed on the machine to choose decode. The next patch will extend the perf tests to supply test cases for each of the supported perf versions. Thanks, Paul Brett
Re: Review Request 36620: Added Non-Freezeer Task Killer.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36620/ --- (Updated Aug. 20, 2015, 11:42 a.m.) Review request for mesos, Benjamin Hindman and Timothy Chen. Summary (updated) - Added Non-Freezeer Task Killer. Bugs: MESOS-3086 https://issues.apache.org/jira/browse/MESOS-3086 Repository: mesos Description --- WIP Added Non-Freezeer Task Killer. Diffs - src/linux/cgroups.cpp 6ef42ed1bc719f334d1ac6e90919a1bc1840d31f Diff: https://reviews.apache.org/r/36620/diff/ Testing --- sudo make check + manual tests Thanks, Joerg Schad
Review Request 37669: Ignore overflow components in version parsing.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37669/ --- Review request for mesos, Isabel Jimenez and Timothy Chen. Bugs: MESOS-2986 https://issues.apache.org/jira/browse/MESOS-2986 Repository: mesos Description --- Ignore overflow components in version parsing. Diffs - 3rdparty/libprocess/3rdparty/stout/include/stout/version.hpp 51010845e8885ac52ce88ec0deb6b65a81122cba 3rdparty/libprocess/3rdparty/stout/tests/version_tests.cpp e8f8358f3c113b4e21e30cb5e9d6a3d229191484 Diff: https://reviews.apache.org/r/37669/diff/ Testing --- Thanks, haosdent huang
Re: Review Request 37466: Update perf tests to including testing the supported perf output formats.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37466/ --- (Updated Aug. 20, 2015, 11 p.m.) Review request for mesos and Ben Mahler. Changes --- Rework to use value parameterized tests. Bugs: MESOS-2834 https://issues.apache.org/jira/browse/MESOS-2834 Repository: mesos Description --- Update perf tests to including testing the supported perf output formats. Diffs (updated) - src/tests/containerizer/perf_tests.cpp 6b3d70f3e7ea8f59f94e6961491d4e9a730e3334 Diff: https://reviews.apache.org/r/37466/diff/ Testing --- sudo make check Thanks, Paul Brett
Re: Review Request 37466: Update perf tests to including testing the supported perf output formats.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37466/ --- (Updated Aug. 20, 2015, 11:51 p.m.) Review request for mesos and Ben Mahler. Changes --- Correct indentation. Bugs: MESOS-2834 https://issues.apache.org/jira/browse/MESOS-2834 Repository: mesos Description --- Update perf tests to including testing the supported perf output formats. Diffs (updated) - src/tests/containerizer/perf_tests.cpp 6b3d70f3e7ea8f59f94e6961491d4e9a730e3334 Diff: https://reviews.apache.org/r/37466/diff/ Testing --- sudo make check Thanks, Paul Brett
Re: Review Request 37657: Document iptable rule need added when use docker bridge network mode.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37657/#review96020 --- docs/docker-containerizer.md (line 21) https://reviews.apache.org/r/37657/#comment151215 I'm also not a native speaker, but i'd suggest to: If *iptables* enabled on slave, make sure *iptables* accept all traffic from docker bridge interface. Example: ``` iptables -A INPUT -s 172.17.0.0/16 -i docker0 -p tcp -j ACCEPT ``` - Klaus Ma On 八月 20, 2015, 5:30 p.m., haosdent huang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37657/ --- (Updated 八月 20, 2015, 5:30 p.m.) Review request for mesos, Klaus Ma and Timothy Chen. Bugs: MESOS-3053 https://issues.apache.org/jira/browse/MESOS-3053 Repository: mesos Description --- Document iptable rule need added when use docker bridge network mode. Diffs - docs/docker-containerizer.md 73f897780a0bb72ab092cb08186a228e3084e798 Diff: https://reviews.apache.org/r/37657/diff/ Testing --- Thanks, haosdent huang
Re: Review Request 37531: MESOS-3070
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37531/ --- (Updated 八月 21, 2015, 2:10 a.m.) Review request for mesos. Bugs: MESOS-3070 https://issues.apache.org/jira/browse/MESOS-3070 Repository: mesos Description --- MESOS-3070 (Master CHECK failure if a framework uses duplicated task id.) Diffs (updated) - include/mesos/mesos.proto 33e1b28 include/mesos/type_utils.hpp dafe1df src/master/master.hpp 0432842 src/master/master.cpp 95207d2 src/master/validation.cpp ffb7bf0 src/messages/messages.proto 8977d8e src/sched/sched.cpp 012af05 src/slave/slave.cpp 2a99abc src/tests/master_tests.cpp 8a6b98b Diff: https://reviews.apache.org/r/37531/diff/ Testing --- Draft code diff, will update UT cases later. Thanks, Klaus Ma
Re: Review Request 37531: MESOS-3070
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37531/#review95670 --- src/master/master.cpp (line 3218) https://reviews.apache.org/r/37531/#comment150763 Yes; the TaskID will send back to framework by statusUpdate, so framework can use the UUID to kill a task which is not included in taskTag logic. - Klaus Ma On 八月 17, 2015, 1:28 p.m., Klaus Ma wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37531/ --- (Updated 八月 17, 2015, 1:28 p.m.) Review request for mesos. Bugs: MESOS-3070 https://issues.apache.org/jira/browse/MESOS-3070 Repository: mesos Description --- MESOS-3070 (Master CHECK failure if a framework uses duplicated task id.) Diffs - include/mesos/mesos.proto 33e1b28f1ccbe227657a14395f81df20e0a9e193 include/mesos/type_utils.hpp dafe1df0cb5d0b83ca0579068916fe7fda848f02 src/master/master.hpp 0432842d77beba024c7895291ca410964bae96be src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf src/master/validation.cpp ffb7bf07b8a40d6e14f922eabcf46045462498b5 src/messages/messages.proto 8977d8e0f3b16003128b6b9cab556a7b224f083c Diff: https://reviews.apache.org/r/37531/diff/ Testing --- Draft code diff, will update UT cases later. Thanks, Klaus Ma
Re: Review Request 37657: Document iptable rule need added when use docker bridge network mode.
On 八月 21, 2015, 1:26 a.m., Klaus Ma wrote: docs/docker-containerizer.md, line 21 https://reviews.apache.org/r/37657/diff/1/?file=1045176#file1045176line21 I'm also not a native speaker, but i'd suggest to: If *iptables* enabled on slave, make sure *iptables* accept all traffic from docker bridge interface. Example: ``` iptables -A INPUT -s 172.17.0.0/16 -i docker0 -p tcp -j ACCEPT ``` haosdent huang wrote: Because the patch is submitted, if it is not urgent, could update next time. :-) sure, it's clear enough to end user :). - Klaus --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37657/#review96020 --- On 八月 20, 2015, 5:30 p.m., haosdent huang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37657/ --- (Updated 八月 20, 2015, 5:30 p.m.) Review request for mesos, Klaus Ma and Timothy Chen. Bugs: MESOS-3053 https://issues.apache.org/jira/browse/MESOS-3053 Repository: mesos Description --- Document iptable rule need added when use docker bridge network mode. Diffs - docs/docker-containerizer.md 73f897780a0bb72ab092cb08186a228e3084e798 Diff: https://reviews.apache.org/r/37657/diff/ Testing --- Thanks, haosdent huang
Re: Review Request 37657: Document iptable rule need added when use docker bridge network mode.
On Aug. 21, 2015, 1:26 a.m., Klaus Ma wrote: docs/docker-containerizer.md, line 21 https://reviews.apache.org/r/37657/diff/1/?file=1045176#file1045176line21 I'm also not a native speaker, but i'd suggest to: If *iptables* enabled on slave, make sure *iptables* accept all traffic from docker bridge interface. Example: ``` iptables -A INPUT -s 172.17.0.0/16 -i docker0 -p tcp -j ACCEPT ``` Because the patch is submitted, if it is not urgent, could update next time. :-) - haosdent --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37657/#review96020 --- On Aug. 20, 2015, 5:30 p.m., haosdent huang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37657/ --- (Updated Aug. 20, 2015, 5:30 p.m.) Review request for mesos, Klaus Ma and Timothy Chen. Bugs: MESOS-3053 https://issues.apache.org/jira/browse/MESOS-3053 Repository: mesos Description --- Document iptable rule need added when use docker bridge network mode. Diffs - docs/docker-containerizer.md 73f897780a0bb72ab092cb08186a228e3084e798 Diff: https://reviews.apache.org/r/37657/diff/ Testing --- Thanks, haosdent huang
Re: Review Request 37462: Add support for version detection and parsing.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37462/ --- (Updated Aug. 20, 2015, 11:49 p.m.) Review request for mesos and Ben Mahler. Changes --- Fix handling of empty fields. Bugs: MESOS-2834 https://issues.apache.org/jira/browse/MESOS-2834 Repository: mesos Description --- Add support for version detection and parsing. Diffs (updated) - src/linux/perf.cpp cdc5f8314a875ababf2b17a32873775d808e1c78 Diff: https://reviews.apache.org/r/37462/diff/ Testing --- sudo make check Perf tests may fail on many machines because the tests are currently using the version of perf installed on the machine to choose decode. The next patch will extend the perf tests to supply test cases for each of the supported perf versions. Thanks, Paul Brett
Re: Review Request 37462: Add support for version detection and parsing.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37462/ --- (Updated Aug. 21, 2015, midnight) Review request for mesos and Ben Mahler. Bugs: MESOS-2834 https://issues.apache.org/jira/browse/MESOS-2834 Repository: mesos Description --- Add support for version detection and parsing. Diffs (updated) - src/linux/perf.cpp cdc5f8314a875ababf2b17a32873775d808e1c78 Diff: https://reviews.apache.org/r/37462/diff/ Testing --- sudo make check Perf tests may fail on many machines because the tests are currently using the version of perf installed on the machine to choose decode. The next patch will extend the perf tests to supply test cases for each of the supported perf versions. Thanks, Paul Brett
Re: Review Request 37657: Document iptable rule need added when use docker bridge network mode.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37657/#review95994 --- Ship it! Ship It! - Timothy Chen On Aug. 20, 2015, 5:30 p.m., haosdent huang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37657/ --- (Updated Aug. 20, 2015, 5:30 p.m.) Review request for mesos, Klaus Ma and Timothy Chen. Bugs: MESOS-3053 https://issues.apache.org/jira/browse/MESOS-3053 Repository: mesos Description --- Document iptable rule need added when use docker bridge network mode. Diffs - docs/docker-containerizer.md 73f897780a0bb72ab092cb08186a228e3084e798 Diff: https://reviews.apache.org/r/37657/diff/ Testing --- Thanks, haosdent huang
Review Request 37657: Document iptable rule need added when use docker bridge network mode.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37657/ --- Review request for mesos and Timothy Chen. Bugs: MESOS-3053 https://issues.apache.org/jira/browse/MESOS-3053 Repository: mesos Description --- Document iptable rule need added when use docker bridge network mode. Diffs - docs/docker-containerizer.md 73f897780a0bb72ab092cb08186a228e3084e798 Diff: https://reviews.apache.org/r/37657/diff/ Testing --- Thanks, haosdent huang
Re: Review Request 37460: Prevent perf test failures from killing the test harness.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37460/ --- (Updated Aug. 20, 2015, 5:52 p.m.) Review request for mesos and Ben Mahler. Changes --- Implement review comment. Bugs: MESOS-2834 https://issues.apache.org/jira/browse/MESOS-2834 Repository: mesos Description --- Prevent perf test failures from killing the test harness. Diffs (updated) - src/tests/containerizer/perf_tests.cpp 6b3d70f3e7ea8f59f94e6961491d4e9a730e3334 Diff: https://reviews.apache.org/r/37460/diff/ Testing --- sudo make check Thanks, Paul Brett
Re: Review Request 37657: Document iptable rule need added when use docker bridge network mode.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37657/#review95995 --- Patch looks great! Reviews applied: [37657] All tests passed. - Mesos ReviewBot On Aug. 20, 2015, 5:30 p.m., haosdent huang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37657/ --- (Updated Aug. 20, 2015, 5:30 p.m.) Review request for mesos, Klaus Ma and Timothy Chen. Bugs: MESOS-3053 https://issues.apache.org/jira/browse/MESOS-3053 Repository: mesos Description --- Document iptable rule need added when use docker bridge network mode. Diffs - docs/docker-containerizer.md 73f897780a0bb72ab092cb08186a228e3084e798 Diff: https://reviews.apache.org/r/37657/diff/ Testing --- Thanks, haosdent huang
Re: Review Request 37442: Factor out the token extraction rules in prepartion for extending them to cope with multiple versions.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37442/ --- (Updated Aug. 20, 2015, 5:31 p.m.) Review request for mesos and Ben Mahler. Changes --- Remove redundant constructor. Repository: mesos Description --- Factor out the token extraction rules in prepartion for extending them to cope with multiple versions. Diffs (updated) - src/linux/perf.cpp cdc5f8314a875ababf2b17a32873775d808e1c78 Diff: https://reviews.apache.org/r/37442/diff/ Testing --- sudo make check Thanks, Paul Brett
Re: Review Request 37655: MESOS-3299: Add protobuf to represent time with integer precision.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37655/#review95981 --- Patch looks great! Reviews applied: [37655] All tests passed. - Mesos ReviewBot On Aug. 20, 2015, 5:03 p.m., Joseph Wu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37655/ --- (Updated Aug. 20, 2015, 5:03 p.m.) Review request for mesos, Benjamin Hindman, Artem Harutyunyan, Joris Van Remoortere, and Vinod Kone. Bugs: MESOS-3299 https://issues.apache.org/jira/browse/MESOS-3299 Repository: mesos Description --- Instead of using doubles, seconds and nanoseconds can be represented like `struct timespec`, with one field for seconds and one for nanoseconds. This will be important if frameworks need to compare times to make decisions (such as for maintenance primitives). Note about the naming: * Time will conflict with the Time class. * Most denominations of time (Seconds, Minutes, Hours, etc) will conflict with Duration. Diffs - include/mesos/mesos.proto 33e1b28f1ccbe227657a14395f81df20e0a9e193 Diff: https://reviews.apache.org/r/37655/diff/ Testing --- `make check` Thanks, Joseph Wu
Re: Review Request 37427: Docker registry: adding TokenManager.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37427/ --- (Updated Aug. 20, 2015, 3:50 p.m.) Review request for mesos, Lily Chen, Joris Van Remoortere, and Timothy Chen. Changes --- rebased with master Repository: mesos Description --- Changes: - Added Token implementation (RFC 7519). - Added TokenManager implementation. This component keeps a cache of tokens requested for any future requests. Diffs (updated) - src/Makefile.am 9fd71d1ddf442712977596e7a13969ff5c1d68db src/slave/containerizer/provisioners/docker/token_manager.hpp PRE-CREATION src/slave/containerizer/provisioners/docker/token_manager.cpp PRE-CREATION src/tests/provisioners/docker_provisioner_tests.cpp PRE-CREATION Diff: https://reviews.apache.org/r/37427/diff/ Testing --- make check. Thanks, Jojy Varghese
Re: Review Request 37462: Add support for version detection and parsing.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37462/ --- (Updated Aug. 20, 2015, 7:17 p.m.) Review request for mesos and Ben Mahler. Changes --- Improve readability for extract function. Bugs: MESOS-2834 https://issues.apache.org/jira/browse/MESOS-2834 Repository: mesos Description --- Add support for version detection and parsing. Diffs (updated) - src/linux/perf.cpp cdc5f8314a875ababf2b17a32873775d808e1c78 Diff: https://reviews.apache.org/r/37462/diff/ Testing (updated) --- sudo make check Perf tests may fail on many machines because the tests are currently using the version of perf installed on the machine to choose decode. The next patch will extend the perf tests to supply test cases for each of the supported perf versions. Thanks, Paul Brett
Re: Review Request 37532: Add QUIESCE call interface to the scheduler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37532/#review95997 --- include/mesos/scheduler.hpp (line 273) https://reviews.apache.org/r/37532/#comment151190 indent is off? include/mesos/scheduler.hpp (line 424) https://reviews.apache.org/r/37532/#comment151195 indent include/mesos/scheduler/scheduler.proto (line 173) https://reviews.apache.org/r/37532/#comment151191 micro-nit: the // is not aligned with lines above. include/mesos/scheduler/scheduler.proto (lines 314 - 315) https://reviews.apache.org/r/37532/#comment151192 This comments does not read well: what is the timeout? also, it would be good to have a bit of info about what the filters are (eg, are they 'inclusion' or 'exclusion' filters? etc.) src/master/master.cpp (line 2507) https://reviews.apache.org/r/37532/#comment151193 IMO we should LOG(INFO) here to state this is not implemented yet or something. Otherwise, say people see this and start using it, then nothing happens, look at the logs (which give the impression that the Processing QUIESCE was successfully called) and will report it as a bug. src/sched/sched.cpp (line 1953) https://reviews.apache.org/r/37532/#comment151194 indent off src/tests/scheduler_tests.cpp (line 1003) https://reviews.apache.org/r/37532/#comment151196 so, this is a good test, but I would really like to see one where we ask Master to keep quiet for a while and we don't get offers during that period of time, then we start getting again. It may require some fiddling around with `Clock`s and all that jazz, but it would give us more confidence that this whole thing works. Also - some tests around more complex filtering (if any? maybe this is there is, then it's fine). - Marco Massenzio On Aug. 17, 2015, 1:47 p.m., Guangya Liu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37532/ --- (Updated Aug. 17, 2015, 1:47 p.m.) Review request for mesos and Vinod Kone. Bugs: MESOS-3037 https://issues.apache.org/jira/browse/MESOS-3037 Repository: mesos Description --- This is just part of MESOS-3037, this patch only add the interface of QUIESCE call. Diffs - include/mesos/scheduler.hpp ee198b6955882f4f31466ca05429ca16fbf2f5cd include/mesos/scheduler/scheduler.proto 89daf8a6b74057ee156b3ad691397e76fcb835b8 include/mesos/v1/scheduler/scheduler.proto bd5e82a614b1163b29f9b20e562208efa1ba4b55 src/master/master.hpp 0432842d77beba024c7895291ca410964bae96be src/master/master.cpp c5e6c6f3304060d4c92d52851951f10bc432500e src/sched/sched.cpp 012af0508eeceeccd168b29f36fa258d20b28c21 src/tests/scheduler_tests.cpp 77c26353afc33f5099be2d1e597ffc630e559968 Diff: https://reviews.apache.org/r/37532/diff/ Testing --- Thanks, Guangya Liu
Re: Review Request 37199: Added store interface and moved store implementation to LocalStore subclass.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37199/#review96006 --- src/slave/containerizer/provisioners/docker/store.cpp (line 56) https://reviews.apache.org/r/37199/#comment151200 Unknown docker store: + flags.docker_Store src/slave/containerizer/provisioners/docker/store.cpp (line 365) https://reviews.apache.org/r/37199/#comment151199 This should be store path not found, local_store.cpp is already part of the log message. - Timothy Chen On Aug. 17, 2015, 9:12 p.m., Lily Chen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37199/ --- (Updated Aug. 17, 2015, 9:12 p.m.) Review request for mesos, Ian Downes, Jie Yu, Jojy Varghese, Timothy Chen, and Jiang Yan Xu. Bugs: MESOS-2849 https://issues.apache.org/jira/browse/MESOS-2849 Repository: mesos Description --- Added store interface and moved store implementation to LocalStore subclass. Diffs - src/slave/containerizer/provisioners/docker/store.hpp PRE-CREATION src/slave/containerizer/provisioners/docker/store.cpp PRE-CREATION Diff: https://reviews.apache.org/r/37199/diff/ Testing --- make check Thanks, Lily Chen
Re: Review Request 37200: Refactored DockerImage struct to store a list of layer ids instead of linked list of DockerLayers.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37200/#review96007 --- src/slave/containerizer/provisioners/docker.hpp (line 54) https://reviews.apache.org/r/37200/#comment151201 I think I commented earlier, a remote registry might be part of the name with a : which specifies port. src/slave/containerizer/provisioners/docker/backend.cpp (line 127) https://reviews.apache.org/r/37200/#comment151202 You can support a std::ostream operator override for the ImageName src/slave/containerizer/provisioners/docker/store.cpp (line 130) https://reviews.apache.org/r/37200/#comment151203 log the image name. src/slave/containerizer/provisioners/docker/store.cpp (line 134) https://reviews.apache.org/r/37200/#comment151204 I think we want to create a staging directory underneath a set staging directory, simialiar how appc does it. src/slave/containerizer/provisioners/docker/store.cpp (line 402) https://reviews.apache.org/r/37200/#comment151205 Use os::rename() instead of calling mv yourself - Timothy Chen On Aug. 19, 2015, 6:44 p.m., Lily Chen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37200/ --- (Updated Aug. 19, 2015, 6:44 p.m.) Review request for mesos, Ian Downes, Jie Yu, Jojy Varghese, Timothy Chen, and Jiang Yan Xu. Bugs: MESOS-2849 https://issues.apache.org/jira/browse/MESOS-2849 Repository: mesos Description --- Refactored DockerImage struct to store a list of layer ids instead of linked list of DockerLayers. Diffs - src/slave/containerizer/provisioners/docker.hpp PRE-CREATION src/slave/containerizer/provisioners/docker.cpp PRE-CREATION src/slave/containerizer/provisioners/docker/backend.hpp PRE-CREATION src/slave/containerizer/provisioners/docker/backend.cpp PRE-CREATION src/slave/containerizer/provisioners/docker/store.hpp PRE-CREATION src/slave/containerizer/provisioners/docker/store.cpp PRE-CREATION src/slave/flags.hpp e56738e2dfd6593ef8f093687919da287af78f77 src/slave/flags.cpp b36710d6d7a7250bc071a57310a2d54bfb3bc624 Diff: https://reviews.apache.org/r/37200/diff/ Testing --- make check Thanks, Lily Chen
Re: Review Request 37245: Refactor Docker Image to exclude path and manifest.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37245/#review96008 --- Can we squash this to ImageName commit? - Timothy Chen On Aug. 17, 2015, 9:13 p.m., Lily Chen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37245/ --- (Updated Aug. 17, 2015, 9:13 p.m.) Review request for mesos, Ian Downes, Jie Yu, Jojy Varghese, Timothy Chen, and Jiang Yan Xu. Bugs: MESOS-2850 https://issues.apache.org/jira/browse/MESOS-2850 Repository: mesos Description --- Refactor Docker Image to exclude path and manifest. Diffs - src/slave/containerizer/provisioners/docker.hpp PRE-CREATION src/slave/containerizer/provisioners/docker.cpp PRE-CREATION src/slave/containerizer/provisioners/docker/backend.hpp PRE-CREATION src/slave/containerizer/provisioners/docker/backend.cpp PRE-CREATION Diff: https://reviews.apache.org/r/37245/diff/ Testing --- make check Thanks, Lily Chen
Re: Review Request 37199: Added store interface and moved store implementation to LocalStore subclass.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37199/#review96010 --- Bad patch! Reviews applied: [37196, 37197, 37198, 37199] Failed command: ./support/apply-review.sh -n -r 37199 Error: 2015-08-20 21:59:29 URL:https://reviews.apache.org/r/37199/diff/raw/ [8196/8196] - 37199.patch [1] error: patch failed: src/slave/containerizer/provisioners/docker/store.hpp:39 error: src/slave/containerizer/provisioners/docker/store.hpp: patch does not apply error: patch failed: src/slave/containerizer/provisioners/docker/store.cpp:48 error: src/slave/containerizer/provisioners/docker/store.cpp: patch does not apply Failed to apply patch - Mesos ReviewBot On Aug. 17, 2015, 9:12 p.m., Lily Chen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37199/ --- (Updated Aug. 17, 2015, 9:12 p.m.) Review request for mesos, Ian Downes, Jie Yu, Jojy Varghese, Timothy Chen, and Jiang Yan Xu. Bugs: MESOS-2849 https://issues.apache.org/jira/browse/MESOS-2849 Repository: mesos Description --- Added store interface and moved store implementation to LocalStore subclass. Diffs - src/slave/containerizer/provisioners/docker/store.hpp PRE-CREATION src/slave/containerizer/provisioners/docker/store.cpp PRE-CREATION Diff: https://reviews.apache.org/r/37199/diff/ Testing --- make check Thanks, Lily Chen
Re: Review Request 37336: Simplified the caller interface to process::Subprocess
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37336/#review95936 --- Patch looks great! Reviews applied: [37336] All tests passed. - Mesos ReviewBot On Aug. 20, 2015, 8:03 a.m., Marco Massenzio wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37336/ --- (Updated Aug. 20, 2015, 8:03 a.m.) Review request for mesos and Joris Van Remoortere. Bugs: MESOS-3035 https://issues.apache.org/jira/browse/MESOS-3035 Repository: mesos Description --- Jira: MESOS-3035 The original API for `process::Subprocess` still left a lot of legwork to do for the caller; we have now added a `wait(Duration timeout)` method that returns a `CommandResult` (also introduced with this patch) which contains useful information about the command invocation; the exit code; stdout and, optionally, stderr too. The `wait()` method will wait for both the process to terminate, and stdout/stderr to be available to read from; it also unpacks them into ready-to-use `string`s. Diffs - 3rdparty/libprocess/include/process/subprocess.hpp d2341a53aac7c779668ee80983f73158fd44bff5 3rdparty/libprocess/src/subprocess.cpp d6ea62ed1c914d34e0e189395831c86fff8aac22 3rdparty/libprocess/src/tests/subprocess_tests.cpp ab7515325e5db0a4fd222bb982f51243d7b7e39d Diff: https://reviews.apache.org/r/37336/diff/ Testing --- make check Thanks, Marco Massenzio
Re: Review Request 37416: Perf supported() should be based on the version of perf, not the version of the kernel.
On Aug. 19, 2015, 1:42 a.m., Ben Mahler wrote: src/linux/perf.cpp, lines 474-478 https://reviews.apache.org/r/37416/diff/3/?file=1043975#file1043975line474 Why `_supported` here that takes a version? Why not just have supported compute the version and then perform the necessary check? We need two variants - perf::supported that computes the version and performs the necessary checks, and a version for testing that we can feed fake version information into so that we can test the parsing against multiple versions of perf output. - Paul --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37416/#review95809 --- On Aug. 20, 2015, 1:13 a.m., Paul Brett wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37416/ --- (Updated Aug. 20, 2015, 1:13 a.m.) Review request for mesos and Ben Mahler. Repository: mesos Description --- Perf supported() should be based on the version of perf, not the version of the kernel. Diffs - src/linux/perf.cpp cdc5f8314a875ababf2b17a32873775d808e1c78 Diff: https://reviews.apache.org/r/37416/diff/ Testing --- sudo make check Thanks, Paul Brett
Re: Review Request 37416: Perf supported() should be based on the version of perf, not the version of the kernel.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37416/ --- (Updated Aug. 20, 2015, 4:39 p.m.) Review request for mesos and Ben Mahler. Changes --- Fix bad patch. Repository: mesos Description --- Perf supported() should be based on the version of perf, not the version of the kernel. Diffs (updated) - src/linux/perf.cpp cdc5f8314a875ababf2b17a32873775d808e1c78 Diff: https://reviews.apache.org/r/37416/diff/ Testing --- sudo make check Thanks, Paul Brett
Re: Review Request 37427: Docker registry: adding TokenManager.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37427/#review95969 --- Patch looks great! Reviews applied: [37426, 37427] All tests passed. - Mesos ReviewBot On Aug. 20, 2015, 3:50 p.m., Jojy Varghese wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37427/ --- (Updated Aug. 20, 2015, 3:50 p.m.) Review request for mesos, Lily Chen, Joris Van Remoortere, and Timothy Chen. Repository: mesos Description --- Changes: - Added Token implementation (RFC 7519). - Added TokenManager implementation. This component keeps a cache of tokens requested for any future requests. Diffs - src/Makefile.am 9fd71d1ddf442712977596e7a13969ff5c1d68db src/slave/containerizer/provisioners/docker/token_manager.hpp PRE-CREATION src/slave/containerizer/provisioners/docker/token_manager.cpp PRE-CREATION src/tests/provisioners/docker_provisioner_tests.cpp PRE-CREATION Diff: https://reviews.apache.org/r/37427/diff/ Testing --- make check. Thanks, Jojy Varghese
Re: Review Request 37416: Perf supported() should be based on the version of perf, not the version of the kernel.
On Aug. 19, 2015, 1:42 a.m., Ben Mahler wrote:
src/linux/perf.cpp, lines 484-490
https://reviews.apache.org/r/37416/diff/3/?file=1043975#file1043975line484
Couple of things:
(1) Let's add a comment as to why we're using await here, since it is
an anti-pattern. Namely, is it because making supported() asynchronous is a
difficult change?
(2) Let's discard the future when it doesn't transition in the timeout.
(3) We're going to silently say false if the future fails, can we log
the failure?
(4) 'if (' :)
(5) Can you add some newlines here to make it less dense?
What is the advantage of calling discard() on version here? At the moment, we
pass version as a const ref, so calling discard is not possible. We could
remove the const constraint, but then we would have to pass an lvalue from
supported.
- Paul
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37416/#review95809
---
On Aug. 20, 2015, 4:39 p.m., Paul Brett wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37416/
---
(Updated Aug. 20, 2015, 4:39 p.m.)
Review request for mesos and Ben Mahler.
Repository: mesos
Description
---
Perf supported() should be based on the version of perf, not the version of
the kernel.
Diffs
-
src/linux/perf.cpp cdc5f8314a875ababf2b17a32873775d808e1c78
Diff: https://reviews.apache.org/r/37416/diff/
Testing
---
sudo make check
Thanks,
Paul Brett
Re: Review Request 37617: Added an error message to MesosContainerizerLaunch command.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37617/#review95960 --- Ship it! Ship It! - Jie Yu On Aug. 19, 2015, 6:26 p.m., Jie Yu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37617/ --- (Updated Aug. 19, 2015, 6:26 p.m.) Review request for mesos, Marco Massenzio and Vinod Kone. Bugs: MESOS-3296 https://issues.apache.org/jira/browse/MESOS-3296 Repository: mesos Description --- Added an error message to MesosContainerizerLaunch command. So that we can triage MESOS-3296. Diffs - src/slave/containerizer/mesos/launch.cpp bd594d3f3d729aa000a1a7fbf9038f6cfcbb169b Diff: https://reviews.apache.org/r/37617/diff/ Testing --- make check Thanks, Jie Yu
