You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Cong Wang <cw...@twopensource.com> on 2015/11/21 00:36:22 UTC

Re: Review Request 39490: Always create non-IP egress filters

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

(Updated Nov. 20, 2015, 11:36 p.m.)


Review request for mesos, Ian Downes and Jie Yu.


Changes
-------

Rebase


Repository: mesos


Description
-------

When rolling out the new flag --egress_unique_flow_per_container, I noticed, on some slaves, only IP egress filters were created as expected, the reset were not. Looking at the code, it looks like we skipped the creation if this is not the first container we create, this is wrong for this case, because egress filters were not created for previous containers yet. We should always create them and ignore error if they exist.


Diffs (updated)
-----

  src/slave/containerizer/mesos/isolators/network/port_mapping.cpp e50616fd609588c547c90bba6d7b3b9b3eb4c6a9 

Diff: https://reviews.apache.org/r/39490/diff/


Testing
-------

Manual tests


Thanks,

Cong Wang


Re: Review Request 39490: Always create non-IP egress filters

Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39490/#review107453
-----------------------------------------------------------

Ship it!



src/slave/containerizer/mesos/isolators/network/port_mapping.cpp (lines 2430 - 2432)
<https://reviews.apache.org/r/39490/#comment166560>

    Please move this comments down (to where you increment the metrics)



src/slave/containerizer/mesos/isolators/network/port_mapping.cpp (line 2450)
<https://reviews.apache.org/r/39490/#comment166557>

    Let's not increment this metrics. Otherwise, you'll have this metrics being increased everytime a new container is launched.
    
    Instead, Please just add a comment and ignore this case.



src/slave/containerizer/mesos/isolators/network/port_mapping.cpp (line 2469)
<https://reviews.apache.org/r/39490/#comment166558>

    Ditto here.


- Jie Yu


On Nov. 20, 2015, 11:36 p.m., Cong Wang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39490/
> -----------------------------------------------------------
> 
> (Updated Nov. 20, 2015, 11:36 p.m.)
> 
> 
> Review request for mesos, Ian Downes and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> When rolling out the new flag --egress_unique_flow_per_container, I noticed, on some slaves, only IP egress filters were created as expected, the reset were not. Looking at the code, it looks like we skipped the creation if this is not the first container we create, this is wrong for this case, because egress filters were not created for previous containers yet. We should always create them and ignore error if they exist.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.cpp e50616fd609588c547c90bba6d7b3b9b3eb4c6a9 
> 
> Diff: https://reviews.apache.org/r/39490/diff/
> 
> 
> Testing
> -------
> 
> Manual tests
> 
> 
> Thanks,
> 
> Cong Wang
> 
>


Re: Review Request 39490: Always create non-IP egress filters

Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39490/#review107455
-----------------------------------------------------------


I'll fix this for you.

- Jie Yu


On Nov. 20, 2015, 11:36 p.m., Cong Wang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39490/
> -----------------------------------------------------------
> 
> (Updated Nov. 20, 2015, 11:36 p.m.)
> 
> 
> Review request for mesos, Ian Downes and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> When rolling out the new flag --egress_unique_flow_per_container, I noticed, on some slaves, only IP egress filters were created as expected, the reset were not. Looking at the code, it looks like we skipped the creation if this is not the first container we create, this is wrong for this case, because egress filters were not created for previous containers yet. We should always create them and ignore error if they exist.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.cpp e50616fd609588c547c90bba6d7b3b9b3eb4c6a9 
> 
> Diff: https://reviews.apache.org/r/39490/diff/
> 
> 
> Testing
> -------
> 
> Manual tests
> 
> 
> Thanks,
> 
> Cong Wang
> 
>