Earlier  
Posted Nick Remark
#openstack-nova - 2021-02-04
15:04:35 gibi thanks
15:23:55 gibi I'm +2 on the fairly simple libvirt metadata feature https://review.opendev.org/c/openstack/nova/+/750552
15:24:35 gibi so if some core has time then it is an easy win
15:26:35 openstackgerrit Sylvain Bauza proposed openstack/nova master: Add network and utils methods for getting routed networks and segments https://review.opendev.org/c/openstack/nova/+/773976
15:26:36 openstackgerrit Sylvain Bauza proposed openstack/nova master: WIP: Add a routed networks scheduler pre-filter https://review.opendev.org/c/openstack/nova/+/749068
15:27:36 bauzas gibi: lemme look
15:28:03 bauzas gibi: btw. thanks for continuing to review the routed networks series
15:28:16 gibi bauzas: thanks
15:28:27 bauzas fwiw, I'm pretty done, just the last change needs to be having UTs and docs
15:33:09 gibi bauzas: ack, I will continue looking at it, actaully the self -1 made me stop so it is good that you stated now that it is basically ready
15:33:29 bauzas gibi: yeah I needed to add UTs
15:33:32 bauzas now it's done
15:33:44 bauzas those are easy peasy
15:37:00 gibi :)
15:44:31 bauzas gibi: concerns with reliability of the guest metadata information in https://review.opendev.org/c/openstack/nova/+/750552
15:45:48 sean-k-mooney bauzas: reliablity?
15:46:03 sean-k-mooney this is an internal debug info
15:46:24 sean-k-mooney so if its a little out of sync i think its ok
15:47:24 bauzas sean-k-mooney: well, if so, we don't need it
15:47:45 sean-k-mooney we dont need it but it does make debuging from logs simpler
15:47:45 bauzas operators could get their infos by other means, right?
15:48:01 sean-k-mooney they could but this would be useful for us reading sosreports
15:48:06 bauzas sean-k-mooney: right, but then we need it to be reliable
15:48:07 sean-k-mooney where we cant
15:48:17 sean-k-mooney ya
15:48:22 sean-k-mooney well
15:48:29 sean-k-mooney it would be preferable
15:48:31 bauzas sean-k-mooney: I personnally voted on the spec because I do agree with the usecase
15:48:55 bauzas but if we go down the road, we need this information to be correct
15:48:59 sean-k-mooney i have not read your concern in context in the review
15:49:07 sean-k-mooney you belive there is a race in the code ?
15:49:15 bauzas right, when detaching
15:49:27 sean-k-mooney i see
15:49:37 bauzas the proposer wrote to delete the info without waiting the neutron event
15:49:37 sean-k-mooney if that can be fixed then i agree it shoudl be.
15:49:47 bauzas which could fail
15:50:17 bauzas and for most of the cases where operators would want to see the IPs, those would be for networking debugging
15:50:20 sean-k-mooney which neutron event? network-vif-unplugged?
15:50:29 bauzas yeah
15:50:36 sean-k-mooney we dont need to wait for that
15:50:37 bauzas sean-k-mooney: see the patch https://review.opendev.org/c/openstack/nova/+/750552
15:50:45 sean-k-mooney we can but we dont need too.
15:51:12 sean-k-mooney once we detach it form libvirt its detacted form the vm
15:51:40 sean-k-mooney what could fail is removing the device owner(vm uuid) form the port
15:53:37 bauzas sean-k-mooney: sean-k-mooney: but then the IP would still be assigned to the instance, right?
15:53:44 sean-k-mooney this is the only place we use network-vif-unplugged i belvie https://opendev.org/openstack/nova/src/branch/master/nova/compute/manager.py#L10079-L10083
15:54:06 sean-k-mooney bauzas: the ip is assigned to the port
15:54:31 sean-k-mooney if the port is not attached to the vm anymroe then even if nueton still thinks the port has teh ip packet wont get to the vm
15:54:48 bauzas sean-k-mooney: the comment is confusing here https://review.opendev.org/c/openstack/nova/+/750552/8/nova/virt/libvirt/driver.py#2329
15:55:04 bauzas we have some internal object that awaits a neutron callback
15:55:09 gibi bauzas: ack, I will check
15:55:33 sean-k-mooney bauzas: the network info cache wont be update until neutron sees the port is removed
15:55:38 gibi but nova meeting starts in 4 minutes on #openstack-meeting-3
15:55:40 sean-k-mooney i belive that is what it is refering too
15:56:05 sean-k-mooney the filter however will remove it from the network info when generating the metadata
15:56:14 sean-k-mooney network_info = list(filter(lambda info: info['id'] != vif['id'],
15:56:16 sean-k-mooney instance.get_network_info()))
15:56:49 sean-k-mooney so regardless of if neutron has sent the event or not to cause the info cache to be refreshed the copy we pass to generate the data has it removed
15:58:37 bauzas sean-k-mooney: my concern is not the fact it filters
15:58:51 bauzas he wrote the filter for a good reason
15:59:17 bauzas my concern is that we remove this information from the metadate while we could still need it
15:59:57 bauzas actually, the question is more, who is the source of truth ? nova or neutron ?
16:00:15 bauzas the IP address is bound to a port, which itself is attached to an instance
16:00:32 bauzas what if the detach event fails in the meantime ?
16:00:49 sean-k-mooney we remove it after libvirt has finished detaching the interface so why would we need it
16:01:04 gibi bauzas: if this info is in the domain xml then I would say that what matters is what the VM sees. so if the vif was removed from the VM then we can remove the metadata too
16:01:34 bauzas gibi: in this case, I could understand this
16:01:47 sean-k-mooney the sequencing is we remove the interface form the domain
16:02:00 sean-k-mooney then we unplug the vif form the backend
16:02:07 sean-k-mooney then we remove it form the metadata
16:02:46 gibi that sequence is OK to me
16:02:48 sean-k-mooney then after that i belive the compute manger update the neutron port and remvoed the device owner
16:04:17 sean-k-mooney by the way we cannot unconditionally wait for network-vif-unplugged here as not all backend will send it if im not mistaken
16:04:26 sean-k-mooney ml2/ovs will
16:04:41 sean-k-mooney after we do self.vif_driver.unplug(instance, vif)
16:04:55 sean-k-mooney but i doint think al backend will
16:07:11 sean-k-mooney yep https://github.com/openstack/nova/blob/788035add9b32fa841389d906a0e307c231456ba/nova/compute/manager.py#L7779-L7794
16:07:23 sean-k-mooney we tell the dirver to detach which is what is being modified
16:07:36 sean-k-mooney and then if we dont raise an exceptio we do _deallocate_port_for_instance
16:09:06 sean-k-mooney that is what does the neutron port update/delete https://github.com/openstack/nova/blob/788035add9b32fa841389d906a0e307c231456ba/nova/network/neutron.py#L1710-L1714
16:11:27 sean-k-mooney bauzas: hopefully ^ that makes sense
16:11:47 bauzas sean-k-mooney: on the nova meeting, catching up
16:13:43 bauzas sean-k-mooney: well, gibi's point sounds reasonable to me
16:14:00 bauzas from a VM perspective, the nic is detached
16:14:18 sean-k-mooney yep before we ever touch the xml to update the metadta
16:14:30 sean-k-mooney so its consitent with novas/libvirt view
16:14:45 bauzas ok, so I'll comment but I'll leave my -1 for other nits
16:14:54 sean-k-mooney cool
16:16:18 bauzas humm, eavesdrop is lagging 15 mins behind, can't just provide a link yet
16:25:29 sean-k-mooney ya it can
16:26:15 sean-k-mooney bauzas: its up to date now
16:26:32 bauzas yup, commented 5 mins before
16:26:41 sean-k-mooney so you did
16:26:45 bauzas it just updated straight while it was lagging
16:27:00 bauzas I guess there are crons behind eavesdrop
16:27:32 bauzas unless it's event-based, which would surprise me
16:27:58 sean-k-mooney i think its a chron/periodic sync ya
16:28:56 sean-k-mooney its rare that it get more then a few minutes out of date
17:06:19 lyarwood stephenfin: mind if I address a nit in https://review.opendev.org/c/openstack/nova/+/751367/2 and rebase the series for you?
17:06:46 stephenfin lyarwood: for sure

Earlier   Later