Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-08-08
21:59:57 melwitt yes. I glossed over that from writing up all the other comments. O tjoml
22:00:05 melwitt *,I think
22:00:13 melwitt yes
22:00:20 mriedem so...
22:00:25 mriedem aren't the first and 2nd tests really the same thing?
22:00:40 mriedem or
22:00:45 mriedem i guess one is saying,
22:00:53 melwitt no because the first test will commit the reservations and the second one won't and will fall into the exception handling and rollback and refresh
22:00:54 mriedem the 2nd is saying,
22:01:15 mriedem between the time _lookup_instance returns the instance, another request deletes the build request, so conductor deletes the instance and then request 1 fails on instance.destroy()?
22:01:21 melwitt the first test never goes into the except InstanceNotFound block
22:01:43 melwitt yes
22:02:06 mriedem i'm not following "no because the first test will commit the reservations and the second one won't and will fall into the exception handling and rollback and refresh"
22:02:27 mriedem but both _lookup_instance and instance.destroy() are in the same try/except InstanceNotFound in _delete_while_booting
22:02:30 melwitt I just mean the first test will not cover the except InstanceNotFound block
22:02:38 mriedem so the point is, each test is covering two ways we could raise in there
22:02:41 melwitt _lookup_instance does not raise InstanceNotFound
22:02:52 melwitt it returns (None, None) if it catches InstanceNotFound
22:03:12 mriedem oh right
22:03:30 mriedem ok
22:04:37 melwitt so first case is _lookup_instance returns (None, None) because it found no instance. second case is instance was found but then instance.destroy() raises because conductor deleted it out from under us after we deleted the build request but before we got to instance.destroy()
22:05:19 melwitt *we deleted the build request, found the instance, and then it was gone in that small window when we got to destroy
22:06:09 mriedem ok
22:06:33 mriedem well, up to you on if you or dansmith want my comments addressed, if you don't care, then i don't really care much either and i'll +W
22:07:07 mriedem also,
22:07:33 dansmith I don't like the test on multiple fronts,
22:07:33 mriedem you're sure you're not hitting this? https://github.com/openstack/nova/blob/stable/ocata/nova/compute/api.py#L1729
22:07:38 mriedem b/c i see you're not using the AllServicesCurrent fixture
22:07:51 dansmith but it seems like the fix is worth getting in, nits on the test aside
22:07:54 melwitt I'd rather not have to convert them to unit tests at this point but if you want me to change it, I will
22:08:29 mriedem no i'm not talking about full on re-writing them as unit tests
22:08:39 mriedem i just meant with stuff like the AllServicesCurrent fixture and NoopConductorFixture
22:09:06 mriedem but we must not be hitting https://github.com/openstack/nova/blob/stable/ocata/nova/compute/api.py#L1729 because your change to the tests in the next patch are ok
22:09:25 melwitt yeah ... seems like it would return early because of the service version but the test does pass after the fix, which means it's getting to the quota calls somehow
22:09:45 dansmith because there are no services?
22:09:53 mriedem the api service is running
22:09:54 dansmith I don't see why we want noopconductor over just not starting it
22:10:29 dansmith but it's all just forced because this should be a unit test where it doesn't matter anyway :)
22:10:49 melwitt SORRY GUYS for my blasphemous functional test
22:10:50 mriedem i'm just surprised that fixture isn't needed is all i'm saying
22:11:03 mriedem like, why do we even need it in the first place in other func tests is my point
22:11:31 mriedem thou shalt be smoten with review nit-pickery
22:11:34 melwitt we have definitely needed it before. I'm not sure what's going on here
22:11:46 melwitt re: ServicesCurrent
22:12:11 dansmith if you have no services you get back the current I think
22:12:18 dansmith we used to return zero,
22:12:23 melwitt the api service doesn't count as a service?
22:12:39 melwitt os_apicompute
22:13:01 mriedem idk, might have something to do with wsgi-intercept usage in OSAPIFixture now
22:13:03 dansmith you're not starting an api service for real here
22:13:05 dansmith right
22:13:14 melwitt O.o
22:13:19 dansmith but anyway if this was A UNIT TEST
22:13:28 mriedem we used to
22:13:29 mriedem self.addCleanup(self.osapi.stop)
22:13:29 mriedem self.osapi.start()
22:13:29 mriedem self.osapi = service.WSGIService("osapi_compute")
22:13:32 melwitt :***(
22:13:37 mriedem which is probably why AllServicesCurrent was added before
22:15:12 mriedem +W
22:15:32 mriedem moving onto soul crusher #2
22:27:51 mriedem bazinga https://review.openstack.org/#/c/582413/8
22:28:04 melwitt argh
22:29:05 melwitt ok that's not so bad
22:29:28 mriedem do you want/need to hold the functional test patch below it out to add checks for core and ram usage?
22:29:57 mriedem or just slap those into this one
22:30:41 melwitt I'm fine with adding them to the second one
22:50:07 openstackgerrit Matt Riedemann proposed openstack/nova master: Docs: Add guide to migrate instance with snapshot https://review.openstack.org/584442
23:00:18 mriedem heh https://review.openstack.org/#/c/382853/
23:00:25 mriedem i knew i'd seen this recently
23:00:26 mriedem https://github.com/starlingx-staging/stx-nova/commit/71acfeae0d1c59fdc77704527d763bd85a276f9a#diff-77f9348ab09642ba46409b6828af4af0R3245
23:00:32 mriedem oct of 2016
23:04:48 openstackgerrit melanie witt proposed openstack/nova stable/ocata: [stable only] Handle quota usage during create/delete races https://review.openstack.org/582413
23:11:45 openstackgerrit Matt Riedemann proposed openstack/nova master: Update nova network info when doing rebuild for evacuate operation https://review.openstack.org/382853
23:12:57 openstackgerrit Matt Riedemann proposed openstack/nova master: Update nova network info when doing rebuild for evacuate operation https://review.openstack.org/382853
23:15:24 mriedem wow so that evacuate fix was approved in march of 2017,
23:15:27 mriedem but apparently needed a rebase
23:15:34 mriedem and then no one ever mentioned needing re-approval
23:17:00 melwitt yikes
23:18:01 openstackgerrit Matt Riedemann proposed openstack/nova stable/pike: Fix instance evacuation with PCI devices https://review.openstack.org/590059
23:29:22 melwitt I'm going to propose stable releases tomorrow. I meant to do it last week but didn't get to it
23:33:38 openstackgerrit Matt Riedemann proposed openstack/nova stable/queens: Update nova network info when doing rebuild for evacuate operation https://review.openstack.org/590062
23:34:41 openstackgerrit Merged openstack/nova master: get provider IDs once when building summaries https://review.openstack.org/589945
23:57:15 openstackgerrit Merged openstack/nova master: libvirt: Reduce calls to qemu-img during update_available_resource https://review.openstack.org/589513
23:57:22 openstackgerrit Merged openstack/nova master: Add tempest-slow job to run the tempest slow tests https://review.openstack.org/567697
23:57:32 openstackgerrit Merged openstack/nova master: Fix bad links for admin-guide https://review.openstack.org/580259
23:57:41 openstackgerrit Merged openstack/nova master: [placement] Avoid rp.get_by_uuid in allocation_candidates https://review.openstack.org/589941
23:57:49 openstackgerrit Merged openstack/nova master: api-ref: fix min_version for parent_provider_uuid in responses https://review.openstack.org/579577
23:57:57 openstackgerrit Merged openstack/nova master: doc: mark the max microversion for rocky https://review.openstack.org/589598
#openstack-nova - 2018-08-09
00:24:58 openstackgerrit Merged openstack/nova master: Add functional test for forced live migration rollback allocs https://review.openstack.org/586636
01:36:59 openstack bug 1783613 in OpenStack Compute (nova) ocata "[ocata only] quota usage not decremented during boot/delete race" [Undecided,In progress] https://launchpad.net/bugs/1783613 - Assigned to melanie witt (melwitt)
01:36:59 openstackgerrit Merged openstack/nova stable/ocata: [stable only] Add functional regression test for bug 1783613 https://review.openstack.org/588416
02:08:17 openstackgerrit Chen proposed openstack/nova stable/queens: Fix bad links for admin-guide https://review.openstack.org/590068
02:13:36 openstackgerrit Matt Riedemann proposed openstack/nova stable/pike: Update nova network info when doing rebuild for evacuate operation https://review.openstack.org/590070
02:16:02 openstackgerrit Chen proposed openstack/nova stable/queens: Fix bad links for admin-guide https://review.openstack.org/590068
02:16:37 openstackgerrit Chen proposed openstack/nova stable/pike: Fix bad links for admin-guide https://review.openstack.org/590072
02:39:05 openstackgerrit Chen proposed openstack/nova master: Update ssh configuration doc https://review.openstack.org/589844
02:45:27 openstackgerrit Ghanshyam Mann proposed openstack/nova master: Update the parameter explain when updating a volume attachment https://review.openstack.org/565181
02:56:50 openstackgerrit Takashi NATSUME proposed openstack/nova master: [placement] api-ref: add description for 1.29 https://review.openstack.org/589407
03:12:36 openstackgerrit Vishakha Agarwal proposed openstack/nova master: Quota details for key_pair "in_use" is 0. https://review.openstack.org/590081

Earlier   Later