You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Ilya Pronin <ip...@twopensource.com> on 2018/07/16 23:14:17 UTC
Review Request 67936: Fixed ephemeral ports deallocation in
network/port_mapping isolator.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67936/
-----------------------------------------------------------
Review request for mesos and Jie Yu.
Bugs: MESOS-9080
https://issues.apache.org/jira/browse/MESOS-9080
Repository: mesos
Description
-------
Ephemeral ports are allocated for a container during the preparation
phase and have to be deallocated during container cleanup regardless of
whether the container process was isolated or not. Otherwise those ports
can be leaked if the container is destroyed during preparation.
Diffs
-----
src/slave/containerizer/mesos/isolators/network/port_mapping.cpp a29282ed0488d83966f222084031ed1d2b6ab1f5
src/tests/containerizer/port_mapping_tests.cpp 553f53043a52cfe235fc2b2e88fb54d0f2725d3e
Diff: https://reviews.apache.org/r/67936/diff/1/
Testing
-------
Added `PortMappingIsolatorTest.ROOT_CleanupNotIsolated` test to verify that ephemeral ports are properly deallocated when the container was not isolated and manually checked that if fails without the fix. Ran `sudo make check`.
Thanks,
Ilya Pronin
Re: Review Request 67936: Fixed ephemeral ports deallocation in
network/port_mapping isolator.
Posted by Ilya Pronin <ip...@twopensource.com>.
> On July 16, 2018, 4:59 p.m., Jie Yu wrote:
> > src/tests/containerizer/port_mapping_tests.cpp
> > Lines 1826 (patched)
> > <https://reviews.apache.org/r/67936/diff/1/?file=2059767#file2059767line1826>
> >
> > instead of hard code 512 which is fragile, i'd get `ephemeral_ports` resources, and use more than half of that.
Fixed.
- Ilya
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67936/#review206138
-----------------------------------------------------------
On July 17, 2018, 1:12 p.m., Ilya Pronin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67936/
> -----------------------------------------------------------
>
> (Updated July 17, 2018, 1:12 p.m.)
>
>
> Review request for mesos and Jie Yu.
>
>
> Bugs: MESOS-9080
> https://issues.apache.org/jira/browse/MESOS-9080
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Ephemeral ports are allocated for a container during the preparation
> phase and have to be deallocated during container cleanup regardless of
> whether the container process was isolated or not. Otherwise those ports
> can be leaked if the container is destroyed during preparation.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/isolators/network/port_mapping.cpp a29282ed0488d83966f222084031ed1d2b6ab1f5
> src/tests/containerizer/port_mapping_tests.cpp 553f53043a52cfe235fc2b2e88fb54d0f2725d3e
>
>
> Diff: https://reviews.apache.org/r/67936/diff/2/
>
>
> Testing
> -------
>
> Added `PortMappingIsolatorTest.ROOT_CleanupNotIsolated` test to verify that ephemeral ports are properly deallocated when the container was not isolated and manually checked that if fails without the fix. Ran `sudo make check`.
>
>
> Thanks,
>
> Ilya Pronin
>
>
Re: Review Request 67936: Fixed ephemeral ports deallocation in
network/port_mapping isolator.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67936/#review206138
-----------------------------------------------------------
Fix it, then Ship it!
src/tests/containerizer/port_mapping_tests.cpp
Lines 1826 (patched)
<https://reviews.apache.org/r/67936/#comment289044>
instead of hard code 512 which is fragile, i'd get `ephemeral_ports` resources, and use more than half of that.
- Jie Yu
On July 16, 2018, 11:14 p.m., Ilya Pronin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67936/
> -----------------------------------------------------------
>
> (Updated July 16, 2018, 11:14 p.m.)
>
>
> Review request for mesos and Jie Yu.
>
>
> Bugs: MESOS-9080
> https://issues.apache.org/jira/browse/MESOS-9080
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Ephemeral ports are allocated for a container during the preparation
> phase and have to be deallocated during container cleanup regardless of
> whether the container process was isolated or not. Otherwise those ports
> can be leaked if the container is destroyed during preparation.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/isolators/network/port_mapping.cpp a29282ed0488d83966f222084031ed1d2b6ab1f5
> src/tests/containerizer/port_mapping_tests.cpp 553f53043a52cfe235fc2b2e88fb54d0f2725d3e
>
>
> Diff: https://reviews.apache.org/r/67936/diff/1/
>
>
> Testing
> -------
>
> Added `PortMappingIsolatorTest.ROOT_CleanupNotIsolated` test to verify that ephemeral ports are properly deallocated when the container was not isolated and manually checked that if fails without the fix. Ran `sudo make check`.
>
>
> Thanks,
>
> Ilya Pronin
>
>
Re: Review Request 67936: Fixed ephemeral ports deallocation in
network/port_mapping isolator.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67936/#review206141
-----------------------------------------------------------
PASS: Mesos patch 67936 was successfully built and tested.
Reviews applied: `['67936']`
All the build artifacts available at: http://dcos-win.westus.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/1939/mesos-review-67936
- Mesos Reviewbot Windows
On July 16, 2018, 11:14 p.m., Ilya Pronin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67936/
> -----------------------------------------------------------
>
> (Updated July 16, 2018, 11:14 p.m.)
>
>
> Review request for mesos and Jie Yu.
>
>
> Bugs: MESOS-9080
> https://issues.apache.org/jira/browse/MESOS-9080
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Ephemeral ports are allocated for a container during the preparation
> phase and have to be deallocated during container cleanup regardless of
> whether the container process was isolated or not. Otherwise those ports
> can be leaked if the container is destroyed during preparation.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/isolators/network/port_mapping.cpp a29282ed0488d83966f222084031ed1d2b6ab1f5
> src/tests/containerizer/port_mapping_tests.cpp 553f53043a52cfe235fc2b2e88fb54d0f2725d3e
>
>
> Diff: https://reviews.apache.org/r/67936/diff/1/
>
>
> Testing
> -------
>
> Added `PortMappingIsolatorTest.ROOT_CleanupNotIsolated` test to verify that ephemeral ports are properly deallocated when the container was not isolated and manually checked that if fails without the fix. Ran `sudo make check`.
>
>
> Thanks,
>
> Ilya Pronin
>
>
Re: Review Request 67936: Fixed ephemeral ports deallocation in
network/port_mapping isolator.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67936/#review206436
-----------------------------------------------------------
Patch looks great!
Reviews applied: [67936]
Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose --disable-libtool-wrappers' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh
- Mesos Reviewbot
On July 17, 2018, 8:12 p.m., Ilya Pronin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67936/
> -----------------------------------------------------------
>
> (Updated July 17, 2018, 8:12 p.m.)
>
>
> Review request for mesos and Jie Yu.
>
>
> Bugs: MESOS-9080
> https://issues.apache.org/jira/browse/MESOS-9080
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Ephemeral ports are allocated for a container during the preparation
> phase and have to be deallocated during container cleanup regardless of
> whether the container process was isolated or not. Otherwise those ports
> can be leaked if the container is destroyed during preparation.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/isolators/network/port_mapping.cpp a29282ed0488d83966f222084031ed1d2b6ab1f5
> src/tests/containerizer/port_mapping_tests.cpp 553f53043a52cfe235fc2b2e88fb54d0f2725d3e
>
>
> Diff: https://reviews.apache.org/r/67936/diff/2/
>
>
> Testing
> -------
>
> Added `PortMappingIsolatorTest.ROOT_CleanupNotIsolated` test to verify that ephemeral ports are properly deallocated when the container was not isolated and manually checked that if fails without the fix. Ran `sudo make check`.
>
>
> Thanks,
>
> Ilya Pronin
>
>
Re: Review Request 67936: Fixed ephemeral ports deallocation in
network/port_mapping isolator.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67936/#review206406
-----------------------------------------------------------
Ship it!
Ship It!
- Jie Yu
On July 17, 2018, 8:12 p.m., Ilya Pronin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67936/
> -----------------------------------------------------------
>
> (Updated July 17, 2018, 8:12 p.m.)
>
>
> Review request for mesos and Jie Yu.
>
>
> Bugs: MESOS-9080
> https://issues.apache.org/jira/browse/MESOS-9080
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Ephemeral ports are allocated for a container during the preparation
> phase and have to be deallocated during container cleanup regardless of
> whether the container process was isolated or not. Otherwise those ports
> can be leaked if the container is destroyed during preparation.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/isolators/network/port_mapping.cpp a29282ed0488d83966f222084031ed1d2b6ab1f5
> src/tests/containerizer/port_mapping_tests.cpp 553f53043a52cfe235fc2b2e88fb54d0f2725d3e
>
>
> Diff: https://reviews.apache.org/r/67936/diff/2/
>
>
> Testing
> -------
>
> Added `PortMappingIsolatorTest.ROOT_CleanupNotIsolated` test to verify that ephemeral ports are properly deallocated when the container was not isolated and manually checked that if fails without the fix. Ran `sudo make check`.
>
>
> Thanks,
>
> Ilya Pronin
>
>
Re: Review Request 67936: Fixed ephemeral ports deallocation in
network/port_mapping isolator.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67936/#review206177
-----------------------------------------------------------
PASS: Mesos patch 67936 was successfully built and tested.
Reviews applied: `['67936']`
All the build artifacts available at: http://dcos-win.westus.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/1942/mesos-review-67936
- Mesos Reviewbot Windows
On July 17, 2018, 8:12 p.m., Ilya Pronin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67936/
> -----------------------------------------------------------
>
> (Updated July 17, 2018, 8:12 p.m.)
>
>
> Review request for mesos and Jie Yu.
>
>
> Bugs: MESOS-9080
> https://issues.apache.org/jira/browse/MESOS-9080
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Ephemeral ports are allocated for a container during the preparation
> phase and have to be deallocated during container cleanup regardless of
> whether the container process was isolated or not. Otherwise those ports
> can be leaked if the container is destroyed during preparation.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/isolators/network/port_mapping.cpp a29282ed0488d83966f222084031ed1d2b6ab1f5
> src/tests/containerizer/port_mapping_tests.cpp 553f53043a52cfe235fc2b2e88fb54d0f2725d3e
>
>
> Diff: https://reviews.apache.org/r/67936/diff/2/
>
>
> Testing
> -------
>
> Added `PortMappingIsolatorTest.ROOT_CleanupNotIsolated` test to verify that ephemeral ports are properly deallocated when the container was not isolated and manually checked that if fails without the fix. Ran `sudo make check`.
>
>
> Thanks,
>
> Ilya Pronin
>
>
Re: Review Request 67936: Fixed ephemeral ports deallocation in
network/port_mapping isolator.
Posted by Ilya Pronin <ip...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67936/
-----------------------------------------------------------
(Updated July 17, 2018, 1:12 p.m.)
Review request for mesos and Jie Yu.
Changes
-------
Addressed review comments.
Bugs: MESOS-9080
https://issues.apache.org/jira/browse/MESOS-9080
Repository: mesos
Description
-------
Ephemeral ports are allocated for a container during the preparation
phase and have to be deallocated during container cleanup regardless of
whether the container process was isolated or not. Otherwise those ports
can be leaked if the container is destroyed during preparation.
Diffs (updated)
-----
src/slave/containerizer/mesos/isolators/network/port_mapping.cpp a29282ed0488d83966f222084031ed1d2b6ab1f5
src/tests/containerizer/port_mapping_tests.cpp 553f53043a52cfe235fc2b2e88fb54d0f2725d3e
Diff: https://reviews.apache.org/r/67936/diff/2/
Changes: https://reviews.apache.org/r/67936/diff/1-2/
Testing
-------
Added `PortMappingIsolatorTest.ROOT_CleanupNotIsolated` test to verify that ephemeral ports are properly deallocated when the container was not isolated and manually checked that if fails without the fix. Ran `sudo make check`.
Thanks,
Ilya Pronin