----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23214/#review47154 -----------------------------------------------------------
Did you consider: hashmap<ContainerID, Option<Info*> > infos This is explicit that we support containers that optionally have network isolation. Then we can disambiguate so e.g., calling watch() or usage() for a container that we know about but doesn't have network isolation can be handled more cleanly and without misleading LOG(WARNING) messages. src/slave/containerizer/isolators/network/port_mapping.cpp <https://reviews.apache.org/r/23214/#comment82740> s/PID/pid/ change language to say this is *possibly* because it's a container that wasn't managed by the network isolator src/slave/containerizer/isolators/network/port_mapping.cpp <https://reviews.apache.org/r/23214/#comment82741> This will get called constantly, even for containers that don't use the network isolator, and thus we'll be logging a lot! Suggest removing the log message. src/tests/isolator_tests.cpp <https://reviews.apache.org/r/23214/#comment82841> This doesn't appear to be used in this file? src/tests/port_mapping_tests.cpp <https://reviews.apache.org/r/23214/#comment82845> Why not just call CreateSlaveFlags again later when you're starting the second containerizer? slave::Flags flags = CreateSlaveFlags() // clear the network isolation parts // start tasks, etc. flags = CreateSlaveFlags(); src/tests/port_mapping_tests.cpp <https://reviews.apache.org/r/23214/#comment82844> s/Reset/Clear/ src/tests/port_mapping_tests.cpp <https://reviews.apache.org/r/23214/#comment82842> s/islator/isolator/ src/tests/port_mapping_tests.cpp <https://reviews.apache.org/r/23214/#comment82846> just name the variable recover? src/tests/port_mapping_tests.cpp <https://reviews.apache.org/r/23214/#comment82847> Add some commenting about why you need three slaves: 1) Start task without network isolator 2) Recover task with slave and run new task with network isolator 3) Recover tasks where one has network isolation and one does not. - Ian Downes On July 1, 2014, 2:22 p.m., Chi Zhang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/23214/ > ----------------------------------------------------------- > > (Updated July 1, 2014, 2:22 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 e3ff6d7 > src/slave/containerizer/isolators/network/port_mapping.cpp a326653 > src/tests/isolator_tests.cpp 4650f97 > src/tests/port_mapping_tests.cpp PRE-CREATION > > Diff: https://reviews.apache.org/r/23214/diff/ > > > Testing > ------- > > > Thanks, > > Chi Zhang > >
