| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-03-01 | |||
| 20:23:41 | temka | mriedem, it's solved in the fake virt driver, but that's different because it's a virt drive | |
| 20:24:00 | temka | fakelibvirt isn't a driver - the real libvirt is the driver | |
| 20:24:17 | temka | fakelibvirt just mocks out the parts that call the libvirt API | |
| 20:24:43 | mriedem | i.e. https://github.com/openstack/nova/blob/master/nova/virt/fake.py#L654 | |
| 20:25:08 | temka | mriedem, right, and virt/fake.py replaces virt/libvirt/driver.py | |
| 20:25:15 | mriedem | yeah i know | |
| 20:25:23 | temka | fakelibvirt.py replaces what virt/libvirt/driver.py calls down to | |
| 20:25:41 | mriedem | right the python-libvirt interface | |
| 20:25:57 | temka | Right | |
| 20:27:04 | mriedem | dansmith: am i correct in my understanding of what you're asking for here? https://review.openstack.org/#/c/633350/7/nova/objects/instance_mapping.py@42 | |
| 20:27:35 | mriedem | on load of instance mapping, of it doesn't have a user_id, try to get it from the request spec, failing that, try the build request, failing *that* try the instance via the cell mapping | |
| 20:27:45 | mriedem | s/of/if/ | |
| 20:28:04 | dansmith | mriedem: right, the nullability has nothing to do with where or how we get the user_id to fill it in | |
| 20:29:01 | temka | I guess I have to mock ComputeManager._get_nodename() | |
| 20:29:14 | temka | ... per service | |
| 20:29:26 | mriedem | dansmith: which means we wouldn't need to update the instance mapping on read of the instance from the api https://review.openstack.org/#/c/638574/4/nova/compute/api.py@2404 | |
| 20:29:42 | mriedem | although, if the request spec doesn't have user_id, and the build request is gone, and the cell the instance is in is down... | |
| 20:30:10 | mriedem | then i guess you're sol? | |
| 20:30:45 | mriedem | temka: you can't override the fakelibvirt.Connection hostname value it uses? | |
| 20:30:46 | aspiers | OK, weekend time. Have a good one everyone o/ | |
| 20:31:22 | temka | mriedem, I guess I could, but I can't make it different depending on which compute service is calling it | |
| 20:32:51 | dansmith | temka: mock service.manager._get_nodename() after it's started to return the value it needs to keep returning, instead of mocking the class | |
| 20:33:15 | temka | dansmith, tried that just now, didn't work | |
| 20:33:25 | temka | I think it needs to be mocked *before* (somehow) it's started | |
| 20:33:35 | dansmith | mriedem: yeah, I mean, making that value nullable for the down-cell-and-not-anywhere-else case is not reasonable I think | |
| 20:33:38 | temka | Because upon startup it reports to placement, and there's a hostname conflict | |
| 20:33:48 | dansmith | temka: you have to mock it both places, | |
| 20:34:00 | dansmith | temka: which is why I said "needs to keep returning" | |
| 20:34:16 | temka | dansmith, which both places? | |
| 20:34:27 | dansmith | the class will catch the startup case, but be invalid once you move to the next service, so you need to mock the resulting instance of the manager class to keep returning the value ongoing | |
| 20:34:50 | temka | Ah, I see | |
| 20:35:08 | dansmith | we used to be able to pass in a different manager class to start_service i think, which would let you just subclass per node.. can we still do that? | |
| 20:35:46 | temka | So mock fakelibvirt's getHostname, start compute0, mock its get_nodename, mock getHostname to the 2nd thing, start compute1, mock its get_nodename() | |
| 20:35:50 | temka | That feels... racy | |
| 20:36:02 | temka | Actually no | |
| 20:36:07 | dansmith | temka: there are no threads here so, no | |
| 20:37:03 | mriedem | heh you still can technically classload a compute manager class it looks like | |
| 20:37:13 | dansmith | mriedem: yeah that's what I was thinking of | |
| 20:38:08 | mriedem | self.start_service('compute', host='fake-host1', manager='artom.special.fake.ComputeManager') | |
| 20:38:28 | dansmith | yeah | |
| 20:38:28 | temka | If anyone says 'dependency injection' there's going to be violence | |
| 20:38:38 | temka | I *still* have no idea what that is | |
| 20:39:01 | temka | Despite university's best attempt to drill it into our heads with massive bloated Java frameworks | |
| 20:39:27 | mriedem | i should have gone to university to learn about hospital | |
| 20:39:38 | temka | dansmith, woot, that worked | |
| 20:39:39 | mriedem | colour flavour | |
| 20:40:44 | mriedem | dansmith: so my one reservation on loading the instance mapping user_id from the turducken is that down cell case because i'm not sure what we'd do, likely 500 from the api, but you can't really online data migrate your way out of that b/c the online data migration relies on, you guessed it, the cells being up | |
| 20:41:02 | dansmith | mriedem: yeah | |
| 20:41:29 | dansmith | you're also migrating from a state where cells had to be up, | |
| 20:41:42 | dansmith | so until all that data is patched up, it's not surprising that...all cells have to be up | |
| 20:44:49 | openstackgerrit | Merged openstack/nova master: Ironic: bump minimum API version to 1.38 https://review.openstack.org/636326 | |
| 20:56:31 | temka | "Allocation for resource provider 'e7fbbec5-7f95-4f4e-895c-a14b691bca6e' that does not exist" | |
| 20:56:40 | temka | Looks like I'm missing a placement-y fixture or something | |
| 20:57:34 | mnaser | does nova cache neutron port info somewhere? | |
| 20:57:50 | mriedem | mnaser: instance_info_caches table | |
| 20:57:51 | mnaser | i had a misconfigured n-ovs-agent which lead to resizes failing with binding_failed on the port | |
| 20:58:00 | mnaser | i've reset the info, but its still grabbing the cached value | |
| 20:58:12 | mnaser | ok, i'll dig there, thanks mriedem | |
| 20:58:47 | mriedem | mnaser: see https://review.openstack.org/#/c/591607/ | |
| 20:59:20 | mriedem | the instance_info_caches.network_info is a json blob string so if you're looking to mess with that manually.... | |
| 20:59:38 | mnaser | mriedem: neat. i assume its not a real cache as in i cant really just drop that row | |
| 20:59:56 | mriedem | ummm | |
| 21:01:35 | mriedem | i don't think that will work | |
| 21:02:04 | mriedem | because anything that tries to refresh the cache from neutron will just rely on the existing cache, which will now be empty... | |
| 21:02:12 | mriedem | which is what https://review.openstack.org/#/c/591607/ was trying to address | |
| 21:03:39 | mriedem | wholesale deleting of the cache would also make nova forget which ports nova created and which were pre-created, so on server delete nova will also delete any pre-existing ports | |
| 21:03:46 | mriedem | which is probably not something your users want | |
| 21:04:50 | mriedem | related change here https://review.openstack.org/#/c/603844/ | |
| 21:05:19 | mriedem | the issue is kind of this code right here https://github.com/openstack/nova/blob/c57d532a306303412d6253f424bee7d665865279/nova/network/neutronv2/api.py#L3055 | |
| 21:05:38 | mriedem | if you wipe out that record in the db, that will result in port_ids being empty | |
| 21:05:46 | mriedem | so https://github.com/openstack/nova/blob/c57d532a306303412d6253f424bee7d665865279/nova/network/neutronv2/api.py#L3061 won't do anything | |
| 21:06:20 | mriedem | it's kind of dumb - if port_ids was empty but current_neutron_port_map is not, we should probably realize "oh we're fully rebuilding the cache b/c the cache was wiped out" | |
| 21:07:08 | mnaser | mriedem: yeah so cache is probably not really cache i guess | |
| 21:07:25 | mnaser | it is tracking state | |
| 21:07:42 | mriedem | well it's a cache in the sense that you don't have to go to neutron every time you want to find out what ports are attached to a server, | |
| 21:07:48 | mriedem | similar to the block_device_mappings table is a "cache" | |
| 21:07:52 | mriedem | of volumes attached to the server | |
| 21:08:02 | mnaser | yeah | |
| 21:08:27 | mriedem | but yeah it's got some state information in there too about the ports nova knows it created | |
| 21:08:45 | mnaser | (ps i totally thought this whole detach => delete port thing is silly but thats another topic) | |
| 21:09:00 | mnaser | this whole thing would be so much easier if i just detached the port and attached it again | |
| 21:09:04 | mnaser | but because it was done on boot, detaching the port = deleting it | |
| 21:09:47 | mriedem | oh there was a spec for that... | |
| 21:09:53 | mriedem | i.e. delete_on_termination for ports | |
| 21:09:58 | mriedem | like volumes | |
| 21:11:09 | mriedem | anyway like i said if you delete the cache via the db, and port_ids is empty but current_neutron_port_map is not https://github.com/openstack/nova/blob/c57d532a306303412d6253f424bee7d665865279/nova/network/neutronv2/api.py#L3061 you could probably just force a cache refresh | |
| 21:13:52 | cfriesen | can someone take a look at http://logs.openstack.org/63/631363/15/check/openstack-tox-py27/6b108d3/testr_results.html.gz and suggest why test_executable_exists_in_path is failing? It works perfectly on my local box. | |
| 21:16:54 | mriedem | mnaser: like this ^ | |
| 21:16:54 | openstackgerrit | Matt Riedemann proposed openstack/nova master: WIP: Force refresh nw info cache if cache is empty but ports attached https://review.openstack.org/640516 | |
| 21:27:15 | mriedem | dansmith: here is another fun side effect of the mox->mock test removal - besides making backports a nightmare, it also makes git blame a real chore | |
| 21:27:18 | mriedem | that is all | |
| 21:27:36 | dansmith | yeah | |
| 21:27:48 | mriedem | plus most parts of nova you can't git blame in github anymore b/c it times out | |
| 21:28:25 | dansmith | ...really? | |
| 21:28:33 | dansmith | with git or github? | |
| 21:28:37 | mriedem | github | |
| 21:28:40 | mriedem | not cli | |
| 21:28:40 | mnaser | github | |
| 21:28:40 | mnaser | lol yeah | |
| 21:28:40 | dansmith | heh nice | |