Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-05-13
19:48:09 dansmith okay, so, cinder yeah?
19:48:23 mriedem i'm honestly not sure if that's enough or not, i.e. if you still need the low-level connection on the host for libvirt/brick
19:48:48 mriedem we maybe don't need that - we don't try to plug vifs on the dest host during pre-validation
19:48:52 dansmith cinder might tell you no, or it might not know and say "sure" but then the actual attach fails I guess
19:49:13 mriedem yeah i'm not sure, that's why i was trying to do it all
19:49:53 mriedem either way i'm not disconnecting the guest / volume from the source host until we've pre-validated the dest
19:50:00 mriedem and start doing the power off / snapshot stuff
19:50:21 dansmith hmm
19:50:42 dansmith in what case is cinder going to tell you no to the attempted attachment change?
19:51:01 dansmith I mean... why do you think you need to do this special for cross-cell?
19:51:39 mriedem i was thinking in case we hit a scenario where the target cell is using a different storage backend from the source cell
19:52:05 mriedem not sure how realistic that is though
19:52:10 dansmith but I mean,
19:52:23 dansmith how is that different from single-cell resize?
19:52:51 sean-k-mooney mriedem: in an edge deployment its likely not that uncommon that they would have different backends
19:52:55 mriedem it's not really, like i said before we could hit this same thing during normal resize and we really don't try to pre-validate that (in prep_resize) or rollback from that kind of failure
19:53:25 dansmith yeah, I'm not sure what the backend difference is that you're talking about
19:53:35 dansmith in this case, the volume is in cinder, on a backend, but migrating won't change what *cinder* backend it's on, regardless of where you move it
19:54:52 sean-k-mooney dansmith: that is ture but that may mean the backen is not reachale from the new host. although if we are using sharing resouce providres to model this in placement it shoudl be reachable
19:55:09 sean-k-mooney assuming you have set up the aggreate correctly
19:55:21 dansmith sean-k-mooney: right, but that's what I'm saying has nothing to do with cross-cell migration
19:55:21 mriedem we are not using sharing providers for any of this, or anything
19:55:47 sean-k-mooney dansmith: sure
19:56:06 sean-k-mooney its not a special case that only comes up with cross cell
19:57:04 mriedem so if this is really not any different from normal resize then i'm probably bending over backwards for no good reason
19:57:26 mriedem but yeah i was thinking kind of like what sean said, the target host can't connect to the storage for the volume
19:58:05 dansmith mriedem: yep, but that's just a thing that can happen any time,
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 virt driver tests. https://review.opendev.org/643894
20:27:54 openstackgerrit Eric Fried proposed openstack/nova master: Remove fake_libvirt_utils from snapshot tests. https://review.opendev.org/642558
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:55 openstackgerrit Eric Fried proposed openstack/nova master: Remove fake_libvirt_utils from libvirt imagebackend tests. https://review.opendev.org/643895
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

Earlier   Later