Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-03-29
15:29:43 gibi fried_rice: trident put together the reproduction and the bugfix as well. I think it is good to go now. So if you have time: https://review.openstack.org/#/c/648653
15:36:17 openstackgerrit Stephen Finucane proposed openstack/nova-specs master: docs: Resolve issues with Sphinx 2.0 https://review.openstack.org/648670
15:43:02 fried_rice gibi, trident: on it
15:46:02 finucannot PapaOurs: CI was happy with https://review.openstack.org/#/c/648670/. I fixed your nit if you want to rehit that
15:46:12 PapaOurs finucannot: yup, just sax
15:46:13 PapaOurs sa
15:46:17 PapaOurs graaah
15:46:19 PapaOurs saw*
16:02:24 openstackgerrit Rodolfo Alonso Hernandez proposed openstack/os-vif master: Refactor functional base test classes https://review.openstack.org/643101
16:05:39 openstackgerrit Balazs Gibizer proposed openstack/nova master: nova-manage: heal port allocations https://review.openstack.org/637955
16:07:16 fried_rice gibi, trident: responded. Do you agree?
16:08:38 gibi fried_rice: do you mean adding another test where we see that the boot succeeds to a host that has the trait?
16:10:16 gibi fried_rice: I have to run. I'm not against adding that extra test, just add a note in the test that it can produce false positives
16:10:47 fried_rice gibi_off: sure, that wfm
16:10:50 fried_rice trident: ^^
16:11:33 fried_rice gibi_off, trident: In that case, I suggest it would be best to do both paths in a single test method.
16:16:51 openstackgerrit Eric Fried proposed openstack/nova master: Poison sleep() in tests https://review.openstack.org/643763
16:23:06 openstackgerrit Eric Fried proposed openstack/nova master: Poison sleep() in tests https://review.openstack.org/643763
16:26:40 openstackgerrit Merged openstack/nova master: Fix exception type in test_boot_reschedule_fill_provider_mapping_raises https://review.openstack.org/648651
16:27:16 openstackgerrit Merged openstack/nova-specs master: docs: Resolve issues with Sphinx 2.0 https://review.openstack.org/648670
16:33:42 trident fried_rice: Thanks. Good points. Will take care of it during the weekend or worst case on monday.
16:33:42 dansmith fried_rice: how much time does that actually save?
16:33:51 dansmith (not sleeping)
16:37:28 fried_rice dansmith: In the gate, I don't know, probably not significant, thirty seconds at best is my guess. That would only be for the unit test suites, which run fast anyway. But IIUC each suite runs on its own node, so getting done some amount of time earlier would free that node up for other runs.
16:37:34 fried_rice dansmith: It's really for local tox
16:38:08 dansmith it saves you a lot locally?
16:39:05 fried_rice "a lot", meh. If I'm working on a test or a whole module or class or whatever, and it's got 7-12s worth of sleeps (which is typical for the cases I fixed) and I have to run it 10x while I'm developing whatever, it can save me minutes.
16:39:08 dansmith the gate workers are only 4T IIRC, so I would expect it to save you less locally
16:39:47 dansmith and what are those sleeps doing? polling a fake cinder or something?
16:39:56 fried_rice mostly retries in the actual code.
16:40:05 fried_rice the functional tests really need to sleep
16:40:11 fried_rice that's like wait_for_state_change stuff
16:40:19 fried_rice it's the sleeps in real code that I'm trying to poison
16:40:24 dansmith right, I mean what are the real code sleeps doing
16:40:32 fried_rice oh, mostly sleeps between retries
16:40:57 fried_rice so a unit test that tests retries is eating all that sleep time, when it really doesn't need that; it can just spin the retries immediately.
16:41:18 fried_rice adding @mock('sleep') is all you need to get that done, so it's easy.
16:41:29 dansmith this just seems kinda crazy invasive to save a few seconds in a few test cases, especially since you have to replace all the calls in the functional tests
16:41:55 fried_rice Understood.
16:41:57 dansmith not to mention the oslo hack and potential impact to other libraries
16:42:11 fried_rice I'm kind of attached to it since I spent a bunch of time on it, but I'll understand if you want to kill it.
16:42:27 dansmith would it not be better to just use that poison fixture where we're testing things that do a bunch of retries?
16:42:49 dansmith or just a short-circuit fixture that collapses them to zero for those cases?
16:43:02 fried_rice that's the problem: you don't necessarily know ahead of time which tests are going to need that.
16:43:08 fried_rice which is why I made it "global".
16:43:45 fried_rice I mean, if you're writing a test that tests retries, you *should* know, but most people don't think about that - which is why those "violations" were there.
16:43:47 dansmith you won't catch all the cases, but you should be able to profile the top ten waiting tests and add those
16:44:04 fried_rice yes, which is what I was doing when I came up with doing it this way instead.
16:44:15 dansmith sure, but...
16:44:23 fried_rice dansmith: https://review.openstack.org/#/c/643760/
16:45:44 fried_rice dansmith: If you hate it, I'll just kill it and propose a patch that has the @mock(sleep) decorators I added there. Same improvement in the short term. No protection in the long term, but <shrug> I'll still sleep at night.
16:45:46 fried_rice Gotta run.
16:46:10 dansmith well, I hate it.. doesn't mean everyone will, but yeah.
16:49:46 finucannot dansmith: Seeing as you're on the subject of hating stuff, how do you feel about looking at something silly that Python does? https://review.openstack.org/#/c/647831/
16:50:00 dansmith when am I not?
16:50:03 dansmith lemme finish this first
16:50:10 finucannot (y)
16:53:03 finucannot fried_rolls: Think I'm going to steal that poisoning technique of yours for privsep'y stuff
16:53:14 finucannot I'm guessing all that should be mocked in functional tests
16:53:24 finucannot though I bet sean-k-mooney would disagree
16:54:31 finucannot Oh, nvm, we do that already (nova/tests/fixtures.py) but it's silently hidden or something. Odd
16:54:51 finucannot igordc: I thought it better captured my general usefulness
16:55:06 igordc finucannot, genius
16:55:26 finucannot I try ¯\_(ツ)_/¯
17:01:24 mnaser is latest python-novaclient broken under py3?
17:01:54 mnaser https://www.irccloud.com/pastebin/wuvud4K7/
17:05:32 mnaser looks like its reported here https://bugs.launchpad.net/ubuntu/+source/python-novaclient/+bug/1777482
17:05:33 openstack Launchpad bug 1777482 in python-novaclient (Ubuntu) "console-log ERROR (TypeError): write() argument must be str, not bytes" [Medium,Triaged]
17:06:31 mnaser python-openstackclient seems to not use the codec fanciness. https://github.com/openstack/python-openstackclient/blob/4bde9af89251431791fc8d69fe09d5e17a8fba8f/openstackclient/compute/v2/console.py#L64
17:06:34 mnaser I dunno what's the best avenue
17:13:14 sean-k-mooney finucannot: of course i disagree. what wat the question so i know what postion i am ment to be taking :)
17:16:23 sean-k-mooney finucannot: oh mocking in functional tests. it depends on what you are mocking but in general you shoul dnot mock in functional test but you might use different backend implementations
17:16:31 sean-k-mooney or test fixtures
17:16:52 finucannot sean-k-mooney: I'm seeing calls through to nova.privsep.utils.supports_direct_io
17:17:05 finucannot from 'File "nova/virt/libvirt/driver.py", line 418, in disk_cachemode'
17:17:23 finucannot sean-k-mooney: I'm guessing we shouldn't be modifying system state
17:18:09 sean-k-mooney it depends on what the test was trying to assert but proably not
17:19:39 sean-k-mooney ah in this case its trying to determin the cache mode default i see
17:19:55 finucannot yup
17:20:02 finucannot There's another one too
17:20:14 finucannot we've got calls to 'nova.pci.utils.get_mac_by_pci_address'
17:20:34 finucannot and they're failing, resulting in lots of "Could not find the expected sysfs file for determining the MAC address of the PCI device ..." messages in logs
17:20:42 sean-k-mooney why are we using the libvir driver here and not the fake one
17:21:23 finucannot It's for e.g. the 'test_create_server_with_pci_dev_and_numa_fails' test
17:21:37 finucannot that wouldn't make sense outside of libvirt
17:21:48 sean-k-mooney right we have sriov support in the fake libvirt driver
17:21:55 finucannot yup
17:22:26 sean-k-mooney so the fake libfir driver should just overrite the disk_cachemode porperty
17:23:33 sean-k-mooney finucannot: hyperv support pci passthough and numa too by the way
17:24:02 finucannot True, but I don't know how that works so building up a fake driver for that would be a lot more work :)
17:24:24 sean-k-mooney its this test https://github.com/openstack/nova/blob/master/nova/tests/functional/libvirt/test_pci_sriov_servers.py#L244
17:25:38 sean-k-mooney so ya you could just add a mock.patch i guess
17:27:42 sean-k-mooney finucannot: https://github.com/openstack/nova/blob/master/nova/tests/functional/libvirt/base.py#L58
17:28:06 sean-k-mooney so those tests are using the real libvirt driver but with a fake connection
17:28:34 finucannot Yeah, that's what I mean when I say fakelibvirt drive
17:28:42 finucannot fakelibvirt is pretty much a simulator
17:28:46 finucannot of libvirt itself
17:29:39 sean-k-mooney i tought you ment https://github.com/openstack/nova/blob/master/nova/tests/unit/virt/libvirt/fakelibvirt.py
17:30:12 sean-k-mooney i guess that just impomentes the fake connection
17:52:56 openstackgerrit Stephen Finucane proposed openstack/nova master: Stub out privsep modules https://review.openstack.org/648747

Earlier   Later