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




src/Makefile.am
Lines 2875 (patched)
<https://reviews.apache.org/r/72728/#comment310831>

    The indent seems not correct.



src/tests/containerizer/volume_csi_isolator_tests.cpp
Lines 21-22 (patched)
<https://reviews.apache.org/r/72728/#comment310832>

    I think we should swap these two lines and add a newline between them.



src/tests/containerizer/volume_csi_isolator_tests.cpp
Lines 95 (patched)
<https://reviews.apache.org/r/72728/#comment310833>

    This one seems unused.



src/tests/containerizer/volume_csi_isolator_tests.cpp
Lines 132-133 (patched)
<https://reviews.apache.org/r/72728/#comment310834>

    A newline between, or we could just swap these two lines.



src/tests/containerizer/volume_csi_isolator_tests.cpp
Lines 238 (patched)
<https://reviews.apache.org/r/72728/#comment310835>

    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`?



src/tests/containerizer/volume_csi_isolator_tests.cpp
Lines 794 (patched)
<https://reviews.apache.org/r/72728/#comment310836>

    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.


- Qian Zhang


On Sept. 2, 2020, 1:57 p.m., Greg Mann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72728/
> -----------------------------------------------------------
> 
> (Updated Sept. 2, 2020, 1:57 p.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
> 
>