| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2022-08-03 | |||
| 09:16:37 | sean-k-mooney[m] | we could have added 2 and removed 2 | |
| 09:16:49 | sean-k-mooney[m] | i think core review is enough to catch that honestly | |
| 09:16:56 | gibi | yes, I hope so | |
| 09:16:58 | sean-k-mooney[m] | we all know that we cant ever remove traits | |
| 09:17:08 | gibi | if not then we need a test in os-traits for it | |
| 09:17:18 | sean-k-mooney[m] | if we need too i would put a hacking test in or similar | |
| 09:17:30 | gibi | as the placement test would be too late to catch it | |
| 09:17:37 | gibi | yeah, hacking would work too | |
| 09:19:26 | sean-k-mooney[m] | ok ill be back in about 10 mins | |
| 09:19:37 | gibi | ack | |
| 09:19:38 | sean-k-mooney[m] | just going to check on fryea and what she is barking at | |
| 09:34:08 | amorin | hello nova team, when shelving an instance, the port binding in neutron is "staying" on the host, I was expecting it to be unbound | |
| 09:34:16 | amorin | am I wrong? | |
| 09:34:37 | sean-k-mooney[m] | amorin: no your are not wrong its a know issue | |
| 09:35:04 | amorin | nice! | |
| 09:35:10 | sean-k-mooney[m] | your expectation alines with mine but we have never actully done it | |
| 09:35:45 | amorin | do you have a launchpad bug already reported for this? | |
| 09:36:02 | sean-k-mooney[m] | https://review.opendev.org/c/openstack/nova/+/832330/7/nova/tests/functional/libvirt/test_pci_sriov_servers.py#1286 | |
| 09:36:09 | sean-k-mooney[m] | amorin no not currently | |
| 09:36:26 | sean-k-mooney[m] | i just came across it whlie wrting functional tests for something else | |
| 09:36:37 | sean-k-mooney[m] | so if you want to file one and or adress it please do | |
| 09:36:58 | sean-k-mooney[m] | amorin: this should not break anything today | |
| 09:37:23 | amorin | ok, I will create the bug report at least | |
| 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_id': '', | |
| 09:41:42 | sean-k-mooney[m] | 'device_owner': '', | |
| 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 | amorin | so i'ts good | |
| 09:44:58 | sean-k-mooney[m] | and or "plugged" | |
| 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 | sean-k-mooney[m] | in partice that was only ever useful for vms without ceph or boot form volume | |
| 09:50:08 | amorin | ok, that's sound like a weird usecase to me | |
| 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 | |