----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68355/#review207304 -----------------------------------------------------------
src/tests/containerizer/cni_isolator_tests.cpp Lines 203 (patched) <https://reviews.apache.org/r/68355/#comment290645> nits: I'd suggest we don't to link break here. Lint won't check c++11 string literals. src/tests/containerizer/cni_isolator_tests.cpp Lines 207 (patched) <https://reviews.apache.org/r/68355/#comment290646> nits: Ditto. Let's use one line for those command src/tests/containerizer/cni_isolator_tests.cpp Lines 216-219 (patched) <https://reviews.apache.org/r/68355/#comment290647> Why you need this? src/tests/containerizer/cni_isolator_tests.cpp Lines 229 (patched) <https://reviews.apache.org/r/68355/#comment290644> I'd suggest we use `trap` to do the cleanup in case any command throw an error: ``` ... ln -sf $CNI_NETNS /var/run/netns/$NETNS function cleanup() { rm -f /var/run/netns/$NETNS } trap cleanup EXIT ``` src/tests/containerizer/cni_isolator_tests.cpp Lines 231-234 (patched) <https://reviews.apache.org/r/68355/#comment290649> I think CNI plugin's error message should be in stdout according to spec (a json) src/tests/containerizer/cni_isolator_tests.cpp Lines 236-240 (patched) <https://reviews.apache.org/r/68355/#comment290648> CNI DEL shouldn't output this? src/tests/containerizer/cni_isolator_tests.cpp Lines 2418-2421 (patched) <https://reviews.apache.org/r/68355/#comment290641> No need for this. src/tests/containerizer/cni_isolator_tests.cpp Lines 2428 (patched) <https://reviews.apache.org/r/68355/#comment290642> No need for this. - Jie Yu On Aug. 15, 2018, 1:47 a.m., Sergey Urbanovich wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/68355/ > ----------------------------------------------------------- > > (Updated Aug. 15, 2018, 1:47 a.m.) > > > Review request for mesos, Jie Yu and Qian Zhang. > > > Bugs: MESOS-5647 > https://issues.apache.org/jira/browse/MESOS-5647 > > > Repository: mesos > > > Description > ------- > > This is a veth CNI plugin that is written in bash. It creates a veth > virtual network pair, one end of the pair will be moved to container > network namespace. > > The veth CNI plugin uses 203.0.113.0/24 subnet, it is reserved for > documentation and examples [rfc5737]. The plugin can allocate up to > 128 veth pairs. > > > Diffs > ----- > > src/tests/containerizer/cni_isolator_tests.cpp > cb22e73b4215b5b0a49ac610e5f657b73d38963b > > > Diff: https://reviews.apache.org/r/68355/diff/1/ > > > Testing > ------- > > bin/mesos-tests.sh --verbose > --gtest_filter="CniIsolatorTest.ROOT_VerifyResourceStatistics" > --gtest_break_on_failure --gtest_repeat=100 > > > Thanks, > > Sergey Urbanovich > >
