mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Qian Zhang <zhq527...@gmail.com>
Subject Re: Review Request 60766: Ignored containers that join CNI networks.
Date Thu, 24 Aug 2017 06:58:26 GMT


> On Aug. 23, 2017, 5:52 p.m., Qian Zhang wrote:
> > src/slave/containerizer/mesos/isolators/network/ports.cpp
> > Line 394 (original), 436 (patched)
> > <https://reviews.apache.org/r/60766/diff/11/?file=1801806#file1801806line436>
> >
> >     So here an empty `Info` object is put into `infos`? I think we need to populate
its `ports` field before that.
> 
> James Peach wrote:
>     The ports are populated in `update()`:
>     
>     ```
>     0823 08:57:15.686208 21125 linux_launcher.cpp:300] Recovered container d9e180c6-c527-431c-b147-f820c788aa7d
>     I0823 08:57:15.686942 21133 ports.cpp:437] recovering container d9e180c6-c527-431c-b147-f820c788aa7d
>     I0823 08:57:15.688449 21123 provisioner.cpp:416] Provisioner recovery complete
>     I0823 08:57:15.689740 21132 slave.cpp:6110] Sending reconnect request to executor
'084372d8-e569-48d9-9bb3-6ce89750207e' of framework ac96cb5d-fbab-402c-8fe8-1042ea1b1986-0000
at executor(1)@17.228.224.108:39141
>     I0823 08:57:15.691100 21459 exec.cpp:283] Received reconnect request from agent ac96cb5d-fbab-402c-8fe8-1042ea1b1986-S0
>     I0823 08:57:15.693174 21131 slave.cpp:4071] Received re-registration message from
executor '084372d8-e569-48d9-9bb3-6ce89750207e' of framework ac96cb5d-fbab-402c-8fe8-1042ea1b1986-0000
>     I0823 08:57:15.693802 21132 ports.cpp:372] updating container d9e180c6-c527-431c-b147-f820c788aa7d
>     I0823 08:57:15.694080 21457 exec.cpp:260] Executor re-registered on agent ac96cb5d-fbab-402c-8fe8-1042ea1b1986-S0
>     ```

Yeah, I know that. I do not think we should rely on that for 2 reasons:
1. This `update()` method will be called when executors reregister agent, but what if executor
does not reregister agent for some reason, but in this case, we still want `network/ports`
to check its listening ports.
2. This `update()` method will not be called for nested container, that means after agent
is restarted, all the nested containers will not be checked by `network/ports` isolator anymore.
And I would suggest to add a test for it.


- Qian


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


On Aug. 22, 2017, 6:01 a.m., James Peach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60766/
> -----------------------------------------------------------
> 
> (Updated Aug. 22, 2017, 6:01 a.m.)
> 
> 
> Review request for mesos, Qian Zhang and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-7675
>     https://issues.apache.org/jira/browse/MESOS-7675
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Working on the assumption that containers with CNI networks will
> get their own IP addresses and don't need port isolation, ignore
> any containers that are joining CNI networks.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/containerizer.cpp 5805dfb4fb6e755e4c23851b0a6b504f2a8b3396

>   src/slave/containerizer/mesos/isolators/network/ports.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/network/ports.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/60766/diff/13/
> 
> 
> Testing
> -------
> 
> make check (Fedora 26).
> 
> 
> Thanks,
> 
> James Peach
> 
>


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