| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-03-01 | |||
| 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 | temka | If anyone says 'dependency injection' there's going to be violence | |
| 20:38:28 | dansmith | yeah | |
| 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 | 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:16:54 | mriedem | mnaser: like this ^ | |
| 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 | dansmith | heh nice | |
| 21:28:40 | mnaser | lol yeah | |
| 21:28:40 | mnaser | github | |
| 21:28:40 | mriedem | not cli | |
| 21:28:41 | mnaser | mriedem: i noticed if you refresh a few times | |
| 21:28:44 | mnaser | it eventually goes through | |
| 21:28:49 | mnaser | i assume that it caches portions of the git blame page | |
| 21:28:50 | mriedem | git blame nova.compute.manager | |
| 21:28:54 | mriedem | or nova.virt.libvirt.driver | |
| 21:29:03 | mnaser | and then eventually caches enough so the request doesnt timeout | |
| 21:29:38 | openstackgerrit | Merged openstack/nova master: Add "links" in the response of "nova show" for a down-cell instance https://review.openstack.org/640302 | |
| 21:31:04 | mriedem | well i see this has come up before https://review.openstack.org/#/q/I3b9f60a2c4aea904f39d6914c8f35c4d682a6b3a | |
| 21:32:16 | mriedem | followed by https://review.openstack.org/#/q/I53d5284907d44ae8b5546993f8fd461b385c39e6 | |
| 21:32:20 | mriedem | we can't seem to make up our mind | |
| 21:40:03 | 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:41:24 | mriedem | melwitt: well i'm kind of stuck on how to proceed with your counting quotas from placement stuff | |
| 21:44:44 | openstackgerrit | Tim Rozet proposed openstack/nova master: Fixes race condition with privsep utime https://review.openstack.org/625741 | |
| 21:49:53 | mriedem | stephenfin: in case you didn't notice it looks like mikal would like you to remove the -2 from https://review.openstack.org/#/c/554438/ | |
| 21:56:48 | openstackgerrit | Merged openstack/nova master: conf: Deprecate 'disable_libvirt_livesnapshot' option https://review.openstack.org/626932 | |
| 21:57:12 | openstackgerrit | Merged openstack/nova master: Stop using "nova" in API samples when creating a server https://review.openstack.org/639874 | |
| 21:57:29 | openstackgerrit | Merged openstack/nova master: Make Claim._claim_test handle SchedulerLimits object https://review.openstack.org/636410 | |
| 22:16:50 | mriedem | is InstanceNUMACell.id a db primary key or something from the actual host...? | |
| 22:17:49 | mriedem | i guess the latter | |