Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-11-15
16:14:24 gibi bauzas: nova.tests.functional.test_nova_manage.TestNovaManagePlacementAudit.test_audit_orphaned_allocations_from_deleted_compute_evacuate failed for me locally now but with a different error than what I saw in the devstack
16:14:27 mriedem that's a slippery slope and bad habit to get into with commands like this
16:14:30 mriedem so just, don't do it
16:14:51 gibi bauzas: File "nova/cmd/manage.py", line 2927, in audit
16:14:51 gibi ctxt, placement, output, provider, delete)
16:14:51 gibi File "nova/cmd/manage.py", line 2781, in _check_orphaned_allocations_for_provider
16:14:55 gibi inst_uuids, mig_uuids = result
16:14:56 bauzas gibi: yeah, I just saw it
16:14:57 gibi TypeError: 'bool' object is not iterable
16:15:02 bauzas I'm on it
16:15:05 gibi bauzas: cool
16:15:35 gibi mriedem: do you mean sanity is slippery slope with nova manage cli ? :)
16:15:58 bauzas gibi: but for *some* reason, the functests should have caught the argument issue
16:16:04 bauzas (for the microversion)
16:16:11 bauzas I mean, the other issue is unrelateed
16:16:19 bauzas or a side effect
16:16:30 bauzas so, mriedem's point is legit
16:16:41 bauzas I'm leaking coverage here
16:17:30 mriedem gibi: heh, no, i just don't really trust unit tests in nova anymore for anything that involves more than one service or even more than a couple of methods interacting within the same service
16:17:38 mriedem it's too easy to mock things that become false positives
16:20:52 bauzas so the bug is fixed, but the argument issue isn't shown
16:26:27 gibi mriedem: I share your view regarding the unit tests in nova
16:27:16 bauzas 2019-11-15 17:26:12,996 INFO [placement.requestlog] 127.0.0.1 "GET /placement/resource_providers" status: 200 len: 840 microversion: 1.0
16:27:21 bauzas magnifico ^
16:27:39 bauzas the report client is blindly accepting the 'microversion' keyword
16:27:48 bauzas but just says "meh" to it
16:27:57 bauzas gibi: mriedem: ^
16:28:40 bauzas I don't see how to catch this unless to mock and assert the call
16:28:50 bauzas which is nonsense
16:29:21 gibi bauzas: then something is wrong with the placement fixture we use in the functional test
16:29:45 bauzas and, with using the 'version' keyword :
16:29:52 bauzas 2019-11-15 17:29:16,087 INFO [placement.requestlog] 127.0.0.1 "GET /placement/resource_providers" status: 200 len: 1684 microversion: 1.14
16:30:05 bauzas we probably kwargs
16:30:07 mriedem def get(self, url, **kwargs): is your problem
16:30:08 bauzas gibi: ^
16:30:13 bauzas heh, this
16:30:13 mriedem like gibi said it's a bug in the fixture
16:30:41 mriedem SchedulerReportClient.get won't let you pass microversion=
16:30:42 mriedem def get(self, url, version=None, global_request_id=None):
16:30:43 gibi then it is a good time to fix the fixture
16:30:51 gibi :)
16:31:42 bauzas aaaaand I love to see a bug about some code with a few TODO(sbauza) around it...
16:33:02 openstackgerrit Stephen Finucane proposed openstack/nova master: functional: Change order of two classes https://review.opendev.org/689178
16:33:03 openstackgerrit Stephen Finucane proposed openstack/nova master: functional: Rework '_delete_server' https://review.opendev.org/689179
16:33:03 openstackgerrit Stephen Finucane proposed openstack/nova master: functional: Make '_wait_for_state_change' behave consistently https://review.opendev.org/689180
16:33:04 openstackgerrit Stephen Finucane proposed openstack/nova master: functional: Unify '_wait_until_deleted' implementations https://review.opendev.org/689181
16:33:04 openstackgerrit Stephen Finucane proposed openstack/nova master: functional: Make 'ServerTestBase' subclass 'InstanceHelperMixin' https://review.opendev.org/689182
16:33:36 mriedem should just remove those todos in the placement fixture about passing a token, that's never going to happen if it hasn't happened by now
16:33:48 mriedem we should like, put a 3 year timer on todos in the code
16:34:05 mriedem "TODO(vishy): do this in grizzly"
16:34:28 gibi :)
16:36:06 openstackgerrit Sylvain Bauza proposed openstack/nova master: Add a placement audit command https://review.opendev.org/670112
16:38:06 openstackgerrit Stephen Finucane proposed openstack/nova master: docs: Remove 'adv-config', 'system-admin' subdocs https://review.opendev.org/684402
16:38:06 openstackgerrit Stephen Finucane proposed openstack/nova master: docs: Replacing underscores with dashes https://review.opendev.org/685929
16:38:07 openstackgerrit Stephen Finucane proposed openstack/nova master: docs: Strip '.rst' suffix https://review.opendev.org/687264
16:42:43 stephenfin melwitt: If you have time before the end of the day, think you could hit this quotas doc patch? https://review.opendev.org/#/c/667165/
16:42:53 stephenfin It's been around for a few months. Would be nice to close it out
16:43:09 KeithMnemonic1 hello all any chance to get some more reviews and a possible WF+1 on this one ? https://review.opendev.org/#/c/683008/
16:43:30 melwitt stephenfin: that's a biggun. I'll try to get to it
16:44:06 stephenfin melwitt: Sure thing. tbh, I'd just forget whatever was there previously and read it like a new doc. If nothing stands out as totally wrong, it's gonna be better than what we have
16:44:09 stephenfin IMO, obv
16:44:51 sean-k-mooney KeithMnemonic1: i think lyarwood should proably review that
16:45:07 melwitt stephenfin: ok, I'll approach it with that in mind
16:47:45 stephenfin ta
16:48:12 stephenfin Have a good weekend, all o/
16:49:02 melwitt happy weekend
16:51:26 gibi bauzas: another bug in https://review.opendev.org/#/c/670112/9/nova/cmd/manage.py@2695 this is also not caught by any functional test as that is green for me on PS9
16:52:10 bauzas gibi: thanks
16:52:34 gibi bauzas: don't be confused by the line numbers in my stack trace. I had to cherry-pick your patch top of the nova master in my devstack
16:52:36 melwitt gibi: I dunno if you saw this but I attempted to do a TODO you mentioned in the NeutronFixture for fun https://review.opendev.org/693453
16:52:47 bauzas gibi: ack no worries
16:53:29 gibi melwitt: yeah I think I saw it during the PTG but then I got distracted. Looking at it now. Thanks for picking that TODO up
16:53:53 melwitt gibi: don't thank me yet haha, I hope I didn't misunderstand
16:56:25 bauzas gibi: and I get why you catched the error and not my tests
16:56:53 bauzas gibi: that's because when I use the cache, I forget to use the cell mapping
16:57:13 bauzas so, unless you have more than one instance for the same compute, you don't have the problem
16:57:29 bauzas (and I only verify one instance... :) )
16:59:00 KeithMnemonic1 thanks sean-k-mooney, lyarwood would you have time in the next few business days to look at it?
17:00:11 KeithMnemonic1 can someone please continue the reviews on mriedem patches for the "openstack list marker hang" https://review.opendev.org/#/c/690721/4
17:01:46 sean-k-mooney KeithMnemonic1: lyarwood for what its worth im +0.5 on it. i dont know htat part of the code well enought to tell if all of the change form queens make sense but the conflcits are called out in the commit and it looks more or less correct
17:03:35 gibi bauzas: I have one instance in shutoff state on the compute
17:03:51 bauzas no worries, I'm just fixing the bug
17:03:55 gibi bauzas: sure
17:04:02 bauzas and thanks, I should actually test it
17:04:35 bauzas it's 6.04pm tho, so I'll just fix the bug and add a TODO in the commit msg for saying I need to add a new func test for it :)
17:04:39 bauzas ie. two instances :-)
17:04:49 bauzas s/add/modify
17:05:01 gibi bauzas: yeah, I will time out soon as well
17:05:13 bauzas actually, you know what ? I'm just gonna amend the test now
17:05:17 bauzas and see whether it's seen
17:17:35 openstackgerrit Merged openstack/nova stable/queens: Revert "openstack server create" to "nova boot" in nova docs https://review.opendev.org/693239
17:26:15 openstackgerrit Sylvain Bauza proposed openstack/nova master: Add a placement audit command https://review.opendev.org/670112
17:30:24 gibi melwitt: two small cleanup suggestion in https://review.opendev.org/#/c/693453/3 then I'm +A
17:31:25 melwitt gibi: thanks, will update
17:31:46 gibi melwitt: I thank you :)
17:35:48 melwitt gibi: thank you for review :)
17:42:45 openstackgerrit Archit Modi proposed openstack/nova stable/pike: doc: fix and clarify --block-device usage in user docs https://review.opendev.org/694450
18:02:43 openstackgerrit melanie witt proposed openstack/nova master: Use wrapper class for NeutronFixture get_client https://review.opendev.org/693453
18:04:17 melwitt gibi: updated ^
18:18:47 openstackgerrit Dustin Cowles proposed openstack/nova-specs master: Update provider config spec for identification conflicts https://review.opendev.org/693414

Earlier   Later