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