You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Joseph Wu <jo...@mesosphere.io> on 2017/12/05 22:06:45 UTC
Re: Review Request 63971: Defined a module interface for translating
volume profiles.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63971/
-----------------------------------------------------------
(Updated Dec. 5, 2017, 2:06 p.m.)
Review request for mesos, Chun-Hung Hsiao and Jie Yu.
Changes
-------
Addressed comments.
Return value of `translate` was changed to only have a single VolumeCapability instead of an array of them.
Bugs: MESOS-8251
https://issues.apache.org/jira/browse/MESOS-8251
Repository: mesos
Description
-------
This module is currently intended for use by the Storage Local
Resource Provider (SLRP), but may be used by other components if
those components use any of the affected Container Storage Interface
(CSI) requests. The affected calls are listed in the module's comments.
Diffs (updated)
-----
include/mesos/mesos.proto 5b93478ed1a8250b05c44ec8b4d1f660a2053b87
include/mesos/resource_provider/volume_profile.hpp PRE-CREATION
include/mesos/v1/mesos.proto 22b3c6889d975147b001ef145ba4fc382debf70a
Diff: https://reviews.apache.org/r/63971/diff/3/
Changes: https://reviews.apache.org/r/63971/diff/2-3/
Testing (updated)
-------
See end of chain.
Thanks,
Joseph Wu
Re: Review Request 63971: Defined a module interface for translating
volume profiles.
Posted by Joseph Wu <jo...@mesosphere.io>.
> On Dec. 6, 2017, 12:04 p.m., Jie Yu wrote:
> > include/mesos/resource_provider/volume_profile.hpp
> > Lines 104-105 (patched)
> > <https://reviews.apache.org/r/63971/diff/3/?file=1909252#file1909252line104>
> >
> > Do we intend to use this module for all LRPs on the agent? If yes, how can we get profiles for a particular LRP? Should profile name be unique cluster wide, or unique per LRP?
> >
> > I guess we can assume it's unique per RP. Some distribution might choose to do cluster wide unique, but that's their choice.
> >
> > With that in mind, maybe include ResourceProviderInfo in the request?
>
> Jie Yu wrote:
> need to do the same for `translate` above
I chose to include a `std::string` corresponding to `CSIPluginInfo::type`. I didn't want to include the entire `CSIPluginInfo` because that includes the ContainerInfos and makes it somewhat ambiguous for the module to choose between `type` and `name` or both.
And since the module only deals with Storage RPs, `ResourceProviderInfo` would be too much info.
- Joseph
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63971/#review193036
-----------------------------------------------------------
On Dec. 6, 2017, 3:37 p.m., Joseph Wu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63971/
> -----------------------------------------------------------
>
> (Updated Dec. 6, 2017, 3:37 p.m.)
>
>
> Review request for mesos, Chun-Hung Hsiao and Jie Yu.
>
>
> Bugs: MESOS-8251
> https://issues.apache.org/jira/browse/MESOS-8251
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This module is currently intended for use by the Storage Local
> Resource Provider (SLRP), but may be used by other components if
> those components use any of the affected Container Storage Interface
> (CSI) requests. The affected calls are listed in the module's comments.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 5b93478ed1a8250b05c44ec8b4d1f660a2053b87
> include/mesos/resource_provider/volume_profile.hpp PRE-CREATION
> include/mesos/v1/mesos.proto 22b3c6889d975147b001ef145ba4fc382debf70a
>
>
> Diff: https://reviews.apache.org/r/63971/diff/4/
>
>
> Testing
> -------
>
> See end of chain.
>
>
> Thanks,
>
> Joseph Wu
>
>
Re: Review Request 63971: Defined a module interface for translating
volume profiles.
Posted by Joseph Wu <jo...@mesosphere.io>.
> On Dec. 6, 2017, 12:04 p.m., Jie Yu wrote:
> > include/mesos/resource_provider/volume_profile.hpp
> > Lines 104-105 (patched)
> > <https://reviews.apache.org/r/63971/diff/3/?file=1909252#file1909252line104>
> >
> > Do we intend to use this module for all LRPs on the agent? If yes, how can we get profiles for a particular LRP? Should profile name be unique cluster wide, or unique per LRP?
> >
> > I guess we can assume it's unique per RP. Some distribution might choose to do cluster wide unique, but that's their choice.
> >
> > With that in mind, maybe include ResourceProviderInfo in the request?
>
> Jie Yu wrote:
> need to do the same for `translate` above
>
> Joseph Wu wrote:
> I chose to include a `std::string` corresponding to `CSIPluginInfo::type`. I didn't want to include the entire `CSIPluginInfo` because that includes the ContainerInfos and makes it somewhat ambiguous for the module to choose between `type` and `name` or both.
>
> And since the module only deals with Storage RPs, `ResourceProviderInfo` would be too much info.
>
> Jie Yu wrote:
> Yeah, the some module might want to use provider ID to do the matching. I guess we can include that later?
>
> Jie Yu wrote:
> Why optional?
I don't forsee this module used by any non-CSI resource provider, so I would definitely leave the ResourceProviderID out for now.
- Joseph
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63971/#review193036
-----------------------------------------------------------
On Dec. 6, 2017, 3:37 p.m., Joseph Wu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63971/
> -----------------------------------------------------------
>
> (Updated Dec. 6, 2017, 3:37 p.m.)
>
>
> Review request for mesos, Chun-Hung Hsiao and Jie Yu.
>
>
> Bugs: MESOS-8251
> https://issues.apache.org/jira/browse/MESOS-8251
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This module is currently intended for use by the Storage Local
> Resource Provider (SLRP), but may be used by other components if
> those components use any of the affected Container Storage Interface
> (CSI) requests. The affected calls are listed in the module's comments.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 5b93478ed1a8250b05c44ec8b4d1f660a2053b87
> include/mesos/resource_provider/volume_profile.hpp PRE-CREATION
> include/mesos/v1/mesos.proto 22b3c6889d975147b001ef145ba4fc382debf70a
>
>
> Diff: https://reviews.apache.org/r/63971/diff/4/
>
>
> Testing
> -------
>
> See end of chain.
>
>
> Thanks,
>
> Joseph Wu
>
>
Re: Review Request 63971: Defined a module interface for translating
volume profiles.
Posted by Jie Yu <yu...@gmail.com>.
> On Dec. 6, 2017, 8:04 p.m., Jie Yu wrote:
> > include/mesos/resource_provider/volume_profile.hpp
> > Lines 104-105 (patched)
> > <https://reviews.apache.org/r/63971/diff/3/?file=1909252#file1909252line104>
> >
> > Do we intend to use this module for all LRPs on the agent? If yes, how can we get profiles for a particular LRP? Should profile name be unique cluster wide, or unique per LRP?
> >
> > I guess we can assume it's unique per RP. Some distribution might choose to do cluster wide unique, but that's their choice.
> >
> > With that in mind, maybe include ResourceProviderInfo in the request?
>
> Jie Yu wrote:
> need to do the same for `translate` above
>
> Joseph Wu wrote:
> I chose to include a `std::string` corresponding to `CSIPluginInfo::type`. I didn't want to include the entire `CSIPluginInfo` because that includes the ContainerInfos and makes it somewhat ambiguous for the module to choose between `type` and `name` or both.
>
> And since the module only deals with Storage RPs, `ResourceProviderInfo` would be too much info.
Yeah, the some module might want to use provider ID to do the matching. I guess we can include that later?
- Jie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63971/#review193036
-----------------------------------------------------------
On Dec. 6, 2017, 11:37 p.m., Joseph Wu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63971/
> -----------------------------------------------------------
>
> (Updated Dec. 6, 2017, 11:37 p.m.)
>
>
> Review request for mesos, Chun-Hung Hsiao and Jie Yu.
>
>
> Bugs: MESOS-8251
> https://issues.apache.org/jira/browse/MESOS-8251
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This module is currently intended for use by the Storage Local
> Resource Provider (SLRP), but may be used by other components if
> those components use any of the affected Container Storage Interface
> (CSI) requests. The affected calls are listed in the module's comments.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 5b93478ed1a8250b05c44ec8b4d1f660a2053b87
> include/mesos/resource_provider/volume_profile.hpp PRE-CREATION
> include/mesos/v1/mesos.proto 22b3c6889d975147b001ef145ba4fc382debf70a
>
>
> Diff: https://reviews.apache.org/r/63971/diff/4/
>
>
> Testing
> -------
>
> See end of chain.
>
>
> Thanks,
>
> Joseph Wu
>
>
Re: Review Request 63971: Defined a module interface for translating
volume profiles.
Posted by Jie Yu <yu...@gmail.com>.
> On Dec. 6, 2017, 8:04 p.m., Jie Yu wrote:
> > include/mesos/resource_provider/volume_profile.hpp
> > Lines 104-105 (patched)
> > <https://reviews.apache.org/r/63971/diff/3/?file=1909252#file1909252line104>
> >
> > Do we intend to use this module for all LRPs on the agent? If yes, how can we get profiles for a particular LRP? Should profile name be unique cluster wide, or unique per LRP?
> >
> > I guess we can assume it's unique per RP. Some distribution might choose to do cluster wide unique, but that's their choice.
> >
> > With that in mind, maybe include ResourceProviderInfo in the request?
need to do the same for `translate` above
- Jie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63971/#review193036
-----------------------------------------------------------
On Dec. 5, 2017, 10:06 p.m., Joseph Wu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63971/
> -----------------------------------------------------------
>
> (Updated Dec. 5, 2017, 10:06 p.m.)
>
>
> Review request for mesos, Chun-Hung Hsiao and Jie Yu.
>
>
> Bugs: MESOS-8251
> https://issues.apache.org/jira/browse/MESOS-8251
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This module is currently intended for use by the Storage Local
> Resource Provider (SLRP), but may be used by other components if
> those components use any of the affected Container Storage Interface
> (CSI) requests. The affected calls are listed in the module's comments.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 5b93478ed1a8250b05c44ec8b4d1f660a2053b87
> include/mesos/resource_provider/volume_profile.hpp PRE-CREATION
> include/mesos/v1/mesos.proto 22b3c6889d975147b001ef145ba4fc382debf70a
>
>
> Diff: https://reviews.apache.org/r/63971/diff/3/
>
>
> Testing
> -------
>
> See end of chain.
>
>
> Thanks,
>
> Joseph Wu
>
>
Re: Review Request 63971: Defined a module interface for translating
volume profiles.
Posted by Jie Yu <yu...@gmail.com>.
> On Dec. 6, 2017, 8:04 p.m., Jie Yu wrote:
> > include/mesos/resource_provider/volume_profile.hpp
> > Lines 104-105 (patched)
> > <https://reviews.apache.org/r/63971/diff/3/?file=1909252#file1909252line104>
> >
> > Do we intend to use this module for all LRPs on the agent? If yes, how can we get profiles for a particular LRP? Should profile name be unique cluster wide, or unique per LRP?
> >
> > I guess we can assume it's unique per RP. Some distribution might choose to do cluster wide unique, but that's their choice.
> >
> > With that in mind, maybe include ResourceProviderInfo in the request?
>
> Jie Yu wrote:
> need to do the same for `translate` above
>
> Joseph Wu wrote:
> I chose to include a `std::string` corresponding to `CSIPluginInfo::type`. I didn't want to include the entire `CSIPluginInfo` because that includes the ContainerInfos and makes it somewhat ambiguous for the module to choose between `type` and `name` or both.
>
> And since the module only deals with Storage RPs, `ResourceProviderInfo` would be too much info.
>
> Jie Yu wrote:
> Yeah, the some module might want to use provider ID to do the matching. I guess we can include that later?
Why optional?
- Jie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63971/#review193036
-----------------------------------------------------------
On Dec. 6, 2017, 11:37 p.m., Joseph Wu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63971/
> -----------------------------------------------------------
>
> (Updated Dec. 6, 2017, 11:37 p.m.)
>
>
> Review request for mesos, Chun-Hung Hsiao and Jie Yu.
>
>
> Bugs: MESOS-8251
> https://issues.apache.org/jira/browse/MESOS-8251
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This module is currently intended for use by the Storage Local
> Resource Provider (SLRP), but may be used by other components if
> those components use any of the affected Container Storage Interface
> (CSI) requests. The affected calls are listed in the module's comments.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 5b93478ed1a8250b05c44ec8b4d1f660a2053b87
> include/mesos/resource_provider/volume_profile.hpp PRE-CREATION
> include/mesos/v1/mesos.proto 22b3c6889d975147b001ef145ba4fc382debf70a
>
>
> Diff: https://reviews.apache.org/r/63971/diff/4/
>
>
> Testing
> -------
>
> See end of chain.
>
>
> Thanks,
>
> Joseph Wu
>
>
Re: Review Request 63971: Defined a module interface for translating
volume profiles.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63971/#review193036
-----------------------------------------------------------
include/mesos/resource_provider/volume_profile.hpp
Lines 104-105 (patched)
<https://reviews.apache.org/r/63971/#comment271529>
Do we intend to use this module for all LRPs on the agent? If yes, how can we get profiles for a particular LRP? Should profile name be unique cluster wide, or unique per LRP?
I guess we can assume it's unique per RP. Some distribution might choose to do cluster wide unique, but that's their choice.
With that in mind, maybe include ResourceProviderInfo in the request?
- Jie Yu
On Dec. 5, 2017, 10:06 p.m., Joseph Wu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63971/
> -----------------------------------------------------------
>
> (Updated Dec. 5, 2017, 10:06 p.m.)
>
>
> Review request for mesos, Chun-Hung Hsiao and Jie Yu.
>
>
> Bugs: MESOS-8251
> https://issues.apache.org/jira/browse/MESOS-8251
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This module is currently intended for use by the Storage Local
> Resource Provider (SLRP), but may be used by other components if
> those components use any of the affected Container Storage Interface
> (CSI) requests. The affected calls are listed in the module's comments.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 5b93478ed1a8250b05c44ec8b4d1f660a2053b87
> include/mesos/resource_provider/volume_profile.hpp PRE-CREATION
> include/mesos/v1/mesos.proto 22b3c6889d975147b001ef145ba4fc382debf70a
>
>
> Diff: https://reviews.apache.org/r/63971/diff/3/
>
>
> Testing
> -------
>
> See end of chain.
>
>
> Thanks,
>
> Joseph Wu
>
>
Re: Review Request 63971: Defined a module interface for translating
volume profiles.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63971/#review193055
-----------------------------------------------------------
include/mesos/resource_provider/volume_profile.hpp
Lines 106 (patched)
<https://reviews.apache.org/r/63971/#comment271552>
I'd suggest we use `ResourceProviderInfo` here so that some module might choose to match based on provider ID.
- Jie Yu
On Dec. 6, 2017, 11:37 p.m., Joseph Wu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63971/
> -----------------------------------------------------------
>
> (Updated Dec. 6, 2017, 11:37 p.m.)
>
>
> Review request for mesos, Chun-Hung Hsiao and Jie Yu.
>
>
> Bugs: MESOS-8251
> https://issues.apache.org/jira/browse/MESOS-8251
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This module is currently intended for use by the Storage Local
> Resource Provider (SLRP), but may be used by other components if
> those components use any of the affected Container Storage Interface
> (CSI) requests. The affected calls are listed in the module's comments.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 5b93478ed1a8250b05c44ec8b4d1f660a2053b87
> include/mesos/resource_provider/volume_profile.hpp PRE-CREATION
> include/mesos/v1/mesos.proto 22b3c6889d975147b001ef145ba4fc382debf70a
>
>
> Diff: https://reviews.apache.org/r/63971/diff/4/
>
>
> Testing
> -------
>
> See end of chain.
>
>
> Thanks,
>
> Joseph Wu
>
>
Re: Review Request 63971: Defined a module interface for translating
volume profiles.
Posted by Chun-Hung Hsiao <ch...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63971/#review193771
-----------------------------------------------------------
Fix it, then Ship it!
include/mesos/resource_provider/volume_profile.hpp
Lines 49 (patched)
<https://reviews.apache.org/r/63971/#comment272414>
I thought the more common spelling is Adapter. Do we prefer Adaptor in the codebase?
- Chun-Hung Hsiao
On Dec. 6, 2017, 11:50 p.m., Joseph Wu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63971/
> -----------------------------------------------------------
>
> (Updated Dec. 6, 2017, 11:50 p.m.)
>
>
> Review request for mesos, Chun-Hung Hsiao and Jie Yu.
>
>
> Bugs: MESOS-8251
> https://issues.apache.org/jira/browse/MESOS-8251
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This module is currently intended for use by the Storage Local
> Resource Provider (SLRP), but may be used by other components if
> those components use any of the affected Container Storage Interface
> (CSI) requests. The affected calls are listed in the module's comments.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 5b93478ed1a8250b05c44ec8b4d1f660a2053b87
> include/mesos/resource_provider/volume_profile.hpp PRE-CREATION
> include/mesos/v1/mesos.proto 22b3c6889d975147b001ef145ba4fc382debf70a
>
>
> Diff: https://reviews.apache.org/r/63971/diff/6/
>
>
> Testing
> -------
>
> See end of chain.
>
>
> Thanks,
>
> Joseph Wu
>
>
Re: Review Request 63971: Defined a module interface for translating
volume profiles.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63971/#review193065
-----------------------------------------------------------
Ship it!
Ship It!
- Jie Yu
On Dec. 6, 2017, 11:50 p.m., Joseph Wu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63971/
> -----------------------------------------------------------
>
> (Updated Dec. 6, 2017, 11:50 p.m.)
>
>
> Review request for mesos, Chun-Hung Hsiao and Jie Yu.
>
>
> Bugs: MESOS-8251
> https://issues.apache.org/jira/browse/MESOS-8251
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This module is currently intended for use by the Storage Local
> Resource Provider (SLRP), but may be used by other components if
> those components use any of the affected Container Storage Interface
> (CSI) requests. The affected calls are listed in the module's comments.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 5b93478ed1a8250b05c44ec8b4d1f660a2053b87
> include/mesos/resource_provider/volume_profile.hpp PRE-CREATION
> include/mesos/v1/mesos.proto 22b3c6889d975147b001ef145ba4fc382debf70a
>
>
> Diff: https://reviews.apache.org/r/63971/diff/5/
>
>
> Testing
> -------
>
> See end of chain.
>
>
> Thanks,
>
> Joseph Wu
>
>
Re: Review Request 63971: Defined a module interface for translating
volume profiles.
Posted by Joseph Wu <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63971/
-----------------------------------------------------------
(Updated Dec. 6, 2017, 3:50 p.m.)
Review request for mesos, Chun-Hung Hsiao and Jie Yu.
Changes
-------
Made the `CSIPluginInfo::type` not optional. The module implementation can just ignore it if it wants to.
Bugs: MESOS-8251
https://issues.apache.org/jira/browse/MESOS-8251
Repository: mesos
Description
-------
This module is currently intended for use by the Storage Local
Resource Provider (SLRP), but may be used by other components if
those components use any of the affected Container Storage Interface
(CSI) requests. The affected calls are listed in the module's comments.
Diffs (updated)
-----
include/mesos/mesos.proto 5b93478ed1a8250b05c44ec8b4d1f660a2053b87
include/mesos/resource_provider/volume_profile.hpp PRE-CREATION
include/mesos/v1/mesos.proto 22b3c6889d975147b001ef145ba4fc382debf70a
Diff: https://reviews.apache.org/r/63971/diff/5/
Changes: https://reviews.apache.org/r/63971/diff/4-5/
Testing
-------
See end of chain.
Thanks,
Joseph Wu
Re: Review Request 63971: Defined a module interface for translating
volume profiles.
Posted by Joseph Wu <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63971/
-----------------------------------------------------------
(Updated Dec. 6, 2017, 3:37 p.m.)
Review request for mesos, Chun-Hung Hsiao and Jie Yu.
Changes
-------
* Renamed module
* Tweaked some comments
* Pulled return type of `translate` into a struct
* Added parameter for optional filtering by CSI plugin type.
Bugs: MESOS-8251
https://issues.apache.org/jira/browse/MESOS-8251
Repository: mesos
Description
-------
This module is currently intended for use by the Storage Local
Resource Provider (SLRP), but may be used by other components if
those components use any of the affected Container Storage Interface
(CSI) requests. The affected calls are listed in the module's comments.
Diffs (updated)
-----
include/mesos/mesos.proto 5b93478ed1a8250b05c44ec8b4d1f660a2053b87
include/mesos/resource_provider/volume_profile.hpp PRE-CREATION
include/mesos/v1/mesos.proto 22b3c6889d975147b001ef145ba4fc382debf70a
Diff: https://reviews.apache.org/r/63971/diff/4/
Changes: https://reviews.apache.org/r/63971/diff/3-4/
Testing
-------
See end of chain.
Thanks,
Joseph Wu
Re: Review Request 63971: Defined a module interface for translating
volume profiles.
Posted by Joseph Wu <jo...@mesosphere.io>.
> On Dec. 6, 2017, 10:37 a.m., Jie Yu wrote:
> > include/mesos/resource_provider/volume_profile.hpp
> > Lines 92 (patched)
> > <https://reviews.apache.org/r/63971/diff/3/?file=1909252#file1909252line92>
> >
> > We chatted about having an ability to define a default profile. To support that, maybe `profile` here needs to be Optional?
I'm hearing mixed messages about providing a default profile.
Sounds like the SLRP could choose to not publish resources if there is no profile.
Or the SLRP could set it's own default somewhere.
Or the default module implementation could return a (basically hard-coded) value for all profiles.
- Joseph
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63971/#review193013
-----------------------------------------------------------
On Dec. 6, 2017, 3:37 p.m., Joseph Wu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63971/
> -----------------------------------------------------------
>
> (Updated Dec. 6, 2017, 3:37 p.m.)
>
>
> Review request for mesos, Chun-Hung Hsiao and Jie Yu.
>
>
> Bugs: MESOS-8251
> https://issues.apache.org/jira/browse/MESOS-8251
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This module is currently intended for use by the Storage Local
> Resource Provider (SLRP), but may be used by other components if
> those components use any of the affected Container Storage Interface
> (CSI) requests. The affected calls are listed in the module's comments.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 5b93478ed1a8250b05c44ec8b4d1f660a2053b87
> include/mesos/resource_provider/volume_profile.hpp PRE-CREATION
> include/mesos/v1/mesos.proto 22b3c6889d975147b001ef145ba4fc382debf70a
>
>
> Diff: https://reviews.apache.org/r/63971/diff/4/
>
>
> Testing
> -------
>
> See end of chain.
>
>
> Thanks,
>
> Joseph Wu
>
>
Re: Review Request 63971: Defined a module interface for translating
volume profiles.
Posted by Jie Yu <yu...@gmail.com>.
> On Dec. 6, 2017, 6:37 p.m., Jie Yu wrote:
> > include/mesos/resource_provider/volume_profile.hpp
> > Lines 92 (patched)
> > <https://reviews.apache.org/r/63971/diff/3/?file=1909252#file1909252line92>
> >
> > We chatted about having an ability to define a default profile. To support that, maybe `profile` here needs to be Optional?
>
> Joseph Wu wrote:
> I'm hearing mixed messages about providing a default profile.
>
> Sounds like the SLRP could choose to not publish resources if there is no profile.
> Or the SLRP could set it's own default somewhere.
> Or the default module implementation could return a (basically hard-coded) value for all profiles.
Discussed offline. Let's don't support default profile yet. The default BLOCK or VOLUME capabilities and access mode will be hard coded in SLRP for now (for publishing). Drop it
- Jie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63971/#review193013
-----------------------------------------------------------
On Dec. 6, 2017, 11:37 p.m., Joseph Wu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63971/
> -----------------------------------------------------------
>
> (Updated Dec. 6, 2017, 11:37 p.m.)
>
>
> Review request for mesos, Chun-Hung Hsiao and Jie Yu.
>
>
> Bugs: MESOS-8251
> https://issues.apache.org/jira/browse/MESOS-8251
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This module is currently intended for use by the Storage Local
> Resource Provider (SLRP), but may be used by other components if
> those components use any of the affected Container Storage Interface
> (CSI) requests. The affected calls are listed in the module's comments.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 5b93478ed1a8250b05c44ec8b4d1f660a2053b87
> include/mesos/resource_provider/volume_profile.hpp PRE-CREATION
> include/mesos/v1/mesos.proto 22b3c6889d975147b001ef145ba4fc382debf70a
>
>
> Diff: https://reviews.apache.org/r/63971/diff/4/
>
>
> Testing
> -------
>
> See end of chain.
>
>
> Thanks,
>
> Joseph Wu
>
>
Re: Review Request 63971: Defined a module interface for translating
volume profiles.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63971/#review193013
-----------------------------------------------------------
include/mesos/resource_provider/volume_profile.hpp
Lines 34 (patched)
<https://reviews.apache.org/r/63971/#comment271487>
This module is not limited to SLRP. It can be used by SERP in the future. I'd suggest we remove the `Local` part in the commejnts.
include/mesos/resource_provider/volume_profile.hpp
Lines 49 (patched)
<https://reviews.apache.org/r/63971/#comment271486>
Looks like we name modules using `XXXer` pattern. `VolumeProfile` might make people think this is a volume profile, not a module to translate volume profiles and get volume profile names.
Suggest to use something like `VolumeProfileAdaptor`.
include/mesos/resource_provider/volume_profile.hpp
Lines 89-91 (patched)
<https://reviews.apache.org/r/63971/#comment271484>
Instead of using a tuple, can we define a struct instead?
include/mesos/resource_provider/volume_profile.hpp
Lines 92 (patched)
<https://reviews.apache.org/r/63971/#comment271485>
We chatted about having an ability to define a default profile. To support that, maybe `profile` here needs to be Optional?
- Jie Yu
On Dec. 5, 2017, 10:06 p.m., Joseph Wu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63971/
> -----------------------------------------------------------
>
> (Updated Dec. 5, 2017, 10:06 p.m.)
>
>
> Review request for mesos, Chun-Hung Hsiao and Jie Yu.
>
>
> Bugs: MESOS-8251
> https://issues.apache.org/jira/browse/MESOS-8251
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This module is currently intended for use by the Storage Local
> Resource Provider (SLRP), but may be used by other components if
> those components use any of the affected Container Storage Interface
> (CSI) requests. The affected calls are listed in the module's comments.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 5b93478ed1a8250b05c44ec8b4d1f660a2053b87
> include/mesos/resource_provider/volume_profile.hpp PRE-CREATION
> include/mesos/v1/mesos.proto 22b3c6889d975147b001ef145ba4fc382debf70a
>
>
> Diff: https://reviews.apache.org/r/63971/diff/3/
>
>
> Testing
> -------
>
> See end of chain.
>
>
> Thanks,
>
> Joseph Wu
>
>