Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-05-13
19:58:28 dansmith so if we're going to do something different, implement this test for regular and cross-cell, and assume they're the same until then, IMHO
19:58:39 tssurya efried: heh was just about to reply to that thread :)
19:59:00 mriedem tssurya: since you're around, does cern use the same cinder for all of its cells? or most anyway?
19:59:02 efried please do tssurya. I'm sure you can provide better insight.
19:59:19 mriedem s/cinder/volume backend/
19:59:21 dansmith mriedem: do you mean the same cinder, or the same backend?
19:59:23 sean-k-mooney dansmith: mriedem for normal resize do we pass a member of parmater to ensure we select a host that is attache to the same sharign resouce provider
19:59:24 dansmith yeah :)
19:59:34 dansmith the latter, I'm sure not, but that's what cross-az-attach is for
19:59:38 tssurya mriedem: cinder for specific cells as far as I know
19:59:50 tssurya but yes same cinder
20:00:27 mriedem i only know of 2 users of cross_az_attach=False (NeCTAR and Target)
20:00:54 tssurya efried: yea what they are saying is true though, just making resource_provider_association_refresh to large value doesn't still solve it for ironic
20:01:24 tssurya we have been having the same problem and mostly might end up adding more computes
20:01:59 tssurya the periodic task locks really long enough (sometimes ~20mins) and doesn't allow boot request to walk past it and we get stuff stuck in scheduling
20:02:26 dansmith mriedem: you mean only two that speak up often or what? we definitely have users of it and are prescribing it for all edge deploys with cinder of course
20:02:58 tssurya so its interesting to see if for them setting update_resources_interval stays fine or not (in terms of things going out of sync)
20:03:00 mriedem right, i only saw target people mention it in a talk once
20:05:54 mriedem well i've commented out the trouble code for now and will have a gate run with a volume-backed cross-cell resize so i'll see if i run into any other bumps along the way; if the only issues with volume-backed cross-cell resize is this pre-validation thing, i'll probably just drop it.
20:09:34 efried tssurya: oh, okay, I thought I might be on the wrong track with refresh interval
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

Earlier   Later