| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2022-08-26 | |||
| 16:15:34 | dansmith | looks like the author dropped some of them from a previous version? | |
| 16:15:48 | sean-k-mooney | on hte user data patch lets see | |
| 16:16:04 | dansmith | yeah | |
| 16:16:17 | sean-k-mooney | ... i jsut realsied i review version 8 not 9 just now | |
| 16:16:21 | sean-k-mooney | and yes | |
| 16:16:25 | sean-k-mooney | https://review.opendev.org/c/openstack/nova/+/816157/8..9 | |
| 16:16:38 | sean-k-mooney | there were more removed then added | |
| 16:17:26 | sean-k-mooney | im stongly tempeted to say we shoudl swap those two if the author of the user data one has not updated it by monday | |
| 16:17:58 | sean-k-mooney | im tempeted to say we do it now but its kind of a pain to update all the micorversion refrences | |
| 16:20:46 | sean-k-mooney | ya so it passed on 8 https://review.opendev.org/c/openstack/nova/+/816157/9#message-87c965d558c8c7b25b2c4d1ebd3232c2627f444a and likely because of os-traits not being released they did not realise it failed on v9 | |
| 16:20:59 | sean-k-mooney | thats a pian | |
| 16:21:03 | dansmith | yeah | |
| 17:07:58 | opendevreview | Merged openstack/nova master: Retry /reshape at provider generation conflict https://review.opendev.org/c/openstack/nova/+/851358 | |
| 17:36:38 | dansmith | gibi: still around? | |
| 17:37:06 | gibi | dansmith: yes, but with limited brain power | |
| 17:37:55 | dansmith | gibi: on the error path.. if we created the new attachment, saved it to the bdm, but failed to delete the old one, is there any reason not to just keep rolling? | |
| 17:38:08 | dansmith | like what does restoring the old attachment_id and aborting get us there? | |
| 17:38:35 | sean-k-mooney | so the db save failed | |
| 17:38:41 | dansmith | no | |
| 17:38:46 | dansmith | the db save of the new one completed | |
| 17:38:56 | dansmith | the delete of the old one failed (in cinderclient) | |
| 17:38:58 | sean-k-mooney | but the delete in cinder failed | |
| 17:39:01 | sean-k-mooney | ah ok | |
| 17:39:17 | sean-k-mooney | am well we could leak it but other then that i think its ok | |
| 17:39:23 | sean-k-mooney | if we have our side in order | |
| 17:39:44 | dansmith | right, we leak an attachment, but presumably we're already sunk if we fail to delete | |
| 17:39:49 | gibi | dansmith: please point me the case in the code | |
| 17:40:42 | dansmith | gibi: https://review.opendev.org/c/openstack/nova/+/820368/32/nova/compute/manager.py in L3437 | |
| 17:40:44 | sean-k-mooney | dansmith: at that point have we "bound" the new attachmet propelry and got all the connector info so we can update the xml and proceed | |
| 17:41:24 | sean-k-mooney | presuamable deleteing the attcoemnt woudl only fail if cidner was unaviable or something like that | |
| 17:41:28 | dansmith | sean-k-mooney: well this is before the reimage, but yeah, I'm saying if we've recorded the new attachment and can't delete the old one, I'm not sure it gets us much to revert our own db record to the old attachment and then try to delete the new one | |
| 17:41:49 | sean-k-mooney | ah | |
| 17:41:55 | gibi | dansmith: so the root_bdm.attachment_id points to the new attachement_id and we just deleted that in cinder | |
| 17:42:06 | dansmith | reverting to the old one and deleting the new one is a more complicated cleanup, which I can do, but I just want to make sure it's worth it | |
| 17:42:07 | gibi | by L3441 | |
| 17:42:46 | dansmith | gibi: that's what I'm saying.. I think if we hit a cinder error in L3437, we should NOT do L3441 and abort | |
| 17:43:05 | gibi | dansmith: OK, that make sense | |
| 17:43:10 | dansmith | the clientexception will catch errors on L3432 *and* 3437 | |
| 17:43:14 | gibi | keep the bdm to use the new attachment id | |
| 17:43:34 | dansmith | ++ okay, it's much cleaner (on our side) to do that, but just wanted to make sure that was reasonable | |
| 17:44:10 | gibi | but we still need to handle the fact that we might detached the volume from the guest already at 3434 | |
| 17:44:54 | sean-k-mooney | i kind of feel like it migh make more sesen to break up that try | |
| 17:45:08 | dansmith | sean-k-mooney: s'what I'm doing (and said in the review) | |
| 17:45:25 | sean-k-mooney | ack | |
| 17:45:25 | dansmith | gibi: yeah, so I'm going to delete the new attachment if we fail to save, but otherwise, I'm going to log both and the situation and plow on | |
| 17:46:33 | gibi | and we say that the instance can be recovered with a hard reboot from this case? | |
| 17:46:49 | gibi | as at that point when the save fail we removed the volume from the guest | |
| 17:47:00 | dansmith | I don't know that we need even that | |
| 17:47:06 | sean-k-mooney | so the exception.InstanceNotFound is coming form the detac potieintally or can it come form the create too | |
| 17:47:58 | sean-k-mooney | gibi: if the bdms are correct and the atachment we created on line 3432 | |
| 17:48:04 | dansmith | sean-k-mooney: only the bdm save, AFAIK | |
| 17:48:07 | sean-k-mooney | is correct then a hard reboot might work | |
| 17:48:17 | dansmith | why do we need a hard reboot? | |
| 17:48:28 | dansmith | we've fixed the instance to point to the new attachment, we can just move on with the reimage | |
| 17:48:29 | dansmith | we' | |
| 17:48:46 | dansmith | we will have leaked an attachment that cinder didn't let us delete but... that doesn't impact the instance anymore right? | |
| 17:48:51 | gibi | ahh OK | |
| 17:48:52 | gibi | I see now | |
| 17:48:59 | gibi | I thought we would abort | |
| 17:49:07 | dansmith | I wish I could push this up, but I'll rebase the userdata one if I do :/ | |
| 17:49:20 | sean-k-mooney | ya that was what i was workign my way thoguht we abort and go to error | |
| 17:49:21 | dansmith | we abort all cases, except the last delete of the old attachment | |
| 17:49:28 | sean-k-mooney | but if we move on | |
| 17:49:32 | gibi | OK, that is fine | |
| 17:49:32 | dansmith | because it doesn't matter at that point | |
| 17:49:34 | sean-k-mooney | as you suggest no reboot needed | |
| 17:49:45 | sean-k-mooney | actully | |
| 17:49:51 | sean-k-mooney | if we get instance not found form our own db | |
| 17:50:02 | sean-k-mooney | well no form libvirt | |
| 17:50:11 | sean-k-mooney | prefumably that means we raced with a delete? | |
| 17:50:17 | dansmith | on bdm save, yeah | |
| 17:50:27 | sean-k-mooney | ya so nothing to try and recover | |
| 17:51:50 | sean-k-mooney | dansmith: you can do git review -R by the way to avoid a rebase | |
| 17:52:13 | dansmith | sean-k-mooney: no, I've already rebased user data locally so I could get this rebased and ready | |
| 17:52:51 | sean-k-mooney | ah ok well it would not be the end fo the world if it was rebased. it needs to be updated for the api sample issue anyway | |
| 17:53:30 | dansmith | I just don't want to step on the other author if they already have things in the middle | |
| 17:54:15 | sean-k-mooney | ack, but yes it sounds like you can just proceed and leak the attacment and may just log it so an op could clean it up later | |
| 17:56:39 | sean-k-mooney | so whats left is gettin gth image form galnce. using that to calualte the size, setting up the wait for the external event and then calling cinder to do the reimage | |
| 17:56:51 | opendevreview | Balazs Gibizer proposed openstack/nova master: Handle PCI dev reconf with allocations https://review.opendev.org/c/openstack/nova/+/852397 | |
| 17:56:52 | opendevreview | Balazs Gibizer proposed openstack/nova master: Allow enabling PCI tracking in Placement https://review.opendev.org/c/openstack/nova/+/850468 | |
| 17:56:52 | opendevreview | Balazs Gibizer proposed openstack/nova master: Generate request_id for Flavor based InstancePCIRequest https://review.opendev.org/c/openstack/nova/+/853835 | |
| 17:56:53 | opendevreview | Balazs Gibizer proposed openstack/nova master: Create RequestGroups from InstancePCIRequests https://review.opendev.org/c/openstack/nova/+/852771 | |
| 17:56:53 | opendevreview | Balazs Gibizer proposed openstack/nova master: Support resource_class and traits in PCI alias https://review.opendev.org/c/openstack/nova/+/853316 | |
| 17:56:54 | opendevreview | Balazs Gibizer proposed openstack/nova master: Split PCI pools per PF https://review.opendev.org/c/openstack/nova/+/854440 | |
| 17:56:54 | opendevreview | Balazs Gibizer proposed openstack/nova master: Map PCI pools to RP UUIDs https://review.opendev.org/c/openstack/nova/+/854118 | |
| 17:56:55 | opendevreview | Balazs Gibizer proposed openstack/nova master: Make allocation candidates available for scheduler filters https://review.opendev.org/c/openstack/nova/+/854119 | |
| 17:56:56 | opendevreview | Balazs Gibizer proposed openstack/nova master: Filter PCI pools based on Placement allocation https://review.opendev.org/c/openstack/nova/+/854120 | |
| 17:56:56 | opendevreview | Balazs Gibizer proposed openstack/nova master: Store allocated RP in InstancePCIRequest https://review.opendev.org/c/openstack/nova/+/854121 | |
| 17:56:58 | opendevreview | Balazs Gibizer proposed openstack/nova master: Func test for PCI in placement scheduling https://review.opendev.org/c/openstack/nova/+/854122 | |
| 17:56:58 | opendevreview | Balazs Gibizer proposed openstack/nova master: Support cold migrate and resize with PCI tracking in placement https://review.opendev.org/c/openstack/nova/+/854247 | |
| 17:57:00 | opendevreview | Balazs Gibizer proposed openstack/nova master: Support evacuate with PCI in placement https://review.opendev.org/c/openstack/nova/+/854615 | |
| 17:57:00 | opendevreview | Balazs Gibizer proposed openstack/nova master: Support unshelve with PCI in placement https://review.opendev.org/c/openstack/nova/+/854616 | |
| 17:57:02 | opendevreview | Balazs Gibizer proposed openstack/nova master: Support same host resize with PCI in placement https://review.opendev.org/c/openstack/nova/+/854441 | |
| 17:57:02 | opendevreview | Balazs Gibizer proposed openstack/nova master: Test reschedule with PCI in placement https://review.opendev.org/c/openstack/nova/+/854626 | |
| 17:57:04 | opendevreview | Balazs Gibizer proposed openstack/nova master: Support multi create with PCI in placement https://review.opendev.org/c/openstack/nova/+/854663 | |
| 17:57:04 | opendevreview | Balazs Gibizer proposed openstack/nova master: Heal allocation for same host resize https://review.opendev.org/c/openstack/nova/+/854822 | |
| 17:58:06 | dansmith | ugh, that causes another failure later, which may be a quirk of our cinder fixture | |
| 17:58:19 | dansmith | the attachment isn't deleted, so much later it complains that the instance is double-attached | |
| 17:58:46 | sean-k-mooney | well form the cinder side i guess it is | |
| 17:59:04 | sean-k-mooney | we could try and recover at that point and delete the old one again | |