----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67195/#review203502 -----------------------------------------------------------
src/slave/containerizer/mesos/isolators/network/ports.hpp Lines 96 (patched) <https://reviews.apache.org/r/67195/#comment285796> Call this ``` Option<IntervalSet<uint16_t>> allocatedPorts; Option<IntervalSet<uint16_t>> activePorts; ``` Do the `/ports/allocatedPorts` in a separate patch to make is easy to distinguish from functional changes. src/slave/containerizer/mesos/isolators/network/ports.hpp Lines 102 (patched) <https://reviews.apache.org/r/67195/#comment285795> Call this `enforceContainerPorts`. src/slave/containerizer/mesos/isolators/network/ports.cpp Lines 577 (patched) <https://reviews.apache.org/r/67195/#comment285794> This is more complicated than it needs to be. You can simply do this: ``` if (!enforceContainerPorts) { if (info->activePorts.isSome() && info->activePorts == listeners) { VLOG(2) << "Skipping container ports violation log"; continue; } // Cache the last listeners sample so that we will // only log new ports resource violations. info->activePorts = listeners; } ``` src/tests/containerizer/ports_isolator_tests.cpp Lines 941 (patched) <https://reviews.apache.org/r/67195/#comment285811> Double blank line before and after this function. - James Peach On May 21, 2018, 4:14 p.m., Xudong Ni wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67195/ > ----------------------------------------------------------- > > (Updated May 21, 2018, 4:14 p.m.) > > > Review request for mesos, James Peach and Jiang Yan Xu. > > > Bugs: MESOS-8340 > https://issues.apache.org/jira/browse/MESOS-8340 > > > Repository: mesos > > > Description > ------- > > To reduce deployment risk, a nonenforce mode is added for network > port isolator. When this flag is set as false(default is false), > even task uses ports not in the container resources, the container > will not raise any limitation. > > Add new test for this flag and update the existing tests > > https://reviews.apache.org/r/67195/ > > > Diffs > ----- > > docs/isolators/network-ports.md ea63968481ce52c46e0a98e242da49baf6962009 > src/slave/containerizer/mesos/isolators/network/ports.hpp > ba71087194a3ae74c7e40dffa9c108b02ffa10ad > src/slave/containerizer/mesos/isolators/network/ports.cpp > 1f84ed4fb2a30fd095e2faec1038de1fa19a15c5 > src/slave/flags.hpp a839591a2b66444ad97fced0620201dde656352d > src/slave/flags.cpp a319b5ea633c41fd8a252c5e1617ac52d1480ba5 > src/tests/containerizer/ports_isolator_tests.cpp > c5b9f926047792e7f9d1f0937fa5355b1dd77965 > > > Diff: https://reviews.apache.org/r/67195/diff/1/ > > > Testing > ------- > > New test added for the flag; Related unit tests passed. > > [ OK ] NetworkPortsIsolatorTest.ROOT_NC_AllocatedPorts (1906 ms) > [ OK ] NetworkPortsIsolatorTest.ROOT_NC_NoPortsResource (1788 ms) > [ OK ] NetworkPortsIsolatorTest.ROOT_NC_NoPortEnforement (17001 ms) > > > Thanks, > > Xudong Ni > >