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/12/05 02:19:55 UTC
Re: Review Request 63388: Handling offer operations in storage local
resource provider.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63388/
-----------------------------------------------------------
(Updated Dec. 5, 2017, 2:19 a.m.)
Review request for mesos, James DeFelice, Jie Yu, Joseph Wu, and Jan Schlicht.
Bugs: MESOS-8108
https://issues.apache.org/jira/browse/MESOS-8108
Repository: mesos
Description
-------
For legacy operations, we just call Resources::apply(). New operations
CREATE_VOLUME, DESTROY_VOLUME, CREATE_BLOCK, DESTROY_BLOCK are
implemented through CSI. Specially, DESTROY_* requires unpublishing
the resources first.
Diffs (updated)
-----
src/resource_provider/storage/provider.cpp d35b0d02992e3730ca47906b34c21e1ba9c653e7
Diff: https://reviews.apache.org/r/63388/diff/4/
Changes: https://reviews.apache.org/r/63388/diff/3-4/
Testing
-------
make
Thanks,
Chun-Hung Hsiao
Re: Review Request 63388: Handling offer operations in storage local
resource provider.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63388/#review192955
-----------------------------------------------------------
src/resource_provider/storage/provider.cpp
Lines 1901-1931 (patched)
<https://reviews.apache.org/r/63388/#comment271347>
See my other comments. Let's move the resource generation/conversion part into a separate helper and make `createVolume` a pure CSI operation.
src/resource_provider/storage/provider.cpp
Lines 1947-1949 (patched)
<https://reviews.apache.org/r/63388/#comment271345>
Hum, i like to keep `deleteVolume` helper simple to just CSI related operations (like you did for publich/unpublish related operations).
The generation of the resources conversion should be in `applyOfferOperation`.
Ditto for `createVolume`
src/resource_provider/storage/provider.cpp
Lines 1971 (patched)
<https://reviews.apache.org/r/63388/#comment271343>
typo?
src/resource_provider/storage/provider.cpp
Lines 2038 (patched)
<https://reviews.apache.org/r/63388/#comment271342>
using iterator in the interface is something we never do in other places. I'd suggest we keep track of a hashmap with key being the operation uuid.
- Jie Yu
On Dec. 5, 2017, 2:19 a.m., Chun-Hung Hsiao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63388/
> -----------------------------------------------------------
>
> (Updated Dec. 5, 2017, 2:19 a.m.)
>
>
> Review request for mesos, James DeFelice, Jie Yu, Joseph Wu, and Jan Schlicht.
>
>
> Bugs: MESOS-8108
> https://issues.apache.org/jira/browse/MESOS-8108
>
>
> Repository: mesos
>
>
> Description
> -------
>
> For legacy operations, we just call Resources::apply(). New operations
> CREATE_VOLUME, DESTROY_VOLUME, CREATE_BLOCK, DESTROY_BLOCK are
> implemented through CSI. Specially, DESTROY_* requires unpublishing
> the resources first.
>
>
> Diffs
> -----
>
> src/resource_provider/storage/provider.cpp d35b0d02992e3730ca47906b34c21e1ba9c653e7
>
>
> Diff: https://reviews.apache.org/r/63388/diff/5/
>
>
> Testing
> -------
>
> make
>
>
> Thanks,
>
> Chun-Hung Hsiao
>
>
Re: Review Request 63388: Handling offer operations in storage local
resource provider.
Posted by Chun-Hung Hsiao <ch...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63388/
-----------------------------------------------------------
(Updated Dec. 7, 2017, 10:47 p.m.)
Review request for mesos, James DeFelice, Jie Yu, Joseph Wu, and Jan Schlicht.
Changes
-------
Rebased and fixed state check.
Bugs: MESOS-8108
https://issues.apache.org/jira/browse/MESOS-8108
Repository: mesos
Description
-------
For legacy operations, we just call Resources::apply(). New operations
CREATE_VOLUME, DESTROY_VOLUME, CREATE_BLOCK, DESTROY_BLOCK are
implemented through CSI. Specially, DESTROY_* requires unpublishing
the resources first.
Diffs (updated)
-----
src/resource_provider/storage/provider.cpp a2794acda7187f78ed4246158da71cc26a7892c8
Diff: https://reviews.apache.org/r/63388/diff/8/
Changes: https://reviews.apache.org/r/63388/diff/7-8/
Testing
-------
make
Thanks,
Chun-Hung Hsiao
Re: Review Request 63388: Handling offer operations in storage local
resource provider.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63388/#review193180
-----------------------------------------------------------
Ship it!
Ship It!
- Jie Yu
On Dec. 7, 2017, 6:38 p.m., Chun-Hung Hsiao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63388/
> -----------------------------------------------------------
>
> (Updated Dec. 7, 2017, 6:38 p.m.)
>
>
> Review request for mesos, James DeFelice, Jie Yu, Joseph Wu, and Jan Schlicht.
>
>
> Bugs: MESOS-8108
> https://issues.apache.org/jira/browse/MESOS-8108
>
>
> Repository: mesos
>
>
> Description
> -------
>
> For legacy operations, we just call Resources::apply(). New operations
> CREATE_VOLUME, DESTROY_VOLUME, CREATE_BLOCK, DESTROY_BLOCK are
> implemented through CSI. Specially, DESTROY_* requires unpublishing
> the resources first.
>
>
> Diffs
> -----
>
> src/resource_provider/storage/provider.cpp d79874290d3de5242fe78cd9e6f72e6878b65882
>
>
> Diff: https://reviews.apache.org/r/63388/diff/7/
>
>
> Testing
> -------
>
> make
>
>
> Thanks,
>
> Chun-Hung Hsiao
>
>
Re: Review Request 63388: Handling offer operations in storage local
resource provider.
Posted by Chun-Hung Hsiao <ch...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63388/
-----------------------------------------------------------
(Updated Dec. 7, 2017, 6:38 p.m.)
Review request for mesos, James DeFelice, Jie Yu, Joseph Wu, and Jan Schlicht.
Changes
-------
Addressed Jie's comments.
Bugs: MESOS-8108
https://issues.apache.org/jira/browse/MESOS-8108
Repository: mesos
Description
-------
For legacy operations, we just call Resources::apply(). New operations
CREATE_VOLUME, DESTROY_VOLUME, CREATE_BLOCK, DESTROY_BLOCK are
implemented through CSI. Specially, DESTROY_* requires unpublishing
the resources first.
Diffs (updated)
-----
src/resource_provider/storage/provider.cpp d79874290d3de5242fe78cd9e6f72e6878b65882
Diff: https://reviews.apache.org/r/63388/diff/7/
Changes: https://reviews.apache.org/r/63388/diff/6-7/
Testing
-------
make
Thanks,
Chun-Hung Hsiao
Re: Review Request 63388: Handling offer operations in storage local
resource provider.
Posted by Chun-Hung Hsiao <ch...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63388/
-----------------------------------------------------------
(Updated Dec. 6, 2017, 8:05 p.m.)
Review request for mesos, James DeFelice, Jie Yu, Joseph Wu, and Jan Schlicht.
Changes
-------
Validation for `createVolume` and `deleteVolume`.
Bugs: MESOS-8108
https://issues.apache.org/jira/browse/MESOS-8108
Repository: mesos
Description
-------
For legacy operations, we just call Resources::apply(). New operations
CREATE_VOLUME, DESTROY_VOLUME, CREATE_BLOCK, DESTROY_BLOCK are
implemented through CSI. Specially, DESTROY_* requires unpublishing
the resources first.
Diffs (updated)
-----
src/resource_provider/storage/provider.cpp d35b0d02992e3730ca47906b34c21e1ba9c653e7
Diff: https://reviews.apache.org/r/63388/diff/6/
Changes: https://reviews.apache.org/r/63388/diff/5-6/
Testing
-------
make
Thanks,
Chun-Hung Hsiao