mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Greg Mann <g...@mesosphere.io>
Subject Re: Review Request 72728: Added tests for the 'volume/csi' isolator.
Date Wed, 02 Sep 2020 15:44:24 GMT


> On Sept. 2, 2020, 1: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`?

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?


> On Sept. 2, 2020, 1: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.

Thanks!!


- Greg


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


On Sept. 2, 2020, 5:57 a.m., Greg Mann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72728/
> -----------------------------------------------------------
> 
> (Updated Sept. 2, 2020, 5:57 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/13/
> 
> 
> Testing
> -------
> 
> `sudo bin/mesos-tests.sh --gtest_filter="*VolumeCSIIsolatorTest*"`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>


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