Re: Review Request 70244: Added new operation reconciliation tests.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70244/#review213872 --- Patch looks great! Reviews applied: [70200, 70208, 70221, 70242,

Re: Review Request 70215: Implemented volume attachment and publishment for v0 `VolumeManager`.

> On March 19, 2019, 5:06 p.m., Benjamin Bannier wrote: > > src/csi/v0_volume_manager.cpp > > Line 120 (original), 142-144 (patched) > > > > > > Shouldn't we evaluate this when we actually send the operation in > >

Re: Review Request 70244: Added new operation reconciliation tests.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70244/#review213870 --- FAIL: Some of the unit tests failed. Please check the relevant

Re: Review Request 70248: Adapted the unversioned `VolumeCapability`.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70248/#review213866 --- Patch looks great! Reviews applied: [70245, 70168, 70169, 70213,

Re: Review Request 70200: Changed operation reconciliation to send updates on the event stream.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70200/ --- (Updated March 20, 2019, 11:24 p.m.) Review request for mesos, Benjamin

Re: Review Request 70200: Changed operation reconciliation to send updates on the event stream.

> On March 20, 2019, 8:55 p.m., Joseph Wu wrote: > > include/mesos/scheduler/scheduler.proto > > Lines 220-228 (original), 223-231 (patched) > > > > > > Is there some reason why the un-versioned protobufs do not

Re: Review Request 70208: Added test for reconciliation of multiple operations.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70208/ --- (Updated March 20, 2019, 11:23 p.m.) Review request for mesos, Gastón Kleiman,

Re: Review Request 70208: Added test for reconciliation of multiple operations.

> On March 20, 2019, 10:07 p.m., Joseph Wu wrote: > > src/tests/operation_reconciliation_tests.cpp > > Lines 233-234 (patched) > > > > > > This is un-used now. Thanks Joseph!! - Greg

Re: Review Request 70233: Allowed for optionally unbundled ZooKeeper for CMake builds.

> On March 19, 2019, 11:07 p.m., Joseph Wu wrote: > > 3rdparty/cmake/FindZOOKEEPER.cmake > > Lines 50 (patched) > > > > > > I wonder if it is worth adding a note here, restating bits of the blurb > > here: > >

Re: Review Request 70248: Adapted the unversioned `VolumeCapability`.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70248/#review213862 --- PASS: Mesos patch 70248 was successfully built and tested.

Re: Review Request 70215: Implemented volume attachment and publishment for v0 `VolumeManager`.

> On March 19, 2019, 5:06 p.m., Benjamin Bannier wrote: > > src/csi/v0_volume_manager.cpp > > Line 120 (original), 142-144 (patched) > > > > > > Shouldn't we evaluate this when we actually send the operation in > >

Re: Review Request 70217: Implemented the remain functionalities for v0 `VolumeManager`.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70217/#review213859 --- src/csi/v0_volume_manager.cpp Lines 322 (patched)

Re: Review Request 70214: Added skeleton code for v0 `VolumeManager`.

> On March 19, 2019, 2:57 p.m., Benjamin Bannier wrote: > > src/csi/v0_volume_manager.hpp > > Lines 100 (patched) > > > > > > Does it make sense to move this into the process? It would lead to > > simpler dispatch

Re: Review Request 70208: Added test for reconciliation of multiple operations.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70208/#review213858 --- Fix it, then Ship it!

Re: Review Request 70244: Added new operation reconciliation tests.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70244/#review213854 --- Patch looks great! Reviews applied: [70200, 70208, 70221, 70242,

Re: Review Request 70248: Adapted the unversioned `VolumeCapability`.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70248/ --- (Updated March 20, 2019, 9:40 p.m.) Review request for mesos, Benjamin

Review Request 70258: Added a unit test for CSI `evolve` and `devolve` helpers.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70258/ --- Review request for mesos, Benjamin Bannier and Jan Schlicht. Bugs: MESOS-9625

Re: Review Request 70247: Added an "unversioned" `VolumeCapability`.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70247/ --- (Updated March 20, 2019, 9:36 p.m.) Review request for mesos, Benjamin

Re: Review Request 70200: Changed operation reconciliation to send updates on the event stream.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70200/#review213851 --- Fix it, then Ship it! Minor question, but other LGTM.

Re: Review Request 70244: Added new operation reconciliation tests.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70244/#review213852 --- FAIL: Some of the unit tests failed. Please check the relevant

Re: Review Request 70244: Added new operation reconciliation tests.

> On March 20, 2019, 7:14 p.m., Greg Mann wrote: > > src/tests/operation_reconciliation_tests.cpp > > Lines 1894 (patched) > > > > > > Whoops, I intended this to be `OPERATION_FINISHED`. There seems to be > > an

Re: Review Request 70244: Added new operation reconciliation tests.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70244/ --- (Updated March 20, 2019, 7:47 p.m.) Review request for mesos, Benjamin

Re: Review Request 70244: Added new operation reconciliation tests.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70244/#review213845 --- src/tests/operation_reconciliation_tests.cpp Lines 1894

Re: Review Request 70243: Improved operation reconciliation for unsubscribed resource providers.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70243/ --- (Updated March 20, 2019, 7:08 p.m.) Review request for mesos, Benjamin

Re: Review Request 70243: Improved operation reconciliation in the agent and RP manager.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70243/ --- (Updated March 20, 2019, 7:07 p.m.) Review request for mesos, Benjamin

Re: Review Request 70247: Added an "unversioned" `VolumeCapability`.

> On March 20, 2019, 2:09 p.m., Benjamin Bannier wrote: > > include/mesos/csi/compat.proto > > Lines 1 (patched) > > > > > > I don't think these definitions need to be public as they never leave > > the agent. Let's

Re: Review Request 70216: Implemented the recovery logic for v0 `VolumeManager`.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70216/#review213840 --- Let's squash the corresponding changes from

Re: Review Request 70248: Adapted the unversioned `VolumeCapability`.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70248/#review213838 --- src/csi/utils.hpp Lines 182-183 (patched)

Re: Review Request 70247: Added an "unversioned" `VolumeCapability`.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70247/#review213837 --- This looks reasonable to me, but I haven't seen yet that it can

Re: Review Request 70169: Refactored SLRP to use `ServiceManager`.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70169/#review213834 --- Fix it, then Ship it! Looking good. Let's squash this with

Re: Review Request 70168: Added `ServiceManager` to manage CSI plugin container lifecycles.

> On March 19, 2019, 2:12 p.m., Benjamin Bannier wrote: > > src/csi/service_manager.cpp > > Lines 184-185 (patched) > > > > > > In Mesos we typically expand types unless they are already mentioned > > somewhere in

Re: Review Request 70168: Added `ServiceManager` to manage CSI plugin container lifecycles.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70168/#review213835 --- Fix it, then Ship it! src/CMakeLists.txt Line 1 (original), 1

Re: Review Request 70250: Made hashmap::containsValue consistent with stout naming convention.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70250/#review213833 --- PASS: Mesos patch 70250 was successfully built and tested.

Re: Review Request 70250: Made hashmap::containsValue consistent with stout naming convention.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70250/#review213832 --- Patch looks great! Reviews applied: [70249, 70250] Passed

Re: Review Request 70245: Moved CSI plugin metrics out from SLRP metrics.

> On March 20, 2019, 6:50 a.m., Chun-Hung Hsiao wrote: > > src/resource_provider/storage/provider_process.hpp > > Line 402 (original), 403 (patched) > > > > > > From r/70169: Benjamin: Composition instead of

Re: Review Request 70245: Moved CSI plugin metrics out from SLRP metrics.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70245/#review213830 --- Ship it! Ship It! - Benjamin Bannier On March 20, 2019,

Re: Review Request 70249: Made hashmap::containsValue consistent with stout naming convention.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70249/#review213828 --- FAIL: Some of the unit tests failed. Please check the relevant

Re: Review Request 70249: Made hashmap::containsValue consistent with stout naming convention.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70249/#review213826 --- Ship it! I agree with the change; `hashmap` extends the

Re: Review Request 70250: Made hashmap::containsValue consistent with stout naming convention.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70250/#review213827 --- Ship it! - Benno Evers On March 20, 2019, 9:56 a.m.,

Review Request 70250: Made hashmap::containsValue consistent with stout naming convention.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70250/ --- Review request for mesos, Benjamin Hindman and Benno Evers. Repository: mesos

Review Request 70249: Made hashmap::containsValue consistent with stout naming convention.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70249/ --- Review request for mesos, Benjamin Hindman and Benno Evers. Repository: mesos

Re: Review Request 70248: Adapted the unversioned `VolumeCapability`.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70248/#review213825 --- Patch looks great! Reviews applied: [70245, 70168, 70169, 70213,

Re: Review Request 70248: Adapted the unversioned `VolumeCapability`.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70248/#review213824 --- PASS: Mesos patch 70248 was successfully built and tested.

Re: Review Request 70245: Moved CSI plugin metrics out from SLRP metrics.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70245/#review213823 --- PASS: Mesos patch 70245 was successfully built and tested.

Re: Review Request 70213: Added the `VolumeManager` interface to manage CSI volumes.

> On March 19, 2019, 2:42 p.m., Benjamin Bannier wrote: > > src/csi/volume_manager.hpp > > Lines 74-75 (patched) > > > > > > Do we have a chance here to translate from CSI semantics to our own > > semantics instead

Review Request 70248: Adapted the unversioned `VolumeCapability`.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70248/ --- Review request for mesos, Benjamin Bannier, James DeFelice, and Jan Schlicht.

Re: Review Request 70244: Added new operation reconciliation tests.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70244/#review213821 --- Patch looks great! Reviews applied: [70200, 70208, 70221, 70242,

Review Request 70247: Added an "unversioned" `VolumeCapability`.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70247/ --- Review request for mesos, Benjamin Bannier, James DeFelice, Jie Yu, and Jan

Re: Review Request 70169: Refactored SLRP to use `ServiceManager`.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70169/ --- (Updated March 20, 2019, 6:03 a.m.) Review request for mesos, Benjamin