| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2022-04-28 | |||
| 10:57:57 | gibi | *train | |
| 10:57:59 | sean-k-mooney | hehe | |
| 10:58:45 | opendevreview | Balazs Gibizer proposed openstack/nova master: Remove unavailable but not reported PCI devices at startup https://review.opendev.org/c/openstack/nova/+/838553 | |
| 10:58:46 | opendevreview | Balazs Gibizer proposed openstack/nova master: Simulate bug 1969496 https://review.opendev.org/c/openstack/nova/+/838554 | |
| 10:58:46 | opendevreview | Balazs Gibizer proposed openstack/nova master: Allow claiming PCI PF if child VF is unavailable https://review.opendev.org/c/openstack/nova/+/838555 | |
| 10:58:59 | gibi | I blame it for the lack of coffein | |
| 11:39:52 | sean-k-mooney | gibi: sorry was pinned for something else runing it locally but ill readd my +2 and review the final patch when it completes | |
| 11:40:45 | gibi | no worries, we have a block in the docs job until https://review.opendev.org/c/openstack/oslo.policy/+/839711 merged and released | |
| 11:41:30 | sean-k-mooney | i breifly looked at that but did not get the full context | |
| 11:42:22 | sean-k-mooney | so oslo-config-generator chagne the kw paramater to a flag? | |
| 11:42:30 | sean-k-mooney | removing the boolean paramater | |
| 11:42:54 | sean-k-mooney | and this is adapting to that backwards incompatible change correct | |
| 11:44:07 | sean-k-mooney | that shoudl have been a major version bump | |
| 11:44:28 | sean-k-mooney | as in oslo.policy=4.0.0 not 3.12.0 | |
| 11:45:02 | sean-k-mooney | gibi: nova.tests.unit.pci.test_manager.PciDevTrackerTestCase.test_set_hvdevs_unavailable_vf_removed is failing for me locally | |
| 11:45:29 | sean-k-mooney | File "/home/sean/repos/openstack/nova-3/nova/objects/pci_device.py", line 238, in _from_db_object | |
| 11:45:30 | sean-k-mooney | setattr(pci_device, key, db_dev[key]) | |
| 11:45:32 | sean-k-mooney | KeyError: 'id' | |
| 11:46:50 | sean-k-mooney | its form the create call | |
| 11:46:53 | sean-k-mooney | File "/home/sean/repos/openstack/nova-3/nova/tests/unit/pci/test_manager.py", line 413, in test_set_hvdevs_unavailable_vf_removed | |
| 11:46:55 | sean-k-mooney | self._create_tracker([fake_db_dev_3, fake_db_dev_4, fake_db_dev_5]) | |
| 11:47:54 | gibi | looking | |
| 11:49:16 | gibi | strange, it is passing for me | |
| 11:49:41 | gibi | do you have commit hash f395e71168 ? | |
| 11:58:36 | sean-k-mooney | f395e71168d843f06c8de7b04874c29f1e10e5a8 | |
| 11:58:55 | sean-k-mooney | ill run it again | |
| 11:58:58 | sean-k-mooney | with -r | |
| 11:59:19 | sean-k-mooney | i think it was a clean venv but we will see if it repoduces | |
| 12:00:24 | sean-k-mooney | oh odd | |
| 12:00:26 | sean-k-mooney | https://zuul.opendev.org/t/openstack/build/f877f8e9afa04b5bb993d6615b8bd558 | |
| 12:00:33 | sean-k-mooney | it failed on the 3.8 arm job | |
| 12:00:37 | sean-k-mooney | but passed on 3.9 | |
| 12:00:55 | gibi | hm I use 3.9 locally | |
| 12:01:08 | sean-k-mooney | let me check which version im using i have 3.8-3.10 locally | |
| 12:01:15 | sean-k-mooney | im proably useing 3.8 | |
| 12:01:39 | gibi | ok it fails iwith 3.8 locally for me too | |
| 12:01:52 | sean-k-mooney | that is super weird | |
| 12:02:15 | gibi | yes | |
| 12:02:48 | sean-k-mooney | this is not in code you are changin i think it appear to be in the fixture/test setup code | |
| 12:03:39 | sean-k-mooney | is it this https://review.opendev.org/c/openstack/nova/+/838553/3/nova/tests/unit/pci/test_manager.py#147 | |
| 12:03:42 | gibi | yes but now I can trace and compare | |
| 12:03:57 | sean-k-mooney | id is technically a reserved keywrod for the id function | |
| 12:04:17 | sean-k-mooney | but when you asign to it as a kwarg it shoudl intoduce it also as a variable | |
| 12:04:25 | sean-k-mooney | its disucuraged but legal | |
| 12:05:40 | sean-k-mooney | hum | |
| 12:06:49 | sean-k-mooney | i wonder if this is realted to not cloning the fake object or soemthing like that | |
| 12:07:30 | sean-k-mooney | i didnt get the failure this time | |
| 12:09:20 | sean-k-mooney | oh this is failing on the first patch | |
| 12:09:41 | sean-k-mooney | oh is it | |
| 12:09:56 | sean-k-mooney | no third i just have the wrong tab open | |
| 12:10:16 | sean-k-mooney | the first patch also hass the same issue | |
| 12:10:49 | gibi | hm, you have a point about cloning | |
| 12:11:06 | gibi | if the id is dropped by our db code then that is now a global change on the db dict | |
| 12:11:21 | sean-k-mooney | yep | |
| 12:11:30 | sean-k-mooney | nova.tests.unit.pci.test_manager.PciDevTrackerTestCase.test_set_hvdevs_unavailable_pf_removed is failing in the first patch | |
| 12:12:48 | sean-k-mooney | you need to do | |
| 12:12:52 | sean-k-mooney | fake_pci_devs = [copy.deepcopy(fake_pci), copy.deepcopy(fake_pci_2), | |
| 12:12:54 | sean-k-mooney | copy.deepcopy(fake_pci_3)] | |
| 12:14:27 | gibi | but non of the test does it so probably the rest of test can be broken too | |
| 12:14:37 | gibi | I can add the deepcopy to _fake_get_pci_devices to fix them all | |
| 12:14:39 | sean-k-mooney | some do | |
| 12:14:59 | sean-k-mooney | like test_set_hvdev_changed_stal | |
| 12:15:38 | sean-k-mooney | many do all_devs = fake_db_devs_tree[:] | |
| 12:15:44 | gibi | that is a shallow copy | |
| 12:15:44 | sean-k-mooney | self._create_tracker(all_devs) | |
| 12:15:47 | gibi | only duplicate the list | |
| 12:15:54 | sean-k-mooney | it is yes | |
| 12:16:25 | sean-k-mooney | but i guess they dont currently modify thigns i guess. | |
| 12:16:26 | gibi | but that does not duplicat the dict having the id field | |
| 12:16:51 | sean-k-mooney | we likely do need to fix other test but i think we have just got lucky | |
| 12:17:43 | sean-k-mooney | thre are certenly sevel test that explcitly do a deepcopy | |
| 12:19:03 | sean-k-mooney | i dont think it s the _create_tracker that copl;es the global state | |
| 12:19:36 | sean-k-mooney | i think its self.tracker._set_hvdevs([fake_db_dev_3, fake_db_dev_4]) | |
| 12:20:52 | sean-k-mooney | _create_tracker does not actully pass the fake devs to the tracker | |
| 12:21:39 | sean-k-mooney | https://github.com/openstack/nova/blob/master/nova/tests/unit/pci/test_manager.py#L144-L147= | |
| 12:22:59 | gibi | yeah you are right | |
| 12:23:09 | gibi | the set_hvdev does the coupling | |
| 12:23:29 | sean-k-mooney | yep so you are not actully initalisign the tracker with the devices you created before using them | |
| 12:26:37 | gibi | I will push a fix | |
| 12:26:50 | gibi | for the new test cases | |
| 12:27:13 | gibi | and a separate commit for the existing ones not having deepcopy before _set_hvdevs | |
| 12:27:55 | sean-k-mooney | so i tihnk it was "working" becaue of this stub https://github.com/openstack/nova/blob/5f5551448dcfcde26095963e223f973b90e6f637/nova/tests/unit/pci/test_manager.py#L153-L154 | |
| 12:28:16 | sean-k-mooney | i was just trying to fiture out how the the tracker had any devices when it was not called | |
| 12:28:53 | sean-k-mooney | that just returns self.fake_devs https://github.com/openstack/nova/blob/5f5551448dcfcde26095963e223f973b90e6f637/nova/tests/unit/pci/test_manager.py#L153-L154 | |
| 12:29:06 | sean-k-mooney | so ya anything not doign a deep copy was sharing state | |
| 12:32:28 | trident | We have had a few policy overrides to allow regular users to create flavors. At some point they seem to have stopped working (probably upgrade to Victoria) - Neither a policy.json with the old format or a policy.yaml seem to help. | |
| 12:33:10 | trident | Any known reasons why a policy override like "os_compute_api:os-flavor-manage:create": "rule:admin_or_owner" (and the same for update and delete) would not work nowadays? | |
| 12:34:17 | trident | Version 22.3.2 | |
| 12:41:55 | sean-k-mooney | it should be possibel to configure that today | |
| 12:42:03 | sean-k-mooney | not advised but possible | |
| 12:45:05 | trident | Weird. I still get a 403 Policy doesn't allow os_compute_api:os-flavor-manage:create to be performed back... | |
| 12:48:38 | trident | sean-k-mooney: You say "not advised" - what would be the advised way to accomplish that? | |
| 12:49:00 | sean-k-mooney | you said it was victoria | |
| 12:49:06 | sean-k-mooney | 22.3.2 | |
| 12:49:41 | sean-k-mooney | 22.3.2 does not exist in git | |
| 12:49:59 | sean-k-mooney | on 22 | |
| 12:50:25 | sean-k-mooney | aslo no does not exist | |
| 12:50:28 | sean-k-mooney | https://github.com/openstack/nova/tree/22.3.2/nova | |
| 12:51:05 | sean-k-mooney | https://github.com/openstack/nova/tree/22.3.0/nova is the most recent release i see | |
| 12:51:42 | sean-k-mooney | same via opendev https://opendev.org/openstack/nova/src/tag/22.3.0 | |