opendevreview | Eduardo Santos proposed openstack/python-manilaclient master: Fix typo in subnet delete description https://review.opendev.org/c/openstack/python-manilaclient/+/826547 | 01:13 |
---|---|---|
opendevreview | Merged openstack/python-manilaclient master: Fix typo in subnet delete description https://review.opendev.org/c/openstack/python-manilaclient/+/826547 | 03:46 |
opendevreview | Fernando Ferraz proposed openstack/manila master: NetApp ONTAP: Add support to multiple subnets per AZ https://review.opendev.org/c/openstack/manila/+/825155 | 03:55 |
*** dviroel|afk is now known as dviroel | 11:20 | |
opendevreview | Victoria Martinez de la Cruz proposed openstack/devstack-plugin-ceph master: Deploy with cephadm https://review.opendev.org/c/openstack/devstack-plugin-ceph/+/826484 | 13:21 |
opendevreview | Victoria Martinez de la Cruz proposed openstack/devstack-plugin-ceph master: Deploy with cephadm https://review.opendev.org/c/openstack/devstack-plugin-ceph/+/826484 | 13:45 |
opendevreview | Victoria Martinez de la Cruz proposed openstack/devstack-plugin-ceph master: Deploy with cephadm https://review.opendev.org/c/openstack/devstack-plugin-ceph/+/826484 | 14:06 |
opendevreview | Carlos Eduardo proposed openstack/manila master: [WIP] Change manila generated key type to ecsda https://review.opendev.org/c/openstack/manila/+/826686 | 14:32 |
opendevreview | Victoria Martinez de la Cruz proposed openstack/devstack-plugin-ceph master: Deploy with cephadm https://review.opendev.org/c/openstack/devstack-plugin-ceph/+/826484 | 15:19 |
ashrodri | o/ anyone around for a riveting discussion on tempest tests? :) | 16:03 |
vkmc | o/ | 16:03 |
ashrodri | \o/ yay | 16:05 |
gouthamr | that's a very clever test case | 16:06 |
* gouthamr misses zhongjun | 16:06 | |
ashrodri | reading through the original bug that lead to this test: https://bugs.launchpad.net/manila/+bug/1774353 | 16:08 |
ashrodri | basically we want to strip .xml/.json from keys, allowing whatever text comes before that to be set as the key | 16:10 |
ashrodri | my failure arises from my router changes, im using mapper.connect to identify each method | 16:11 |
*** dviroel is now known as dviroel|lunch | 16:13 | |
ashrodri | hmmm still thinking about how to address this sorry | 16:13 |
gouthamr | hmm, one thing we could do here is to preserve the behavior for show/delete | 16:17 |
gouthamr | if there is a “.xml|.json” suffix, respond with 404 | 16:17 |
gouthamr | or not - this would be incorrect if “foo.xml” and “foo” are both present in the metadata | 16:18 |
gouthamr | But that’s going to be incorrect even today | 16:19 |
ashrodri | when i tested this yesterday, i could set a metadata key value pair with foo.xml=bar and it would show up when i did manila show | 16:20 |
ashrodri | but i couldnt delete it with unset foo.xml | 16:20 |
gouthamr | Hmm, did you test with the client or did you use cURL ? | 16:21 |
ashrodri | but with a key like foo.bar i can set and unset normally | 16:21 |
ashrodri | client | 16:22 |
ashrodri | i can try again with a curl | 16:22 |
ashrodri | ah wait, i mightve been testing on my changes not on master haha let me try again | 16:22 |
gouthamr | Yep; do confirm - because this seems to be behavior associated with the URL ending with .xml|.json | 16:23 |
gouthamr | Which should happen with GET, PUT or DELETE against a specific metadata item | 16:23 |
ashrodri | ah okay just tested again. i set key foo=bar and foo.xml=value when i try metadata unset foo.xml, foo is deleted | 16:27 |
ashrodri | which means, foo.xml is there to stay. theres no way to me to delete that with the client | 16:29 |
gouthamr | yes | 16:29 |
gouthamr | this is a bug | 16:30 |
opendevreview | Victoria Martinez de la Cruz proposed openstack/devstack-plugin-ceph master: Deploy with cephadm https://review.opendev.org/c/openstack/devstack-plugin-ceph/+/826484 | 16:30 |
gouthamr | so i'm okay deleting that tempest test | 16:30 |
ashrodri | curling with the correct url also doesnt delete the foo.xml btw | 16:30 |
gouthamr | yes | 16:30 |
gouthamr | because .xml (and .json) are stripped before the API method sees the key | 16:31 |
ashrodri | okay so what behavior do we actually want when it comes to these types of keys | 16:31 |
gouthamr | the behavior must be that we don't mess with them | 16:31 |
gouthamr | i.e., the URL for share metadata is /shares/{share_id}/metadata/{key} | 16:32 |
gouthamr | we're deconstructing the {key} the user has and stripping .xml or .json - which isn't correct | 16:32 |
gouthamr | your change is fixing this behavior to do the right thing, in turn causing the tempest test to fail | 16:33 |
ashrodri | that deconstructing is from zhongjun's changes? so do i revert it? | 16:33 |
gouthamr | most of our API resources are going to be words (e.g.: "shares", "share-replicas", "servers", ...) or UUIDs (e.g.: /shares/d1e7af20-a412-46d9-a406-d43075fe0073, ...) | 16:35 |
gouthamr | s/to be/to end with | 16:35 |
gouthamr | the only place this differs is metadata and extra-specs | 16:35 |
gouthamr | where the resource locator is allowed to be ascii text upto 255 characters because these are key=value structures | 16:36 |
gouthamr | in the latter, we've allowed users to use any legal keys, and "something.json" seems legal enough to me | 16:37 |
ashrodri | me too | 16:37 |
gouthamr | https://bugs.launchpad.net/manila/+bug/1774353 | 16:38 |
gouthamr | ^ the bug says the spec allows ".xml" and ".json" are accepted | 16:38 |
gouthamr | i wonder where that language is | 16:38 |
ashrodri | i cant find that in the original spec | 16:41 |
gouthamr | dunno i wonder if it is just copy-pasta :) | 16:41 |
gouthamr | https://review.opendev.org/c/openstack/cinder/+/231663 | 16:41 |
gouthamr | ^ i don't recall there being an xml api with manila | 16:42 |
gouthamr | that's tangential | 16:42 |
gouthamr | ashrodri: without reverting that change, removing the metadata tempest test should also help your case? | 16:43 |
ashrodri | yes, but i figured fixing the bug regarding deleting these keys would also help. could also be done in a separate patch | 16:44 |
gouthamr | hmmm, how would you do that? | 16:45 |
ashrodri | not sure, id have to look to see where the stripping is happening and why it only happens on DELETE not PUT/POST | 16:46 |
ashrodri | removing the test can be one patch, then have my api changes depend on it would let it pass zuul. but then ive just entangled my patch in a chain a week away from FF so... | 16:47 |
gouthamr | yes, not a fan of patch chains - but this should hopefully be a short one | 16:48 |
gouthamr | you're hitting this with GET too, correct? i'd expect the same behavior with PUT/POST -- that | 16:48 |
gouthamr | unless we're not allowing PUT/POST against a specific metadata ite | 16:48 |
gouthamr | item* | 16:48 |
ashrodri | the router only identifies the route for update_all which is a PUT with no key at the end of the URL | 16:51 |
ashrodri | the api does have a get item but not the client, let me try curling | 16:51 |
ashrodri | nope, curling with foo.xml doesnt work | 16:52 |
ashrodri | curling with PUT and foo.xml gives me a URI mismatch so its also stripping there | 16:54 |
opendevreview | Merged openstack/manila master: Force disk wipe when running lvcreate https://review.opendev.org/c/openstack/manila/+/817206 | 16:54 |
ashrodri | POST is the only place that could allow foo.xml since its in the body not URL GET PUT and DELETE all strip which causes an error | 16:55 |
ashrodri | removing the strip completely would probably fix the behavior, and we can either remove or alter the tempest test to check it but i think itll be fine since without stripping foo.xml would be treated the same as foo.foo and i know that works | 16:56 |
ashrodri | both with client and curling | 16:57 |
opendevreview | Victoria Martinez de la Cruz proposed openstack/devstack-plugin-ceph master: Deploy with cephadm https://review.opendev.org/c/openstack/devstack-plugin-ceph/+/826484 | 16:58 |
opendevreview | Carlos Eduardo proposed openstack/manila master: WIP Add FIPS testing jobs https://review.opendev.org/c/openstack/manila/+/810953 | 17:11 |
opendevreview | Victoria Martinez de la Cruz proposed openstack/devstack-plugin-ceph master: Deploy with cephadm https://review.opendev.org/c/openstack/devstack-plugin-ceph/+/826484 | 17:24 |
*** dviroel|lunch is now known as dviroel | 17:26 | |
opendevreview | Ashley Rodriguez proposed openstack/manila-tempest-plugin master: remove share metadata test https://review.opendev.org/c/openstack/manila-tempest-plugin/+/826747 | 17:59 |
opendevreview | Ashley Rodriguez proposed openstack/manila master: WIP: Metadata for Share Resource https://review.opendev.org/c/openstack/manila/+/824648 | 18:21 |
opendevreview | Merged openstack/manila stable/victoria: early return for _share_replica_update() if there is no active replica https://review.opendev.org/c/openstack/manila/+/825066 | 19:02 |
*** dviroel is now known as dviroel|out | 20:20 | |
opendevreview | Carlos Eduardo proposed openstack/manila master: [WIP] Change manila generated key type to ecsda https://review.opendev.org/c/openstack/manila/+/826686 | 20:48 |
opendevreview | Carlos Eduardo proposed openstack/manila stable/victoria: Modify docker instalation for fedora systems https://review.opendev.org/c/openstack/manila/+/826621 | 20:50 |
opendevreview | Carlos Eduardo proposed openstack/manila stable/victoria: Adapt CephFS driver to do not try to escape export ip https://review.opendev.org/c/openstack/manila/+/826622 | 20:51 |
opendevreview | Merged openstack/manila master: Fix note in the share manager https://review.opendev.org/c/openstack/manila/+/824652 | 21:10 |
opendevreview | Merged openstack/manila stable/xena: [doc] Fix config and install guide for the generic driver https://review.opendev.org/c/openstack/manila/+/817092 | 21:31 |
opendevreview | Merged openstack/manila stable/wallaby: [doc] Fix config and install guide for the generic driver https://review.opendev.org/c/openstack/manila/+/817093 | 21:52 |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!