You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Greg Mann <gr...@mesosphere.io> on 2016/12/02 01:08:52 UTC
Re: Review Request 46229: Add unit tests for adding a user for
persistent volumes.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46229/#review157656
-----------------------------------------------------------
src/tests/containerizer/linux_filesystem_isolator_tests.cpp (line 1531)
<https://reviews.apache.org/r/46229/#comment228301>
Nit:
s/use (and assume the presence) of/use (and assume the presence of)/
src/tests/persistent_volume_tests.cpp (lines 554 - 559)
<https://reviews.apache.org/r/46229/#comment228295>
I think that providing some validation of the nobody user is a good idea, but I wonder if this is the best way to accomplish it (and in the test below as well). It looks like this block also appears in `SlaveTest.DISABLED_ROOT_RunTaskWithCommandInfoWithUser`, but other tests which make use of 'nobody' (`SlaveTest.ROOT_LaunchTaskInfoWithContainerInfo`, for example) do not have such a check.
We could instead add a new filter to the test filters already present in `src/tests/environment.cpp` which would filter out all tests which make use of the `nobody` user when it's not present.
I would advocate a more general solution like that, I think. If you opt for such an approach, you could do it as part of this work, or just place a TODO here which points to a Mesos JIRA for that task.
Thoughts?
- Greg Mann
On Nov. 20, 2016, 4:51 p.m., Anindya Sinha wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46229/
> -----------------------------------------------------------
>
> (Updated Nov. 20, 2016, 4:51 p.m.)
>
>
> Review request for mesos and Jiang Yan Xu.
>
>
> Bugs: MESOS-4893
> https://issues.apache.org/jira/browse/MESOS-4893
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Add unit tests for adding a user for persistent volumes.
>
>
> Diffs
> -----
>
> src/tests/containerizer/linux_filesystem_isolator_tests.cpp 24d1a6971d412aaf7bdc20efcc1197b120de51ce
> src/tests/hierarchical_allocator_tests.cpp edd5cea8996d7c616cf9428f0f1c05d70c37c307
> src/tests/master_validation_tests.cpp f893067859425967654401f3226149268b51cf57
> src/tests/mesos.hpp f94882f44e7fd35f6e9aaa381656af5c5a58ff9e
> src/tests/persistent_volume_tests.cpp 8651b2c5455089041f16d091c90a7e0d948191b8
> src/tests/resources_tests.cpp d6eb7787bac58c1133a4bab0fc17df49117fed87
> src/tests/sorter_tests.cpp 9f48abe65c011acfaf79f3d0d79f1d032fd6a4af
>
> Diff: https://reviews.apache.org/r/46229/diff/
>
>
> Testing
> -------
>
> All tests passed (including the newly added tests).
>
>
> Thanks,
>
> Anindya Sinha
>
>