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 2016/01/07 00:41:15 UTC
Re: Review Request 41820: Pulled out provisioner from linux filesystem
isolator.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41820/
-----------------------------------------------------------
(Updated Jan. 6, 2016, 3:41 p.m.)
Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.
Bugs: MESOS-4240
https://issues.apache.org/jira/browse/MESOS-4240
Repository: mesos
Description
-------
Pulled out provisioner from linux filesystem isolator.
Diffs (updated)
-----
src/slave/containerizer/mesos/containerizer.hpp 89d18624b08f2c953b9fa21663aeda694d4aac12
src/slave/containerizer/mesos/containerizer.cpp f3c370aeb331beb6202fd30cd0278877da0b42e0
src/slave/containerizer/mesos/isolators/filesystem/linux.hpp a29f9d184a0d1088577b3168a12bc8c06493a477
src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 8af2cecdca947e55d8d39e26d2fc3d42212f36c3
src/tests/containerizer/filesystem_isolator_tests.cpp 5bb85034c22caef64054c1629f6fd55d227e48b1
Diff: https://reviews.apache.org/r/41820/diff/
Testing
-------
make check (ubuntu14.04 + clang-3.6)
Thanks,
Gilbert Song
Re: Review Request 41820: Pulled out provisioner from linux filesystem
isolator.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41820/#review113375
-----------------------------------------------------------
src/slave/containerizer/mesos/containerizer.cpp (lines 482 - 483)
<https://reviews.apache.org/r/41820/#comment173953>
We should recover the isolators first before recover provisioner (the reverse order of launching a container because recover might do cleanup on unknown containers).
YOu might want to add `recoverIsolators` and `recoverProvisioner`. Change the `_recover` function to be:
```
return recoverIsolators(recoverable, orphans)
.then(defer(self(), &Self::recoverProvisioner, recoverable, orphans))
.then(defer(self(), &Self::__recover, recoverable, orphans));
```
src/slave/containerizer/mesos/containerizer.cpp (lines 689 - 690)
<https://reviews.apache.org/r/41820/#comment173983>
We mutate the executorInfo here and that mutated executorInfo here needs to be passed to `prepare` and `_launch`. This is important for command executors.
I guess what you needs to do here is: rename the existing `_launch` to `__launch` and add a new `_launch`. In `launch`, do the following:
```
return provisioner->provision(containerId, executorInfo)
.then(defer(self(),
&Self::_launch,
containerId,
executorInfo,
directory,
user,
slaveId,
slavePid,
checkpoint,
lambda::_1));
```
The new `_launch` should be similar to what you have here for `_provision`, but in the end, if will call `__launch` with mutated executorInfo.
src/slave/containerizer/mesos/containerizer.cpp (line 711)
<https://reviews.apache.org/r/41820/#comment173985>
I realized another issue with command executors. We don't have to resolve it in the patch, but I just want to mention it here so that we don't forget. You probably can add a TODO comments here.
For command executors, we modify the executorInfo so that the user image will be mounted in as a volume. However, we also need to figure out a way to support passing and handling those runtime configurations in the image.
cc @tnachen
src/slave/containerizer/mesos/containerizer.cpp (lines 1447 - 1451)
<https://reviews.apache.org/r/41820/#comment173984>
Again, we should call provisioner->destroy after all isolators have been cleaned up. Also, it does not make sense to put provisioner->destroy in cleanupIsolators.
src/slave/containerizer/mesos/isolators/filesystem/linux.hpp (lines 45 - 46)
<https://reviews.apache.org/r/41820/#comment173955>
This fits in one line?
src/slave/containerizer/mesos/isolators/filesystem/linux.hpp (lines 77 - 78)
<https://reviews.apache.org/r/41820/#comment173956>
Ditto.
src/slave/containerizer/mesos/isolators/filesystem/linux.cpp (lines 62 - 63)
<https://reviews.apache.org/r/41820/#comment173957>
Ditto.
src/slave/containerizer/mesos/isolators/filesystem/linux.cpp (lines 173 - 174)
<https://reviews.apache.org/r/41820/#comment173958>
Fits in one line?
src/tests/containerizer/filesystem_isolator_tests.cpp (lines 157 - 158)
<https://reviews.apache.org/r/41820/#comment173959>
Fits in one line?
src/tests/containerizer/filesystem_isolator_tests.cpp (lines 1008 - 1009)
<https://reviews.apache.org/r/41820/#comment173960>
FIts in one line?
- Jie Yu
On Jan. 6, 2016, 11:41 p.m., Gilbert Song wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41820/
> -----------------------------------------------------------
>
> (Updated Jan. 6, 2016, 11:41 p.m.)
>
>
> Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.
>
>
> Bugs: MESOS-4240
> https://issues.apache.org/jira/browse/MESOS-4240
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Pulled out provisioner from linux filesystem isolator.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/containerizer.hpp 89d18624b08f2c953b9fa21663aeda694d4aac12
> src/slave/containerizer/mesos/containerizer.cpp f3c370aeb331beb6202fd30cd0278877da0b42e0
> src/slave/containerizer/mesos/isolators/filesystem/linux.hpp a29f9d184a0d1088577b3168a12bc8c06493a477
> src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 8af2cecdca947e55d8d39e26d2fc3d42212f36c3
> src/tests/containerizer/filesystem_isolator_tests.cpp 5bb85034c22caef64054c1629f6fd55d227e48b1
>
> Diff: https://reviews.apache.org/r/41820/diff/
>
>
> Testing
> -------
>
> make check (ubuntu14.04 + clang-3.6)
>
>
> Thanks,
>
> Gilbert Song
>
>
Re: Review Request 41820: Pulled out provisioner from linux filesystem
isolator.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41820/#review113923
-----------------------------------------------------------
Bad patch!
Reviews applied: [41814, 41999, 41815, 41816, 41817, 41819]
Failed command: ./support/apply-review.sh -n -r 41819
Error:
2016-01-12 03:21:08 URL:https://reviews.apache.org/r/41819/diff/raw/ [21646/21646] -> "41819.patch" [1]
error: patch failed: src/slave/containerizer/mesos/containerizer.cpp:702
error: src/slave/containerizer/mesos/containerizer.cpp: patch does not apply
error: patch failed: src/slave/containerizer/mesos/isolators/filesystem/posix.cpp:91
error: src/slave/containerizer/mesos/isolators/filesystem/posix.cpp: patch does not apply
- Mesos ReviewBot
On Jan. 11, 2016, 8:15 p.m., Gilbert Song wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41820/
> -----------------------------------------------------------
>
> (Updated Jan. 11, 2016, 8:15 p.m.)
>
>
> Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.
>
>
> Bugs: MESOS-4240
> https://issues.apache.org/jira/browse/MESOS-4240
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Pulled out provisioner from linux filesystem isolator.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/containerizer.hpp 89d18624b08f2c953b9fa21663aeda694d4aac12
> src/slave/containerizer/mesos/containerizer.cpp f3c370aeb331beb6202fd30cd0278877da0b42e0
> src/slave/containerizer/mesos/isolators/filesystem/linux.hpp a29f9d184a0d1088577b3168a12bc8c06493a477
> src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 8af2cecdca947e55d8d39e26d2fc3d42212f36c3
> src/tests/containerizer/filesystem_isolator_tests.cpp 5bb85034c22caef64054c1629f6fd55d227e48b1
>
> Diff: https://reviews.apache.org/r/41820/diff/
>
>
> Testing
> -------
>
> make check (ubuntu14.04 + clang-3.6)
>
>
> Thanks,
>
> Gilbert Song
>
>
Re: Review Request 41820: Pulled out provisioner from linux filesystem
isolator.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41820/#review114255
-----------------------------------------------------------
Ship it!
Thanks! THis is great!
src/slave/containerizer/mesos/containerizer.cpp (line 1365)
<https://reviews.apache.org/r/41820/#comment175058>
We are not destroying the 'provisioner'. So
```
Failed to destroy the provisioned filesystem when...
```
- Jie Yu
On Jan. 11, 2016, 8:15 p.m., Gilbert Song wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41820/
> -----------------------------------------------------------
>
> (Updated Jan. 11, 2016, 8:15 p.m.)
>
>
> Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.
>
>
> Bugs: MESOS-4240
> https://issues.apache.org/jira/browse/MESOS-4240
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Pulled out provisioner from linux filesystem isolator.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/containerizer.hpp 89d18624b08f2c953b9fa21663aeda694d4aac12
> src/slave/containerizer/mesos/containerizer.cpp f3c370aeb331beb6202fd30cd0278877da0b42e0
> src/slave/containerizer/mesos/isolators/filesystem/linux.hpp a29f9d184a0d1088577b3168a12bc8c06493a477
> src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 8af2cecdca947e55d8d39e26d2fc3d42212f36c3
> src/tests/containerizer/filesystem_isolator_tests.cpp 5bb85034c22caef64054c1629f6fd55d227e48b1
>
> Diff: https://reviews.apache.org/r/41820/diff/
>
>
> Testing
> -------
>
> make check (ubuntu14.04 + clang-3.6)
>
>
> Thanks,
>
> Gilbert Song
>
>
Re: Review Request 41820: Pulled out provisioner from linux filesystem
isolator.
Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41820/#review114279
-----------------------------------------------------------
Ship it!
Ship It!
- Timothy Chen
On Jan. 11, 2016, 8:15 p.m., Gilbert Song wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41820/
> -----------------------------------------------------------
>
> (Updated Jan. 11, 2016, 8:15 p.m.)
>
>
> Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.
>
>
> Bugs: MESOS-4240
> https://issues.apache.org/jira/browse/MESOS-4240
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Pulled out provisioner from linux filesystem isolator.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/containerizer.hpp 89d18624b08f2c953b9fa21663aeda694d4aac12
> src/slave/containerizer/mesos/containerizer.cpp f3c370aeb331beb6202fd30cd0278877da0b42e0
> src/slave/containerizer/mesos/isolators/filesystem/linux.hpp a29f9d184a0d1088577b3168a12bc8c06493a477
> src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 8af2cecdca947e55d8d39e26d2fc3d42212f36c3
> src/tests/containerizer/filesystem_isolator_tests.cpp 5bb85034c22caef64054c1629f6fd55d227e48b1
>
> Diff: https://reviews.apache.org/r/41820/diff/
>
>
> Testing
> -------
>
> make check (ubuntu14.04 + clang-3.6)
>
>
> Thanks,
>
> Gilbert Song
>
>
Re: Review Request 41820: Pulled out provisioner from linux filesystem
isolator.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41820/#review114340
-----------------------------------------------------------
Patch looks great!
Reviews applied: [41814, 41999, 41815, 41816, 41817, 41819, 41820]
Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export COMPILER=gcc; ./support/docker_build.sh
- Mesos ReviewBot
On Jan. 13, 2016, 9:19 p.m., Gilbert Song wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41820/
> -----------------------------------------------------------
>
> (Updated Jan. 13, 2016, 9:19 p.m.)
>
>
> Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.
>
>
> Bugs: MESOS-4240
> https://issues.apache.org/jira/browse/MESOS-4240
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Pulled out provisioner from linux filesystem isolator.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/containerizer.hpp 89d18624b08f2c953b9fa21663aeda694d4aac12
> src/slave/containerizer/mesos/containerizer.cpp f3c370aeb331beb6202fd30cd0278877da0b42e0
> src/slave/containerizer/mesos/isolators/filesystem/linux.hpp a29f9d184a0d1088577b3168a12bc8c06493a477
> src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 8af2cecdca947e55d8d39e26d2fc3d42212f36c3
> src/tests/containerizer/filesystem_isolator_tests.cpp 5bb85034c22caef64054c1629f6fd55d227e48b1
>
> Diff: https://reviews.apache.org/r/41820/diff/
>
>
> Testing
> -------
>
> make check (ubuntu14.04 + clang-3.6)
>
>
> Thanks,
>
> Gilbert Song
>
>
Re: Review Request 41820: Pulled out provisioner from linux filesystem
isolator.
Posted by Gilbert Song <so...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41820/
-----------------------------------------------------------
(Updated Jan. 13, 2016, 1:19 p.m.)
Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.
Bugs: MESOS-4240
https://issues.apache.org/jira/browse/MESOS-4240
Repository: mesos
Description
-------
Pulled out provisioner from linux filesystem isolator.
Diffs (updated)
-----
src/slave/containerizer/mesos/containerizer.hpp 89d18624b08f2c953b9fa21663aeda694d4aac12
src/slave/containerizer/mesos/containerizer.cpp f3c370aeb331beb6202fd30cd0278877da0b42e0
src/slave/containerizer/mesos/isolators/filesystem/linux.hpp a29f9d184a0d1088577b3168a12bc8c06493a477
src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 8af2cecdca947e55d8d39e26d2fc3d42212f36c3
src/tests/containerizer/filesystem_isolator_tests.cpp 5bb85034c22caef64054c1629f6fd55d227e48b1
Diff: https://reviews.apache.org/r/41820/diff/
Testing
-------
make check (ubuntu14.04 + clang-3.6)
Thanks,
Gilbert Song
Re: Review Request 41820: Pulled out provisioner from linux filesystem
isolator.
Posted by Gilbert Song <so...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41820/
-----------------------------------------------------------
(Updated Jan. 11, 2016, 12:15 p.m.)
Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.
Bugs: MESOS-4240
https://issues.apache.org/jira/browse/MESOS-4240
Repository: mesos
Description
-------
Pulled out provisioner from linux filesystem isolator.
Diffs (updated)
-----
src/slave/containerizer/mesos/containerizer.hpp 89d18624b08f2c953b9fa21663aeda694d4aac12
src/slave/containerizer/mesos/containerizer.cpp f3c370aeb331beb6202fd30cd0278877da0b42e0
src/slave/containerizer/mesos/isolators/filesystem/linux.hpp a29f9d184a0d1088577b3168a12bc8c06493a477
src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 8af2cecdca947e55d8d39e26d2fc3d42212f36c3
src/tests/containerizer/filesystem_isolator_tests.cpp 5bb85034c22caef64054c1629f6fd55d227e48b1
Diff: https://reviews.apache.org/r/41820/diff/
Testing
-------
make check (ubuntu14.04 + clang-3.6)
Thanks,
Gilbert Song
Re: Review Request 41820: Pulled out provisioner from linux filesystem
isolator.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41820/#review113667
-----------------------------------------------------------
Bad patch!
Reviews applied: [41814, 41999, 41815, 41816, 41817, 41819]
Failed command: ./support/apply-review.sh -n -r 41819
Error:
2016-01-11 01:39:47 URL:https://reviews.apache.org/r/41819/diff/raw/ [21646/21646] -> "41819.patch" [1]
error: patch failed: src/slave/containerizer/mesos/containerizer.cpp:702
error: src/slave/containerizer/mesos/containerizer.cpp: patch does not apply
error: patch failed: src/slave/containerizer/mesos/isolators/filesystem/posix.cpp:91
error: src/slave/containerizer/mesos/isolators/filesystem/posix.cpp: patch does not apply
- Mesos ReviewBot
On Jan. 9, 2016, 2:14 a.m., Gilbert Song wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41820/
> -----------------------------------------------------------
>
> (Updated Jan. 9, 2016, 2:14 a.m.)
>
>
> Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.
>
>
> Bugs: MESOS-4240
> https://issues.apache.org/jira/browse/MESOS-4240
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Pulled out provisioner from linux filesystem isolator.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/containerizer.hpp 89d18624b08f2c953b9fa21663aeda694d4aac12
> src/slave/containerizer/mesos/containerizer.cpp f3c370aeb331beb6202fd30cd0278877da0b42e0
> src/slave/containerizer/mesos/isolators/filesystem/linux.hpp a29f9d184a0d1088577b3168a12bc8c06493a477
> src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 8af2cecdca947e55d8d39e26d2fc3d42212f36c3
> src/tests/containerizer/filesystem_isolator_tests.cpp 5bb85034c22caef64054c1629f6fd55d227e48b1
>
> Diff: https://reviews.apache.org/r/41820/diff/
>
>
> Testing
> -------
>
> make check (ubuntu14.04 + clang-3.6)
>
>
> Thanks,
>
> Gilbert Song
>
>
Re: Review Request 41820: Pulled out provisioner from linux filesystem
isolator.
Posted by Gilbert Song <so...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41820/
-----------------------------------------------------------
(Updated Jan. 8, 2016, 6:14 p.m.)
Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.
Bugs: MESOS-4240
https://issues.apache.org/jira/browse/MESOS-4240
Repository: mesos
Description
-------
Pulled out provisioner from linux filesystem isolator.
Diffs (updated)
-----
src/slave/containerizer/mesos/containerizer.hpp 89d18624b08f2c953b9fa21663aeda694d4aac12
src/slave/containerizer/mesos/containerizer.cpp f3c370aeb331beb6202fd30cd0278877da0b42e0
src/slave/containerizer/mesos/isolators/filesystem/linux.hpp a29f9d184a0d1088577b3168a12bc8c06493a477
src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 8af2cecdca947e55d8d39e26d2fc3d42212f36c3
src/tests/containerizer/filesystem_isolator_tests.cpp 5bb85034c22caef64054c1629f6fd55d227e48b1
Diff: https://reviews.apache.org/r/41820/diff/
Testing
-------
make check (ubuntu14.04 + clang-3.6)
Thanks,
Gilbert Song
Re: Review Request 41820: Pulled out provisioner from linux filesystem
isolator.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41820/#review113210
-----------------------------------------------------------
Bad patch!
Reviews applied: [41814, 41999, 41815, 41816, 41817]
Failed command: ./support/apply-review.sh -n -r 41817
Error:
2016-01-07 07:13:03 URL:https://reviews.apache.org/r/41817/diff/raw/ [11190/11190] -> "41817.patch" [1]
Total errors found: 0
Checking 6 files
Error: Commit message summary (the first line) must not exceed 72 characters.
- Mesos ReviewBot
On Jan. 6, 2016, 11:41 p.m., Gilbert Song wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41820/
> -----------------------------------------------------------
>
> (Updated Jan. 6, 2016, 11:41 p.m.)
>
>
> Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.
>
>
> Bugs: MESOS-4240
> https://issues.apache.org/jira/browse/MESOS-4240
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Pulled out provisioner from linux filesystem isolator.
>
>
> Diffs
> -----
>
> src/slave/containerizer/mesos/containerizer.hpp 89d18624b08f2c953b9fa21663aeda694d4aac12
> src/slave/containerizer/mesos/containerizer.cpp f3c370aeb331beb6202fd30cd0278877da0b42e0
> src/slave/containerizer/mesos/isolators/filesystem/linux.hpp a29f9d184a0d1088577b3168a12bc8c06493a477
> src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 8af2cecdca947e55d8d39e26d2fc3d42212f36c3
> src/tests/containerizer/filesystem_isolator_tests.cpp 5bb85034c22caef64054c1629f6fd55d227e48b1
>
> Diff: https://reviews.apache.org/r/41820/diff/
>
>
> Testing
> -------
>
> make check (ubuntu14.04 + clang-3.6)
>
>
> Thanks,
>
> Gilbert Song
>
>