----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23214/#review47246 -----------------------------------------------------------
src/slave/containerizer/isolators/network/port_mapping.cpp <https://reviews.apache.org/r/23214/#comment82891> Change that to VLOG(1)? src/slave/containerizer/isolators/network/port_mapping.cpp <https://reviews.apache.org/r/23214/#comment82894> We probably shouldn't combine these two cases. if (unmanaged.contains(containerId)) { return Failure("Prepare an unmanaged container"); } src/slave/containerizer/isolators/network/port_mapping.cpp <https://reviews.apache.org/r/23214/#comment82895> "Trying to isolate an unmanaged container" src/slave/containerizer/isolators/network/port_mapping.cpp <https://reviews.apache.org/r/23214/#comment82897> Ditto. Split these two cases. if (unmanaged.contains(containerId)) { LOG(WARNING) << "Ignoring watch for unmanaged container " << containerId; } if (!infos.contains(containerId)) { ... } src/slave/containerizer/isolators/network/port_mapping.cpp <https://reviews.apache.org/r/23214/#comment82900> Include the containerId. src/tests/port_mapping_tests.cpp <https://reviews.apache.org/r/23214/#comment82911> Add a comment here saying that this is to check if network isolator can be run on this host. src/tests/port_mapping_tests.cpp <https://reviews.apache.org/r/23214/#comment82907> is not supported on this host. src/tests/port_mapping_tests.cpp <https://reviews.apache.org/r/23214/#comment82912> Should we do the cleanup in the test fixture the same way as we did in PortMappingIsolatorTest? - Jie Yu On July 2, 2014, 10:55 p.m., Chi Zhang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/23214/ > ----------------------------------------------------------- > > (Updated July 2, 2014, 10:55 p.m.) > > > Review request for mesos, Ian Downes, Jie Yu, and Vinod Kone. > > > Bugs: https://issues.apache.org/jira/browse/MESOS-1557 > > https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/MESOS-1557 > > > Repository: mesos-git > > > Description > ------- > > This eases deployment where a slave could be upgraded to use Network Isolator > without removing all the existing tasks. > > - Added a new test. > - Moved all portmapping tests to a new file. > > > Diffs > ----- > > src/Makefile.am 6438939 > src/slave/containerizer/isolators/network/port_mapping.hpp ac3bee3 > src/slave/containerizer/isolators/network/port_mapping.cpp a326653 > src/tests/isolator_tests.cpp 4650f97 > src/tests/mesos.cpp 0d7f335 > src/tests/port_mapping_tests.cpp PRE-CREATION > > Diff: https://reviews.apache.org/r/23214/diff/ > > > Testing > ------- > > > Thanks, > > Chi Zhang > >
