mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From James Peach <jpe...@apache.org>
Subject Re: Review Request 67195: Adding enforce_container_ports flag for network ports isolation.
Date Thu, 24 May 2018 20:30:45 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67195/#review203805
-----------------------------------------------------------


Fix it, then Ship it!




Commit subject should be "Added ..."


docs/isolators/network-ports.md
Lines 49 (patched)
<https://reviews.apache.org/r/67195/#comment286126>

    How about this:
    
    ```
    The `--enforce_container_ports` flag, specifies
    whether the network ports isolator should terminate
    tasks that listen on ports they have not been assigned.
    If enforcement is disabled, the isolator will log violations
    but will not terminate tasks. By default, network port enforcement
    is disabled.
    ```



src/slave/containerizer/mesos/isolators/network/ports.cpp
Lines 582 (patched)
<https://reviews.apache.org/r/67195/#comment286117>

    "while port enforcement is disabled"



src/slave/containerizer/mesos/isolators/network/ports.cpp
Lines 586 (patched)
<https://reviews.apache.org/r/67195/#comment286118>

    Newline after the '}'.



src/slave/flags.cpp
Lines 1123 (patched)
<https://reviews.apache.org/r/67195/#comment286125>

    "is used by the `network/ports` isolator."
    
    Note the backquotes and fill stop.



src/tests/containerizer/ports_isolator_tests.cpp
Lines 834 (patched)
<https://reviews.apache.org/r/67195/#comment286122>

    Needs a full-stop "is false."



src/tests/containerizer/ports_isolator_tests.cpp
Lines 835 (patched)
<https://reviews.apache.org/r/67195/#comment286121>

    `ROOT_NC_NoPortEnforcement`



src/tests/containerizer/ports_isolator_tests.cpp
Lines 929 (patched)
<https://reviews.apache.org/r/67195/#comment286123>

    Add a comment here:
    ```
    // Since container ports are not being enforced, we expect that the task
    // should still be running after the check and that we should be able to
    // explicitly kill it.
    ```


- James Peach


On May 23, 2018, 6:33 p.m., Xudong Ni wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67195/
> -----------------------------------------------------------
> 
> (Updated May 23, 2018, 6:33 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
> 
> 
> 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/3/
> 
> 
> 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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message