----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46097/#review131758 -----------------------------------------------------------
Fix it, then Ship it! src/tests/containerizer/cni_isolator_tests.cpp (line 31) <https://reviews.apache.org/r/46097/#comment195788> No need for `#ifdef __linux__` because the file is guarded by `if OS_LINUX` in makefile. src/tests/containerizer/cni_isolator_tests.cpp (line 37) <https://reviews.apache.org/r/46097/#comment195789> Please move `{` to the next line. Ditto for others. src/tests/containerizer/cni_isolator_tests.cpp (lines 40 - 51) <https://reviews.apache.org/r/46097/#comment195790> Instead of escaping special characters, can you use the new C++11 literal? See example here: https://github.com/apache/mesos/blob/master/src/tests/containerizer/provisioner_appc_tests.cpp#L282 src/tests/containerizer/cni_isolator_tests.cpp (line 60) <https://reviews.apache.org/r/46097/#comment195791> Ditto on tailing `{` src/tests/containerizer/cni_isolator_tests.cpp (line 72) <https://reviews.apache.org/r/46097/#comment195793> This test requires INTERNET access. Can you add other filters needed: ``` ROOT_INTERNET_CURL_... ``` Also, I think we should have a test to test the case for a container that uses the host filesystem (i.e., no container image). src/tests/containerizer/cni_isolator_tests.cpp (line 80) <https://reviews.apache.org/r/46097/#comment195794> let's replace os::getcwd() with sandbox.get() in this test. src/tests/containerizer/cni_isolator_tests.cpp (line 92) <https://reviews.apache.org/r/46097/#comment195795> I would kill this line. src/tests/containerizer/cni_isolator_tests.cpp (line 119) <https://reviews.apache.org/r/46097/#comment195796> Can you do something related to network like 'ifconfig'? - Jie Yu On April 26, 2016, 3:53 p.m., Qian Zhang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/46097/ > ----------------------------------------------------------- > > (Updated April 26, 2016, 3:53 p.m.) > > > Review request for mesos, Avinash sridharan and Jie Yu. > > > Bugs: MESOS-5167 > https://issues.apache.org/jira/browse/MESOS-5167 > > > Repository: mesos > > > Description > ------- > > Added the test "CniIsolatorTest.ROOT_LaunchCommandTask". > > > Diffs > ----- > > src/Makefile.am e024c6d65608a55765e527a8668c415723dcfcca > src/tests/containerizer/cni_isolator_tests.cpp PRE-CREATION > > Diff: https://reviews.apache.org/r/46097/diff/ > > > Testing > ------- > > [ RUN ] CniIsolatorTest.ROOT_LaunchCommandTask > + /home/stack/workspace/mesos/build/src/mesos-containerizer mount > --help=false --operation=make-rslave --path=/ > + grep+ -E /tmp/CniIsolatorTest_ROOT_LaunchCommandTask_HRK4Dz/.+ > /proc/self/mountinfo > + grepcut -v -d -f5 > d06b117d-518b-41e2-b8e0-62a12083773c > + xargs --no-run-if-empty umount -l > + mount -n --rbind > /tmp/CniIsolatorTest_ROOT_LaunchCommandTask_HRK4Dz/provisioner/containers/d06b117d-518b-41e2-b8e0-62a12083773c/backends/copy/rootfses/7ea27011-cd3a-43b0-8301-b0b94d9f9b47 > > /tmp/CniIsolatorTest_ROOT_LaunchCommandTask_HRK4Dz/slaves/18dea042-5bb5-4336-8bc8-358ed1fbf6dd-S0/frameworks/18dea042-5bb5-4336-8bc8-358ed1fbf6dd-0000/executors/60e6d35d-6d33-47ae-9c23-d2e5c913c892/runs/d06b117d-518b-41e2-b8e0-62a12083773c/.rootfs > I0420 22:26:00.924844 9305 exec.cpp:150] Version: 0.29.0 > I0420 22:26:00.942319 9375 exec.cpp:225] Executor registered on agent > 18dea042-5bb5-4336-8bc8-358ed1fbf6dd-S0 > Registered executor on mesos > Starting task 60e6d35d-6d33-47ae-9c23-d2e5c913c892 > Forked command at 9382 > sh -c 'ls /' > bin dev etc home lib linuxrc media mnt proc > root run sbin sys tmp usr var > Command exited with status 0 (pid: 9382) > I0420 22:26:01.098331 9380 exec.cpp:399] Executor asked to shutdown > [ OK ] CniIsolatorTest.ROOT_LaunchCommandTask (42603 ms) > > > Thanks, > > Qian Zhang > >
