| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2022-08-26 | |||
| 15:22:10 | opendevreview | Merged openstack/nova master: Add locked_memory extra spec and image property https://review.opendev.org/c/openstack/nova/+/778347 | |
| 15:22:15 | sean-k-mooney | elodilles++ thanks very much :) | |
| 15:24:54 | elodilles | sean-k-mooney: do you need to release it ASAP? I'm asking because if really needed then I'll +W with single +2 (i'm the only release manager on duty today o:)) otherwise we need to wait till Monday | |
| 15:27:45 | sean-k-mooney | am ideally yes. it could wait but we have 2 api changes blocked by it currently. | |
| 15:28:11 | sean-k-mooney | its at your disgression ultimately dansmith or gibi might have a stonger prefernce | |
| 15:28:52 | gibi | I'm fine releasing that today with a single release mgmt +2 | |
| 15:29:07 | dansmith | yeah, it'd be a lot better if it could be today, unfortunately | |
| 15:29:45 | gibi | PTO + FF is not a good combination | |
| 16:06:05 | elodilles | sean-k-mooney gibi dansmith: ack, os-traits release is on the way | |
| 16:06:13 | dansmith | elodilles: thanks! | |
| 16:06:15 | gibi | elodilles: thank oyu | |
| 16:06:16 | gibi | you | |
| 16:06:23 | elodilles | np | |
| 16:07:09 | sean-k-mooney | once that is release we shuld bump our min required version https://github.com/openstack/nova/blob/master/requirements.txt#L56 right in the change that adds the userdata feature | |
| 16:07:56 | sean-k-mooney | like i know we will get it automaticaly but we should raise our dep in the feature that requires the new trait | |
| 16:09:15 | opendevreview | Dan Smith proposed openstack/nova master: DNM: Test for rosmaita https://review.opendev.org/c/openstack/nova/+/854815 | |
| 16:10:02 | opendevreview | Dan Smith proposed openstack/nova master: DNM: Test for rosmaita https://review.opendev.org/c/openstack/nova/+/854815 | |
| 16:10:55 | gibi | the gate queue is 53 patches long, nice | |
| 16:12:18 | elodilles | :-o | |
| 16:12:42 | sean-k-mooney | oh the oauth 2 feature is mergin in keystone | |
| 16:12:54 | sean-k-mooney | thats cool | |
| 16:15:09 | dansmith | sean-k-mooney: I'm getting functional fails on missing api samples for 2.93 | |
| 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 | 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:45:25 | sean-k-mooney | ack | |
| 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 | dansmith | because it doesn't matter at that point | |
| 17:49:32 | gibi | OK, that is fine | |
| 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 | |