> On April 22, 2016, 5:19 a.m., Avinash sridharan wrote:
> > src/tests/containerizer/cni_isolator_tests.cpp, line 24
> > <https://reviews.apache.org/r/46097/diff/3/?file=1353232#file1353232line24>
> >
> >     I think `mesos` definitions should come after `process`.

No, I think we should do it in alphabet order, 'm' should come before 'p'. You 
can check command_executor_tests.cpp for an example.


> On April 22, 2016, 5:19 a.m., Avinash sridharan wrote:
> > src/tests/containerizer/cni_isolator_tests.cpp, line 31
> > <https://reviews.apache.org/r/46097/diff/3/?file=1353232#file1353232line31>
> >
> >     Move the `std` definitions above process.

We should do it in alphabet order.


> On April 22, 2016, 5:19 a.m., Avinash sridharan wrote:
> > src/tests/containerizer/cni_isolator_tests.cpp, line 38
> > <https://reviews.apache.org/r/46097/diff/3/?file=1353232#file1353232line38>
> >
> >     We should have an #ifdef for the whole file? This looks a bit odd. I 
> > would suggest put this conditional in the Makefile, so that this doesn't 
> > get compiled for non-linux platforms.

You can check runtime_isolator_tests.cpp which does the same.


> On April 22, 2016, 5:19 a.m., Avinash sridharan wrote:
> > src/tests/containerizer/cni_isolator_tests.cpp, line 53
> > <https://reviews.apache.org/r/46097/diff/3/?file=1353232#file1353232line53>
> >
> >     Do we need to explicitly specify this registry? I thought this would be 
> > the default?

Yeah, I agree!


> On April 22, 2016, 5:19 a.m., Avinash sridharan wrote:
> > src/tests/containerizer/cni_isolator_tests.cpp, line 65
> > <https://reviews.apache.org/r/46097/diff/3/?file=1353232#file1353232line65>
> >
> >     Do you want to make this into a helper function? I am guessing this 
> > will be used in other test cases too ?

Agree!


> On April 22, 2016, 5:19 a.m., Avinash sridharan wrote:
> > src/tests/containerizer/cni_isolator_tests.cpp, line 82
> > <https://reviews.apache.org/r/46097/diff/3/?file=1353232#file1353232line82>
> >
> >     Might Want to convert this into a helper function as well.

Agree!


> On April 22, 2016, 5:19 a.m., Avinash sridharan wrote:
> > src/tests/containerizer/cni_isolator_tests.cpp, line 74
> > <https://reviews.apache.org/r/46097/diff/3/?file=1353232#file1353232line74>
> >
> >     Should you be reading the /etc/resolv.conf and returning the DNS info 
> > as well. Else, the other test cases would never ever create a DNS file in 
> > the <container DIR>?

Agree!


- Qian


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46097/#review129957
-----------------------------------------------------------


On April 20, 2016, 10:26 p.m., Qian Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46097/
> -----------------------------------------------------------
> 
> (Updated April 20, 2016, 10:26 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 5a2b4efa781752863d6751f98614fb78bece73ac 
>   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