You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Benjamin Bannier <be...@mesosphere.io> on 2018/08/22 10:30:49 UTC
Review Request 68472: Removed `ROOT` requirement some
`StorageLocalResourceProvider` tests.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68472/
-----------------------------------------------------------
Review request for mesos and Chun-Hung Hsiao.
Repository: mesos
Description
-------
These tests required `ROOT` in order to use `filesystem/linux`
isolation this is not a requirement anymore so we can run the tests in
general. These tests appear to be able to run in parallel as well.
Diffs
-----
src/tests/storage_local_resource_provider_tests.cpp 9b88d366df958d938a3a0cac4f64a937c6178ee3
Diff: https://reviews.apache.org/r/68472/diff/1/
Testing
-------
`make check`
`GTEST_FILTER='*StorageLocalResourceProviderTest*' ../support/mesos-gtest-runner.py ./src/mesos-tests`
Thanks,
Benjamin Bannier
Re: Review Request 68472: Removed `ROOT` requirement some
`StorageLocalResourceProvider` tests.
Posted by Chun-Hung Hsiao <ch...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68472/#review207903
-----------------------------------------------------------
Ship it!
Ship It!
- Chun-Hung Hsiao
On Aug. 22, 2018, 10:30 a.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68472/
> -----------------------------------------------------------
>
> (Updated Aug. 22, 2018, 10:30 a.m.)
>
>
> Review request for mesos and Chun-Hung Hsiao.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> These tests required `ROOT` in order to use `filesystem/linux`
> isolation this is not a requirement anymore so we can run the tests in
> general. These tests appear to be able to run in parallel as well.
>
>
> Diffs
> -----
>
> src/tests/storage_local_resource_provider_tests.cpp 9b88d366df958d938a3a0cac4f64a937c6178ee3
>
>
> Diff: https://reviews.apache.org/r/68472/diff/1/
>
>
> Testing
> -------
>
> `make check`
> `GTEST_FILTER='*StorageLocalResourceProviderTest*' ../support/mesos-gtest-runner.py ./src/mesos-tests`
>
>
> Thanks,
>
> Benjamin Bannier
>
>
Re: Review Request 68472: Removed `ROOT` requirement some
`StorageLocalResourceProvider` tests.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68472/#review207958
-----------------------------------------------------------
Patch looks great!
Reviews applied: [68472]
Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose --disable-libtool-wrappers' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh
- Mesos Reviewbot
On Aug. 27, 2018, 7:48 a.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68472/
> -----------------------------------------------------------
>
> (Updated Aug. 27, 2018, 7:48 a.m.)
>
>
> Review request for mesos and Chun-Hung Hsiao.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> These tests required `ROOT` in order to use `filesystem/linux`
> isolation this is not a requirement anymore so we can run the tests in
> general. These tests appear to be able to run in parallel as well.
>
>
> Diffs
> -----
>
> src/tests/storage_local_resource_provider_tests.cpp 65b1464733926cc0ddaa1f0d518411b70daa6a03
>
>
> Diff: https://reviews.apache.org/r/68472/diff/2/
>
>
> Testing
> -------
>
> `make check`
> `GTEST_FILTER='*StorageLocalResourceProviderTest*' ../support/mesos-gtest-runner.py ./src/mesos-tests`
>
>
> Thanks,
>
> Benjamin Bannier
>
>
Re: Review Request 68472: Removed `ROOT` requirement some
`StorageLocalResourceProvider` tests.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68472/#review207953
-----------------------------------------------------------
PASS: Mesos patch 68472 was successfully built and tested.
Reviews applied: `['68472']`
All the build artifacts available at: http://dcos-win.westus.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2241/mesos-review-68472
- Mesos Reviewbot Windows
On Aug. 27, 2018, 7:48 a.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68472/
> -----------------------------------------------------------
>
> (Updated Aug. 27, 2018, 7:48 a.m.)
>
>
> Review request for mesos and Chun-Hung Hsiao.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> These tests required `ROOT` in order to use `filesystem/linux`
> isolation this is not a requirement anymore so we can run the tests in
> general. These tests appear to be able to run in parallel as well.
>
>
> Diffs
> -----
>
> src/tests/storage_local_resource_provider_tests.cpp 65b1464733926cc0ddaa1f0d518411b70daa6a03
>
>
> Diff: https://reviews.apache.org/r/68472/diff/2/
>
>
> Testing
> -------
>
> `make check`
> `GTEST_FILTER='*StorageLocalResourceProviderTest*' ../support/mesos-gtest-runner.py ./src/mesos-tests`
>
>
> Thanks,
>
> Benjamin Bannier
>
>
Re: Review Request 68472: Removed `ROOT` requirement some
`StorageLocalResourceProvider` tests.
Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68472/
-----------------------------------------------------------
(Updated Aug. 27, 2018, 9:48 a.m.)
Review request for mesos and Chun-Hung Hsiao.
Changes
-------
Rebased.
Repository: mesos
Description
-------
These tests required `ROOT` in order to use `filesystem/linux`
isolation this is not a requirement anymore so we can run the tests in
general. These tests appear to be able to run in parallel as well.
Diffs (updated)
-----
src/tests/storage_local_resource_provider_tests.cpp 65b1464733926cc0ddaa1f0d518411b70daa6a03
Diff: https://reviews.apache.org/r/68472/diff/2/
Changes: https://reviews.apache.org/r/68472/diff/1-2/
Testing
-------
`make check`
`GTEST_FILTER='*StorageLocalResourceProviderTest*' ../support/mesos-gtest-runner.py ./src/mesos-tests`
Thanks,
Benjamin Bannier
Re: Review Request 68472: Removed `ROOT` requirement some
`StorageLocalResourceProvider` tests.
Posted by Chun-Hung Hsiao <ch...@apache.org>.
> On Aug. 22, 2018, 11:37 p.m., Chun-Hung Hsiao wrote:
> > src/tests/storage_local_resource_provider_tests.cpp
> > Line 299 (original)
> > <https://reviews.apache.org/r/68472/diff/1/?file=2076113#file2076113line299>
> >
> > The reason to enable this isolator is because we create new mounts when launching the CSI plugin container:
> > https://github.com/apache/mesos/blob/da13db36d9735ec6f03817fb7da8d7e2ef00831c/src/resource_provider/storage/provider.cpp#L1947
> > https://github.com/apache/mesos/blob/da13db36d9735ec6f03817fb7da8d7e2ef00831c/src/resource_provider/storage/provider.cpp#L1965
> > If we don't use this isolator, wouldn't the test pollute the host mount table? Or are they already taken care of by the test environment teardown?
> >
> > If not, we need to manually call `unmountAll` in the test teardown instead.
It seems I'm mistaken. If the `posix/filesystem` isolator is used instead the volumes will be just symlinked. Dropping this.
- Chun-Hung
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68472/#review207778
-----------------------------------------------------------
On Aug. 22, 2018, 10:30 a.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68472/
> -----------------------------------------------------------
>
> (Updated Aug. 22, 2018, 10:30 a.m.)
>
>
> Review request for mesos and Chun-Hung Hsiao.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> These tests required `ROOT` in order to use `filesystem/linux`
> isolation this is not a requirement anymore so we can run the tests in
> general. These tests appear to be able to run in parallel as well.
>
>
> Diffs
> -----
>
> src/tests/storage_local_resource_provider_tests.cpp 9b88d366df958d938a3a0cac4f64a937c6178ee3
>
>
> Diff: https://reviews.apache.org/r/68472/diff/1/
>
>
> Testing
> -------
>
> `make check`
> `GTEST_FILTER='*StorageLocalResourceProviderTest*' ../support/mesos-gtest-runner.py ./src/mesos-tests`
>
>
> Thanks,
>
> Benjamin Bannier
>
>
Re: Review Request 68472: Removed `ROOT` requirement some
`StorageLocalResourceProvider` tests.
Posted by Chun-Hung Hsiao <ch...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68472/#review207778
-----------------------------------------------------------
src/tests/storage_local_resource_provider_tests.cpp
Line 299 (original)
<https://reviews.apache.org/r/68472/#comment291282>
The reason to enable this isolator is because we create new mounts when launching the CSI plugin container:
https://github.com/apache/mesos/blob/da13db36d9735ec6f03817fb7da8d7e2ef00831c/src/resource_provider/storage/provider.cpp#L1947
https://github.com/apache/mesos/blob/da13db36d9735ec6f03817fb7da8d7e2ef00831c/src/resource_provider/storage/provider.cpp#L1965
If we don't use this isolator, wouldn't the test pollute the host mount table? Or are they already taken care of by the test environment teardown?
If not, we need to manually call `unmountAll` in the test teardown instead.
src/tests/storage_local_resource_provider_tests.cpp
Line 371 (original), 369 (patched)
<https://reviews.apache.org/r/68472/#comment291283>
This test and some other tests have been disabled on the master branch. Please rebase.
- Chun-Hung Hsiao
On Aug. 22, 2018, 10:30 a.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68472/
> -----------------------------------------------------------
>
> (Updated Aug. 22, 2018, 10:30 a.m.)
>
>
> Review request for mesos and Chun-Hung Hsiao.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> These tests required `ROOT` in order to use `filesystem/linux`
> isolation this is not a requirement anymore so we can run the tests in
> general. These tests appear to be able to run in parallel as well.
>
>
> Diffs
> -----
>
> src/tests/storage_local_resource_provider_tests.cpp 9b88d366df958d938a3a0cac4f64a937c6178ee3
>
>
> Diff: https://reviews.apache.org/r/68472/diff/1/
>
>
> Testing
> -------
>
> `make check`
> `GTEST_FILTER='*StorageLocalResourceProviderTest*' ../support/mesos-gtest-runner.py ./src/mesos-tests`
>
>
> Thanks,
>
> Benjamin Bannier
>
>
Re: Review Request 68472: Removed `ROOT` requirement some
`StorageLocalResourceProvider` tests.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68472/#review207740
-----------------------------------------------------------
PASS: Mesos patch 68472 was successfully built and tested.
Reviews applied: `['68472']`
All the build artifacts available at: http://dcos-win.westus.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2216/mesos-review-68472
- Mesos Reviewbot Windows
On Aug. 22, 2018, 10:30 a.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68472/
> -----------------------------------------------------------
>
> (Updated Aug. 22, 2018, 10:30 a.m.)
>
>
> Review request for mesos and Chun-Hung Hsiao.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> These tests required `ROOT` in order to use `filesystem/linux`
> isolation this is not a requirement anymore so we can run the tests in
> general. These tests appear to be able to run in parallel as well.
>
>
> Diffs
> -----
>
> src/tests/storage_local_resource_provider_tests.cpp 9b88d366df958d938a3a0cac4f64a937c6178ee3
>
>
> Diff: https://reviews.apache.org/r/68472/diff/1/
>
>
> Testing
> -------
>
> `make check`
> `GTEST_FILTER='*StorageLocalResourceProviderTest*' ../support/mesos-gtest-runner.py ./src/mesos-tests`
>
>
> Thanks,
>
> Benjamin Bannier
>
>
Re: Review Request 68472: Removed `ROOT` requirement some
`StorageLocalResourceProvider` tests.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68472/#review207742
-----------------------------------------------------------
Patch looks great!
Reviews applied: [68472]
Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose --disable-libtool-wrappers' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh
- Mesos Reviewbot
On Aug. 22, 2018, 10:30 a.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68472/
> -----------------------------------------------------------
>
> (Updated Aug. 22, 2018, 10:30 a.m.)
>
>
> Review request for mesos and Chun-Hung Hsiao.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> These tests required `ROOT` in order to use `filesystem/linux`
> isolation this is not a requirement anymore so we can run the tests in
> general. These tests appear to be able to run in parallel as well.
>
>
> Diffs
> -----
>
> src/tests/storage_local_resource_provider_tests.cpp 9b88d366df958d938a3a0cac4f64a937c6178ee3
>
>
> Diff: https://reviews.apache.org/r/68472/diff/1/
>
>
> Testing
> -------
>
> `make check`
> `GTEST_FILTER='*StorageLocalResourceProviderTest*' ../support/mesos-gtest-runner.py ./src/mesos-tests`
>
>
> Thanks,
>
> Benjamin Bannier
>
>