| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2021-02-04 | |||
| 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 | |
| 17:06:49 | stephenfin | go for it | |
| 17:07:17 | stephenfin | I missed the AR | |