----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/72690/#review221272 -----------------------------------------------------------
src/CMakeLists.txt Lines 338 (patched) <https://reviews.apache.org/r/72690/#comment310145> Why put the csi isolator implementation in a separate subdirectory, while the other volume isolators are in the same directory? src/slave/containerizer/mesos/isolators/volume/csi/isolator.hpp Lines 31 (patched) <https://reviews.apache.org/r/72690/#comment310146> No `using` directives in headers. src/slave/containerizer/mesos/isolators/volume/csi/isolator.hpp Lines 43 (patched) <https://reviews.apache.org/r/72690/#comment310147> I don't think that this isolator needs a secret generator; just the secret resolver is sufficient. src/slave/containerizer/mesos/isolators/volume/csi/isolator.hpp Lines 76 (patched) <https://reviews.apache.org/r/72690/#comment310148> Nit: I would suggest the slightly more concise "Stores the CSI plugin configurations, keyed by plugin type." src/slave/containerizer/mesos/isolators/volume/csi/isolator.cpp Lines 91 (patched) <https://reviews.apache.org/r/72690/#comment310149> Nit: is the namespace qualification here necessary? `::Error` src/slave/containerizer/mesos/isolators/volume/csi/isolator.cpp Lines 103 (patched) <https://reviews.apache.org/r/72690/#comment310154> Nit: s/is/are/ src/slave/containerizer/mesos/isolators/volume/csi/isolator.cpp Lines 104 (patched) <https://reviews.apache.org/r/72690/#comment310155> Missing a closing quote after the path. src/slave/containerizer/mesos/isolators/volume/csi/isolator.cpp Lines 105 (patched) <https://reviews.apache.org/r/72690/#comment310156> Quotes around the type maybe? src/slave/containerizer/mesos/isolators/volume/csi/isolator.cpp Lines 110-112 (patched) <https://reviews.apache.org/r/72690/#comment310150> Should we return an error here if no valid plugin configurations were found, in other words if `csiPluginConfigs.empty() == true`? - Greg Mann On July 20, 2020, 8 a.m., Qian Zhang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/72690/ > ----------------------------------------------------------- > > (Updated July 20, 2020, 8 a.m.) > > > Review request for mesos, Andrei Budnik and Greg Mann. > > > Bugs: MESOS-10152 > https://issues.apache.org/jira/browse/MESOS-10152 > > > Repository: mesos > > > Description > ------- > > Implemented the framework and `create` method of "volume/csi" isolator. > > > Diffs > ----- > > src/CMakeLists.txt bcb5128f0e61af0d541502e4ed833da0487b7792 > src/Makefile.am a89919dd7d5ccbc4c5fa79d9a83616608f84ef49 > src/slave/containerizer/mesos/isolators/volume/csi/isolator.hpp > PRE-CREATION > src/slave/containerizer/mesos/isolators/volume/csi/isolator.cpp > PRE-CREATION > > > Diff: https://reviews.apache.org/r/72690/diff/2/ > > > Testing > ------- > > > Thanks, > > Qian Zhang > >
