| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2022-08-03 | |||
| 04:53:43 | opendevreview | Amit Uniyal proposed openstack/nova master: For evacuation, ignore if task_state is not None https://review.opendev.org/c/openstack/nova/+/848886 | |
| 07:22:14 | gibi | good morning | |
| 07:33:09 | Uggla | gibi, o/ | |
| 07:33:17 | opendevreview | Merged openstack/nova master: Updated Suspend definition in server concepts doc https://review.opendev.org/c/openstack/nova/+/851511 | |
| 07:35:04 | gibi | \o/ the unittest.mock series landed \o/ | |
| 07:35:30 | gibi | (hope nobody too mad about the merge conflicts we generated with that :D) | |
| 07:47:11 | gibi | sean-k-mooney[m]: left feedback in the vdpa patch https://review.opendev.org/c/openstack/nova/+/832330 | |
| 08:04:51 | gibi | sean-k-mooney[m]: could you look at https://review.opendev.org/c/openstack/nova/+/851909 it is needed for the sdk 0.100 release here https://review.opendev.org/c/openstack/requirements/+/849986 | |
| 08:12:51 | sean-k-mooney[m] | im just about to have a quick call downstream but ill look at it shortly thanks for reviewing the vdpa patch ill adress your feedback when im adressing stephens | |
| 08:13:42 | sean-k-mooney[m] | if i can get the first patch at least to a point where it can merge today that would be ideall then i can focus on just the ones that wont be backported and thos can hopefully merge seperately as a group | |
| 08:15:52 | gibi | sean-k-mooney[m]: sure. ping me and I will re-review the vdpa patch | |
| 08:16:02 | gibi | if stephenfin is also available the we can land that today | |
| 08:36:29 | opendevreview | Balazs Gibizer proposed openstack/placement master: Func test for os-traits and os-resource-classes lib sync https://review.opendev.org/c/openstack/placement/+/851966 | |
| 08:37:22 | gibi | sean-k-mooney[m], melwitt: as we agreed on the Zed PTG I change the how placement tests for the lib sync so that we don't need to do the test disable/enable dance any more at a lib release ^^ | |
| 08:41:55 | sean-k-mooney[m] | ack almost done with internal call | |
| 08:42:02 | sean-k-mooney[m] | ill look at that next | |
| 08:42:17 | sean-k-mooney[m] | +2 on the safeconnect fix | |
| 08:58:46 | gibi | thanks | |
| 09:04:20 | sean-k-mooney[m] | i was going to ask for the placement change to be done slightly differntly | |
| 09:04:48 | sean-k-mooney[m] | but i realise now that you are loading the tratis/resouce classes directly form the lib and assertign the api returns the same content | |
| 09:04:58 | sean-k-mooney[m] | which should always be in sync | |
| 09:05:25 | sean-k-mooney[m] | i was going to ask that you check that the api respocne contains the lib content and the could was greater or equal | |
| 09:05:45 | sean-k-mooney[m] | but the exact comparison should work as ultimately they have the same data source | |
| 09:05:51 | sean-k-mooney[m] | the lib | |
| 09:09:06 | sean-k-mooney[m] | the only test coverage we currntly loose by not doing the >= check is if we acidentally delete a trait | |
| 09:09:47 | sean-k-mooney[m] | but we are aware that that is not allowed so im not really concerned by that | |
| 09:14:32 | gibi | do you mean accidentally deleting a trait from os-traits? yeah that is something we need to cover elsewhere | |
| 09:15:32 | gibi | today we also not checking for that. Also a >= would not cover that as a os-traits lib version bump migh add 10 new traits and remove 2 but the change is net positive so >= would pass | |
| 09:16:18 | sean-k-mooney[m] | we were checking for it indirectly by asserting the exact number | |
| 09:16:30 | sean-k-mooney[m] | but ya | |
| 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_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 | |