| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2021-02-04 | |||
| 13:01:28 | sean-k-mooney | i can do both too so its not either or | |
| 13:02:11 | openstackgerrit | Lee Yarwood proposed openstack/nova master: WIP libvirt: Record the machine_type of instances in system_metadata https://review.opendev.org/c/openstack/nova/+/767533 | |
| 13:02:13 | openstackgerrit | Lee Yarwood proposed openstack/nova master: WIP nova-status: Add hw_machine_type check for libvirt instances https://review.opendev.org/c/openstack/nova/+/770643 | |
| 13:02:13 | openstackgerrit | Lee Yarwood proposed openstack/nova master: WIP nova-manage: Add commands for managing instance machine type https://review.opendev.org/c/openstack/nova/+/769548 | |
| 13:04:24 | lyarwood | sean-k-mooney: https://review.opendev.org/c/openstack/nova/+/767533/5/nova/tests/functional/libvirt/test_machine_type.py - comments on the tests here would be appreciated btw | |
| 13:05:25 | sean-k-mooney | sure. i think i owe bauzas a review of his routed stuff first but i now have your open in front of me :) | |
| 13:08:51 | lyarwood | thanks | |
| 13:09:25 | bauzas | sean-k-mooney: I'll upload a new revision https://review.opendev.org/c/openstack/nova/+/773976 today later | |
| 13:10:37 | sean-k-mooney | bauzas: ok ill review lyarwood patch then now and ill look at yours when you push it | |
| 13:11:20 | bauzas | thanks | |
| 13:26:48 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Remove __unicode__() from nova unit test Exception https://review.opendev.org/c/openstack/nova/+/769894 | |
| 14:05:51 | openstackgerrit | Lee Yarwood proposed openstack/nova master: Add regression test for bug #1908075 https://review.opendev.org/c/openstack/nova/+/766976 | |
| 14:05:54 | openstackgerrit | Lee Yarwood proposed openstack/nova master: fup: Merge duplicate volume attachment checks https://review.opendev.org/c/openstack/nova/+/773380 | |
| 14:05:54 | openstackgerrit | Lee Yarwood proposed openstack/nova master: api: Reject volume attach requests when an active bdm exists https://review.opendev.org/c/openstack/nova/+/768472 | |
| 14:05:54 | openstack | bug 1908075 in OpenStack Compute (nova) "Nova allows a non-multiattach volume to be attached to multiple instances *if* its volume state is reset by an admin" [Undecided,In progress] https://launchpad.net/bugs/1908075 - Assigned to Lee Yarwood (lyarwood) | |
| 14:05:56 | lyarwood | stephenfin: ^ updated, would you mind hitting the changes below that fup as well? | |
| 14:37:03 | openstackgerrit | Ghanshyam proposed openstack/placement master: DNM: testing direct l-c https://review.opendev.org/c/openstack/placement/+/773813 | |
| 14:44:40 | lyarwood | sean-k-mooney: ^ sorry forgot to update the unit tests in that fist change | |
| 14:44:54 | lyarwood | argh git-review is still slow | |
| 14:45:15 | openstackgerrit | Lee Yarwood proposed openstack/nova master: WIP nova-manage: Add commands for managing instance machine type https://review.opendev.org/c/openstack/nova/+/769548 | |
| 14:45:15 | openstackgerrit | Lee Yarwood proposed openstack/nova master: WIP libvirt: Record the machine_type of instances in system_metadata https://review.opendev.org/c/openstack/nova/+/767533 | |
| 14:45:16 | openstackgerrit | Lee Yarwood proposed openstack/nova master: WIP nova-status: Add hw_machine_type check for libvirt instances https://review.opendev.org/c/openstack/nova/+/770643 | |
| 14:55:32 | gibi | lyarwood, stephenfin: do we need both? https://review.opendev.org/c/openstack/nova/+/773727 https://review.opendev.org/c/openstack/nova/+/769920 | |
| 14:56:17 | stephenfin | Yes, I think so. The fixtures proves the stubbing isn't complete and would be useful even when it is to prevent regressions | |
| 14:56:40 | lyarwood | yeah what stephenfin said, already has shown a few things we missed AFAICT | |
| 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) | |