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