| opendevreview | Jonathan Koerber proposed openstack/manila master: Adding OpenAPI Schemas to Quota Sets Adding OpenAPi Shemas to User Massages Adding function decorators for User Massages => manila/manila/v2/massages.py Change-Id: I80282c2c146b63080f143946c8f0144dd356d4a4 https://review.opendev.org/c/openstack/manila/+/922566 | 00:02 |
|---|---|---|
| opendevreview | Volodymyr Boiko proposed openstack/manila master: Add share driver for VastData storage https://review.opendev.org/c/openstack/manila/+/915380 | 00:14 |
| opendevreview | Jonathan Koerber proposed openstack/manila master: Adding OpenAPI Schemas to Quota Sets Adding OpenAPi Shemas to User Massages Adding function decorators for User Massages => manila/manila/v2/massages.py Change-Id: I80282c2c146b63080f143946c8f0144dd356d4a4 https://review.opendev.org/c/openstack/manila/+/922566 | 01:19 |
| opendevreview | Joel Capitao proposed openstack/manila master: WIP decode with utf-8 https://review.opendev.org/c/openstack/manila/+/923029 | 08:26 |
| opendevreview | Joel Capitao proposed openstack/manila master: Explicitly decode with utf-8 in validation helpers https://review.opendev.org/c/openstack/manila/+/923029 | 09:40 |
| opendevreview | Volodymyr Boiko proposed openstack/manila master: Add share driver for VastData storage https://review.opendev.org/c/openstack/manila/+/915380 | 15:23 |
| kpdev | discussion for share metadata spec | 15:57 |
| kpdev | original idea was to propogate metadata API changes to driver. | 15:57 |
| kpdev | but now we have extended the discussion.. to have metadata as more granular property of share dervied from share type | 15:58 |
| kpdev | @goutham: as I understand, we will have share type extra spec and when we create share from that type, we copy share type extra spec driver properties in share metadata. Now which properties need to copy are decided based on entries in manila.conf.. whether those goes into default section or driver specific section ? | 15:58 |
| kpdev | we can not copy all properties due to security reason.. | 15:59 |
| * gouthamr is extracting out of a call | 16:03 | |
| gouthamr | kpdev: yes, taht's the suggestion | 16:04 |
| gouthamr | kpdev: i'd use the default section | 16:05 |
| kpdev | Also on PR you said ---- User creates a share with your share type; the netapp driver sets that in share metadata. I assume its job of manila API layer or share manager and not of NetApp driver. | 16:05 |
| gouthamr | kpdev: sure; i meant that in case the values are modified (or not modified after a request to modify them), the NetApp driver can return these values in the "update_metadata" driver call you'll be implementing | 16:06 |
| gouthamr | kpdev: but yes, if you're creating a share with some share type extra-specs that we the API knows are updatable metadata properties, the API must create those metadata key-value pairs into the db | 16:07 |
| * gouthamr edits badly | 16:07 | |
| gouthamr | kpdev: but yes, if a user is creating a share with some share type extra-specs that the API knows are updatable metadata properties, the API must create those metadata key-value pairs into the db | 16:07 |
| kpdev | so i believe this the flow | 16:08 |
| kpdev | user create share type and add extra spec | 16:08 |
| kpdev | user create share... manila copy driver properties from default section of manila.conf to share metadata with default value as that of specififed in extra spec | 16:09 |
| kpdev | user modify share metadata, manila update in db and propogate updated metadata i.e. delta to backend driver and backend then can take appropriate action if supported | 16:10 |
| kpdev | Is this flow correct ? | 16:10 |
| kpdev | or anything missing | 16:11 |
| gouthamr | Yep sounds right | 16:12 |
| kpdev | one more thing, backend (if performing) any action with success or failure, will be reported using message API | 16:12 |
| kpdev | thanks for clarifying, i will update the spec | 16:15 |
| gouthamr | yes, and my thought was that: | 16:20 |
| gouthamr | when creating shares with specs that the driver can’t fulfill, you’ll see an error - that’s happening today - there’s no changes needed here | 16:21 |
| gouthamr | when updating metadata, if the driver/share manager fail to do so, it could also revert the metadata - along with creating a user message | 16:22 |
| opendevreview | Jonathan Koerber proposed openstack/manila master: Adding OpenAPi Shemas to User Massages https://review.opendev.org/c/openstack/manila/+/922566 | 16:30 |
| opendevreview | Volodymyr Boiko proposed openstack/manila master: Add share driver for VastData storage https://review.opendev.org/c/openstack/manila/+/915380 | 16:38 |
| ashrodri | kped "user create share... manila copy driver properties from default section of manila.conf to share metadata with default value as that of specififed in extra spec" I would like to point out here that if a share is created with metadata in --property, would it override the default? | 16:40 |
| ashrodri | m/kped/kpdev | 16:41 |
| gouthamr | ^ not override; the API must handle that incompatibility.. can you comment with this on the spec, ashrodri: https://review.opendev.org/c/openstack/manila-specs/+/916595 | 16:42 |
| gouthamr | i mean there must be an error.. | 16:42 |
| gouthamr | can't set metadata XYZZY to SPOON because the share type doesn't allow it | 16:43 |
| kpdev | so share type has extra spec, and share metadata passed during share creation, this will be error ? | 16:47 |
| gouthamr | if the values are incompatible | 16:47 |
| kpdev | if metadata key matches with what is available in share type then ? .. it will first copy default and then over-write the specified ? | 16:48 |
| gouthamr | kpdev: i think that's the concern.. if we see that a user is creating a share with incompatible metadata (incompatible meaning that the metadata values differ from the share type's extra-specs's values), the API would raise an error instead of creating the share... | 16:53 |
| kpdev | why would it be ? | 16:54 |
| kpdev | if values are different but keys are same, means share wants something different than default value and instead of using update metadata API, user try to achieve the same in create API call | 16:55 |
| gouthamr | kpdev: if a user is creating a share with metadata that matches the extra-specs, we'd could just ignore the metadata; the values from the extra-specs would be copied into the metadata anyway. the metadata that the user's providing is just redundant | 16:55 |
| kpdev | this means, metadata can not be used in share create API. | 16:56 |
| kpdev | i think, it should not be an error.. because that is what we want to achiever | 16:56 |
| kpdev | default values from share type extra-spec and share specific values from share metadata | 16:57 |
| gouthamr | hmmm, my thought is that administrators may want to enforce the defaults through share type extra-specs | 16:58 |
| ashrodri | if the extra-specs contains keys which are in supported_admin_metadata_keys then there should be an error raised. otherwise, we can override the support_metadata_keys default value with what is given during creation | 16:58 |
| ashrodri | if we're still using those options from the spec | 16:58 |
| gouthamr | yeah ^ unless the user is an admin, they can't set metadata that's configured as "admin_only_metadata" | 16:59 |
| gouthamr | > because that is what we want to achiever | 16:59 |
| gouthamr | ^ can you please add this to the use cases section of your spec | 16:59 |
| gouthamr | kpdev: it feels like this is a new use case... | 17:00 |
| kpdev | so .. if share is created with metadata, if its admin metadata raise error, if its non-admin metadata over-write default values copied from extra-spec ... correct ? | 17:00 |
| gouthamr | i was under the impression you wanted the ability for users to change things that were configured by the administrator (through extra-specs) | 17:00 |
| ashrodri | that what i am thinking yes | 17:00 |
| gouthamr | but now you're implying that you want users to have the ability to override the administrator's configuration | 17:01 |
| kpdev | no | 17:01 |
| ashrodri | gouthamr: only for keys which the administrator ALLOWS user to override them | 17:01 |
| gouthamr | hmmm; i don't see anything *wrong* with this... yet :) | 17:02 |
| kpdev | the admin config will not be over-written, only non-admin config (supported_metadata_keys) will be overwritten | 17:02 |
| kpdev | the admin config then can be over-written using update metadata API by admin.. right ? | 17:03 |
| kpdev | that is out of scope of this spec though | 17:03 |
| gouthamr | yes; that is clear in my head btw.. ^ i was using less words for brevity | 17:03 |
| ashrodri | administrators would have to think carefully over what keys they include in supported or admin_supported lists is all. | 17:03 |
| gouthamr | +1 | 17:03 |
| kpdev | ok | 17:03 |
| gouthamr | please, lets decide on the naming of these configuration options as well | 17:03 |
| gouthamr | there's one already: "admin_only_metadata" | 17:04 |
| gouthamr | so you only need one more: "driver_updatable_metadata" perhaps? | 17:04 |
| kpdev | driver_updateable_metadata/driver_updatable_admin_metadata.. | 17:05 |
| gouthamr | why would you need "driver_updatable_admin_metadata" | 17:05 |
| kpdev | you mean driver_updatable_admin_metadata and admin_only_metadata are same ? | 17:05 |
| ashrodri | supported_admin_metadata_keys = admin_only_metadata that already exists | 17:06 |
| kpdev | will there be scenario where driver_updatable_admin_metadata is subset of admin_only_metadata.. | 17:06 |
| ashrodri | admin_only_metadata currently by default includes scheduler hints. but can be configured to include more | 17:06 |
| kpdev | yes,, the hints need not to pass to backend driver | 17:07 |
| gouthamr | ^ yes and the mount_options thingy carloss introduced | 17:07 |
| kpdev | i mean this metadata is specific to achive two things | 17:07 |
| kpdev | 1. more granularity of metadata | 17:07 |
| kpdev | 2. pass to backend driver | 17:07 |
| kpdev | not all things from admin_only_metadata need to go to backed, I believe | 17:08 |
| gouthamr | sure… here’s the deal: admin_only_metadata is a list of things only admins can modify… driver_updatable_metadata is the list of all the metadata that involve the driver when they are updated | 17:09 |
| gouthamr | when a user modifies metadata, you’ll first look into “admin_only_metadata” to see if they are modifying any of it and if they *can modify* - this is done today via policy checks | 17:11 |
| gouthamr | This check will fail early if you don’t have permission to modify something…Once you’re past this check, you’d only need to look into the “driver_updatable_metadata” list to pass on the modification to the driver | 17:11 |
| gouthamr | I don’t like the names of these options btw | 17:12 |
| kpdev | ok | 17:12 |
| ashrodri | can we identify admin_only_metadata in config as driver_updateable_metadata? Like inheritance of a variable and not an explicit list, or is that too sophisticated to parse? | 17:13 |
| kpdev | also in driver_update_metadata= "netapp:max_files" or drver_updated_metadata="max_files" ? | 17:13 |
| gouthamr | Feel free to change “driver_updatable_metadata” to anything that makes more sense… but I wanted to be consistent with the config opt “admin_only_metadata” that we have a just a pet peeve to introduce too many config opts and then have to clarify them with their descriptions | 17:13 |
| kpdev | >can we identify admin_only_metadata in config as driver_updateable_metadata | 17:14 |
| kpdev | means ? | 17:14 |
| gouthamr | ashrodri: no need to inherit; let the admin please configure things in both lists | 17:14 |
| gouthamr | ashrodri: not all “admin_only_metadata” needs to go to the driver | 17:14 |
| ashrodri | like admin_only = [driver_update_list] and driver_update_list = [key1, key2, key3...] | 17:15 |
| ashrodri | admin_only would always include scheduler hints, so we're just appending driver_update_list to those | 17:16 |
| kpdev | that means driver update list only updated by admin... | 17:16 |
| kpdev | this is not achieveing purpose.. | 17:17 |
| kpdev | so better be seprate config options | 17:17 |
| ashrodri | driver_update_list only updated by admin. right, this is in config option, is that not just an admin file when starting service? | 17:17 |
| gouthamr | it’s a config opt | 17:18 |
| gouthamr | there are no smarts to do what you’re suggesting with oslo.config | 17:18 |
| kpdev | >> driver_update_list only updated by admin. | 17:19 |
| kpdev | no... this is list we need to consider to pass to backed on metadata update API call.. and regular user can do so | 17:19 |
| ashrodri | regular user can change values, but they cannot identify what those keys that are passable are | 17:20 |
| ashrodri | is moot point anyways as oslo.config cant handle inheritance. for the specs purpose in config, admin will have what we initially thought of as "supported_admin_metadata keys" to the already established "admin_only_metadata". and there will be a "driver_updatable_metadata" list that includes those keys that are passable to driver. | 17:24 |
| kpdev | ok, so adding new config option. | 17:25 |
| ashrodri | OH i See what you mean. driver_updatable is not subset of admin_only, my mistake | 17:25 |
| kpdev | when user call update API, if admin_metadata and supported, we continue to check if its present in driver_updatable and if its, we pass to backend | 17:26 |
| kpdev | if admin_metadata_and not supported, throw error (this happens today, no new logic) | 17:26 |
| kpdev | if non-admin metadata and presnet in driver_updated, pass to backend | 17:26 |
| kpdev | if non-admin metadata and not present in driver_updated list, just update db. | 17:27 |
| ashrodri | "if admin_metadata and supported" it will throw error for user, will pass for admin | 17:27 |
| kpdev | yes, supported.. mean supported by policy | 17:27 |
| kpdev | confirmed ? | 17:28 |
| ashrodri | yes | 17:30 |
| gouthamr | kpdev: ashrodri let’s please take this discussion to gerrit | 17:30 |
| kpdev | thanks for discussion. i will update spec accordingly | 17:31 |
| gouthamr | kpdev: if you’d be kind, please link this chat when you make the updates | 17:31 |
| gouthamr | the log location is in the topic for this channel | 17:31 |
| kpdev | ok | 17:35 |
| carloss | kpdev++ gouthamr++ ashrodri++ | 17:36 |
| * carloss missed out on some of this fun | 17:37 | |
| carloss | great discussion | 17:37 |
| carloss | and I like the config option being proposed as well :) | 17:37 |
| opendevreview | Chuan Miao proposed openstack/manila master: svm migration across physical networks with multiple port bindings https://review.opendev.org/c/openstack/manila/+/869720 | 19:43 |
| opendevreview | Chuan Miao proposed openstack/manila master: Improve scheduler performance on thin provisioning https://review.opendev.org/c/openstack/manila/+/898306 | 20:29 |
| opendevreview | Merged openstack/manila master: Explicitly decode with utf-8 in validation helpers https://review.opendev.org/c/openstack/manila/+/923029 | 21:39 |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!