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