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

[Jie Yu]

---
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)


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



src/slave/containerizer/mesos/isolators/network/port_mapping.cpp (line 2450)


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)


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

[Jie Yu]

---
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
> 
>

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

[Cong Wang]

---
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

[Ian Downes]

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



src/slave/containerizer/isolators/network/port_mapping.cpp (line 2431)


Why is this no longer a failure?



src/slave/containerizer/isolators/network/port_mapping.cpp (line 2450)


ditto?


- Ian Downes


On Oct. 20, 2015, 11:57 a.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39490/
> ---
> 
> (Updated Oct. 20, 2015, 11:57 a.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/isolators/network/port_mapping.cpp 
> e6bb75e6f5ba48a0c4cf6dd8f353e5f5843d0eef 
> 
> Diff: https://reviews.apache.org/r/39490/diff/
> 
> 
> Testing
> ---
> 
> Manual tests
> 
> 
> Thanks,
> 
> Cong Wang
> 
>

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

[Cong Wang]

> On Oct. 27, 2015, 5:38 p.m., Ian Downes wrote:
> > src/slave/containerizer/isolators/network/port_mapping.cpp, line 2431
> > 
> >
> > Why is this no longer a failure?

They could be already created by the slave if the flag were enabled before 
restarting.


- Cong


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


On Oct. 20, 2015, 6:57 p.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39490/
> ---
> 
> (Updated Oct. 20, 2015, 6:57 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/isolators/network/port_mapping.cpp 
> e6bb75e6f5ba48a0c4cf6dd8f353e5f5843d0eef 
> 
> Diff: https://reviews.apache.org/r/39490/diff/
> 
> 
> Testing
> ---
> 
> Manual tests
> 
> 
> Thanks,
> 
> Cong Wang
> 
>

Review Request 39490: Always create non-IP egress filters

[Cong Wang]

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

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/isolators/network/port_mapping.cpp 
e6bb75e6f5ba48a0c4cf6dd8f353e5f5843d0eef 

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


Testing
---

Manual tests


Thanks,

Cong Wang

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

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [39490]

All tests passed.

- Mesos ReviewBot


On Oct. 20, 2015, 6:57 p.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39490/
> ---
> 
> (Updated Oct. 20, 2015, 6:57 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/isolators/network/port_mapping.cpp 
> e6bb75e6f5ba48a0c4cf6dd8f353e5f5843d0eef 
> 
> Diff: https://reviews.apache.org/r/39490/diff/
> 
> 
> Testing
> ---
> 
> Manual tests
> 
> 
> Thanks,
> 
> Cong Wang
> 
>