| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2021-08-26 | |||
| 12:10:37 | artom | Ah, gate | |
| 12:13:36 | sean-k-mooney | ya i want to see the results | |
| 12:13:40 | sean-k-mooney | ill likely squash it | |
| 13:26:20 | gibi | something is happening with placement concurrent allocation update issue https://paste.opendev.org/show/808348/ | |
| 13:27:02 | gibi | the amount of job failure due to that getting significantly worse | |
| 13:27:21 | sean-k-mooney | hehe that one way to shorten a url | |
| 13:27:38 | gibi | if you have a hammer ... | |
| 13:27:49 | sean-k-mooney | yes it hit melwitt patches yesterday i think | |
| 13:28:05 | sean-k-mooney | i this the same issue that is cause by a concurent delete request | |
| 13:28:20 | sean-k-mooney | if so melwitt has patches for that as im sure your aware | |
| 13:28:25 | gibi | probably we always had this as in http://status.openstack.org/elastic-recheck/#1836754 | |
| 13:28:33 | gibi | but now it become significant | |
| 13:32:57 | sean-k-mooney | https://review.opendev.org/c/openstack/nova/+/688802 this is what i was thinking of | |
| 13:33:35 | sean-k-mooney | although i think we shoudl jsut go back to calling delete on the allocation and ignoring the version personally instead of doing a put | |
| 13:35:50 | opendevreview | sean mooney proposed openstack/nova master: Add autopep8 to tox and pre-commit https://review.opendev.org/c/openstack/nova/+/806182 | |
| 13:41:50 | artom | sean-k-mooney, we may need for melwitt to wake up for this one, but... does the database fixture actually mock out anything? | |
| 13:41:58 | artom | I don't grok how it's actually used | |
| 13:42:35 | sean-k-mooney | its creating a real db using sqlight in memory database feature | |
| 13:43:10 | artom | OK... So for instance if you pass it a connection URL, it'll create a new context and use its engine: https://github.com/openstack/nova/blob/master/nova/tests/fixtures/nova.py#L625 | |
| 13:43:43 | sean-k-mooney | its an interesting question though if i need it now that i have added the other fixture for get engine | |
| 13:43:49 | artom | But then the migration code will always just use the main_db_api.get_engine: https://github.com/openstack/nova/blob/master/nova/db/migration.py#L47 | |
| 13:44:23 | sean-k-mooney | ill try droping it now that i have added the addtional fixture and see if that is enough | |
| 13:44:43 | artom | Reason I'm asking is - I've tried using the connection=url kwarg to the Database fixture to avoid your mocking while still fixing the global engine issue | |
| 13:44:48 | sean-k-mooney | i am using this to run a db sync after all so i dont need the db fixture to set other things up for me | |
| 13:45:06 | sean-k-mooney | artom: ya i trided that too but it wont work for 2 reaons | |
| 13:45:16 | artom | And they are definitely different engines - but the test uses the main_db_api engine, and not the fixture engine | |
| 13:46:02 | sean-k-mooney | artom: im mocking out get_enging in the migration | |
| 13:46:05 | sean-k-mooney | module | |
| 13:46:17 | sean-k-mooney | so it shoudl be useing the new fixtures engine | |
| 13:46:43 | artom | Right, I get that. I'm trying to understand why it's even necessary with the database fixture | |
| 13:47:08 | sean-k-mooney | because the engine.url is imutable | |
| 13:47:15 | sean-k-mooney | and its reused between multipel tests | |
| 13:47:26 | sean-k-mooney | so it end up with a different connection string then the one we pass | |
| 13:47:32 | artom | Right - but we can't use different engines? | |
| 13:47:40 | sean-k-mooney | even when we use the connection parmater in the db fixture | |
| 13:47:45 | artom | Per test, for tests that need it? | |
| 13:48:30 | sean-k-mooney | that is what https://review.opendev.org/c/openstack/nova/+/805663/6/nova/tests/unit/db/test_migration.py#50 is doing | |
| 13:49:15 | sean-k-mooney | artom: the engine url used to be mutable im not sure our db fixtuer has worked with custom connections since that change in sqlalcahme about a year or 2 agao | |
| 13:49:24 | artom | Yeah... I just thought the fixture would handle that | |
| 13:49:25 | artom | ? | |
| 13:49:53 | artom | Especially with the enticingly named connection=url kwarg :) | |
| 13:50:16 | sean-k-mooney | i think it used too but it definetly does not now | |
| 13:50:50 | artom | Weird. | |
| 13:50:59 | artom | And it's only used in 1 spot, the kwarg | |
| 13:51:25 | sean-k-mooney | ok so i dont need the db fixture just my one for 'nova.db.migration._get_engine' | |
| 13:51:48 | sean-k-mooney | the test will still work if i remove the db fixture | |
| 13:52:21 | sean-k-mooney | good catch ill update that when i squah things | |
| 13:53:16 | sean-k-mooney | ooo is still runnign so i dont want to interupt that job | |
| 13:56:27 | sean-k-mooney | artom: you mentioned its used in only one place? | |
| 13:56:34 | sean-k-mooney | where is that | |
| 13:56:49 | sean-k-mooney | maybe i should remove the parmater in a follow up and fix that | |
| 13:56:50 | opendevreview | Jorhson Deng proposed openstack/nova master: recheck the attachment_id after the reschedule successful https://review.opendev.org/c/openstack/nova/+/796209 | |
| 13:57:35 | sean-k-mooney | or well make the connnection parmater work by creating an new engin instnace | |
| 14:14:10 | artom | sean-k-mooney, well, it appear to correctly create a new engine | |
| 14:14:20 | artom | .. which then never gets used anywhere, AFAICT | |
| 14:15:01 | artom | nova/tests/functional/db/test_connection_switch.py is where it gets used | |
| 14:15:05 | artom | self.useFixture(nova_fixtures.Database(connection=self.fake_conn)) | |
| 14:18:08 | lyarwood | stupid question if anyone with vif/neutron background can help, I'm trying to work out how to find a default for hw_vif_model when it's unset | |
| 14:18:27 | lyarwood | I thought osinfo.HardwareProperties(instance.image_meta).network_model was what I wanted but I'm seeing it return None | |
| 14:18:48 | sean-k-mooney | lyarwood: its virtio | |
| 14:18:56 | lyarwood | just hardcoded? | |
| 14:19:06 | sean-k-mooney | i think so i think i know where it is | |
| 14:19:17 | lyarwood | kk I've missed it then | |
| 14:19:33 | sean-k-mooney | its virt dirver dependent | |
| 14:19:50 | sean-k-mooney | vmware default to a differnt thing then libvirt | |
| 14:20:17 | opendevreview | Stephan Pampel proposed openstack/nova master: docs: admin/networking rename neutron_tunneled to neutron_tunnel https://review.opendev.org/c/openstack/nova/+/806197 | |
| 14:20:34 | lyarwood | https://github.com/openstack/nova/blob/master/nova/virt/osinfo.py#L99-L112 was where I was looking FWIW | |
| 14:20:55 | lyarwood | just without os_distro set this doesn't seem to do anything | |
| 14:21:11 | sean-k-mooney | https://github.com/openstack/nova/blob/master/nova/virt/libvirt/vif.py#L211-L215 | |
| 14:21:32 | sean-k-mooney | sorry wrong link | |
| 14:21:42 | sean-k-mooney | but its not commig form osinfo its form vif.py | |
| 14:21:58 | lyarwood | kk | |
| 14:22:01 | sean-k-mooney | https://github.com/openstack/nova/blob/master/nova/virt/libvirt/vif.py#L162-L178 | |
| 14:22:45 | sean-k-mooney | we do check os_info but its more complicated then that | |
| 14:23:38 | lyarwood | ah | |
| 14:24:05 | sean-k-mooney | this is where its then called in get_base_config https://github.com/openstack/nova/blob/master/nova/virt/libvirt/vif.py#L205 | |
| 14:25:06 | sean-k-mooney | lyarwood: out of interest why were you looking for that | |
| 14:25:34 | lyarwood | sean-k-mooney: for the device bus migration stuff, the initial part is to register the current values | |
| 14:26:04 | lyarwood | sean-k-mooney: so I was trying to see if I can pull the default for this easily like I can with hw_disk_bus/hw_cdrom_bus from disk_info | |
| 14:26:14 | sean-k-mooney | ah right ya so you can jsut call that driver function to get the model | |
| 14:26:16 | lyarwood | sean-k-mooney: without inspecting the instance itself etc | |
| 14:26:37 | lyarwood | sean-k-mooney: any idea how I'd get an instance of that VIF class? | |
| 14:26:53 | sean-k-mooney | we construct it in the driver | |
| 14:26:57 | lyarwood | kk | |
| 14:27:22 | lyarwood | ah self.vif_driver, awesome | |
| 14:27:33 | sean-k-mooney | if you need too though you can just constuct it with no args | |
| 14:28:05 | sean-k-mooney | get_vif_model does not rely on self.host but ya self.vif_driver | |
| 14:28:39 | sean-k-mooney | this used to be plugable | |
| 14:28:57 | sean-k-mooney | i dont think it is anymore | |
| 14:59:51 | melwitt | gibi, sean-k-mooney: I support reverting the patch that changed from DELETE of allocations and I've been thinking about it more lately and think it's the better thing to do anyway | |
| 15:00:59 | gibi | melwitt, sean-k-mooney: I have to load context for that. I only saw the placement conflict error appearnig more frequently in the last ~24 hours. | |
| 15:01:23 | melwitt | gibi: yes, it's because of consumer types I'm afraid :( | |
| 15:03:05 | gibi | melwitt: is there a connection? | |
| 15:03:44 | melwitt | gibi: I think there is but I want to look through to make sure it makes sense | |
| 15:05:48 | gibi | OK, I can also spend some time on it tomorrow | |
| 15:06:53 | melwitt | lyarwood: ack will look again | |
| 15:07:23 | melwitt | sean-k-mooney: ack will look at the db url patch again | |
| 15:54:31 | sean-k-mooney | melwitt: i have one update to push for that which is the db fixture is not required | |
| 15:54:59 | sean-k-mooney | and im going to suqah the follow up patch in to adress stephens nits but the main content of hte patch wont change | |
| 15:55:37 | sean-k-mooney | melwitt: actully the ooo job has now completed so ill push those change in the next 5 mins or so | |