| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2022-08-03 | |||
| 09:37:30 | sean-k-mooney[m] | ack | |
| 09:37:35 | amorin | do you know if neutron already implement such API? | |
| 09:37:40 | amorin | that would allow nova to unbound the port? | |
| 09:37:51 | sean-k-mooney[m] | yes it does | |
| 09:38:04 | sean-k-mooney[m] | and nova has some code to do it too | |
| 09:38:10 | amorin | in the binding extension probably | |
| 09:38:14 | sean-k-mooney[m] | it currently however does too much | |
| 09:38:24 | sean-k-mooney[m] | well unbinding is jsut doing 2 things | |
| 09:38:28 | amorin | oh nice, any hint where it's located in nova code? | |
| 09:38:34 | sean-k-mooney[m] | one settign bind-host=None | |
| 09:38:47 | sean-k-mooney[m] | and second clearing the files we set in the binding profile | |
| 09:38:56 | sean-k-mooney[m] | what we shoudl not do is clear the device id | |
| 09:39:11 | sean-k-mooney[m] | that is the bit our current unbind code is doing that it should not be | |
| 09:39:27 | sean-k-mooney[m] | our current unbind is only used for detaching a port or when deleteing a vm | |
| 09:39:43 | amorin | ack, yes, the device_id should stay | |
| 09:40:38 | sean-k-mooney[m] | https://github.com/openstack/nova/blob/master/nova/network/neutron.py#L616 | |
| 09:41:21 | amorin | perfect, that where I was also looking :) | |
| 09:41:24 | sean-k-mooney[m] | so this should not be part of unbind https://github.com/openstack/nova/blob/master/nova/network/neutron.py#L642 | |
| 09:41:42 | sean-k-mooney[m] | 'device_owner': '', | |
| 09:41:42 | sean-k-mooney[m] | 'device_id': '', | |
| 09:41:46 | sean-k-mooney[m] | is acttully detach | |
| 09:42:08 | amorin | ack, we should maybe split that in 2 separate function | |
| 09:42:15 | amorin | where detach would call unbind | |
| 09:42:24 | sean-k-mooney[m] | that or add a detach kwarg | |
| 09:42:47 | amorin | ok, maybe easier | |
| 09:43:16 | amorin | I think I have all info, I'll do some tests, open the bug, and come back | |
| 09:43:17 | sean-k-mooney[m] | did this cause you any issues or did you just notice it and find it ood | |
| 09:43:51 | amorin | it's causing us issues because we are monitoring the ports attached to a compute, and we found some bound ports without instances | |
| 09:44:25 | amorin | and then, digging into that, we were thinking that it's odd to keep the binding :) | |
| 09:44:39 | sean-k-mooney[m] | that should not result in the ports being bound on the compute node | |
| 09:44:44 | sean-k-mooney[m] | sorry not bound | |
| 09:44:48 | sean-k-mooney[m] | but present on ovs | |
| 09:44:53 | amorin | it's not anymore | |
| 09:44:58 | sean-k-mooney[m] | and or "plugged" | |
| 09:44:58 | amorin | so i'ts good | |
| 09:45:21 | sean-k-mooney[m] | ya okj | |
| 09:45:30 | sean-k-mooney[m] | just making sure we were not leaking the neutron ports | |
| 09:45:42 | sean-k-mooney[m] | we acctuly do call unplug_vifs | |
| 09:45:46 | sean-k-mooney[m] | during shelve offload | |
| 09:45:54 | amorin | yes, everything is actually fine on the compute | |
| 09:46:01 | sean-k-mooney[m] | incidentally shelve offload is where it should be unbound | |
| 09:46:05 | sean-k-mooney[m] | not shelve itslef | |
| 09:46:11 | amorin | it's just that the database (ml2_port_binding) does not reflect what is on the compute | |
| 09:46:24 | amorin | yes | |
| 09:46:30 | sean-k-mooney[m] | by defualt we automaticaly shelve offload with a time out of 0 seconds | |
| 09:46:44 | sean-k-mooney[m] | yep that makes sense | |
| 09:46:49 | amorin | true, we kept this parameter to 0 here | |
| 09:47:34 | sean-k-mooney[m] | personally i would prefer to eventurlaly remove the config option and always just offload but that proably wont happen | |
| 09:48:00 | amorin | what's the purpose of delaying the offload? | |
| 09:48:55 | sean-k-mooney[m] | in rare cases if you unshleve quickly enough it can sometimes be useful to delay. the use case was for bustable instances | |
| 09:49:14 | sean-k-mooney[m] | basically in some cases where shelve/unshele is used to scale in/out | |
| 09:49:28 | sean-k-mooney[m] | its nice to have say a 15min delay incase the load spike | |
| 09:49:42 | sean-k-mooney[m] | as its faster to unshleve if the vms is still in place | |
| 09:50:08 | amorin | ok, that's sound like a weird usecase to me | |
| 09:50:08 | sean-k-mooney[m] | in partice that was only ever useful for vms without ceph or boot form volume | |
| 09:50:19 | amorin | shelve / unshelve is not suppose to happen a quick way | |
| 09:50:31 | sean-k-mooney[m] | well that depends | |
| 09:50:36 | sean-k-mooney[m] | shelve should be very very quick | |
| 09:50:45 | sean-k-mooney[m] | if you are using rbd or boot form volume | |
| 09:50:55 | sean-k-mooney[m] | sicne there is no data to copy | |
| 09:51:18 | sean-k-mooney[m] | but ya it was a pretty niche usecase which is why i would prefer to simplify the code | |
| 09:51:29 | amorin | ack | |
| 09:51:36 | sean-k-mooney[m] | that or remove the config option but make it an api parmater | |
| 09:51:54 | sean-k-mooney[m] | so the user can say delay for up to x time | |
| 09:52:18 | sean-k-mooney[m] | i do not like config driven api behavior shich this currently is | |
| 10:47:27 | opendevreview | Merged openstack/nova master: Remove the PowerVM driver https://review.opendev.org/c/openstack/nova/+/850346 | |
| 11:23:09 | sean-k-mooney[m] | stephenfin: your lines of code stats will never not be negitive ^ | |
| 11:23:57 | gibi | don't encourage him, he will find a way to replace the content of the nova repo with a simple readme file :D | |
| 11:23:58 | sean-k-mooney[m] | cells v1, nova networks xen and power vm. | |
| 11:24:06 | sean-k-mooney[m] | whats next on the list :) | |
| 11:24:44 | sean-k-mooney[m] | oh i for got the docs project :) | |
| 11:25:04 | sean-k-mooney[m] | @gibi: that would be one way to close out all the bugs | |
| 11:25:57 | gibi | what an elegant way :) | |
| 11:25:58 | sean-k-mooney[m] | @gibi speaking of which i marked the custom device_owner bug as invalid | |
| 11:26:17 | gibi | heh, I guessed you would | |
| 11:26:26 | gibi | and I agree | |
| 11:28:58 | gibi | their use case can be solved without the requested change | |
| 11:30:14 | sean-k-mooney[m] | if we wanted to support it we should do it properly as a minor feature with the filtering of events on the neutron side and filtering of port on the nova side so taht they are never stored in our netrowk info cache ectra | |
| 11:30:48 | opendevreview | Merged openstack/nova master: [trivial] Simplify dict get call by removing unused default https://review.opendev.org/c/openstack/nova/+/850450 | |
| 11:37:40 | gibi | sean-k-mooney[m]: I agree | |
| 12:12:33 | gibi | during live migration what triggers the creation of the domain on the dest host? Is it the call to migrateToURI3 from the source host? or nova somehow pre-create a domain on the dest host? | |
| 12:12:46 | sean-k-mooney[m] | yes | |
| 12:13:06 | sean-k-mooney[m] | its created by libvirt as a result of our call to libvirt on the source node | |
| 12:13:11 | gibi | thanks | |
| 12:13:16 | sean-k-mooney[m] | via the migrateToURI3 function | |
| 12:13:39 | gibi | so even though our func test env simulate libvirt it does not simulate that 14:12 < gibi> during live migration what triggers the creation of the domain on the dest host? Is it the call to migrateToURI3 from the source host? or nova somehow pre-create a domain on the dest host? | |
| 12:13:44 | gibi | 14:12 < sean-k-mooney[m]> yes | |
| 12:13:45 | gibi | again | |
| 12:13:47 | gibi | sorry | |
| 12:13:53 | sean-k-mooney[m] | correct | |
| 12:14:04 | sean-k-mooney[m] | we just mock out that call | |
| 12:14:07 | gibi | so even though our func test env simulate libvirt it does not simulate that migrateToURI3 triggers the domain creation on the dest | |
| 12:14:15 | gibi | ack | |
| 12:14:43 | gibi | that explains some func test failures I see with https://review.opendev.org/c/openstack/nova/+/851832/3 | |
| 12:14:47 | sean-k-mooney[m] | we dont really simulate the domins in general in the libvirt fixture | |
| 12:14:58 | sean-k-mooney[m] | at least not for the move ops like that | |
| 12:15:18 | sean-k-mooney[m] | we proably could mock it differntly adn have it do that to be fair | |
| 12:15:38 | gibi | three is some vm tracking in the fixture but yes, that seems to be incomplete | |
| 13:13:54 | opendevreview | Balazs Gibizer proposed openstack/nova master: Prevent instance.save() under mutated migration context https://review.opendev.org/c/openstack/nova/+/850746 | |
| 13:13:54 | opendevreview | Balazs Gibizer proposed openstack/nova master: Do not mutate migration context for rollback_live_migration_at_destination https://review.opendev.org/c/openstack/nova/+/851832 | |