Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2022-11-17
10:25:10 johnthetubaguy actually, I remember seeing some os-traits error actually
10:25:20 bauzas yeah, we hardfail on the number of traits
10:25:40 johnthetubaguy it was a missing attribute error I think, which is basically the same thing
10:25:48 bauzas cool
10:25:58 johnthetubaguy well mystery solved, thank you!
10:26:07 bauzas np, glad you fixed it by yourself :D
10:26:31 johnthetubaguy (makes bashing with a hammer noises)
10:27:06 bauzas :)
10:44:00 opendevreview John Garbutt proposed openstack/nova master: Functional test test_boot_reschedule_with_proper_pci_device_count https://review.opendev.org/c/openstack/nova/+/760354
10:44:01 opendevreview John Garbutt proposed openstack/nova master: Fix PCI passthrough race on reschedule (claims) https://review.opendev.org/c/openstack/nova/+/710847
10:44:01 opendevreview John Garbutt proposed openstack/nova master: Fix PCI passthrough race on reschedule (refresh) https://review.opendev.org/c/openstack/nova/+/710848
11:02:59 johnthetubaguy gibi: I think you reviewed those in the past, I am not sure it answers all your questions, but I put the functional test first, in an attempt to work out which patches are needed. Its a nasty bug that on re-schedule you try to get the wrong PCI device, but fail with in-use errors.
11:03:54 sean-k-mooney johnthetubaguy: so i was thinking about your ironic patch for reserving on schedule over night
11:04:04 sean-k-mooney i think in general its a good idea
11:04:32 sean-k-mooney there was some concern about if cleaning was used or not in large cloud right extendign the time they would be unavaiable
11:05:04 johnthetubaguy yeah, gibi was mentioning that, and well, I don't disagree
11:05:11 sean-k-mooney if we wanted to cater for that we coudl make this configurable but i think the its proably ok ot reserve by default or uncondtionally
11:05:39 johnthetubaguy yeah, it feels like a future workaround config, if its a problem for people
11:05:56 johnthetubaguy interestingly, I think it fixes an extra case I should add to the commit...
11:06:25 sean-k-mooney oh what one
11:07:12 johnthetubaguy when you mark an in-use node as in maintenance mode as its broken, user gets to delete their instance when they are ready, and that goes into clean failed (depending on your ironic config), we don't hit the race with our placement updates any more either
11:07:27 johnthetubaguy we had the same window with that, once the allocation is removed when the instance is deleted
11:08:05 sean-k-mooney oh ok so clean failed happens because it in mantainance
11:08:14 sean-k-mooney and cant actully start cleaning?
11:08:55 johnthetubaguy more that, we start sending new instances to the node that is in maintainance, shortly after the user deletes their nova server
11:09:26 johnthetubaguy its basically the same race condition, but with a slightly different reason
11:09:45 sean-k-mooney nice i alwasys like it when one fix fixes multiple bugs
11:10:13 johnthetubaguy totally
11:11:10 sean-k-mooney so between https://review.opendev.org/c/openstack/nova/+/842478 (the retry) and https://review.opendev.org/c/openstack/nova/+/864773 (reserving) we have two fixes. they retry is certinaly backportable
11:11:33 sean-k-mooney reserving honelsy proably is too but to backport that i think we would need the workaround option
11:12:05 johnthetubaguy yeah, I think we need both, although the new one makes the older one less important
11:12:55 johnthetubaguy i.e. the older one only matters where available nodes go no longer available, and we don't spot it right away, since we remove the issue with automatic cleaning also causing that problem
11:14:12 sean-k-mooney yes so i was about to approve the old patch and then soft -1 the second one askign for the workaround option if that works for you.
11:14:33 sean-k-mooney the only thing i was wonderign about for the first patch is shoudl it have a release note
11:14:44 sean-k-mooney although its only apartial fix
11:14:51 sean-k-mooney the second patch should have one
11:15:01 johnthetubaguy yeah, second patch needs one for sure
11:15:37 johnthetubaguy the first one might be worth advertising via a release note I guess, it is handy
11:16:24 johnthetubaguy but merging is better for me, obviously :)
11:16:41 sean-k-mooney shall i hold +2w for you to add one or just go for it
11:16:46 sean-k-mooney i think its fine as is
11:17:03 johnthetubaguy yeah, lets get that first one in, I will add some workaround stuff in the second one
11:17:05 sean-k-mooney i like to have releas notes for close-bug
11:17:18 sean-k-mooney i treat them as optional for paritals or related
11:17:22 johnthetubaguy ah, fair enough
11:17:45 sean-k-mooney cool first one is on its way
11:17:52 sean-k-mooney ill comment on the second
11:18:43 johnthetubaguy sweet, thank you
11:19:19 sean-k-mooney i owe you a review of the ironic spec too, i didnt get to it on the review day so once im done with this ill take a look at that next
11:24:55 johnthetubaguy Ah, that would be great, thank you. I am sure it needs some refinement. I have half a plan to do some POC work on that soon, but these other bugs keep distracting me.
11:31:38 sean-k-mooney your looking at a thrid bug related to pci claims too right. did you pick that up form mark?
11:32:22 sean-k-mooney ya its https://review.opendev.org/q/topic:bug%252F1860555
11:32:54 johnthetubaguy sort of yes, trying to restack that on top of the functional test
11:33:21 johnthetubaguy I need to decide which of these we backport in various downstreams
11:34:53 sean-k-mooney i havent looked at it in a while but i tought it would be backportable upstream
11:35:47 sean-k-mooney in case that helps.
11:36:04 johnthetubaguy yeah, I agree, I think it should be
11:36:29 johnthetubaguy I am not totally sure about the claims stuff, and how critical that is, its very possible we leak PCI devices without that fix up
11:37:54 sean-k-mooney leak is not quite right
11:38:08 sean-k-mooney we can end up claiming more then we request
11:38:17 sean-k-mooney they will all get freed when the vm is deleted
11:38:19 sean-k-mooney but not before
11:38:25 sean-k-mooney without it
11:38:39 johnthetubaguy yeah, without that object refresh, that is certainly true
11:39:06 sean-k-mooney we have seen this persiste even after the vm is shelved in downstream bug reports
11:39:07 johnthetubaguy the functional test probably needs more checks on the PCI claims I guess
11:39:26 johnthetubaguy ah, good to know, certainly believeable
11:39:26 sean-k-mooney not nessisalry because fo the rescdule there were issue with resize in the past that had a similar effect
11:40:01 johnthetubaguy ah, interesting
11:40:28 sean-k-mooney i have not look i added RP+1 to get it on my list i proably wont get to it this week but ill try and see if i can take a look again on monday
11:41:35 johnthetubaguy thank you
11:43:14 opendevreview Konrad Gube proposed openstack/nova-specs master: Add API for assisted volume extend https://review.opendev.org/c/openstack/nova-specs/+/855490
12:21:25 sean-k-mooney elodilles: im plannign to fix a trivial docs issuw with one of our config options but i want to also test this in ci https://bugs.launchpad.net/nova/+bug/1996094
12:22:07 sean-k-mooney i chatted to bauzas about this a bit downstream and we wer eunsure how you and other stabel cores would feel about when it comes to backporting
12:22:30 sean-k-mooney elodilles: is it ok to do both in one patch or woudl you prefer we did not backport the ci change
12:22:56 sean-k-mooney i woudl prefer to backport both and if we are backportign both i would prefer to have it be in one patch
12:23:45 sean-k-mooney my curent plan is to set heal_instance_info_cache_interval=0 in nova-next
12:36:54 opendevreview John Garbutt proposed openstack/nova master: Ironic nodes with instance reserved in placement https://review.opendev.org/c/openstack/nova/+/864773
12:39:29 johnthetubaguy gibi sean-k-mooney I have added a release note and the workaround config, slightly more intrusive, but not by much I guess.
12:40:37 sean-k-mooney ack most of the way though the spec not much feeback so far beyond what is already there
12:40:59 sean-k-mooney just got to the nova manage command
12:48:47 bauzas sean-k-mooney: elodilles: yup, I just wondered if we were ok for backporting a .zuul file :)
12:49:57 sean-k-mooney im pretty sure i have dont that before
12:50:11 sean-k-mooney backported a job change we defintly did it to fix the train gate
13:01:19 elodilles sean-k-mooney bauzas : as far as i remember, traditionally, doc change backports were not accepted, but i think it is OK to backport them. about the CI I'm a bit hesitant, though. but it depends on the change, i would say
13:01:25 gibi johnthetubaguy: left reply in the pci re-schedule bugfix
13:01:47 gibi johnthetubaguy: that single instance.refresh() feels strange to me
13:01:58 sean-k-mooney elodilles: it littrally is disablelng a perodic task
13:02:36 sean-k-mooney elodilles: we perodicaly heal the network info cache but that in practic should not be required as neutron tells us when something chagnes
13:02:46 sean-k-mooney elodilles: so the ci change is setting one config value to 0
13:03:04 sean-k-mooney in one job nova-next
13:03:04 johnthetubaguy gibi: agreed, its crazy that fixes so much, drops all the transient changes before a save, roughly. I want more of that claims stuff in the functional test I think
13:03:25 elodilles sean-k-mooney: so it won't be a new CI job, but a config value change in 1(?) job as I understand then
13:03:46 sean-k-mooney yes just one addtionall config override in the nova-next job
13:03:52 gibi johnthetubaguy: if the PciDevice.instance_uuid field is already update to point to this instance then simply resetting the instance with instance.refresh() cannot be enough
13:03:58 sean-k-mooney to set the existing config option to 0 instead of the defautl 60
13:04:02 elodilles sean-k-mooney: and you state that it won't introduce any instability o:)
13:04:24 sean-k-mooney well thats why we want to have it runing in ci
13:04:29 sean-k-mooney but i dont belive it will
13:04:53 elodilles maby let's see it first in master banch then :)

Earlier   Later