-----------------------------------------------------------
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
> 
>

Reply via email to