Earlier  
Posted Nick Remark
#openstack-nova - 2021-11-23
14:36:03 sean-k-mooney bauzas: we proably shoudl jsut not use libvirt for mdevs at all honestly but fun
14:36:09 gibi mnaser: I'm still digging oslo.messaging to understand more how to fit this in
14:36:32 bauzas sean-k-mooney: well, I'd prefer the other way, ie. asking libvirt to create mdevs
14:36:41 mnaser gibi: something i found interesting was - https://docs.celeryproject.org/projects/kombu/en/stable/_modules/kombu/connection.html#Connection.ensure_connection
14:36:42 bauzas sean-k-mooney: but they won't do it
14:36:58 mnaser it looks like kombu has it's own error handling inside ensure_connection that maybe didn't exist back when openstack started using it
14:37:11 sean-k-mooney bauzas: well the reason i say that is libvirt does not define that as a stable interface
14:37:29 sean-k-mooney and createing/remvoing mdevs via the filesystem is pretty trivial
14:37:35 gibi mnaser: yeah komubo has the flags in the interface
14:37:40 gibi kombu
14:37:54 bauzas anyway, when I'm seeing it, I wonder whether libvirt folks know there are some upper services that use them
14:37:59 gibi mnaser: so we could configure kombu to stop after x retries
14:38:12 gibi mnaser: we just don't have the scaffolding in oslo.messaging to use that
14:38:23 mnaser gibi: yeah but i think then that brings the other interesting issue of 'does that mean it gives up forever for all future notifications'
14:38:24 sean-k-mooney bauzas: they are partly aware i had a converstaion with them about this in relateino to vdpa
14:38:42 sean-k-mooney and mac adresses
14:38:58 sean-k-mooney they do not guarenttee the names are stabel and said we shoudl avoid realying on them
14:40:11 bauzas meh
14:40:13 bauzas OK
14:40:16 sean-k-mooney bauzas: to me this is incorrect behavior in mdevctl and the libvirt chagne should be reverted
14:40:22 bauzas gibi: heh https://bugs.launchpad.net/nova/+bug/1951623
14:40:36 bauzas gibi: I guess this will be fixed by your change, right ?
14:41:40 bauzas oh, nevermind, it was for reboot
14:43:52 mnaser i do see a point in 'if notifications are down, stop doing things that can be billed'
14:45:10 gibi bauzas: unfortunatly that is a different problem
14:45:16 sean-k-mooney mnaser: that is anyting other then delete including allowing the instance to continue running :)
14:45:26 bauzas gibi: looks like, yes
14:45:39 mnaser sean-k-mooney: but that means more $$$ am i rite? =ap
14:46:09 gibi mnaser: yepp, I agree, but I think this behavior needs to be configurable
14:46:40 gibi mnaser: to handle the case when notification only just good to have
14:46:43 gibi but not must have
14:47:06 opendevreview Artom Lifshitz proposed openstack/nova master: DNM: Explode libvirt builds to trigger tempest error code https://review.opendev.org/c/openstack/nova/+/818932
14:47:16 sean-k-mooney mnaser: :) i think the correct behavior if notification cant be sent is either to just log an error and drop them or continue to have the service work but set its healtch check to degraded
14:47:34 sean-k-mooney the only other option i see would be for the service to terminate
14:48:01 opendevreview Artom Lifshitz proposed openstack/nova master: DNM: Explode libvirt builds to trigger tempest error code https://review.opendev.org/c/openstack/nova/+/818932
14:48:20 sean-k-mooney mnaser: if we are going to stop some operation from workign i dont think we shoudl be reportign the srvice as up in our current status filed
14:48:37 sean-k-mooney but that is based on a heatbeat which might still be working
14:48:44 sean-k-mooney so that not really something we can change
14:49:40 mnaser sean-k-mooney: historically i rather a failure and consistent behaviour than a degraded 'pass' that leaves a bit of a mess
14:49:45 mnaser (see: ignore cinder volume deletes)
14:49:59 sean-k-mooney well notifocation are generally considerd besteffort
14:50:09 sean-k-mooney its not a mission cirtical part of the cloud
14:50:57 sean-k-mooney so we could make the service exit as an opt in behavior but in generally i think logging an error and reportign a degrated health status then allowing something external to decied what to do i think is better
14:51:15 mnaser i guess notifications being critical will dpeend on who you ask ;p
14:51:45 sean-k-mooney form a nova point of view i dont think they ever have had any guarentees
14:52:32 sean-k-mooney i know people do consume them for billing and other uses but the in ablity to send a notificaotn shoudl not have any impact on a user operation by default
14:53:59 sean-k-mooney mnaser: as a customer if i tried to reboot a vm and it failed because fo a notrifcaiton issue i wold expect a 500 or other error code to be logged showing its an infra issue so i can file a ticket with my cloud provider for downtime
14:54:34 mnaser sean-k-mooney: that's true, but i guess since we don't have any buffering right now, it means notifications can be lost .. and in theory, they'll get a 500 anyways now if the notifications are down =P
14:54:53 sean-k-mooney mnaser:basicly in cloud where notificaiotn are treated as critical it would be a cloud outage from a customer point of ivew
14:55:17 sean-k-mooney mnaser: hehe i guess they will get the 500 now ture
14:55:32 sean-k-mooney although only if you dont set a retry limit right
14:55:40 mnaser nope, even if you set retry=0 it fails :(
14:55:51 mnaser see what gibi mentioned above, it gets stuck on trying to connect and that has no retries
14:55:51 sean-k-mooney oh ok then that is a bug
14:55:56 mnaser it loops forever on trying to connect
14:56:04 sean-k-mooney ah right
14:56:27 sean-k-mooney because its the amqp connection that is down its not an issue with sendign to a queue or exchange
14:56:32 mnaser i captured GMR and updated info here - https://bugs.launchpad.net/nova/+bug/1917645
14:57:00 sean-k-mooney mnaser: well that proably should be converted to an oslo messaging bug at this point
14:57:22 mnaser sean-k-mooney: good point actually
14:57:47 sean-k-mooney nova might be able to work around this but eventually we will want to adress this there i suspect
15:01:11 opendevreview Dmitrii Shcherbakov proposed openstack/os-traits master: Add a trait for remote_managed port-capable nodes https://review.opendev.org/c/openstack/os-traits/+/818514
15:03:42 gibi sean-k-mooney: sure oslo.messaging lacks a configuration possibility to stop retrying the connect, but I don't think it is purelya bug
15:03:58 sean-k-mooney pro tip dont put a folder with 2412 sub directories in your test path if you want test discovery to happen before the heat death of the universe
15:04:00 gibi sean-k-mooney: on nova side it is sure a bug if the instances stuck in some tranisitional state due to hanging
15:04:45 sean-k-mooney gibi: yes but i dont think im sold that we should intoduction config driven api behavior
15:04:53 gibi sean-k-mooney: not in nova no
15:05:00 gibi sean-k-mooney: we need oslo.messaging configurability
15:05:12 sean-k-mooney right so we cant block opertaion because we failed to send a notification
15:05:35 sean-k-mooney gibi: well not i think not at all nova need to proceed even if the notificaiton cant be sent
15:05:55 sean-k-mooney we can log an error ro report it via a healthcheck or similar
15:06:11 gibi sean-k-mooney: I don't agree. If notifications are used for billing then I can imagine that the operator want the instance boot to fail if the notification bus is down
15:06:28 sean-k-mooney gibi: its a grey area i guess
15:06:35 gibi sure, don't stuck, but fail cleanly
15:06:53 sean-k-mooney to me unless we can report a 500 to the user im not sure we should block the operation
15:07:22 bauzas gibi: sean-k-mooney: thoughts abotu https://bugs.launchpad.net/nova/+bug/1951623 be a nova or neutron issue ?
15:08:04 sean-k-mooney ..... both
15:08:05 bauzas nova wouldn't know whether the port is disabled
15:08:10 gibi bauzas: nova, as nova waits unconditionally
15:08:15 sean-k-mooney nova can check that
15:08:15 bauzas yeah
15:08:16 gibi bauzas: nova can query the port sate
15:08:18 gibi state
15:08:32 sean-k-mooney nova because this is not checked today
15:08:40 sean-k-mooney and neutron because this change depending on the backend
15:08:53 bauzas gibi: mmm, that means that we would call neutron to get the port state
15:09:15 sean-k-mooney bauzas: i think we have that in the network info cache but yes we have a check for this in some of the code already
15:09:39 bauzas sean-k-mooney: ok, then it's a nova bug if we already call it
15:09:50 sean-k-mooney we just dont in that part of the code
15:10:19 sean-k-mooney but part of me is wondering if we would be better off just not using those event until we actully define a contract with neutorn on when they will be sent
15:10:59 gibi sean-k-mooney: but thing might never happen based on past experience ^^ :)
15:11:07 sean-k-mooney https://github.com/openstack/nova/blob/master/nova/virt/libvirt/driver.py#L7221-L7228
15:11:21 bauzas gibi: sean-k-mooney: I just triaged the bug https://bugs.launchpad.net/nova/+bug/1951623
15:11:28 sean-k-mooney so we check if its active here but we dont for the migration events
15:12:31 sean-k-mooney https://github.com/openstack/nova/blob/master/nova/network/model.py#L488-L508 or here https://github.com/openstack/nova/blob/master/nova/network/model.py#L567-L586
15:12:59 sean-k-mooney bauzas: gibi so we just need to update the model fucntion to skip disbaled ports
15:13:15 bauzas ack
15:13:22 bauzas bug confirmed anyway
15:13:37 sean-k-mooney the issue is that in some cases we will get event if the port is disabled
15:13:49 sean-k-mooney which we will now be ignorign but thats is likely ok

Earlier   Later