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