Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-03-29
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
17:52:56 openstackgerrit Stephen Finucane proposed openstack/nova master: tests: Stub out os_vif https://review.openstack.org/648748
17:53:00 finucannot sean-k-mooney: ^
17:53:04 finucannot and with that, I take my leave
17:53:17 sean-k-mooney mdbooth: your porbaly sane and have left for the weekend but shoundt https://github.com/openstack/nova/blob/master/nova/virt/libvirt/driver.py#L3945-L3969 use the disk cachemode set in the config https://github.com/openstack/nova/blob/master/nova/conf/libvirt.py#L639-L695
17:53:30 sean-k-mooney mdbooth: becasue im pretty sure it does not
17:55:42 sean-k-mooney stephenfin: not sure that returing NONE is what you want there but ill leave a comment in thereview
19:14:00 openstackgerrit Ghanshyam Mann proposed openstack/nova-specs master: Spec for API policy updates https://review.openstack.org/547850
19:14:41 fried_rolls stephenfin: You may also be interested in PrivsepFixture: https://review.openstack.org/#/c/647849/
19:17:47 fried_rice stephenfin: If your test hits an *actual* privsep-wrapped method, it'll blow up on its own.
19:29:52 openstackgerrit Eric Fried proposed openstack/nova master: Mock time.sleep() in unit tests https://review.openstack.org/648762
19:35:26 mnaser I've got a really interesting use case where I need to schedule on different GPUs with things like different *steppings*, with the possibility that a host might have different GPUs with different steppings but the *same* vendor id
19:36:18 mnaser so my questions are: does placement store records for all gpus? can I use traits to 'decorate' those gpus with those different steppings/etc? is there anything in nova that allows me to schedule based on "find me a gpu that has this trait"
19:39:42 dansmith fried_rice: answer my two little questions in there so I can make nice and +2 that
19:40:18 fried_rice ack
19:40:51 fried_rice mnaser: We don't have support for multiple VGPUs yet...
19:40:56 fried_rice and physical GPUs are not tracked in placement yet.
19:41:12 mnaser fried_rice: okay, those are physical GPUs.. hmm okay, so this might probably have to be done in some other fashion
19:41:15 fried_rice cyborg is working on making the latter happen in Train (<== Sundar)
19:41:36 fried_rice mnaser: You should be able to swing it with PCI passthrough as long as you know which PCI IDs are which
19:41:45 fried_rice mnaser: make different PCI aliases for each group.
19:41:47 dansmith same pci id I think
19:41:54 mnaser fried_rice: well in this case, I have devices that identify the same pai id
19:41:55 dansmith so you'd need aggregates I think
19:41:57 mnaser PCI ID*
19:42:12 fried_rice Sorry, not PCI IDs
19:42:17 fried_rice PCI *addresses*
19:42:25 mnaser oh that's interesting
19:42:33 mnaser I didn't know we could do it based on PCI addresses
19:42:48 dansmith that'd be pretty laborious
19:42:50 fried_rice almost certain, lmk if you need help finding that.
19:43:11 mnaser dansmith: I have automation in place to 'identify' these cards
19:43:17 fried_rice Yes, it would be laborious, especially because you now need to do individual confs per compute.
19:43:27 fried_rice no more single conf sprayed around on the computes

Earlier   Later