----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23214/#review47258 -----------------------------------------------------------
src/slave/containerizer/isolators/network/port_mapping.cpp <https://reviews.apache.org/r/23214/#comment82915> s/isolator/isolator, because it was launched by the slave with network isolation disabled)/ src/slave/containerizer/isolators/network/port_mapping.cpp <https://reviews.apache.org/r/23214/#comment82916> previous run of what? maybe just kill the "in the previous run" part. src/tests/port_mapping_tests.cpp <https://reviews.apache.org/r/23214/#comment82918> can you use ASSERT_NE? Also, how is this possible? Aren't we controlling whether this isolation is set or not? src/tests/port_mapping_tests.cpp <https://reviews.apache.org/r/23214/#comment82925> This is only used in one test. Not sure it belongs in the fixture. src/tests/port_mapping_tests.cpp <https://reviews.apache.org/r/23214/#comment82921> s/recover/_recover/ src/tests/port_mapping_tests.cpp <https://reviews.apache.org/r/23214/#comment82922> Why do you need framework 2? Why can't framework 1 launch the next task? src/tests/port_mapping_tests.cpp <https://reviews.apache.org/r/23214/#comment82926> How are you making sure it recovered both the tasks? You should probably call "containers()" and make sure it's size is 2? src/tests/port_mapping_tests.cpp <https://reviews.apache.org/r/23214/#comment82927> Do you always want to test update() and usage() work fine w/ mixed containers? what about destroy()? you already do driver::stop() below which should destroy the containers. you probably want to do some expectations that show that they are properly destroyed. src/tests/port_mapping_tests.cpp <https://reviews.apache.org/r/23214/#comment82923> Can you add a comment on what this test is doing? - Vinod Kone 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 > >
