| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-02-14 | |||
| 19:12:27 | mriedem | here is an easy example test https://review.openstack.org/#/c/543257/ | |
| 19:12:29 | melwitt | yeah, I mean, in previous revisions I didn't need it | |
| 19:12:46 | mnaser | i think `test_delete_error_state_with_no_host` covers this case, which makes sure that local_delete is actually being called (and local delete does clean up bdms) | |
| 19:13:33 | mriedem | mnaser: that's a unit test | |
| 19:13:48 | mriedem | we really should have a functional regression test for this, given the various service interactions and cell0 stuff | |
| 19:14:08 | mnaser | is there an example of functional regression tests involving other services (i.e. cinder) | |
| 19:14:17 | mnaser | i dont mind trying to write one if that helps | |
| 19:14:17 | mriedem | https://review.openstack.org/#/c/543257/ | |
| 19:16:17 | mnaser | melwitt: if you have other things at hand, i can write a functional regression test for this right now | |
| 19:17:12 | melwitt | mnaser: sure, sounds good, thanks for the help | |
| 19:17:40 | mnaser | ok perfect, i'll work on it locally and then i can tack it on top of the change | |
| 19:21:02 | mriedem | ideally the regression test patch comes before the fix, | |
| 19:21:13 | mriedem | to show that the volume is still 'attaching' even after we delete the instance | |
| 19:21:30 | mriedem | then mel's patch goes on top of that and changes the test to show the volume is properly unreserved | |
| 19:21:59 | mriedem | the cinder fixture we have should be able to give you the volume state tracking and API support for figuring that out | |
| 19:24:56 | mnaser | https://git.openstack.org/cgit/openstack/nova/tree/nova/tests/functional/regressions/test_bug_1675570.py | |
| 19:25:03 | mnaser | well this is nice to have, heh | |
| 19:26:13 | openstackgerrit | Dan Smith proposed openstack/nova-specs master: Add placement-req-filter spec https://review.openstack.org/544585 | |
| 19:33:02 | mriedem | mnaser: yeah you could probably build on that with a bfv scenario, and nix the shelve offload part | |
| 19:36:18 | openstackgerrit | Dan Smith proposed openstack/nova-specs master: Add placement-req-filter spec https://review.openstack.org/544585 | |
| 19:37:36 | mriedem | dansmith: couple small docs issues and then i'm +2 | |
| 19:38:50 | mriedem | i really like the idea of using this for multiattach bfv | |
| 19:39:07 | mriedem | assuming people don't hate the idea of storing the compute driver capabilities as traits | |
| 19:39:23 | mriedem | we can also use it for tagged bfv | |
| 19:39:27 | mriedem | which artom should like | |
| 19:40:17 | artom | I hope I wouldn't be the only one | |
| 19:40:30 | artom | It seems that sort of thing has come up for other problems | |
| 19:40:39 | dansmith | mriedem: jaypipes was also saying this will let us clean up the pci request -> nets mapping thing, which sounds nice | |
| 19:40:44 | dansmith | I have to go dig back into that stuff to see though | |
| 19:41:23 | openstackgerrit | Dan Smith proposed openstack/nova-specs master: Add placement-req-filter spec https://review.openstack.org/544585 | |
| 19:45:10 | openstackgerrit | Jackie Truong proposed openstack/nova master: Implement certificate_utils https://review.openstack.org/479949 | |
| 19:45:10 | openstackgerrit | Jackie Truong proposed openstack/nova master: Add trusted_certs object https://review.openstack.org/489408 | |
| 19:45:10 | openstackgerrit | Jackie Truong proposed openstack/nova master: Add trusted_certs to instance_extra https://review.openstack.org/537897 | |
| 19:45:11 | openstackgerrit | Jackie Truong proposed openstack/nova master: Add trusted_image_certificates to REST API https://review.openstack.org/486204 | |
| 19:49:51 | mriedem | jackie should be in irc | |
| 19:50:02 | openstackgerrit | melanie witt proposed openstack/nova-specs master: Re-propose convert consoles code to use objects framework https://review.openstack.org/543662 | |
| 19:53:17 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Remove old flavor_get db api method https://review.openstack.org/544621 | |
| 19:54:44 | dansmith | mriedem: related to ^, I'll remove aggregate-in-cell-db compatibility before I add the methods to AggregateList that I need | |
| 19:54:55 | dansmith | just so you don't do it if you were looking for deprecations to remove | |
| 19:59:00 | mriedem | dansmith: i had just been slowly working on this list https://github.com/openstack/nova/blob/master/nova/cmd/manage.py#L385 | |
| 19:59:05 | mriedem | last thing i removed was the flavor compat | |
| 19:59:26 | mriedem | or maybe it was keypairs, don't remmeber | |
| 19:59:30 | dansmith | mriedem: okay well, I've got the two aggregate ones in my sights | |
| 19:59:44 | mriedem | ok wfm, | |
| 19:59:50 | mriedem | we can definitely remove these old newton compat things | |
| 19:59:55 | mriedem | s/compat/migration/ | |
| 20:00:02 | dansmith | yeah | |
| 20:02:23 | openstackgerrit | melanie witt proposed openstack/nova master: Add periodic task to clean expired console tokens https://review.openstack.org/325381 | |
| 20:02:24 | openstackgerrit | melanie witt proposed openstack/nova master: Convert websocketproxy to use db for token validation https://review.openstack.org/333990 | |
| 20:02:24 | openstackgerrit | melanie witt proposed openstack/nova master: Use ConsoleAuthToken object to generate authorizations https://review.openstack.org/325414 | |
| 20:03:46 | mriedem | anyone want to send this home and the one below it? https://review.openstack.org/#/c/544066/ - the cinder admin guide patch for multiattach is dependent on this | |
| 20:04:00 | mriedem | i think we should probably backport that to queens too | |
| 20:07:44 | mnaser | hey .. is this a bug in the cinder fixtures? https://github.com/openstack/nova/blob/master/nova/tests/fixtures.py#L1390-L1394 .. shouldn't it modify volume['attach_status'] and not volume['status']? | |
| 20:07:59 | mnaser | so it should be volume['status'] = 'in-use' if has_attachment else 'available' | |
| 20:08:07 | mnaser | volume['attach_status'] = 'attached' if has_attachment else 'detached' | |
| 20:08:09 | mriedem | jogo: can you remove your -2 on this? https://review.openstack.org/#/c/159382/1 | |
| 20:08:29 | mnaser | my little functional test so far is failing because: HTTP exception thrown: Invalid volume: volume 'f8580fcb-b22d-4e50-b69a-d3d91f8f3a6b' status must be 'available'. Currently in 'detached' | |
| 20:08:33 | mriedem | mnaser: yes | |
| 20:08:51 | mnaser | ok, ill fix that first | |
| 20:08:53 | mriedem | mnaser: same problem here https://github.com/openstack/nova/blob/master/nova/tests/fixtures.py#L1574 | |
| 20:09:14 | melwitt | mnaser: is that what I proposed here? https://review.openstack.org/#/c/539758/ | |
| 20:09:28 | mnaser | melwitt: exactly that | |
| 20:09:38 | mnaser | ill rebasemy work on that, thank you | |
| 20:09:42 | mriedem | lemme look, we could rebase quick | |
| 20:09:50 | melwitt | I ran into the same problem recently working on func tests | |
| 20:09:58 | mnaser | melwitt: do we want to add in-use for volume status too? | |
| 20:10:22 | mnaser | but that might be a bit more work | |
| 20:10:36 | mnaser | because im not sure what the status is when using multiattach (in-use or available?) | |
| 20:10:51 | melwitt | mnaser: we might. I didn't try to go further with changing the fixture | |
| 20:10:59 | mriedem | i just left that comment | |
| 20:11:05 | mriedem | if there are attachments, the status should be at least 'in-use' | |
| 20:11:11 | melwitt | once it's attached, it should be 'in-use' from what I understand | |
| 20:11:18 | mriedem | it might not be technically correct, but it's more correct than saying 'available' | |
| 20:11:25 | mnaser | yeah, makes sense | |
| 20:11:47 | mriedem | with the new fixture, there can be 'empty' attachments, which puts the volume status in 'attaching' status, and the volume doesn't go to in-use until the attachment is 'completed' | |
| 20:11:59 | mriedem | in the old fixture, completed means calling os-attach | |
| 20:12:18 | mriedem | but we likely don't need to worry about those details in these fixtures | |
| 20:12:31 | melwitt | mnaser: you want to add 'in-use' and 'available' to that patch while you're rebasing? | |
| 20:12:39 | mnaser | melwitt: sure, i can do that | |
| 20:12:44 | melwitt | cool, thanks | |
| 20:13:15 | mriedem | actually, the loop above deals with this already | |
| 20:13:19 | mriedem | so likely don't need a change | |
| 20:14:19 | mriedem | i don't even know why we have this block of code now | |
| 20:14:24 | mriedem | the loop above handles both of those status values | |
| 20:16:01 | openstackgerrit | Peter Hamilton proposed openstack/nova-specs master: Add support for certificate validation https://review.openstack.org/540879 | |
| 20:20:30 | openstackgerrit | Jay Pipes proposed openstack/nova-specs master: Account for host agg allocation ratio in placement https://review.openstack.org/544683 | |
| 20:25:46 | openstackgerrit | Eric Berglund proposed openstack/nova master: WIP: PowerVM Driver: Snapshot https://review.openstack.org/543023 | |
| 20:30:53 | cfriesen | mriedem: are you aware of any work going on as far as live migration monitoring? (related to the broken progress monitor) | |
| 20:31:12 | mnaser | sorry for the questions, is there a way to turn on debug logging for functional tests? | |
| 20:32:34 | mriedem | mnaser: run with OS_DEBUG=True | |
| 20:32:40 | mnaser | mriedem: thank you | |
| 20:32:41 | mriedem | cfriesen: nope | |
| 20:40:39 | mriedem | jaypipes: thanks for starting https://review.openstack.org/#/c/544683/ - a few thoughts inline | |
| 20:41:04 | mriedem | will definitely want to talk with dansmith about the new upcall | |
| 20:49:14 | mriedem | efried: i moved https://blueprints.launchpad.net/nova/+spec/nested-resource-providers back to queens and completed it | |
| 20:49:22 | mriedem | let us never speak of it again | |
| 20:49:29 | efried | ack | |
| 20:49:58 | efried | Someone still owes a NRP-in-alloc-cands spec for Rocky. | |
| 20:50:10 | efried | I think I'm still on the hook for that. | |
| 20:50:24 | mriedem | you just smelled it | |