Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-09-04
18:40:19 mriedem so we had to go to his house b/c he coudn't leave, but then we had to watch renegade
18:42:01 mriedem who is lorenzo roundhouse kicking today?
18:43:36 melwitt heh
18:54:09 zbr efried: does https://review.opendev.org/#/c/674057/ look ok now?
18:59:17 artom I'm a dumbass
18:59:42 artom I spent literally hours, at this point, trying to stub, mock, or hack __dict__ to pin a compute RPC object
19:00:09 artom (Because def router is an @property that can't be set/stubbed, and it's using a global singleton)
19:00:30 artom When what I *should* have done is just inherited from ComputeAPI and overwrote def router >_<
19:02:58 dansmith can't you just kill router on the instance that each one gets?
19:03:09 dansmith service.manager.rpcapi.router = mock.Mock() ?
19:03:14 dansmith or whatever
19:03:54 artom @property makes it un-settable
19:04:09 dansmith can you delattr it?
19:04:35 artom Huh, didn't think to try that
19:04:39 dansmith anyway, doesn't matter
19:30:36 openstackgerrit Artom Lifshitz proposed openstack/nova master: NUMA live migration support https://review.opendev.org/634606
19:30:37 openstackgerrit Artom Lifshitz proposed openstack/nova master: Functional tests for NUMA live migration https://review.opendev.org/672595
19:30:37 openstackgerrit Artom Lifshitz proposed openstack/nova master: Deprecate CONF.workarounds.enable_numa_live_migration https://review.opendev.org/640021
19:31:51 artom dansmith, ^^
19:32:20 artom (FWIW, I had the -patch27 in my local branch name for some reason, fixed that)
19:36:16 dansmith artom: one more thing I didn't catch last time, but it's easy
19:36:28 dansmith artom: otherwise I think it looks good, probably time for mriedem to have a go at it
19:41:07 aspiers can anyone point me at where the fake server instance object comes from in https://github.com/openstack/nova/blob/master/nova/tests/unit/api/openstack/compute/test_migrate_server.py#L579 ?
19:41:18 aspiers I've trawled through the test setup code and can't find it
19:43:04 aspiers ah
19:43:12 aspiers admin_only_action_common.py
19:53:09 aspiers still don't get it
19:53:38 aspiers if I use _stub_instance_get() then I get "Exception: This test uses methods that set internal oslo_db state, but it does not claim to use the database. This will conflict with the setup of tests that do use the database and cause failures later."
19:59:20 melwitt aspiers: is there a link to a patch you're working on that will give more context? my first guess is maybe your new test is accessing an attribute on instance that isn't populated, so it's trying to lazy-load it from the database, which is flagging your test as USES_DB = True
19:59:27 openstackgerrit Merged openstack/nova master: Remove unused methods https://review.opendev.org/679918
20:00:17 aspiers melwitt: I'll upload
20:01:06 melwitt that error comes from a test inheriting from NoDBTestCase and then accessing the database
20:05:16 aspiers melwitt: does this test avoid it by patching out the meat of the code path (i.e. resize)? https://github.com/openstack/nova/blob/master/nova/tests/unit/api/openstack/compute/test_migrate_server.py#L124
20:05:34 aspiers so it's effectively only testing the HTTP stuff in the top layer?
20:07:06 melwitt probably, that stub will fill in most of the instance objects attributes. so if anything in the code path accesses the attributes, they are already filled in,
20:07:42 melwitt with objects, if an attribute is *not* filled in and it is accessed, the object will fall back on a read from the database to get the value
20:08:07 openstackgerrit Merged openstack/nova master: Convert nova-lvm job to zuul v3 https://review.opendev.org/674345
20:08:29 melwitt so it will test the full code path, just not the database accesses (if there were going to be any)
20:08:30 efried sean-k-mooney: in case this doesn't show up in your inbox/dashboard/whatever, would you please respond to this os-vif release patch: https://review.opendev.org/#/c/680094/
20:09:10 aspiers melwitt: ahhh maybe that's it
20:10:09 aspiers melwitt: maybe it's missing the flavor or image_meta
20:10:39 melwitt aspiers: so sometimes what happens is you're working on a new test whose code path will access an attribute that's not stubbed out/filled in, and then it falls back on a read from the db, and if you're inherited from NoDBTestCase, that error will raise
20:11:18 aspiers no, just adding a call to self._stub_instance_get() is enough to trigger this db-related failure, *even* if I don't use the returned fake instance
20:11:31 aspiers so it's not my test code which is triggering the error
20:11:54 melwitt yeah if you look in https://github.com/openstack/nova/blob/master/nova/tests/unit/fake_instance.py#L45 you can see what's been filled in
20:11:59 aspiers yeah I spotted that
20:12:04 melwitt oh, hm
20:12:12 aspiers but this makes no sense, because other tests happily call that same method
20:13:40 aspiers melwitt: uploading now
20:13:52 openstackgerrit Adam Spiers proposed openstack/nova master: Reject live migration and suspend on SEV guests https://review.opendev.org/680158
20:13:56 aspiers there ^^^
20:14:40 aspiers melwitt: https://review.opendev.org/#/c/680158/1/nova/tests/unit/api/openstack/compute/test_migrate_server.py@592 causes this db error
20:14:51 aspiers why do other identical lines in the same file not cause it?
20:16:22 melwitt at a glance, I notice the other tests calling _stub_instance_get are also mocking the live_migrate method in compute_api
20:16:35 aspiers right, that's what I was saying earlier
20:16:43 aspiers they mock the meat of the code path
20:17:19 aspiers but the call to _stub_instance_get() isn't inside the mocking?!
20:17:49 aspiers so it can't be that
20:17:57 aspiers super weird
20:18:06 openstackgerrit Adam Spiers proposed openstack/nova master: Reject live migration and suspend on SEV guests https://review.opendev.org/680158
20:18:28 melwitt you're saying the _stub_instance_get call itself will not return and raises that error?
20:18:33 aspiers yes
20:18:39 melwitt O.o
20:18:53 melwitt ok, that is unexpected
20:19:24 aspiers oh no wait
20:19:32 aspiers OK I get it now
20:19:42 aspiers _stub_instance_get() has side effects of course
20:19:47 aspiers it patches stuff
20:19:53 aspiers so it doesn't matter if I don't use the return value
20:20:55 aspiers yeah it's the lookup of instance.image_meta which fails
20:21:33 mriedem dansmith: artom: ack, probably going to have to be in the morning at this point, just got off a call
20:21:42 melwitt aspiers: where's the lookup? I'm curious
20:22:04 aspiers melwitt: https://review.opendev.org/#/c/680158/2/nova/compute/api.py@218
20:22:13 aspiers melwitt: I think my mocking of that hardware method is broken probably
20:22:24 aspiers mocking in the wrong place is my guess
20:22:44 aspiers need to mock in nova.compute.api
20:22:54 melwitt ok, I thought you said _stub_instance_get was blowing up itself
20:22:59 aspiers yeah I was wrong
20:23:03 melwitt and I was curious where in there it accesses db
20:23:04 melwitt ok
20:24:44 aspiers ohh doh
20:25:05 aspiers patching hardware.get_mem_encryption_constraint doesn't stop the calculation of the parameters being passed to it
20:25:16 aspiers so I have to patch instance.image_meta too, or just set it
20:31:29 dansmith mriedem: np
20:41:39 aspiers melwitt: got it working finally
20:41:48 aspiers melwitt: thanks for your help :)
20:41:51 melwitt yay
20:43:03 openstackgerrit Merged openstack/nova master: Modernize nova-lvm job https://review.opendev.org/674356
20:45:55 openstackgerrit Adam Spiers proposed openstack/nova master: Reject live migration and suspend on SEV guests https://review.opendev.org/680158
20:46:17 aspiers melwitt: https://review.opendev.org/#/c/680158/2..3/nova/tests/unit/api/openstack/compute/test_suspend_server.py@47
20:47:09 openstackgerrit Adam Spiers proposed openstack/nova master: Enable booting of libvirt guests with AMD SEV memory encryption https://review.opendev.org/666616
20:48:10 melwitt aspiers: oh, so you didn't need to stub compute_api
20:48:19 aspiers melwitt: right, it was just image_meta
20:48:43 aspiers I didn't want to stub compute_api because that's where the desired exception gets thrown
20:49:15 melwitt yeah, makes sense
20:55:27 openstackgerrit Matt Riedemann proposed openstack/nova master: FUP for I66d8f06f19c5c631e33208580428aa843abb38d2 https://review.opendev.org/678951
20:57:14 aspiers oh crap, circular imports
21:01:38 aspiers what's the ASCII smiley for a facepalm?
21:09:34 aspiers efried: make sense if I move sev_enabled() from libvirt.utils to virt.hardware to break the circular import?
21:10:04 efried aspiers: without looking specifically at the patch, that sounds like a reasonable thing.

Earlier   Later