mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Qian Zhang <zhq527...@gmail.com>
Subject Re: Review Request 72728: Added tests for the 'volume/csi' isolator.
Date Thu, 03 Sep 2020 08:26:24 GMT


> On Sept. 2, 2020, 9:35 p.m., Qian Zhang wrote:
> > src/tests/containerizer/volume_csi_isolator_tests.cpp
> > Lines 238 (patched)
> > <https://reviews.apache.org/r/72728/diff/13/?file=2239017#file2239017line238>
> >
> >     Do we really need this method? I see it is only called when we create CSI server
in `ROOT_PluginConfigAddedAtRuntime`, can we just create the CSI server with NULL secret generator
like what we do in `ROOT_InvalidPluginConfig`?
> 
> Greg Mann wrote:
>     I'd rather leave it in, since it is also used by the subsequent tests in the next
patch in this chain. In order to run these tests with authentication enabled, we need to create
a secret generator. Since we have some nontrivial authentication code in these code paths,
keeping authentication enabled seems like a good idea to me. What do you think?

Yeah, I agree.


> On Sept. 2, 2020, 9:35 p.m., Qian Zhang wrote:
> > src/tests/containerizer/volume_csi_isolator_tests.cpp
> > Lines 794 (patched)
> > <https://reviews.apache.org/r/72728/diff/13/?file=2239017#file2239017line794>
> >
> >     Usually we use `SUDO_USER` as the non-root user in our unit tests, see https://github.com/apache/mesos/blob/1.10.0/src/tests/containerizer/docker_volume_isolator_tests.cpp#L1335
. And the test name should have the `UNPRIVILEGED_USER_` prefix.
> >     
> >     Just realized in a couple of your tests here, we need to pull Docker image `alpine`
from Docker Hub, right? Then I think we need to include the prefix `INTERNET_CURL_` in the
test name.
> 
> Greg Mann wrote:
>     Thanks!!
> 
> Greg Mann wrote:
>     I ended up switching the tests to use a mixture of tasks with a rootfs and tasks
without - let me know what you think.

SGTM!


- Qian


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72728/#review221754
-----------------------------------------------------------


On Sept. 3, 2020, 3:05 a.m., Greg Mann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72728/
> -----------------------------------------------------------
> 
> (Updated Sept. 3, 2020, 3:05 a.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Qian Zhang.
> 
> 
> Bugs: MESOS-10163
>     https://issues.apache.org/jira/browse/MESOS-10163
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added tests for the 'volume/csi' isolator.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am 1043c7b860372a17dba1e84fe5547388cb6a3b63 
>   src/tests/CMakeLists.txt 6b420d03e85470c16de85c1cbb81ec339142e226 
>   src/tests/cluster.cpp 3c8685565f9de6c314f6be758368f1eff46e2370 
>   src/tests/containerizer/volume_csi_isolator_tests.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/72728/diff/14/
> 
> 
> Testing
> -------
> 
> `sudo bin/mesos-tests.sh --gtest_filter="*VolumeCSIIsolatorTest*"`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message