You are viewing a plain text version of this content. The canonical link for it is here.

Posted to reviews@mesos.apache.org by Chun-Hung Hsiao <ch...@mesosphere.io> on 2017/10/17 00:53:03 UTC

Re: Review Request 63022: Imported resources from CSI plugins in storage local resource provider.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63022/
-----------------------------------------------------------

(Updated Oct. 17, 2017, 12:53 a.m.)


Review request for mesos, James DeFelice, Jie Yu, Joseph Wu, and Jan Schlicht.


Summary (updated)
-----------------

Imported resources from CSI plugins in storage local resource provider.


Bugs: MESOS-8101
    https://issues.apache.org/jira/browse/MESOS-8101


Repository: mesos


Description (updated)
-------

The following lists the steps to import resources from a CSI plugin:
1. Launch the node plugin
  1.1 GetSupportedVersions
  1.2 GetPluginInfo
  1.3 ProbeNode
  1.4 GetNodeCapabilities
2. Launch the controller plugin
  2.1 GetSuportedVersions
  2.2 GetPluginInfo
  2.3 GetControllerCapabilities
3. GetCapacity
4. ListVolumes
5. Report to the resource provider through UPDATE_TOTAL_RESOURCES


Diffs (updated)
-----

  src/resource_provider/storage/provider.cpp 46224997430ac0c568904d80014166a6f059907f 


Diff: https://reviews.apache.org/r/63022/diff/2/

Changes: https://reviews.apache.org/r/63022/diff/1-2/


Testing
-------


Thanks,

Chun-Hung Hsiao

Re: Review Request 63022: Imported and reconcile resources from CSI plugins.

Posted by Jie Yu <yu...@gmail.com>.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63022/#review192943
-----------------------------------------------------------


Ship it!




Ship It!

- Jie Yu


On Dec. 5, 2017, 5:57 a.m., Chun-Hung Hsiao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63022/
> -----------------------------------------------------------
> 
> (Updated Dec. 5, 2017, 5:57 a.m.)
> 
> 
> Review request for mesos, James DeFelice, Jie Yu, Joseph Wu, and Jan Schlicht.
> 
> 
> Bugs: MESOS-8101
>     https://issues.apache.org/jira/browse/MESOS-8101
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The following lists the steps to reconcile resources:
> 
> 1. Import resources from the CSI plugin:
>   a. Get preprovisioned volumes:
>        ListVolumes
>        ValidateVolumeCapabilities for each volume
>   b. GetCapacity for each profile to get RAW resources
> 2. For each resource in the checkpointed resources:
>   a. Strip away metadata (reservation, persistence, etc)
>   b. Additional resources after subtracting the stripped resources
>      from the imported resources are new resources.
> 3. Report the reconciled resources through UPDATE_STATE
> 
> 
> Diffs
> -----
> 
>   src/resource_provider/storage/provider.cpp d35b0d02992e3730ca47906b34c21e1ba9c653e7 
> 
> 
> Diff: https://reviews.apache.org/r/63022/diff/16/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 63022: Imported and reconcile resources from CSI plugins.

Posted by Jie Yu <yu...@gmail.com>.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63022/#review193038
-----------------------------------------------------------




src/resource_provider/storage/provider.cpp
Lines 1286-1293 (patched)
<https://reviews.apache.org/r/63022/#comment271532>

    We can use `protobuf::createOfferOperation` here.


- Jie Yu


On Dec. 6, 2017, 7:50 p.m., Chun-Hung Hsiao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63022/
> -----------------------------------------------------------
> 
> (Updated Dec. 6, 2017, 7:50 p.m.)
> 
> 
> Review request for mesos, James DeFelice, Jie Yu, Joseph Wu, and Jan Schlicht.
> 
> 
> Bugs: MESOS-8101
>     https://issues.apache.org/jira/browse/MESOS-8101
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The following lists the steps to reconcile resources:
> 
> 1. Import resources from the CSI plugin:
>   a. Get preprovisioned volumes:
>        ListVolumes
>        ValidateVolumeCapabilities for each volume
>   b. GetCapacity for each profile to get RAW resources
> 2. For each resource in the checkpointed resources:
>   a. Strip away metadata (reservation, persistence, etc)
>   b. Additional resources after subtracting the stripped resources
>      from the imported resources are new resources.
> 3. Report the reconciled resources through UPDATE_STATE
> 
> 
> Diffs
> -----
> 
>   src/resource_provider/storage/provider.cpp d35b0d02992e3730ca47906b34c21e1ba9c653e7 
> 
> 
> Diff: https://reviews.apache.org/r/63022/diff/17/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 63022: Imported and reconcile resources from CSI plugins.

Posted by Chun-Hung Hsiao <ch...@mesosphere.io>.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63022/
-----------------------------------------------------------

(Updated Dec. 6, 2017, 7:50 p.m.)


Review request for mesos, James DeFelice, Jie Yu, Joseph Wu, and Jan Schlicht.


Changes
-------

Added `sendResourceProviderStateUpdate` and sending pending operations.


Bugs: MESOS-8101
    https://issues.apache.org/jira/browse/MESOS-8101


Repository: mesos


Description
-------

The following lists the steps to reconcile resources:

1. Import resources from the CSI plugin:
  a. Get preprovisioned volumes:
       ListVolumes
       ValidateVolumeCapabilities for each volume
  b. GetCapacity for each profile to get RAW resources
2. For each resource in the checkpointed resources:
  a. Strip away metadata (reservation, persistence, etc)
  b. Additional resources after subtracting the stripped resources
     from the imported resources are new resources.
3. Report the reconciled resources through UPDATE_STATE


Diffs (updated)
-----

  src/resource_provider/storage/provider.cpp d35b0d02992e3730ca47906b34c21e1ba9c653e7 


Diff: https://reviews.apache.org/r/63022/diff/17/

Changes: https://reviews.apache.org/r/63022/diff/16-17/


Testing
-------


Thanks,

Chun-Hung Hsiao

Re: Review Request 63022: Imported and reconcile resources from CSI plugins.

Posted by Jie Yu <yu...@gmail.com>.


> On Dec. 5, 2017, 2:08 p.m., James DeFelice wrote:
> > src/resource_provider/storage/provider.cpp
> > Lines 1203 (patched)
> > <https://reviews.apache.org/r/63022/diff/15/?file=1908557#file1908557line1211>
> >
> >     some plugins might not implement create/delete, but DO implement list. such a plugin might only support "block"- (or only support "mount"-) type volumes. in such cases, it could be useful to report those resources as BLOCK (or MOUNT) volumes instead of RAW. I don't *think* it's a blocker for MVP, but we've had a couple of conversations recently about exposing low-level primitives to specialized storage frameworks, and that it might be useful to report block devices as BLOCK disk resources, by default.

The framework should decide whether to convert the RAW into either MOUNT or BLOCK based on how it intends to use the volume.

This does raise one valid case for default profile. Currently, the default profile will call `ValidateVolumeCapabilities` using `MountVolume` capability. It does not test for if the volume supports BlockVolume or not. That mean for pre-existing volume, CreateVolume/CreateBlock handler will need to call `ValidateVolumeCapabilities` before it converts the resources to either MOUNT/PATH, or BLOCK. That makes me wonder if we need to call `ValidateVolumeCapabilities` at all for volumes that does not have a profile.


- Jie


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63022/#review192864
-----------------------------------------------------------


On Dec. 5, 2017, 5:57 a.m., Chun-Hung Hsiao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63022/
> -----------------------------------------------------------
> 
> (Updated Dec. 5, 2017, 5:57 a.m.)
> 
> 
> Review request for mesos, James DeFelice, Jie Yu, Joseph Wu, and Jan Schlicht.
> 
> 
> Bugs: MESOS-8101
>     https://issues.apache.org/jira/browse/MESOS-8101
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The following lists the steps to reconcile resources:
> 
> 1. Import resources from the CSI plugin:
>   a. Get preprovisioned volumes:
>        ListVolumes
>        ValidateVolumeCapabilities for each volume
>   b. GetCapacity for each profile to get RAW resources
> 2. For each resource in the checkpointed resources:
>   a. Strip away metadata (reservation, persistence, etc)
>   b. Additional resources after subtracting the stripped resources
>      from the imported resources are new resources.
> 3. Report the reconciled resources through UPDATE_STATE
> 
> 
> Diffs
> -----
> 
>   src/resource_provider/storage/provider.cpp d35b0d02992e3730ca47906b34c21e1ba9c653e7 
> 
> 
> Diff: https://reviews.apache.org/r/63022/diff/16/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 63022: Imported and reconcile resources from CSI plugins.

Posted by James DeFelice <ja...@gmail.com>.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63022/#review192864
-----------------------------------------------------------




src/resource_provider/storage/provider.cpp
Lines 1203 (patched)
<https://reviews.apache.org/r/63022/#comment271190>

    some plugins might not implement create/delete, but DO implement list. such a plugin might only support "block"- (or only support "mount"-) type volumes. in such cases, it could be useful to report those resources as BLOCK (or MOUNT) volumes instead of RAW. I don't *think* it's a blocker for MVP, but we've had a couple of conversations recently about exposing low-level primitives to specialized storage frameworks, and that it might be useful to report block devices as BLOCK disk resources, by default.


- James DeFelice


On Dec. 5, 2017, 5:57 a.m., Chun-Hung Hsiao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63022/
> -----------------------------------------------------------
> 
> (Updated Dec. 5, 2017, 5:57 a.m.)
> 
> 
> Review request for mesos, James DeFelice, Jie Yu, Joseph Wu, and Jan Schlicht.
> 
> 
> Bugs: MESOS-8101
>     https://issues.apache.org/jira/browse/MESOS-8101
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The following lists the steps to reconcile resources:
> 
> 1. Import resources from the CSI plugin:
>   a. Get preprovisioned volumes:
>        ListVolumes
>        ValidateVolumeCapabilities for each volume
>   b. GetCapacity for each profile to get RAW resources
> 2. For each resource in the checkpointed resources:
>   a. Strip away metadata (reservation, persistence, etc)
>   b. Additional resources after subtracting the stripped resources
>      from the imported resources are new resources.
> 3. Report the reconciled resources through UPDATE_STATE
> 
> 
> Diffs
> -----
> 
>   src/resource_provider/storage/provider.cpp d35b0d02992e3730ca47906b34c21e1ba9c653e7 
> 
> 
> Diff: https://reviews.apache.org/r/63022/diff/16/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 63022: Imported and reconcile resources from CSI plugins.

Posted by Chun-Hung Hsiao <ch...@mesosphere.io>.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63022/
-----------------------------------------------------------

(Updated Dec. 5, 2017, 5:57 a.m.)


Review request for mesos, James DeFelice, Jie Yu, Joseph Wu, and Jan Schlicht.


Changes
-------

Addressed Jie's comments.


Bugs: MESOS-8101
    https://issues.apache.org/jira/browse/MESOS-8101


Repository: mesos


Description (updated)
-------

The following lists the steps to reconcile resources:

1. Import resources from the CSI plugin:
  a. Get preprovisioned volumes:
       ListVolumes
       ValidateVolumeCapabilities for each volume
  b. GetCapacity for each profile to get RAW resources
2. For each resource in the checkpointed resources:
  a. Strip away metadata (reservation, persistence, etc)
  b. Additional resources after subtracting the stripped resources
     from the imported resources are new resources.
3. Report the reconciled resources through UPDATE_STATE


Diffs (updated)
-----

  src/resource_provider/storage/provider.cpp d35b0d02992e3730ca47906b34c21e1ba9c653e7 


Diff: https://reviews.apache.org/r/63022/diff/16/

Changes: https://reviews.apache.org/r/63022/diff/15-16/


Testing
-------


Thanks,

Chun-Hung Hsiao

Re: Review Request 63022: Imported and reconcile resources from CSI plugins.

Posted by Jie Yu <yu...@gmail.com>.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63022/#review192806
-----------------------------------------------------------




src/resource_provider/storage/provider.cpp
Lines 439 (patched)
<https://reviews.apache.org/r/63022/#comment271099>

    Please don't use `*` for default profile.



src/resource_provider/storage/provider.cpp
Lines 654 (patched)
<https://reviews.apache.org/r/63022/#comment271097>

    typo



src/resource_provider/storage/provider.cpp
Lines 670-714 (patched)
<https://reviews.apache.org/r/63022/#comment271098>

    Can you combine these two loops. Let's not worried about the case where resources disappear. We need to handle that in the future using maintainance primitives maybe.



src/resource_provider/storage/provider.cpp
Lines 766 (patched)
<https://reviews.apache.org/r/63022/#comment271096>

    Can you add a TODO to do the reconciliation early.



src/resource_provider/storage/provider.cpp
Lines 1207 (patched)
<https://reviews.apache.org/r/63022/#comment271095>

    Let's not use `*` to indicate a default profile. Please just not set the profile field.


- Jie Yu


On Dec. 5, 2017, 2:18 a.m., Chun-Hung Hsiao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63022/
> -----------------------------------------------------------
> 
> (Updated Dec. 5, 2017, 2:18 a.m.)
> 
> 
> Review request for mesos, James DeFelice, Jie Yu, Joseph Wu, and Jan Schlicht.
> 
> 
> Bugs: MESOS-8101
>     https://issues.apache.org/jira/browse/MESOS-8101
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The following lists the steps to reconcile resources:
> 
> 1. Import resources from the CSI plugin:
>   a. Get preprovisioned volumes:
>        ListVolumes
>        ValidateVolumeCapabilities for each volume
>   b. GetCapacity for each profile to get RAW resources
> 2. For each resource in the checkpointed resources:
>   a. Strip away metadata for offer operations
>   b. Keep the persistent volumes and all resources contained in the
>      imported resources
>   c. Additional imported resources are new resources
> 3. Report the reconciled resources through UPDATE_STATE
> 
> 
> Diffs
> -----
> 
>   src/resource_provider/storage/provider.cpp d35b0d02992e3730ca47906b34c21e1ba9c653e7 
> 
> 
> Diff: https://reviews.apache.org/r/63022/diff/15/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 63022: Imported and reconcile resources from CSI plugins.

Posted by Chun-Hung Hsiao <ch...@mesosphere.io>.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63022/
-----------------------------------------------------------

(Updated Dec. 5, 2017, 2:18 a.m.)


Review request for mesos, James DeFelice, Jie Yu, Joseph Wu, and Jan Schlicht.


Bugs: MESOS-8101
    https://issues.apache.org/jira/browse/MESOS-8101


Repository: mesos


Description
-------

The following lists the steps to reconcile resources:

1. Import resources from the CSI plugin:
  a. Get preprovisioned volumes:
       ListVolumes
       ValidateVolumeCapabilities for each volume
  b. GetCapacity for each profile to get RAW resources
2. For each resource in the checkpointed resources:
  a. Strip away metadata for offer operations
  b. Keep the persistent volumes and all resources contained in the
     imported resources
  c. Additional imported resources are new resources
3. Report the reconciled resources through UPDATE_STATE


Diffs (updated)
-----

  src/resource_provider/storage/provider.cpp d35b0d02992e3730ca47906b34c21e1ba9c653e7 


Diff: https://reviews.apache.org/r/63022/diff/15/

Changes: https://reviews.apache.org/r/63022/diff/14-15/


Testing
-------


Thanks,

Chun-Hung Hsiao

Re: Review Request 63022: Imported and reconcile resources from CSI plugins.

Posted by Chun-Hung Hsiao <ch...@mesosphere.io>.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63022/
-----------------------------------------------------------

(Updated Dec. 1, 2017, 1:52 a.m.)


Review request for mesos, James DeFelice, Jie Yu, Joseph Wu, and Jan Schlicht.


Changes
-------

Moved `prepareControllerService` and `prepareNodeService` to another patch. Bug fix.


Summary (updated)
-----------------

Imported and reconcile resources from CSI plugins.


Bugs: MESOS-8101
    https://issues.apache.org/jira/browse/MESOS-8101


Repository: mesos


Description (updated)
-------

The following lists the steps to reconcile resources:

1. Import resources from the CSI plugin:
  a. Get preprovisioned volumes:
       ListVolumes
       ValidateVolumeCapabilities for each volume
  b. GetCapacity for each profile to get RAW resources
2. For each resource in the checkpointed resources:
  a. Strip away metadata for offer operations
  b. Keep the persistent volumes and all resources contained in the
     imported resources
  c. Additional imported resources are new resources
3. Report the reconciled resources through UPDATE_STATE


Diffs (updated)
-----

  src/resource_provider/storage/provider.cpp f586afc256fbcb2f2bf2451ffb7e7ba1d59cfa78 


Diff: https://reviews.apache.org/r/63022/diff/14/

Changes: https://reviews.apache.org/r/63022/diff/13-14/


Testing
-------


Thanks,

Chun-Hung Hsiao