You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Gilbert Song <so...@gmail.com> on 2018/10/05 00:37:36 UTC
Review Request 68929: Fixed the nested container launch failure on
the agent upgrade case.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68929/
-----------------------------------------------------------
Review request for mesos, Greg Mann, Jie Yu, Meng Zhu, Qian Zhang, and Vinod Kone.
Bugs: MESOS-9295
https://issues.apache.org/jira/browse/MESOS-9295
Repository: mesos
Description
-------
Fixed the nested container launch failure on the agent upgrade case.
Diffs
-----
src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 11dfbab182487c45fa494c41e31f777272b4b5d0
Diff: https://reviews.apache.org/r/68929/diff/1/
Testing
-------
make check
Thanks,
Gilbert Song
Re: Review Request 68929: Fixed the nested container launch failure
on the agent upgrade case.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68929/#review209250
-----------------------------------------------------------
PASS: Mesos patch 68929 was successfully built and tested.
Reviews applied: `['68929']`
All the build artifacts available at: http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2421/mesos-review-68929
- Mesos Reviewbot Windows
On Oct. 5, 2018, 12:37 a.m., Gilbert Song wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68929/
> -----------------------------------------------------------
>
> (Updated Oct. 5, 2018, 12:37 a.m.)
>
>
> Review request for mesos, Greg Mann, Jie Yu, Meng Zhu, Qian Zhang, and Vinod Kone.
>
>
> Bugs: MESOS-9295
> https://issues.apache.org/jira/browse/MESOS-9295
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Fixed the nested container launch failure on the agent upgrade case.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 11dfbab182487c45fa494c41e31f777272b4b5d0
>
>
> Diff: https://reviews.apache.org/r/68929/diff/1/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Gilbert Song
>
>
Re: Review Request 68929: Fixed the nested container launch failure
on the agent upgrade case.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68929/#review209278
-----------------------------------------------------------
Bad patch!
Reviews applied: [68929]
Failed command: python support/apply-reviews.py -n -r 68929
Error:
Traceback (most recent call last):
File "support/apply-reviews.py", line 35, in <module>
import urllib.request
ImportError: No module named request
Full log: https://builds.apache.org/job/Mesos-Reviewbot/23455/console
- Mesos Reviewbot
On Oct. 5, 2018, 12:37 a.m., Gilbert Song wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68929/
> -----------------------------------------------------------
>
> (Updated Oct. 5, 2018, 12:37 a.m.)
>
>
> Review request for mesos, Greg Mann, Jie Yu, Meng Zhu, Qian Zhang, and Vinod Kone.
>
>
> Bugs: MESOS-9295
> https://issues.apache.org/jira/browse/MESOS-9295
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Fixed the nested container launch failure on the agent upgrade case.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 11dfbab182487c45fa494c41e31f777272b4b5d0
>
>
> Diff: https://reviews.apache.org/r/68929/diff/1/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Gilbert Song
>
>
Re: Review Request 68929: Fixed the nested container launch failure
on the agent upgrade case.
Posted by Qian Zhang <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68929/#review209300
-----------------------------------------------------------
Fix it, then Ship it!
src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp
Lines 692-696 (patched)
<https://reviews.apache.org/r/68929/#comment293607>
Indent seems not correct.
src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp
Lines 689-691 (original), 707-709 (patched)
<https://reviews.apache.org/r/68929/#comment293608>
Could you add the `containerId` in this message like what you did in the above one?
- Qian Zhang
On Oct. 6, 2018, 7:30 a.m., Gilbert Song wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68929/
> -----------------------------------------------------------
>
> (Updated Oct. 6, 2018, 7:30 a.m.)
>
>
> Review request for mesos, Greg Mann, Jie Yu, Meng Zhu, Qian Zhang, and Vinod Kone.
>
>
> Bugs: MESOS-9295
> https://issues.apache.org/jira/browse/MESOS-9295
>
>
> Repository: mesos
>
>
> Description
> -------
>
> If there are new cgroup subsystems are added after the agent upgrad
> or recovery, new nested container launched under old containers that
> are launched before the recovery would fail, because it cannot assign
> its pid to the non-existed cgroup hierarchy. We should skip those
> new cgroup subsystems for nested containers under old containers.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 11dfbab182487c45fa494c41e31f777272b4b5d0
>
>
> Diff: https://reviews.apache.org/r/68929/diff/2/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Gilbert Song
>
>
Re: Review Request 68929: Fixed the nested container launch failure
on the agent upgrade case.
Posted by Gilbert Song <so...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68929/
-----------------------------------------------------------
(Updated Oct. 8, 2018, 10:36 a.m.)
Review request for mesos, Greg Mann, Jie Yu, Meng Zhu, Qian Zhang, and Vinod Kone.
Bugs: MESOS-9295
https://issues.apache.org/jira/browse/MESOS-9295
Repository: mesos
Description
-------
If there are new cgroup subsystems are added after the agent upgrad
or recovery, new nested container launched under old containers that
are launched before the recovery would fail, because it cannot assign
its pid to the non-existed cgroup hierarchy. We should skip those
new cgroup subsystems for nested containers under old containers.
Diffs (updated)
-----
src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 11dfbab182487c45fa494c41e31f777272b4b5d0
Diff: https://reviews.apache.org/r/68929/diff/3/
Changes: https://reviews.apache.org/r/68929/diff/2-3/
Testing
-------
make check
Thanks,
Gilbert Song
Re: Review Request 68929: Fixed the nested container launch failure
on the agent upgrade case.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68929/#review209282
-----------------------------------------------------------
PASS: Mesos patch 68929 was successfully built and tested.
Reviews applied: `['68929']`
All the build artifacts available at: http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2429/mesos-review-68929
- Mesos Reviewbot Windows
On Oct. 5, 2018, 11:30 p.m., Gilbert Song wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68929/
> -----------------------------------------------------------
>
> (Updated Oct. 5, 2018, 11:30 p.m.)
>
>
> Review request for mesos, Greg Mann, Jie Yu, Meng Zhu, Qian Zhang, and Vinod Kone.
>
>
> Bugs: MESOS-9295
> https://issues.apache.org/jira/browse/MESOS-9295
>
>
> Repository: mesos
>
>
> Description
> -------
>
> If there are new cgroup subsystems are added after the agent upgrad
> or recovery, new nested container launched under old containers that
> are launched before the recovery would fail, because it cannot assign
> its pid to the non-existed cgroup hierarchy. We should skip those
> new cgroup subsystems for nested containers under old containers.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 11dfbab182487c45fa494c41e31f777272b4b5d0
>
>
> Diff: https://reviews.apache.org/r/68929/diff/2/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Gilbert Song
>
>
Re: Review Request 68929: Fixed the nested container launch failure
on the agent upgrade case.
Posted by Gilbert Song <so...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68929/
-----------------------------------------------------------
(Updated Oct. 5, 2018, 4:30 p.m.)
Review request for mesos, Greg Mann, Jie Yu, Meng Zhu, Qian Zhang, and Vinod Kone.
Bugs: MESOS-9295
https://issues.apache.org/jira/browse/MESOS-9295
Repository: mesos
Description (updated)
-------
If there are new cgroup subsystems are added after the agent upgrad
or recovery, new nested container launched under old containers that
are launched before the recovery would fail, because it cannot assign
its pid to the non-existed cgroup hierarchy. We should skip those
new cgroup subsystems for nested containers under old containers.
Diffs (updated)
-----
src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 11dfbab182487c45fa494c41e31f777272b4b5d0
Diff: https://reviews.apache.org/r/68929/diff/2/
Changes: https://reviews.apache.org/r/68929/diff/1-2/
Testing
-------
make check
Thanks,
Gilbert Song
Re: Review Request 68929: Fixed the nested container launch failure
on the agent upgrade case.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68929/#review209269
-----------------------------------------------------------
Ship it!
Ship It!
- Greg Mann
On Oct. 5, 2018, 12:37 a.m., Gilbert Song wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68929/
> -----------------------------------------------------------
>
> (Updated Oct. 5, 2018, 12:37 a.m.)
>
>
> Review request for mesos, Greg Mann, Jie Yu, Meng Zhu, Qian Zhang, and Vinod Kone.
>
>
> Bugs: MESOS-9295
> https://issues.apache.org/jira/browse/MESOS-9295
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Fixed the nested container launch failure on the agent upgrade case.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 11dfbab182487c45fa494c41e31f777272b4b5d0
>
>
> Diff: https://reviews.apache.org/r/68929/diff/1/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Gilbert Song
>
>
Re: Review Request 68929: Fixed the nested container launch failure
on the agent upgrade case.
Posted by Meng Zhu <mz...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68929/#review209252
-----------------------------------------------------------
We probably want to udpate the comment here.
https://github.com/apache/mesos/blob/6654b26918b464764ae0329cba3669b4566a8d60/src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp#L343-L349
src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp
Lines 680 (patched)
<https://reviews.apache.org/r/68929/#comment293554>
Looks like `infos[rootContainerId]->cgroup` is valid through out the scope, `const string&` to save a copy
src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp
Lines 689 (patched)
<https://reviews.apache.org/r/68929/#comment293555>
Since we expect to rarely hit here, I suggest to `LOG(INFO)` this. We might be grateful someday :)
- Meng Zhu
On Oct. 4, 2018, 5:37 p.m., Gilbert Song wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68929/
> -----------------------------------------------------------
>
> (Updated Oct. 4, 2018, 5:37 p.m.)
>
>
> Review request for mesos, Greg Mann, Jie Yu, Meng Zhu, Qian Zhang, and Vinod Kone.
>
>
> Bugs: MESOS-9295
> https://issues.apache.org/jira/browse/MESOS-9295
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Fixed the nested container launch failure on the agent upgrade case.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 11dfbab182487c45fa494c41e31f777272b4b5d0
>
>
> Diff: https://reviews.apache.org/r/68929/diff/1/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Gilbert Song
>
>