You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Ian Downes <ia...@gmail.com> on 2014/08/01 08:06:30 UTC
Review Request 24177: Pass executor directory to Isolator::prepare().
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24177/
-----------------------------------------------------------
Review request for mesos, Ben Mahler and Jie Yu.
Repository: mesos-git
Description
-------
Pass executor directory to Isolator::prepare().
Will be used for FilesystemIsolator.
Diffs
-----
src/slave/containerizer/isolator.hpp e52e8b15c740c62ef64b49897d3d6ae5179d4719
src/slave/containerizer/isolator.cpp 5e61bf2e3cf14be53d41aa657b4a78ab2dd6ecb0
src/slave/containerizer/isolators/cgroups/cpushare.hpp 780037b093cb5b61254e61ddcfb9ecd4b0551a77
src/slave/containerizer/isolators/cgroups/cpushare.cpp 3265a8084dfab6f3c9244f8f227e4024d8afe68a
src/slave/containerizer/isolators/cgroups/mem.hpp 8c476c7847baa923821bc89a1e04f50f019c6ecb
src/slave/containerizer/isolators/cgroups/mem.cpp e8d1e35a1acdb3beb03a91cfd734a59417657b20
src/slave/containerizer/isolators/cgroups/perf_event.hpp 4ceb07a6e4f171788cfbabca78c14ae23ff183db
src/slave/containerizer/isolators/cgroups/perf_event.cpp 6f65b722c5b22a71a7c334c2b63db1584cb9fe29
src/slave/containerizer/isolators/posix.hpp 17bbd10c565aa88048ec3a845014ceee582396d2
src/slave/containerizer/mesos/containerizer.cpp 2c394e2c8702166266f5d20ff005abb218da8a6c
src/tests/isolator.hpp 89df4c4959c680354b002fa12e3a270a358087af
src/tests/isolator_tests.cpp c38f87632cb6984543cb3767dbd656cde7459610
Diff: https://reviews.apache.org/r/24177/diff/
Testing
-------
make check
Thanks,
Ian Downes
Re: Review Request 24177: Pass executor directory to Isolator::prepare().
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24177/#review50352
-----------------------------------------------------------
src/tests/isolator_tests.cpp
<https://reviews.apache.org/r/24177/#comment88054>
Remove 'dir' when test finishes?
src/tests/isolator_tests.cpp
<https://reviews.apache.org/r/24177/#comment88055>
Ditto.
src/tests/isolator_tests.cpp
<https://reviews.apache.org/r/24177/#comment88056>
Ditto.
src/tests/isolator_tests.cpp
<https://reviews.apache.org/r/24177/#comment88057>
Ditto.
- Jie Yu
On Aug. 1, 2014, 6:06 a.m., Ian Downes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24177/
> -----------------------------------------------------------
>
> (Updated Aug. 1, 2014, 6:06 a.m.)
>
>
> Review request for mesos, Ben Mahler and Jie Yu.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Pass executor directory to Isolator::prepare().
>
> Will be used for FilesystemIsolator.
>
>
> Diffs
> -----
>
> src/slave/containerizer/isolator.hpp e52e8b15c740c62ef64b49897d3d6ae5179d4719
> src/slave/containerizer/isolator.cpp 5e61bf2e3cf14be53d41aa657b4a78ab2dd6ecb0
> src/slave/containerizer/isolators/cgroups/cpushare.hpp 780037b093cb5b61254e61ddcfb9ecd4b0551a77
> src/slave/containerizer/isolators/cgroups/cpushare.cpp 3265a8084dfab6f3c9244f8f227e4024d8afe68a
> src/slave/containerizer/isolators/cgroups/mem.hpp 8c476c7847baa923821bc89a1e04f50f019c6ecb
> src/slave/containerizer/isolators/cgroups/mem.cpp e8d1e35a1acdb3beb03a91cfd734a59417657b20
> src/slave/containerizer/isolators/cgroups/perf_event.hpp 4ceb07a6e4f171788cfbabca78c14ae23ff183db
> src/slave/containerizer/isolators/cgroups/perf_event.cpp 6f65b722c5b22a71a7c334c2b63db1584cb9fe29
> src/slave/containerizer/isolators/posix.hpp 17bbd10c565aa88048ec3a845014ceee582396d2
> src/slave/containerizer/mesos/containerizer.cpp 2c394e2c8702166266f5d20ff005abb218da8a6c
> src/tests/isolator.hpp 89df4c4959c680354b002fa12e3a270a358087af
> src/tests/isolator_tests.cpp c38f87632cb6984543cb3767dbd656cde7459610
>
> Diff: https://reviews.apache.org/r/24177/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Ian Downes
>
>
Re: Review Request 24177: Pass executor directory to
Isolator::prepare().
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24177/#review54458
-----------------------------------------------------------
Ship it!
Ship It!
- Jie Yu
On Sept. 23, 2014, 11:42 p.m., Ian Downes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24177/
> -----------------------------------------------------------
>
> (Updated Sept. 23, 2014, 11:42 p.m.)
>
>
> Review request for mesos, Ben Mahler and Jie Yu.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Pass executor directory to Isolator::prepare().
>
> Will be used for FilesystemIsolator.
>
>
> Diffs
> -----
>
> src/slave/containerizer/isolator.hpp e52e8b15c740c62ef64b49897d3d6ae5179d4719
> src/slave/containerizer/isolator.cpp 5e61bf2e3cf14be53d41aa657b4a78ab2dd6ecb0
> src/slave/containerizer/isolators/cgroups/cpushare.hpp 2187c296ea9b1a7de9ae3f09fdf1983f98a3d01b
> src/slave/containerizer/isolators/cgroups/cpushare.cpp 7164ecc0f068d4a72248521e3cbd345958efa880
> src/slave/containerizer/isolators/cgroups/mem.hpp b1b4f5a2bd9e01b03fdfa74f187f7dee8119b812
> src/slave/containerizer/isolators/cgroups/mem.cpp b3d4a5daa90a842e501bc6be2f0cf20fe22906ac
> src/slave/containerizer/isolators/cgroups/perf_event.hpp f7283d830cd6af7b3c9006c098de0a6ad48b7c82
> src/slave/containerizer/isolators/cgroups/perf_event.cpp 4ced508e600e13f3e5ae9d12ea199de743def652
> src/slave/containerizer/isolators/posix.hpp f120aafef96343d84f93c5636484509dc972a0a8
> src/tests/isolator.hpp 89df4c4959c680354b002fa12e3a270a358087af
> src/tests/isolator_tests.cpp c38f87632cb6984543cb3767dbd656cde7459610
>
> Diff: https://reviews.apache.org/r/24177/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Ian Downes
>
>
Re: Review Request 24177: Pass executor directory to
Isolator::prepare().
Posted by Ian Downes <ia...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24177/
-----------------------------------------------------------
(Updated Oct. 23, 2014, 10:40 a.m.)
Review request for mesos, Ben Mahler and Jie Yu.
Repository: mesos-git
Description
-------
Pass executor directory to Isolator::prepare().
Will be used for FilesystemIsolator.
Diffs (updated)
-----
src/slave/containerizer/isolator.hpp e52e8b15c740c62ef64b49897d3d6ae5179d4719
src/slave/containerizer/isolator.cpp 5e61bf2e3cf14be53d41aa657b4a78ab2dd6ecb0
src/slave/containerizer/isolators/cgroups/cpushare.hpp 2187c296ea9b1a7de9ae3f09fdf1983f98a3d01b
src/slave/containerizer/isolators/cgroups/cpushare.cpp 7164ecc0f068d4a72248521e3cbd345958efa880
src/slave/containerizer/isolators/cgroups/mem.hpp b1b4f5a2bd9e01b03fdfa74f187f7dee8119b812
src/slave/containerizer/isolators/cgroups/mem.cpp b3d4a5daa90a842e501bc6be2f0cf20fe22906ac
src/slave/containerizer/isolators/cgroups/perf_event.hpp f7283d830cd6af7b3c9006c098de0a6ad48b7c82
src/slave/containerizer/isolators/cgroups/perf_event.cpp 4ced508e600e13f3e5ae9d12ea199de743def652
src/slave/containerizer/isolators/network/port_mapping.hpp b3fd331903ff610e35ce329c35dfe7e8d4c67f7d
src/slave/containerizer/isolators/network/port_mapping.cpp 62ca9bca08de55f19bfff7b8828dfa4fb6c51f30
src/slave/containerizer/isolators/posix.hpp 6038e92312f58ac1db3fc58673da2d07d2527966
src/tests/isolator.hpp 89df4c4959c680354b002fa12e3a270a358087af
src/tests/isolator_tests.cpp 52b38a38eaafde3c42d464caa7bb028ba970a291
src/tests/port_mapping_tests.cpp 973bdef884e3e0db2ab305c3a99ccc4e7c1542ef
Diff: https://reviews.apache.org/r/24177/diff/
Testing
-------
make check
Thanks,
Ian Downes
Re: Review Request 24177: Pass executor directory to
Isolator::prepare().
Posted by Ian Downes <ia...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24177/
-----------------------------------------------------------
(Updated Oct. 2, 2014, 11:18 a.m.)
Review request for mesos, Ben Mahler and Jie Yu.
Changes
-------
Updated use of temporary directory to be relative to test dir.
Repository: mesos-git
Description
-------
Pass executor directory to Isolator::prepare().
Will be used for FilesystemIsolator.
Diffs (updated)
-----
src/slave/containerizer/isolator.hpp e52e8b15c740c62ef64b49897d3d6ae5179d4719
src/slave/containerizer/isolator.cpp 5e61bf2e3cf14be53d41aa657b4a78ab2dd6ecb0
src/slave/containerizer/isolators/cgroups/cpushare.hpp 2187c296ea9b1a7de9ae3f09fdf1983f98a3d01b
src/slave/containerizer/isolators/cgroups/cpushare.cpp 7164ecc0f068d4a72248521e3cbd345958efa880
src/slave/containerizer/isolators/cgroups/mem.hpp b1b4f5a2bd9e01b03fdfa74f187f7dee8119b812
src/slave/containerizer/isolators/cgroups/mem.cpp b3d4a5daa90a842e501bc6be2f0cf20fe22906ac
src/slave/containerizer/isolators/cgroups/perf_event.hpp f7283d830cd6af7b3c9006c098de0a6ad48b7c82
src/slave/containerizer/isolators/cgroups/perf_event.cpp 4ced508e600e13f3e5ae9d12ea199de743def652
src/slave/containerizer/isolators/network/port_mapping.hpp b624c4d2f2b2d635bb3fd49db46d3f23262312e4
src/slave/containerizer/isolators/network/port_mapping.cpp 2766a00ff81dc550c21387f920666f81705db4f0
src/slave/containerizer/isolators/posix.hpp f120aafef96343d84f93c5636484509dc972a0a8
src/tests/isolator.hpp 89df4c4959c680354b002fa12e3a270a358087af
src/tests/isolator_tests.cpp c38f87632cb6984543cb3767dbd656cde7459610
src/tests/port_mapping_tests.cpp 0389f4034e2582691068855305e23f2364f72f4a
Diff: https://reviews.apache.org/r/24177/diff/
Testing
-------
make check
Thanks,
Ian Downes
Re: Review Request 24177: Pass executor directory to
Isolator::prepare().
Posted by Ian Downes <ia...@gmail.com>.
> On Sept. 25, 2014, 3:27 p.m., Vinod Kone wrote:
> > src/tests/isolator_tests.cpp, line 446
> > <https://reviews.apache.org/r/24177/diff/3/?file=703418#file703418line446>
> >
> > probably not part of this review, but we avoid this pattern because it could lead to leaked temp directories if the test fails.
> >
> > since you are already inside a sandbox (because you inherit from TemporaryDirectoryTest), you should use relative paths to create temporary directories.
> >
> > s/os::mkdtemp()/os::mkdtemp("./XXXXXX")/
> >
> > mind adding a TODO?
FYI - relative paths don't work so path::join(os::getcwd(), "XXXXXX"))
- Ian
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24177/#review54618
-----------------------------------------------------------
On Oct. 2, 2014, 11:18 a.m., Ian Downes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24177/
> -----------------------------------------------------------
>
> (Updated Oct. 2, 2014, 11:18 a.m.)
>
>
> Review request for mesos, Ben Mahler and Jie Yu.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Pass executor directory to Isolator::prepare().
>
> Will be used for FilesystemIsolator.
>
>
> Diffs
> -----
>
> src/slave/containerizer/isolator.hpp e52e8b15c740c62ef64b49897d3d6ae5179d4719
> src/slave/containerizer/isolator.cpp 5e61bf2e3cf14be53d41aa657b4a78ab2dd6ecb0
> src/slave/containerizer/isolators/cgroups/cpushare.hpp 2187c296ea9b1a7de9ae3f09fdf1983f98a3d01b
> src/slave/containerizer/isolators/cgroups/cpushare.cpp 7164ecc0f068d4a72248521e3cbd345958efa880
> src/slave/containerizer/isolators/cgroups/mem.hpp b1b4f5a2bd9e01b03fdfa74f187f7dee8119b812
> src/slave/containerizer/isolators/cgroups/mem.cpp b3d4a5daa90a842e501bc6be2f0cf20fe22906ac
> src/slave/containerizer/isolators/cgroups/perf_event.hpp f7283d830cd6af7b3c9006c098de0a6ad48b7c82
> src/slave/containerizer/isolators/cgroups/perf_event.cpp 4ced508e600e13f3e5ae9d12ea199de743def652
> src/slave/containerizer/isolators/network/port_mapping.hpp b624c4d2f2b2d635bb3fd49db46d3f23262312e4
> src/slave/containerizer/isolators/network/port_mapping.cpp 2766a00ff81dc550c21387f920666f81705db4f0
> src/slave/containerizer/isolators/posix.hpp f120aafef96343d84f93c5636484509dc972a0a8
> src/tests/isolator.hpp 89df4c4959c680354b002fa12e3a270a358087af
> src/tests/isolator_tests.cpp c38f87632cb6984543cb3767dbd656cde7459610
> src/tests/port_mapping_tests.cpp 0389f4034e2582691068855305e23f2364f72f4a
>
> Diff: https://reviews.apache.org/r/24177/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Ian Downes
>
>
Re: Review Request 24177: Pass executor directory to
Isolator::prepare().
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24177/#review54618
-----------------------------------------------------------
Ship it!
src/tests/isolator_tests.cpp
<https://reviews.apache.org/r/24177/#comment94817>
probably not part of this review, but we avoid this pattern because it could lead to leaked temp directories if the test fails.
since you are already inside a sandbox (because you inherit from TemporaryDirectoryTest), you should use relative paths to create temporary directories.
s/os::mkdtemp()/os::mkdtemp("./XXXXXX")/
mind adding a TODO?
- Vinod Kone
On Sept. 23, 2014, 11:42 p.m., Ian Downes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24177/
> -----------------------------------------------------------
>
> (Updated Sept. 23, 2014, 11:42 p.m.)
>
>
> Review request for mesos, Ben Mahler and Jie Yu.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Pass executor directory to Isolator::prepare().
>
> Will be used for FilesystemIsolator.
>
>
> Diffs
> -----
>
> src/slave/containerizer/isolator.hpp e52e8b15c740c62ef64b49897d3d6ae5179d4719
> src/slave/containerizer/isolator.cpp 5e61bf2e3cf14be53d41aa657b4a78ab2dd6ecb0
> src/slave/containerizer/isolators/cgroups/cpushare.hpp 2187c296ea9b1a7de9ae3f09fdf1983f98a3d01b
> src/slave/containerizer/isolators/cgroups/cpushare.cpp 7164ecc0f068d4a72248521e3cbd345958efa880
> src/slave/containerizer/isolators/cgroups/mem.hpp b1b4f5a2bd9e01b03fdfa74f187f7dee8119b812
> src/slave/containerizer/isolators/cgroups/mem.cpp b3d4a5daa90a842e501bc6be2f0cf20fe22906ac
> src/slave/containerizer/isolators/cgroups/perf_event.hpp f7283d830cd6af7b3c9006c098de0a6ad48b7c82
> src/slave/containerizer/isolators/cgroups/perf_event.cpp 4ced508e600e13f3e5ae9d12ea199de743def652
> src/slave/containerizer/isolators/posix.hpp f120aafef96343d84f93c5636484509dc972a0a8
> src/tests/isolator.hpp 89df4c4959c680354b002fa12e3a270a358087af
> src/tests/isolator_tests.cpp c38f87632cb6984543cb3767dbd656cde7459610
>
> Diff: https://reviews.apache.org/r/24177/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Ian Downes
>
>
Re: Review Request 24177: Pass executor directory to
Isolator::prepare().
Posted by Ian Downes <ia...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24177/
-----------------------------------------------------------
(Updated Sept. 23, 2014, 4:42 p.m.)
Review request for mesos, Ben Mahler and Jie Yu.
Repository: mesos-git
Description
-------
Pass executor directory to Isolator::prepare().
Will be used for FilesystemIsolator.
Diffs (updated)
-----
src/slave/containerizer/isolator.hpp e52e8b15c740c62ef64b49897d3d6ae5179d4719
src/slave/containerizer/isolator.cpp 5e61bf2e3cf14be53d41aa657b4a78ab2dd6ecb0
src/slave/containerizer/isolators/cgroups/cpushare.hpp 2187c296ea9b1a7de9ae3f09fdf1983f98a3d01b
src/slave/containerizer/isolators/cgroups/cpushare.cpp 7164ecc0f068d4a72248521e3cbd345958efa880
src/slave/containerizer/isolators/cgroups/mem.hpp b1b4f5a2bd9e01b03fdfa74f187f7dee8119b812
src/slave/containerizer/isolators/cgroups/mem.cpp b3d4a5daa90a842e501bc6be2f0cf20fe22906ac
src/slave/containerizer/isolators/cgroups/perf_event.hpp f7283d830cd6af7b3c9006c098de0a6ad48b7c82
src/slave/containerizer/isolators/cgroups/perf_event.cpp 4ced508e600e13f3e5ae9d12ea199de743def652
src/slave/containerizer/isolators/posix.hpp f120aafef96343d84f93c5636484509dc972a0a8
src/tests/isolator.hpp 89df4c4959c680354b002fa12e3a270a358087af
src/tests/isolator_tests.cpp c38f87632cb6984543cb3767dbd656cde7459610
Diff: https://reviews.apache.org/r/24177/diff/
Testing
-------
make check
Thanks,
Ian Downes
Re: Review Request 24177: Pass executor directory to
Isolator::prepare().
Posted by Ian Downes <ia...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24177/
-----------------------------------------------------------
(Updated Sept. 15, 2014, 12:59 p.m.)
Review request for mesos, Ben Mahler and Jie Yu.
Repository: mesos-git
Description
-------
Pass executor directory to Isolator::prepare().
Will be used for FilesystemIsolator.
Diffs (updated)
-----
src/slave/containerizer/isolator.hpp e52e8b15c740c62ef64b49897d3d6ae5179d4719
src/slave/containerizer/isolator.cpp 5e61bf2e3cf14be53d41aa657b4a78ab2dd6ecb0
src/slave/containerizer/isolators/cgroups/cpushare.hpp d4df5f37e8d2e356d35ca40d799197a47393fa9a
src/slave/containerizer/isolators/cgroups/cpushare.cpp b1cad472a561e81422f980182fd24eb95701140a
src/slave/containerizer/isolators/cgroups/mem.hpp b1b4f5a2bd9e01b03fdfa74f187f7dee8119b812
src/slave/containerizer/isolators/cgroups/mem.cpp fb3db88af7b2ffa79272743f571c4c021c619c48
src/slave/containerizer/isolators/cgroups/perf_event.hpp f7283d830cd6af7b3c9006c098de0a6ad48b7c82
src/slave/containerizer/isolators/cgroups/perf_event.cpp ff047d37c1b2e659b18b5d4a1e97301192d05e55
src/slave/containerizer/isolators/posix.hpp f120aafef96343d84f93c5636484509dc972a0a8
src/tests/isolator.hpp 89df4c4959c680354b002fa12e3a270a358087af
src/tests/isolator_tests.cpp c38f87632cb6984543cb3767dbd656cde7459610
Diff: https://reviews.apache.org/r/24177/diff/
Testing
-------
make check
Thanks,
Ian Downes