Re: Review Request 72716: Added implementation of the CSI server.

2020-08-10 Thread Greg Mann
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/72716/ --- (Updated Aug. 11, 2020, 4:22 a.m.) Review request for mesos, Andrei Budnik and

Re: Review Request 72716: Added implementation of the CSI server.

2020-08-10 Thread Qian Zhang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/72716/#review221531 --- Ship it! Ship It! - Qian Zhang On Aug. 11, 2020, 10:30 a.m.,

Re: Review Request 72716: Added implementation of the CSI server.

2020-08-10 Thread Greg Mann
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/72716/ --- (Updated Aug. 11, 2020, 2:30 a.m.) Review request for mesos, Andrei Budnik and

Re: Review Request 72716: Added implementation of the CSI server.

2020-08-10 Thread Greg Mann
> On Aug. 7, 2020, 12:01 p.m., Qian Zhang wrote: > > src/slave/csi_server.cpp > > Lines 336 (patched) > > > > > > Do we need to check if `volume/csi` isolator is enabled? Like: > > ``` > > if (!strings::conta

Re: Review Request 72716: Added implementation of the CSI server.

2020-08-09 Thread Qian Zhang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/72716/#review221517 --- Ship it! Ship It! - Qian Zhang On Aug. 7, 2020, 3 p.m., Greg

Re: Review Request 72716: Added implementation of the CSI server.

2020-08-09 Thread Qian Zhang
> On Aug. 7, 2020, 8:01 p.m., Qian Zhang wrote: > > src/slave/csi_server.cpp > > Lines 200 (patched) > > > > > > Do we need to include `info.type()` in the container prefix? Otherwise > > the container prefix for al

Re: Review Request 72716: Added implementation of the CSI server.

2020-08-09 Thread Qian Zhang
> On July 29, 2020, 11:27 p.m., Qian Zhang wrote: > > src/slave/csi_server.cpp > > Lines 233-235 (patched) > > > > > > I am just curious what would happen if any of the initialization logic > > fail, how will the fa

Re: Review Request 72716: Added implementation of the CSI server.

2020-08-07 Thread Greg Mann
> On July 29, 2020, 3:27 p.m., Qian Zhang wrote: > > src/slave/csi_server.cpp > > Lines 233-235 (patched) > > > > > > I am just curious what would happen if any of the initialization logic > > fail, how will the fai

Re: Review Request 72716: Added implementation of the CSI server.

2020-08-07 Thread Greg Mann
> On Aug. 7, 2020, 12:01 p.m., Qian Zhang wrote: > > src/slave/csi_server.cpp > > Lines 200 (patched) > > > > > > Do we need to include `info.type()` in the container prefix? Otherwise > > the container prefix for a

Re: Review Request 72716: Added implementation of the CSI server.

2020-08-07 Thread Qian Zhang
> On July 29, 2020, 11:27 p.m., Qian Zhang wrote: > > src/slave/csi_server.cpp > > Lines 233-235 (patched) > > > > > > I am just curious what would happen if any of the initialization logic > > fail, how will the fa

Re: Review Request 72716: Added implementation of the CSI server.

2020-08-07 Thread Qian Zhang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/72716/#review221499 --- src/slave/csi_server.cpp Lines 99-103 (patched)

Re: Review Request 72716: Added implementation of the CSI server.

2020-08-07 Thread Greg Mann
> On Aug. 4, 2020, 3:07 p.m., Qian Zhang wrote: > > src/slave/csi_server.cpp > > Lines 278 (patched) > > > > > > Should we do `"return plugins.at(name).volumeManager->recover();"` > > instead? Yep, thanks! - Greg

Re: Review Request 72716: Added implementation of the CSI server.

2020-08-07 Thread Greg Mann
> On Aug. 3, 2020, 8:46 p.m., Andrei Budnik wrote: > > src/slave/csi_server.cpp > > Lines 294 (patched) > > > > > > Could you please add a comment clarifying why we overwrite (re-assign) > > `startupComplete` here?

Re: Review Request 72716: Added implementation of the CSI server.

2020-08-07 Thread Greg Mann
> On July 29, 2020, 3:27 p.m., Qian Zhang wrote: > > src/slave/csi_server.cpp > > Lines 233-235 (patched) > > > > > > I am just curious what would happen if any of the initialization logic > > fail, how will the fai

Re: Review Request 72716: Added implementation of the CSI server.

2020-08-07 Thread Greg Mann
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/72716/ --- (Updated Aug. 7, 2020, 7 a.m.) Review request for mesos, Andrei Budnik and Qian

Re: Review Request 72716: Added implementation of the CSI server.

2020-08-06 Thread Greg Mann
> On July 29, 2020, 3:27 p.m., Qian Zhang wrote: > > src/slave/csi_server.cpp > > Lines 233-235 (patched) > > > > > > I am just curious what would happen if any of the initialization logic > > fail, how will the fai

Re: Review Request 72716: Added implementation of the CSI server.

2020-08-06 Thread Greg Mann
> On July 29, 2020, 3:27 p.m., Qian Zhang wrote: > > src/slave/csi_server.cpp > > Lines 244-245 (patched) > > > > > > Do we have to use `started` and `initializationCallbacks`? Can we do > > the similar with > > ht

Re: Review Request 72716: Added implementation of the CSI server.

2020-08-06 Thread Greg Mann
> On July 29, 2020, 3:27 p.m., Qian Zhang wrote: > > src/slave/csi_server.cpp > > Lines 233-235 (patched) > > > > > > I am just curious what would happen if any of the initialization logic > > fail, how will the fai

Re: Review Request 72716: Added implementation of the CSI server.

2020-08-04 Thread Qian Zhang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/72716/#review221466 --- src/slave/csi_server.cpp Lines 278 (patched)

Re: Review Request 72716: Added implementation of the CSI server.

2020-08-04 Thread Greg Mann
> On July 29, 2020, 3:27 p.m., Qian Zhang wrote: > > src/slave/csi_server.cpp > > Lines 244-245 (patched) > > > > > > Do we have to use `started` and `initializationCallbacks`? Can we do > > the similar with > > ht

Re: Review Request 72716: Added implementation of the CSI server.

2020-08-04 Thread Qian Zhang
> On July 29, 2020, 11:27 p.m., Qian Zhang wrote: > > src/slave/csi_server.cpp > > Lines 233-235 (patched) > > > > > > I am just curious what would happen if any of the initialization logic > > fail, how will the fa

Re: Review Request 72716: Added implementation of the CSI server.

2020-08-03 Thread Andrei Budnik
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/72716/#review221452 --- src/slave/csi_server.cpp Lines 294 (patched)

Re: Review Request 72716: Added implementation of the CSI server.

2020-08-03 Thread Greg Mann
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/72716/ --- (Updated Aug. 3, 2020, 6:58 p.m.) Review request for mesos, Andrei Budnik and Q

Re: Review Request 72716: Added implementation of the CSI server.

2020-08-03 Thread Greg Mann
> On July 29, 2020, 3:27 p.m., Qian Zhang wrote: > > src/slave/csi_server.cpp > > Lines 194 (patched) > > > > > > I think we should set this based on the CSI services in the related > > `CSIPluginInfo` rather than h

Re: Review Request 72716: Added implementation of the CSI server.

2020-08-02 Thread Qian Zhang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/72716/#review221447 --- src/slave/csi_server.cpp Lines 114-115 (patched)

Re: Review Request 72716: Added implementation of the CSI server.

2020-08-02 Thread Qian Zhang
> On July 29, 2020, 11:27 p.m., Qian Zhang wrote: > > src/slave/csi_server.cpp > > Lines 194 (patched) > > > > > > I think we should set this based on the CSI services in the related > > `CSIPluginInfo` rather than

Re: Review Request 72716: Added implementation of the CSI server.

2020-07-31 Thread Greg Mann
> On July 29, 2020, 3:27 p.m., Qian Zhang wrote: > > src/slave/csi_server.cpp > > Lines 123 (patched) > > > > > > So we use a single runtime for all the plugins managed by CSI server, > > will that cause any problem

Re: Review Request 72716: Added implementation of the CSI server.

2020-07-31 Thread Greg Mann
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/72716/ --- (Updated July 31, 2020, 7 p.m.) Review request for mesos, Andrei Budnik and Qia

Re: Review Request 72716: Added implementation of the CSI server.

2020-07-29 Thread Qian Zhang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/72716/#review221405 --- src/Makefile.am Lines 1300-1301 (patched)

Re: Review Request 72716: Added implementation of the CSI server.

2020-07-28 Thread Mesos Reviewbot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/72716/#review221402 --- Patch looks great! Reviews applied: [72660, 72661, 72672, 72681,

Re: Review Request 72716: Added implementation of the CSI server.

2020-07-28 Thread Greg Mann
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/72716/ --- (Updated July 28, 2020, 9:50 p.m.) Review request for mesos, Andrei Budnik and

Review Request 72716: Added implementation of the CSI server.

2020-07-28 Thread Greg Mann
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/72716/ --- Review request for mesos, Andrei Budnik and Qian Zhang. Bugs: MESOS-10163 h