| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-05-13 | |||
| 19:40:06 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Add CrossCellWeigher https://review.opendev.org/614353 | |
| 19:40:06 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Support cross-cell moves in external_instance_event https://review.opendev.org/658478 | |
| 19:40:07 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Add cross-cell resize policy rule and enable in API https://review.opendev.org/638269 | |
| 19:40:07 | openstackgerrit | Matt Riedemann proposed openstack/nova master: WIP: Enable cross-cell resize in the nova-multi-cell job https://review.opendev.org/656656 | |
| 19:40:08 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Robustify attachment tracking in CinderFixtureNewAttachFlow https://review.opendev.org/658904 | |
| 19:41:11 | mriedem | dansmith: i've hit a not-so-fun snag in pre-validating the target host wrt attached volumes in the cross-cell resize stuff and i'd like to brain dump | |
| 19:41:25 | dansmith | okay | |
| 19:42:06 | mriedem | so we want to try and connect the volume on the dest host to see if it will work before starting the whole process, | |
| 19:42:22 | mriedem | to do that, i need to update the volume attachment with the dest host connector to get the connection_info dict that the virt driver uses, | |
| 19:42:58 | mriedem | the problem is, at the point i'm doing that, the source host volume attachment still exists and cinder pukes saying you essentially can't have more than one connected attachment for the same volume at the same time | |
| 19:43:26 | mriedem | which makes sense i guess, and is not something we hit in a normal move because we delete the source host attachment before finishing on the dest | |
| 19:44:02 | mriedem | granted normal resize could fail the same thing today - if you can't connect the volume on the dest after we've transferred the disks and stuff | |
| 19:44:34 | dansmith | cross-cell is actually easier in this case because you have two separate instances and attachments on the nova side | |
| 19:44:56 | dansmith | normal resize would need to stash that in case it needs to revert right? or maybe just ask cinder again.. tbh, I dunno how that attachment stuff works really :) | |
| 19:45:28 | mriedem | attachments in cinder are essentially ephemeral, we can create and delete them as needed, or that's the idea anyway | |
| 19:45:39 | mriedem | so if i want to make this work, | |
| 19:46:09 | openstackgerrit | Merged openstack/python-novaclient master: Microversion 2.73: Support adding the reason behind a server lock https://review.opendev.org/648659 | |
| 19:46:09 | mriedem | i'd need to do something like delete the source host attachment, update the attachment with the dest host connector so we can test the connection on the dest host, | |
| 19:46:39 | mriedem | and then if something fails and we're rolling back, i'd have to create a new attachment and complete it for the source host (using the source host connector, which we don't stash away) | |
| 19:46:48 | dansmith | which entity is going to give you the go/no-go on the new attachment? cinder? brick? libvirt? | |
| 19:46:52 | mriedem | so not impossible, but kinda gross | |
| 19:47:38 | mriedem | updating the attachment with the host connector is what creates the exports in the storage backend | |
| 19:47:45 | mriedem | and gives us back the connection_info dict | |
| 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 | mriedem | we are not using sharing providers for any of this, or anything | |
| 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: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 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 | |