| *** amoralej|off is now known as amoralej | 07:04 | |
| opendevreview | Hervé Beraud proposed openstack/python-cinderclient master: Remove python-dev from bindep https://review.opendev.org/c/openstack/python-cinderclient/+/863842 | 10:04 |
|---|---|---|
| *** dviroel|out is now known as dviroel | 10:16 | |
| opendevreview | Rajat Dhasmana proposed openstack/cinder master: Correct help text of target configs https://review.opendev.org/c/openstack/cinder/+/859191 | 10:16 |
| opendevreview | Jean Pierre Roquesalane proposed openstack/cinder master: Dell PowerFlex: Additionnal params for enabling self signed certificates https://review.opendev.org/c/openstack/cinder/+/858370 | 13:00 |
| *** amoralej is now known as amoralej|lunch | 13:20 | |
| *** amoralej|lunch is now known as amoralej | 14:12 | |
| *** dviroel is now known as dviroel|lunch | 15:08 | |
| opendevreview | Rajat Dhasmana proposed openstack/cinder master: Add storage_protocol to FS drivers https://review.opendev.org/c/openstack/cinder/+/852725 | 16:01 |
| *** sfinucan is now known as stephenfin | 16:10 | |
| *** amoralej is now known as amoralej|off | 16:13 | |
| hemna- | I have found an odd failure. using the latest cinderclient released 9.10 against wallaby. I can't create a volume snapshot. it worked against train | 16:19 |
| hemna- | the openstack client works! | 16:19 |
| hemna- | https://paste.openstack.org/show/bFgOix4jYvOJRkptI8QY/ | 16:20 |
| hemna- | the cinderclient's default for the force flag is None, which turns into a string value of 'null' inside the cinder api, which fails schema validation | 16:21 |
| hemna- | The API LOG --> Action Body: {"snapshot": {"volume_id": "6ba26261-6f02-4cab-8d96-6febcbbd89b3", "force": null, "name": "walt-wallaby", "description": null, "metadata": {}}} | 16:22 |
| eharney | that seems bad :/ | 16:22 |
| eharney | i think we had a cinderclient bug very similar to this a little bit ago, but i forget on what operation | 16:22 |
| hemna- | https://github.com/openstack/python-cinderclient/blob/9.1.0/cinderclient/v3/shell.py#L2203-L2221 | 16:24 |
| eharney | yeah i was just eyeballing that same code in https://review.opendev.org/c/openstack/python-cinderclient/+/806817/ | 16:24 |
| hemna- | client downgraded the microversion to 3.65 | 16:24 |
| eharney | maybe the default=None doesn't do what was expected there | 16:25 |
| hemna- | yah I'll try hacking it and see what happens | 16:25 |
| hemna- | not sure why that default=None is right there ever | 16:26 |
| hemna- | https://github.com/openstack/python-cinderclient/blob/9.1.0/cinderclient/v3/volume_snapshots.py#L92-L93 | 16:26 |
| hemna- | the default should always be the same as that definition, which is False | 16:27 |
| hemna- | weird. the shell.py 3.66 microversion is what is called | 16:30 |
| hemna- | https://github.com/openstack/python-cinderclient/blob/9.1.0/cinderclient/v3/shell.py#L2292 | 16:30 |
| hemna- | that's what is called from shell.py | 16:30 |
| hemna- | adding a default=False to the definition of --force there fixed it | 16:31 |
| eharney | i think the idea of None was to have it not send the parameter to the API | 16:32 |
| eharney | sending "description": null seems fishy too | 16:32 |
| hemna- | https://pastebin.com/7gAVuFKs | 16:35 |
| *** dviroel|lunch is now known as dviroel | 16:35 | |
| hemna- | I tried using openstack paste to make that paste but it gave me an error every time | 16:35 |
| hemna- | anyway, you can see my patch against the client in that paste and the 2 attempts | 16:35 |
| hemna- | whats odd is the 3.66 do_snapshot_create is what is called, but the @api_versions.wraps("3.0", "3.65") is what is used | 16:36 |
| hemna- | that seems really weird | 16:36 |
| hemna- | I guess I'll put up a bug fix patch for it | 16:39 |
| hemna- | also this | 16:44 |
| hemna- | https://github.com/openstack/python-cinderclient/blob/master/cinderclient/v3/shell.py#L2309 | 16:44 |
| hemna- | bah | 16:51 |
| hemna- | if I set a default in the definition, I get a tox error ValueError: Since microversion 3.66 of the Block Storage API, the 'force' option is invalid for this request. For backward compatibility, however, when the 'force' flag is passed with a value evaluating to True, it is silently ignored. | 16:54 |
| hemna- | hrmm, so I set the default to True? | 16:57 |
| hemna- | I'm not sure what the 'correct' way to fix this is | 16:59 |
| opendevreview | Walt proposed openstack/python-cinderclient master: Fix passing None/null to snapshot create https://review.opendev.org/c/openstack/python-cinderclient/+/863923 | 18:57 |
| dansmith | rosmaita: geguileo: am I reading correctly that there's no job for me to see this working: https://review.opendev.org/c/openstack/devstack/+/814193 | 19:09 |
| dansmith | specifically because it doesn't work? | 19:09 |
| rosmaita | dansmith: that's right | 19:11 |
| rosmaita | we need this in place to test the code that isn't working yet | 19:12 |
| dansmith | is it worth running all the nova and glance jobs against this to make sure none of those other cinder jobs break? | 19:13 |
| dansmith | and/or did you do a DNM patch against cinder to make sure all the known cinder configs still work? | 19:13 |
| dansmith | there's enough changing there, and even (almost) trivially touching nova, so .. seems worth the diligence to make sure we don't land this and find out other projects have broken jobs | 19:14 |
| dansmith | if so I can do the DNMs against glance and nova | 19:14 |
| dansmith | rosmaita: ^ | 19:15 |
| rosmaita | dansmith: (in a meeting, looking) | 19:16 |
| rosmaita | dansmith: yeah, i guess the patches on that patch aren't enough | 19:21 |
| dansmith | rosmaita: np, I put up the glance/nova changes just to be extra careful | 19:21 |
| rosmaita | cool | 19:21 |
| dansmith | if they pass, I'll +2 | 19:22 |
| rosmaita | i'll put up a cinder patch, just to make sure the old config opts are respected | 19:22 |
| dansmith | probably a good plan :) | 19:22 |
| rosmaita | thanks! | 19:22 |
| dansmith | \np | 19:22 |
| whoami-rajat | rosmaita, dansmith actually we've a job to see this working https://review.opendev.org/c/openstack/cinder-tempest-plugin/+/855576 | 19:24 |
| rosmaita | whoami-rajat: thanks, i couldn't find that patch | 19:24 |
| rosmaita | except, it's failing on that patch | 19:25 |
| whoami-rajat | rosmaita, yeah i think there are still things to be figured out, tosky would know better | 19:25 |
| opendevreview | Brian Rosmaita proposed openstack/cinder master: DNM: test LVM NVMe support config patch https://review.opendev.org/c/openstack/cinder/+/863927 | 19:26 |
| dansmith | whoami-rajat: cool, but at a minimum we want to make sure it doesn't break everything else I think | 19:26 |
| whoami-rajat | dansmith, agreed, +1 for testing it against glance and nova | 19:27 |
| dansmith | ++ | 19:28 |
| *** dviroel is now known as dviroel|afk | 19:40 | |
| opendevreview | Eric Harney proposed openstack/python-cinderclient master: Add test coverage for shell snapshot-create w/ metadata https://review.opendev.org/c/openstack/python-cinderclient/+/863299 | 19:47 |
| tosky | rosmaita, dansmith : I had a testing job https://review.opendev.org/c/openstack/cinder-tempest-plugin/+/855576 | 20:19 |
| tosky | whoami-rajat: ^^ | 20:19 |
| tosky | it depends on a long stack of patches but it seems to be doing something | 20:20 |
| tosky | only a few tests were failing in the last iteration | 20:20 |
| rosmaita | tosky: ack | 20:21 |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!