| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-05-13 | |||
| 20:09:50 | efried | please do correct me on the thred. | |
| 20:09:56 | efried | thread | |
| 20:10:01 | tssurya | yep no problem | |
| 20:27:53 | openstackgerrit | Eric Fried proposed openstack/nova master: Remove fake_libvirt_utils from connection tests. https://review.opendev.org/642557 | |
| 20:27:54 | openstackgerrit | Eric Fried proposed openstack/nova master: Remove fake_libvirt_utils from snapshot tests. https://review.opendev.org/642558 | |
| 20:27:54 | openstackgerrit | Eric Fried proposed openstack/nova master: Remove fake_libvirt_utils from virt driver tests. https://review.opendev.org/643894 | |
| 20:27:55 | openstackgerrit | Eric Fried proposed openstack/nova master: Remove fake_libvirt_utils from libvirt imagebackend tests. https://review.opendev.org/643895 | |
| 20:27:55 | openstackgerrit | Eric Fried proposed openstack/nova master: Remove remaining vestiges of fake_libvirt_utils from unit tests. https://review.opendev.org/643896 | |
| 20:27:56 | openstackgerrit | Eric Fried proposed openstack/nova master: Remove fake_libvirt_utils users in functional testing. https://review.opendev.org/644793 | |
| 21:08:10 | sean-k-mooney | mriedem: so regarding https://review.opendev.org/#/c/653506/ https://review.opendev.org/#/c/653506/4/nova/network/neutronv2/api.py@3358 is really the only part i would proably change but i think the updated code is more or less correct. | |
| 21:09:03 | sean-k-mooney | the pci requst update code for the non migration flow i belive is correct i just got confused by the comment | |
| 21:13:22 | mriedem | ganso: figured it out | |
| 21:13:49 | ganso | yay \o/ mriedem: what was the problem? | |
| 21:14:08 | mriedem | tight coupling in _delete_allocation_after_move which was dropped in stein, | |
| 21:14:19 | mriedem | details are in https://review.opendev.org/#/c/658834/1/nova/tests/functional/test_servers.py | |
| 21:15:28 | mriedem | i think i've got a fix | |
| 21:15:54 | mriedem | your persistence has paid off | |
| 21:16:56 | mriedem | ganso: so if you want, i can cleanup your rocky-only test with the proper context and such in the commit message, make the test assert the bug, and then stack the fix on top | |
| 21:17:08 | mriedem | and also explain why it's not a problem in stein | |
| 21:18:44 | ganso | mriedem: hmmm would the fix be a) or b) ? I am not sure I understand which case (a) does not cover | |
| 21:20:27 | ganso | mriedem: as far as I understand, we would always have a migration-based allocation if confirm_migration ever fails | |
| 21:22:09 | mriedem | the fix i'm going with is (b) | |
| 21:22:23 | mriedem | which is safe for rocky and queens | |
| 21:22:47 | mriedem | migration-based allocation during resize means that conductor moves the source node allocations to the migration record, which was new in queens and only happens iff the computes in the migration are new enough to handle that logic | |
| 21:22:57 | mriedem | which might not be the case if you're doing a rolling upgrade in queens | |
| 21:23:09 | mriedem | so (b) is safer i guess for backports | |
| 21:23:33 | mriedem | it's also how that code is meant to work assuming you hit _delete_allocation_after_move without driver.confirm_migration failing | |
| 21:25:52 | mriedem | so i guess we just need to figure out what to do about your recreate test | |
| 21:26:14 | ganso | mriedem: hmm I understand now | |
| 21:27:28 | mriedem | i think we could land your test in master, backport to stein, and backport to rocky but in rocky we lay the rocky-only fix on it (and then backport that to queens as well), or we change the test patch in rocky to assert it fails until we have a rocky-only fix on top | |
| 21:27:53 | ganso | mriedem: so, about the test in master, I am not sure it is 100% the same. If it is the same, we would need to make a lot of changes to backport it to rocky | |
| 21:28:17 | mriedem | you already made those changes right? and it's not the same as the point i pointed out | |
| 21:28:32 | mriedem | it's close, but in the existing test in master the PUT /allocations call to placement is stubbed to fail with a 409 | |
| 21:28:46 | mriedem | so not really what we'd want for asserting this fix in rocky/queens | |
| 21:28:54 | mriedem | *not the same as the test i pointed out | |
| 21:28:57 | ganso | mriedem: oh sorry, just saw your response! great we can merge it then =D | |
| 21:30:19 | mriedem | yeah given https://review.opendev.org/#/c/652153/ hasn't merged in queens, we can hold that up until your recreate test is ready in queens | |
| 21:30:55 | ganso | yes | |
| 21:31:29 | ganso | mriedem: ok sounds like a good plan, I can improve the rocky patch once the stein backport lands, so it makes sense | |
| 21:31:39 | mriedem | ack ok | |
| 21:31:52 | mriedem | dansmith: can you hit this easy test-only patch? https://review.opendev.org/#/c/657870/ | |
| 21:32:26 | mriedem | actually hold up | |
| 21:33:07 | mriedem | ganso: one thing in https://review.opendev.org/#/c/657870/ - you can remove that self.failed_host thing right? | |
| 21:33:08 | mriedem | it's not used | |
| 21:33:14 | mriedem | must have been copy/paste | |
| 21:33:39 | ganso | mriedem: yea was copy paste | |
| 21:33:45 | mriedem | ok remove that and i'm +2 | |
| 21:33:46 | ganso | mriedem: will remove and push an update | |
| 21:33:50 | ganso | mriedem: great =D | |
| 21:33:56 | ganso | mriedem: doing that now | |
| 21:34:58 | mriedem | sean-k-mooney: so on https://review.opendev.org/#/c/653506/4/nova/network/neutronv2/api.py@3358 - what do you want to do instead? just remove the try/except and let the PortBindingActivationFailed raise up? if so i'm cool with that. | |
| 21:35:46 | sean-k-mooney | mriedem: i was just responding. either that or as i previous said in stead of just doing the update delete the inactive port bidning then leave it fall back to updating the souce binding | |
| 21:36:16 | openstackgerrit | Rodrigo Barbieri proposed openstack/nova master: Add functional confirm_migration_error test https://review.opendev.org/657870 | |
| 21:36:30 | mriedem | sean-k-mooney: just letting that raise up is easier | |
| 21:36:46 | mriedem | this is all workaround for weird edge casing anyway | |
| 21:36:57 | sean-k-mooney | ya that is ture. | |
| 21:37:04 | mriedem | true even | |
| 21:37:10 | sean-k-mooney | :) | |
| 21:37:16 | mriedem | i'll just do that then | |
| 21:37:32 | mriedem | dansmith: ok i'm +2 on https://review.opendev.org/#/c/657870/ | |
| 21:38:04 | sean-k-mooney | cool in which case the last test can just be an assertRaises | |
| 21:38:07 | mriedem | dansmith: tl;dr i had fixed an allocation leak in master and stein but ganso found it it didn't really fix the bug before stein b/c of some code refactor stuff that was missed in the backport | |
| 21:38:33 | dansmith | okay | |
| 21:38:34 | mriedem | the test is used to recreate the bug in rocky and queens https://review.opendev.org/#/q/I9d6478f492351b58aa87b8f56e907ee633d6d1c6 | |
| 21:45:13 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Handle inactive port binding in _update_port_binding_for_instance https://review.opendev.org/653506 | |
| 21:52:58 | openstackgerrit | Matt Riedemann proposed openstack/nova stable/stein: Add functional confirm_migration_error test https://review.opendev.org/658929 | |
| 21:53:01 | mriedem | ganso: there is the stein backport ^ | |
| 21:54:19 | ganso | mriedem_afk: cool, ok! I will improve the text message of the other ones | |
| 22:00:20 | aspiers | is it just me or is https://etherpad.openstack.org/p/nova-ptg-train borked? is that why there was a https://etherpad.openstack.org/p/nova-ptg-train-2 ? but that seems borked too | |
| 22:03:10 | openstackgerrit | Rodrigo Barbieri proposed openstack/nova stable/rocky: Add functional confirm_migration_error test https://review.opendev.org/658834 | |
| 22:07:50 | sean-k-mooney | aspiers: -4 was working last week | |
| 22:09:24 | sean-k-mooney | looks like that is broken too. efried has an offline copy. i have one on my personal laptop too | |
| 22:09:30 | aspiers | sean-k-mooney: yikes, etherpad zombies everywhere | |
| 22:09:33 | sean-k-mooney | we proably should jsut email it to the mailing list | |
| 22:09:38 | aspiers | did it just get too big for etherpad to handle? | |
| 22:09:41 | efried | I made -5 last week | |
| 22:09:52 | aspiers | does the infra team know about this? | |
| 22:10:01 | sean-k-mooney | efried: -5 is not loading either | |
| 22:10:01 | efried | aspiers: I've seen way bigger etherpads that have survived, so I doubt it's the size. | |
| 22:10:22 | aspiers | yeah, -5 not loading for me either | |
| 22:10:58 | sean-k-mooney | there is proably some content in it that it does not like | |
| 22:11:01 | efried | urm, looks like I might have killed my open window to -5 | |
| 22:11:16 | aspiers | I've just notified infra | |
| 22:11:21 | efried | was there anything in particular you needed from there? | |
| 22:11:21 | sean-k-mooney | i have a copy i saved offline | |
| 22:11:30 | sean-k-mooney | i can email it to the list | |
| 22:12:38 | sean-k-mooney | if i was to guess at anything i woudl guess it was the non ascii chater that was added to the start of the etherpad | |
| 22:23:59 | sean-k-mooney | aspiers: i sent a mail to the list with subject " [nova][ptg] main etherpad backup" | |
| 22:24:54 | sean-k-mooney | which is apparently held for moderator apporval becasue apprently its over 40KB... | |
| 22:25:05 | sean-k-mooney | this is 2019 right | |
| 22:25:37 | openstackgerrit | Eric Fried proposed openstack/nova master: WIP: TC Vision Reflection https://review.opendev.org/658932 | |
| 22:30:16 | sean-k-mooney | efried: am i missing somthing or is the reflect missing form ^ | |
| 22:30:34 | efried | sean-k-mooney: You mean did I forget to git-add | |
| 22:30:35 | efried | yes | |
| 22:30:37 | efried | yes I did. | |
| 22:30:40 | efried | thanks | |
| 22:30:48 | sean-k-mooney | ah ok :) | |
| 22:30:55 | efried | doesn't do me much good to "back up my work" and "save my place" if I don't upload the freakin document. | |
| 22:31:10 | openstackgerrit | Eric Fried proposed openstack/nova master: WIP: TC Vision Reflection https://review.opendev.org/658932 | |
| 22:31:36 | efried | sean-k-mooney: ^ | |