You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Qian Zhang <zh...@gmail.com> on 2019/07/29 01:30:33 UTC
Review Request 71172: Looked up parent's IP for the non-checkpointed
nested containers.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71172/
-----------------------------------------------------------
Review request for mesos.
Repository: mesos
Description
-------
Looked up parent's IP for the non-checkpointed nested containers.
Diffs
-----
src/slave/containerizer/mesos/isolators/network/cni/cni.cpp f2989cf7a2161154bb7d9bf2112bee8dd3cc5cf5
Diff: https://reviews.apache.org/r/71172/diff/1/
Testing
-------
sudo make check
Thanks,
Qian Zhang
Re: Review Request 71172: Looked up parent's IP for the
non-checkpointed nested containers.
Posted by Gilbert Song <so...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71172/#review217094
-----------------------------------------------------------
Ship it!
Ship It!
- Gilbert Song
On July 29, 2019, 1:40 a.m., Qian Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71172/
> -----------------------------------------------------------
>
> (Updated July 29, 2019, 1:40 a.m.)
>
>
> Review request for mesos, Andrei Budnik and Gilbert Song.
>
>
> Bugs: MESOS-9868
> https://issues.apache.org/jira/browse/MESOS-9868
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Looked up parent's IP for the non-checkpointed nested containers.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/isolators/network/cni/cni.cpp f2989cf7a2161154bb7d9bf2112bee8dd3cc5cf5
>
>
> Diff: https://reviews.apache.org/r/71172/diff/1/
>
>
> Testing
> -------
>
> sudo make check
>
>
> Thanks,
>
> Qian Zhang
>
>
Re: Review Request 71172: Looked up parent's IP for the
non-checkpointed nested containers.
Posted by Qian Zhang <zh...@gmail.com>.
> On July 31, 2019, 9:15 a.m., Gilbert Song wrote:
> > This part of code is fragile. Probably we need a unit test.
> >
> > The complication comes from the factor that we only checkpointed partial information. Probably we should just check point all containers.
Unit test: https://reviews.apache.org/r/71235
> On July 31, 2019, 9:15 a.m., Gilbert Song wrote:
> > src/slave/containerizer/mesos/isolators/network/cni/cni.cpp
> > Lines 1445-1457 (original), 1447-1472 (patched)
> > <https://reviews.apache.org/r/71172/diff/1/?file=2158035#file2158035line1447>
> >
> > Can review this patch https://reviews.apache.org/r/65987/ again and make sure there is no similar logic mistake in other methods
Yeah, I reviewed it and I think there is no similar logci mistake in other methods.
- Qian
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71172/#review216976
-----------------------------------------------------------
On July 29, 2019, 4:40 p.m., Qian Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71172/
> -----------------------------------------------------------
>
> (Updated July 29, 2019, 4:40 p.m.)
>
>
> Review request for mesos, Andrei Budnik and Gilbert Song.
>
>
> Bugs: MESOS-9868
> https://issues.apache.org/jira/browse/MESOS-9868
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Looked up parent's IP for the non-checkpointed nested containers.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/isolators/network/cni/cni.cpp f2989cf7a2161154bb7d9bf2112bee8dd3cc5cf5
>
>
> Diff: https://reviews.apache.org/r/71172/diff/1/
>
>
> Testing
> -------
>
> sudo make check
>
>
> Thanks,
>
> Qian Zhang
>
>
Re: Review Request 71172: Looked up parent's IP for the
non-checkpointed nested containers.
Posted by Qian Zhang <zh...@gmail.com>.
> On July 31, 2019, 9:15 a.m., Gilbert Song wrote:
> > src/slave/containerizer/mesos/isolators/network/cni/cni.cpp
> > Lines 1448-1461 (patched)
> > <https://reviews.apache.org/r/71172/diff/1/?file=2158035#file2158035line1448>
> >
> > if a nested container has its own network namespace, I guess its INFO will be checkpointed?
Yes!
- Qian
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71172/#review216976
-----------------------------------------------------------
On July 29, 2019, 4:40 p.m., Qian Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71172/
> -----------------------------------------------------------
>
> (Updated July 29, 2019, 4:40 p.m.)
>
>
> Review request for mesos, Andrei Budnik and Gilbert Song.
>
>
> Bugs: MESOS-9868
> https://issues.apache.org/jira/browse/MESOS-9868
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Looked up parent's IP for the non-checkpointed nested containers.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/isolators/network/cni/cni.cpp f2989cf7a2161154bb7d9bf2112bee8dd3cc5cf5
>
>
> Diff: https://reviews.apache.org/r/71172/diff/1/
>
>
> Testing
> -------
>
> sudo make check
>
>
> Thanks,
>
> Qian Zhang
>
>
Re: Review Request 71172: Looked up parent's IP for the
non-checkpointed nested containers.
Posted by Gilbert Song <so...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71172/#review216976
-----------------------------------------------------------
Ship it!
This part of code is fragile. Probably we need a unit test.
The complication comes from the factor that we only checkpointed partial information. Probably we should just check point all containers.
src/slave/containerizer/mesos/isolators/network/cni/cni.cpp
Lines 1445-1457 (original), 1447-1472 (patched)
<https://reviews.apache.org/r/71172/#comment304202>
Can review this patch https://reviews.apache.org/r/65987/ again and make sure there is no similar logic mistake in other methods
src/slave/containerizer/mesos/isolators/network/cni/cni.cpp
Lines 1445-1457 (original), 1447-1472 (patched)
<https://reviews.apache.org/r/71172/#comment304203>
another option is:
```
if (isNestedContainer) {
if (!infos.contains(containerId) || infos[containerId]->joinsParentsNetwork) {
return status(containerId.parent());
}
}
if (!infos.contains(containerId)) {
return ContainerStatus();
}
```
up to you to pick the style that is easier to read.
src/slave/containerizer/mesos/isolators/network/cni/cni.cpp
Lines 1448-1461 (patched)
<https://reviews.apache.org/r/71172/#comment304201>
if a nested container has its own network namespace, I guess its INFO will be checkpointed?
- Gilbert Song
On July 29, 2019, 1:40 a.m., Qian Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71172/
> -----------------------------------------------------------
>
> (Updated July 29, 2019, 1:40 a.m.)
>
>
> Review request for mesos, Andrei Budnik and Gilbert Song.
>
>
> Bugs: MESOS-9868
> https://issues.apache.org/jira/browse/MESOS-9868
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Looked up parent's IP for the non-checkpointed nested containers.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/isolators/network/cni/cni.cpp f2989cf7a2161154bb7d9bf2112bee8dd3cc5cf5
>
>
> Diff: https://reviews.apache.org/r/71172/diff/1/
>
>
> Testing
> -------
>
> sudo make check
>
>
> Thanks,
>
> Qian Zhang
>
>
Re: Review Request 71172: Looked up parent's IP for the
non-checkpointed nested containers.
Posted by Qian Zhang <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71172/
-----------------------------------------------------------
(Updated July 29, 2019, 4:40 p.m.)
Review request for mesos, Andrei Budnik and Gilbert Song.
Bugs: MESOS-9868
https://issues.apache.org/jira/browse/MESOS-9868
Repository: mesos
Description
-------
Looked up parent's IP for the non-checkpointed nested containers.
Diffs
-----
src/slave/containerizer/mesos/isolators/network/cni/cni.cpp f2989cf7a2161154bb7d9bf2112bee8dd3cc5cf5
Diff: https://reviews.apache.org/r/71172/diff/1/
Testing
-------
sudo make check
Thanks,
Qian Zhang
Re: Review Request 71172: Looked up parent's IP for the
non-checkpointed nested containers.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71172/#review216904
-----------------------------------------------------------
Bad review!
Reviews applied: []
Error:
No reviewers specified. Please find a reviewer by asking on JIRA or the mailing list.
- Mesos Reviewbot
On July 29, 2019, 1:30 a.m., Qian Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71172/
> -----------------------------------------------------------
>
> (Updated July 29, 2019, 1:30 a.m.)
>
>
> Review request for mesos.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Looked up parent's IP for the non-checkpointed nested containers.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/isolators/network/cni/cni.cpp f2989cf7a2161154bb7d9bf2112bee8dd3cc5cf5
>
>
> Diff: https://reviews.apache.org/r/71172/diff/1/
>
>
> Testing
> -------
>
> sudo make check
>
>
> Thanks,
>
> Qian Zhang
>
>