----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60496/#review184492 -----------------------------------------------------------
src/slave/containerizer/mesos/isolators/network/ports.cpp Lines 137 (patched) <https://reviews.apache.org/r/60496/#comment260662> Kill this empty line. src/slave/containerizer/mesos/isolators/network/ports.cpp Lines 271-284 (patched) <https://reviews.apache.org/r/60496/#comment260674> Mind to explain why running the loop inside the isolator process (since you supplied `pid` as the first parameter of `process::loop()`) and making `collectContainerListeners()` a static function? Can we follow the similar way of what DRF allocator process does? I mean we can run the loop outside the isolator process and make `collectContainerListeners()` a member method of `NetworkPortsIsolatorProcess`, and then invoke `collectContainerListeners()` with a `dispatch()`. In this way, we do not need to pass those parameters (like `portsProcess->infos.keys()`) to `collectContainerListeners()` since as a member method it can access them naturally. src/slave/containerizer/mesos/isolators/network/ports.cpp Lines 367-368 (patched) <https://reviews.apache.org/r/60496/#comment260676> I think this check is a bit confusing, we should instead do: ``` CHECK(resources.empty()); ``` The reason is, as you mentioned in the above comment, the resources are attached to the root container, so the resources here for nested container must be empty. src/slave/containerizer/mesos/isolators/network/ports.cpp Lines 368-369 (patched) <https://reviews.apache.org/r/60496/#comment260675> A newline between. - Qian Zhang On Aug. 31, 2017, 7:19 a.m., James Peach wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/60496/ > ----------------------------------------------------------- > > (Updated Aug. 31, 2017, 7:19 a.m.) > > > Review request for mesos, Qian Zhang and Jiang Yan Xu. > > > Bugs: MESOS-7675 > https://issues.apache.org/jira/browse/MESOS-7675 > > > Repository: mesos > > > Description > ------- > > Implemented ports resource restrictions in the network ports isolator. > Periodically, scan for listening sockets and match them up to all > the open sockets in the containers we are tracking in the network. > Check any sockets we find against the ports resource and trigger a > resource limitation if the port has not been allocated. > > > Diffs > ----- > > src/slave/containerizer/mesos/isolators/network/ports.hpp PRE-CREATION > src/slave/containerizer/mesos/isolators/network/ports.cpp PRE-CREATION > > > Diff: https://reviews.apache.org/r/60496/diff/16/ > > > Testing > ------- > > make check (Fedora 26) > > > Thanks, > > James Peach > >
