Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2017-01-06
12:15:05 mdbooth Is anybody working on the gate snafu, btw?
12:16:09 mdbooth nova.tests.unit.test_cinder.CinderV1TestCase.test_cinderclient_unsupported_v1
12:17:07 mdbooth cdent: You currently have a zuul job failing in ^^^ btw
12:17:52 cdent mdbooth: that was talked about last night
12:18:11 cdent https://bugs.launchpad.net/nova/+bug/1654393
12:18:11 openstack Launchpad bug 1654393 in OpenStack Compute (nova) ""keystoneauth1.exceptions.catalog.EmptyCatalog: The service catalog is empty." in test_cinderclient_unsupported_v1" [High,In progress] - Assigned to Matt Riedemann (mriedem)
12:18:22 cdent matt's got a fix pending
12:18:37 mdbooth cdent: Impact is nothing can land, right?
12:18:54 cdent I was under the impression it was racy
12:19:27 johnthetubaguy sfinucan: I wasn't sure about this one: https://review.openstack.org/#/c/413658/2 does my comment make sense?
12:21:30 sfinucan johnthetubaguy: Yup, `null` would be the JSON variant
12:23:58 sfinucan johnthetubaguy: In turn, any idea if the comments on this patch correct? I've no idea, I'm afraid :/ https://review.openstack.org/#/c/416959/1/nova/conf/quota.py
12:25:04 johnthetubaguy sfinucan: oh, that is correct I think
12:25:16 johnthetubaguy sfinucan: all the network ones, there is a bit in the code that would confirm that, one sec
12:26:04 sdague_ I'm back now, what's up?
12:26:17 sfinucan johnthetubaguy: Yeah, I've found in rather touch to delineate between nova-net and neutron in 'nova/network' where most of these variables are used
12:27:25 sfinucan sdague: Any idea about the the validity of comments on https://review.openstack.org/#/c/416959/1/nova/conf/quota.py ?
12:29:34 sdague sfinucan: I'm pretty sure Takashi is right, those also only have an effect when nova-network is in play
12:31:56 sfinucan sdague: johnthetubaguy: Cool cool. I'll respin that to drop those so
13:14:25 jaypipes good morning Novas.
13:36:15 openstackgerrit Stephen Finucane proposed openstack/nova: libvirt: Make 'get_domain' private https://review.openstack.org/417378
13:54:43 openstackgerrit Ed Leafe proposed openstack/nova: Remove the Allocation.create() method. https://review.openstack.org/416751
13:54:44 openstackgerrit Ed Leafe proposed openstack/nova: Add check for invalid allocation amounts https://review.openstack.org/416752
13:55:43 openstackgerrit Stephen Finucane proposed openstack/nova: rbd: Remove unnecessary 'encode' calls https://review.openstack.org/412356
13:59:40 openstackgerrit Stephen Finucane proposed openstack/nova: conf: Deprecate yet more nova-net options https://review.openstack.org/416959
14:10:33 openstackgerrit melissaml proposed openstack/nova: Fix a typo in documents https://review.openstack.org/417389
14:14:25 cdent mdbooth: maybe you have some thoughts on this: Do you know to what extent eventlet gets involved when running the unit tests?
14:15:12 mdbooth cdent: I think sdague would be a better authority on this stuff, but to the best of my knowledge: not really.
14:15:33 mdbooth I understand that tox fires up several workers in independent processes, which then run tests serially.
14:15:49 cdent (I was asking in relation to your comment on matt's fix for the gate failure)
14:15:55 mdbooth So 'races' shouldn't really occur.
14:16:16 mdbooth However, you can get ordering artifacts if tests don't clean up after themselves properly.
14:16:19 cdent yeah, that was my thinking too, but if eventlet does getinvolved, then game over
14:16:31 cdent s/over/confused/
14:16:42 mdbooth However, I'd defer entirely to sdague :)
14:16:56 sdague cdent: what is the eventlet concern when tests are run?
14:17:10 cdent So I've been assuming it doesn't (as that would be bad as it introduces ambiguity).
14:17:43 mdbooth Incidentally, I'm just starting to look at this myself.
14:17:44 cdent sdague: If eventlet is present and monkey patching file io in some sense, you might get an unexpected yield to an unexpected greenthread?
14:18:11 cdent I don't really know, I'm experiening curiosity. Expressing it to see if anyone else knows what's up.
14:18:20 sdague cdent: eventlet is definitely present
14:20:06 sdague but, there are going to need to be spawns to have greenlets running, right? We use that to our advantage on the functional tests with the single process cloud
14:21:29 cdent That's effectively my question: If your first statement/question is true, not an issue.
14:22:20 sdague the parallel running of tests is via testr spawning dedicated processes with subsets of the tests, and running subunit serially in each
14:23:03 cdent yeah, that part I understand (and know too well, dealing with trying to get gabbi to behave properly in that scenario (it does))
14:23:07 sdague so... as long as tests clean up after themselves, there shouldn't be interference between tests. But cleaning up after themselves is sometimes not super clear, especially if globals are set
14:38:18 openstackgerrit John Garbutt proposed openstack/nova: correct misleading wording https://review.openstack.org/413658
14:38:44 openstackgerrit John Garbutt proposed openstack/nova: correct misleading wording https://review.openstack.org/413658
14:49:54 mdbooth cdent: https://bugs.launchpad.net/nova/+bug/1654393/comments/2
14:49:54 openstack Launchpad bug 1654393 in OpenStack Compute (nova) ""keystoneauth1.exceptions.catalog.EmptyCatalog: The service catalog is empty." in test_cinderclient_unsupported_v1" [High,In progress] - Assigned to Matt Riedemann (mriedem)
14:50:30 cdent go you. mdbooth++
14:51:37 johnthetubaguy was just looking at matt's patch he has up for that
14:54:01 mdbooth johnthetubaguy: Yeah, I wasn't convinced.
14:55:02 mdbooth So, it looks like it fails because _SESSION already exists
14:55:48 johnthetubaguy yeah, someone already set it up somehow
14:56:03 mdbooth johnthetubaguy: See my comment
14:56:21 mdbooth Looks like it's pretty easy to initialise
14:56:33 johnthetubaguy so if _SESSION is set, the old mock would never get run
14:56:39 johnthetubaguy but thats not true after his patch
14:56:41 johnthetubaguy I think?
14:58:17 johnthetubaguy hang on, I am missreading this
14:58:36 mdbooth johnthetubaguy: I'm going to submit an alt patch which does cinder.reset_globals() instead
14:58:51 johnthetubaguy mdbooth: ah, gotcha
14:59:06 mdbooth Because that's the real issue here. It's not that mock is stamping on things, it's that something previously initialised global state.
14:59:30 johnthetubaguy yeah, its global state already being tweaked
14:59:52 mdbooth Ah... and BaseCinderTestCase also does that
15:00:33 openstackgerrit Stephen Finucane proposed openstack/nova: console: Move proxies to 'console/proxy' https://review.openstack.org/408192
15:00:34 openstackgerrit Stephen Finucane proposed openstack/nova: conf: Deprecate 'console_driver' https://review.openstack.org/408193
15:00:34 openstackgerrit Stephen Finucane proposed openstack/nova: tests: Replace use of CONF with monkey patching https://review.openstack.org/408191
15:04:14 cdent Is it fair to ask "Why do we have global state?"?
15:04:45 mriedem i'm going to be sending off the user survey feedback question today so it's last call for questions
15:05:46 johnthetubaguy cdent: its a cache I believe
15:06:06 openstackgerrit Matthew Booth proposed openstack/nova: Reset global state in CinderV1TestCase https://review.openstack.org/417408
15:06:21 johnthetubaguy cdent: still a valid question
15:06:22 cdent mriedem: "Do you wish there was an orchestration layer?"
15:06:23 mriedem mdbooth: i've already had a patch up for that since yesterday
15:06:48 mdbooth mriedem: Yeah, I read it earlier.
15:07:02 mriedem mdbooth: my first attempt at it failed, but the one that you -1ed has passed jenkins several times already
15:07:52 mdbooth mriedem: See ^^^. I'm doing reset_globals(). Your patch mocks _SESSION. It has the same effect, but reset_globals() is how the other tests do it.
15:08:04 mdbooth And it's more explicit as to why it's being done.
15:08:11 mriedem mdbooth: the reset_globals() thing seems like a hack to begin with
15:08:31 mriedem mdbooth: we shouldn't have to deal with a global between concurrently running tests
15:08:34 mdbooth You need a hack like this everywhere you have global state.
15:08:37 mriedem which is why i'm mocking out _SESSION altogether
15:08:45 mdbooth We don't have concurrently running tests.
15:09:02 mriedem sure we do, between test classes at least
15:09:10 mdbooth That's why I was -1 on your patch, because I think it obscured the issue.
15:09:34 mdbooth I don't think so. They run serially in multiple processes.
15:09:46 mdbooth The issue here wasn't a race. It was that something else previously initialised the global state.
15:10:00 mriedem mdbooth: then how is it that it was never reproduced locally?
15:10:04 mdbooth It is.
15:10:05 mriedem if it was dirty state?
15:10:12 mriedem i never hit it locally
15:10:17 mdbooth See my comment 2 on the bug.
15:10:38 mdbooth Running those 2 tests in order reproduces the bug deterministically.
15:11:03 sdague mriedem: test grouping and order is an awkward level of deterministic.
15:11:38 sdague so probably your .testrepository past results are impacting test run order for you and you just aren't getting the bad sequence
15:12:16 mriedem so why not just get rid of the need for reset_globals() altogether?
15:12:22 mriedem if we have to rely on that it seems we've failed
15:12:24 sdague mtreinish: do you know if there is a way for us to turn off the order optimizer?

Earlier   Later