Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2017-01-06
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?
15:13:08 mriedem mtreinish: isn't awake yet :)
15:13:26 mdbooth mriedem: To do that you'd need to remove the global state, i.e. not have a global _SESSION
15:13:56 mdbooth I think perhaps we could have a central repository for resetting global state, though.
15:14:04 mdbooth We currently do it piecemeal all over the place.
15:14:37 mdbooth If every time we added global state we also added it to a super-global reset_globals()?
15:16:33 sdague yeh, that would be less prone to errors. A general _reset_globals cleanup in the base test case
15:16:38 mriedem i guess i don't care at this point, i don't see a problem with how i've fixed this, but don't really want to argue (2 sips into coffee), so i'll let others on the core team decide which should go in
15:17:45 sdague so I'm inclined at this point to take mriedem's approach, because that good practice for that test in question - https://review.openstack.org/#/c/417206
15:18:31 sdague but I also think a global cleanup on test.py (especially if it came with a warning about what globals it found that it was reseting so you could figure out what test cases weren't cleaning up) would be good
15:18:53 mdbooth WFM. Is it worth fixing up the commit message to not refer to races? That's what confused me, and is why I kept looking.
15:19:32 cdent Unfortunately we have a tendency to overload "race" to mean anything that is non-deterministic in practice.
15:19:43 mtreinish mriedem: I am awake
15:19:59 mriedem :)
15:20:13 mtreinish sdague: you mean the scheduler's optimization based on previous times
15:20:20 sdague mtreinish: yeh
15:20:32 mtreinish sdague: the easiest way is just to nuke times.dbm
15:20:41 mtreinish I don't think there is a flag in testr run to disable it
15:20:45 sdague mtreinish: right, but I want to turn it off in tox
15:20:54 mriedem fwiw i deleted all of .tox and .testrepository within the last week
15:21:21 sdague mriedem: sure, but anything there impacts the run order
15:21:27 mtreinish sdague: add rm .testrepository/times.dbm to the command in the tox.ini?
15:21:48 mtreinish I can push a patch up to testr to skip it, but it'll just go in my pile of open PRs
15:22:20 mdbooth sdague: It's easy enough to pull the test ordering out of the subunit output
15:22:26 mdbooth Which is what I did
15:22:50 mdbooth https://wiki.openstack.org/wiki/Testr#Reproducing_Failures
15:22:50 sdague mdbooth: yeh, it's a little less fun to replicate that run order
15:23:10 sdague oh, there it is
15:24:08 mriedem johnthetubaguy: dansmith: can you guys get this in unless there are any issues? https://review.openstack.org/#/c/415511/ - jay's API code is ready for +2 and i got the novaclient change up for it last night so would like to move forward
15:24:32 openstackgerrit Samantha Blanco proposed openstack/nova: Add missing CLI commands in support-matrix.ini https://review.openstack.org/417232
15:24:48 johnthetubaguy mriedem: looking now ish
15:25:03 mriedem thanks
15:25:14 mriedem johnthetubaguy: all i added in the latest PS was the response example
15:33:48 openstackgerrit Ed Leafe proposed openstack/nova: Trivial indentation fix https://review.openstack.org/417422
15:38:19 sdague mriedem: +A
15:38:38 sdague thanks for adding that example in there
15:38:43 mriedem sdague: thanks
15:50:40 kashyap If anyone got a moment, https://review.openstack.org/#/c/411936/ ("libvirt: avoid generating script with empty path")
15:52:10 openstackgerrit Ed Leafe proposed openstack/nova: Remove the Allocation.create() method. https://review.openstack.org/416751
15:52:11 openstackgerrit Ed Leafe proposed openstack/nova: Add check for invalid allocation amounts https://review.openstack.org/416752
15:55:25 mriedem kashyap: looking, have an upgrade question coming
15:55:42 kashyap mriedem: Sure, I assume you'll write on the review
15:55:47 mriedem yup
15:56:39 kashyap mdbooth: When you have time, your eyes are also appreciated (https://review.openstack.org/#/c/411936/)
15:57:03 mriedem kashyap: posted
15:58:47 openstack Launchpad bug 1649527 in OpenStack Compute (nova) "nova creates an invalid ethernet/bridge interface definition in virsh xml" [Undecided,In progress] - Assigned to Neil Jerram (neil-jerram)
15:58:47 mriedem kashyap: yeah see sgordon's comment in the bug https://bugs.launchpad.net/nova/+bug/1649527/comments/1
15:59:15 sahid jamielennox|away:
15:59:26 sahid oops, sorry for that
15:59:34 kashyap mriedem: Yes, the upgrade question is a good one.
15:59:49 kashyap Will have to bring it up with upstream libvir list
16:00:26 kashyap mriedem: Thanks for the quick look. I'll post there once I learn more from the upstream folks.
16:00:48 mriedem kashyap: i guess the question would be how to recreate an upgrade issue where we hit that, i.e. create a vm via nova with libvirt 1.3.1, then upgrade libvirt to 2.0.0, and then try to do something with the vm (restart? stop/start?)
16:01:03 mriedem kashyap: if nova just blows the domain away and recreates it, then we probably don't care once we have the fix
16:01:10 mriedem it's just the xml parse part that i worry about
16:01:17 sgordon mriedem, it also has to be the specific networking solutions that use that specific vif path
16:01:39 sgordon mriedem, i think neil had identified that was midonet, vrouter, and one or two others
16:02:18 mriedem ok, either way the fix looks good, just thinking we should handle '' or None in the parse_dom
16:02:35 mriedem hard to say what else we can do w/o an upgrade recreate
16:02:57 openstackgerrit Balazs Gibizer proposed openstack/nova: handle uninited fields in notification payload https://review.openstack.org/415857
16:04:19 openstackgerrit Balazs Gibizer proposed openstack/nova: Transform missing delete notifications https://review.openstack.org/410297
16:06:17 kashyap mriedem: Yeah, the upgrade scenario you outlined, with specific VIF path sounds correct. I haven't tested it myself, just noticed on the upstream thread.
16:06:27 kashyap Will see if I can test it early next week w/ an upgrade setup
16:06:50 openstackgerrit Matt Riedemann proposed openstack/python-novaclient: Fix "Message object do not support addition". https://review.openstack.org/414529
16:34:26 openstackgerrit Balazs Gibizer proposed openstack/nova: handle uninited fields in notification payload https://review.openstack.org/415857
16:36:29 mriedem BobBall: can we get a run of tempest.api.compute.servers.test_attach_interfaces.AttachInterfacesTestJSON.test_create_list_show_delete_interfaces in the xenserver ci on patch https://review.openstack.org/#/c/365505/ ?
16:36:50 mriedem BobBall: nevermind https://review.openstack.org/#/c/416797/
16:37:28 BobBall :)
16:37:44 mriedem that's a pretty slick way to test that stuff out
16:38:41 BobBall I guess other CIs have a fixed exclusion list in their jenkins job configuration?
16:38:53 mriedem not sure how others do it

Earlier   Later