| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2022-08-26 | |||
| 15:13:48 | dansmith | sigh | |
| 15:13:56 | sean-k-mooney | im not sure it would take much to add ironci supprot but testin gwould be hard | |
| 15:14:23 | sean-k-mooney | the ironic specific behavior is that it has an api parmater that allows the ephemerl disk to not be erased when you rebild with a new image | |
| 15:14:47 | sean-k-mooney | we could support that for libvirt too but we dotn since the default impl does not support it | |
| 15:15:14 | dansmith | SIGH | |
| 15:16:17 | dansmith | so yeah, so much of rebuild is done by ironic, perhaps just refusing to do rebuild on bfv if we're instructed to wipe the root disk is the right approach | |
| 15:16:31 | dansmith | but we'll really need that implemented | |
| 15:17:09 | sean-k-mooney | well we can check that in the api fairly simplely | |
| 15:17:18 | sean-k-mooney | we can check the hypervior type i belive | |
| 15:17:38 | dansmith | which would be terrible | |
| 15:17:40 | sean-k-mooney | and just reject it with a 400 | |
| 15:17:55 | dansmith | we should not have the API behaving differently depending on the virt driver | |
| 15:18:17 | sean-k-mooney | it also should not behave differntly based on if the instnace is boot form volume | |
| 15:18:18 | dansmith | that's compute stuff, and while it sucks to fail late like that, I much prefer that than building virt-specific stuff into the api | |
| 15:18:22 | sean-k-mooney | but that the situration we are in | |
| 15:18:45 | sean-k-mooney | due to the hacky (only update metadat if the image is the same and its BFV) legacy | |
| 15:19:03 | dansmith | the api has to behave differently for bfv because it historically did, but we shouldn't be building *new* stuff that drags virt specifics into the api | |
| 15:19:26 | sean-k-mooney | ack i agree with that | |
| 15:19:38 | sean-k-mooney | the preserve_ephemeral filed looks like it predates microveriosn by the way | |
| 15:19:45 | dansmith | basically all the functionals on that user data patch fail because of that missing trait, | |
| 15:19:47 | sean-k-mooney | i was trying to figure out when we added that | |
| 15:20:06 | dansmith | which makes it hard to work on the functional tests | |
| 15:20:13 | sean-k-mooney | i belvie the pased in the previsous version of the patch that did not have the trait | |
| 15:20:40 | sean-k-mooney | locally we can just pip install the os-traits git repo | |
| 15:20:51 | sean-k-mooney | but ya that why i was hopign to do the release yesterday | |
| 15:20:53 | dansmith | yeah, I'll have to do that | |
| 15:21:37 | sean-k-mooney | elodilles: any chance we can merge https://review.opendev.org/c/openstack/releases/+/854617 | |
| 15:21:49 | sean-k-mooney | to adress ^ | |
| 15:22:00 | elodilles | sean-k-mooney: ack, will look into it | |
| 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 | 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 | |