Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-10-02
14:20:01 artom Actually yeah
14:20:19 dansmith fine, where is your fail,call ? :)
14:20:20 gibi dansmith: is there a nicer way to code that?
14:20:26 artom What we're doing in https://review.opendev.org/#/c/685950/3/nova/tests/functional/regressions/test_bug_1843090.py is exactly what we said we shouldn't do in https://review.opendev.org/#/c/685950/2/nova/tests/functional/compute/test_live_migration.py@23
14:21:13 dansmith gibi: well, for one thing you should keep track of your mock calls outside of the runtime objects you're handling
14:21:37 efried dansmith: Checking the actual mock's call_count is hard because you have to define the method before you create the mock before you define the method, but you have to reference the mock from within the method.
14:21:44 dansmith gibi: point being I'm not sure what spy is making nicer about that test because it seems like the same ickiness, but with a different calling convention
14:22:03 gibi dansmith: it hides the scope magic efried is just described
14:22:15 gibi dansmith: it automates the call to the original function
14:22:16 dansmith efried: everywhere else we keep a list above the scope and chuck things into there inside the handler
14:22:36 dansmith gibi: so it saves the one line of saving a reference to the original method?
14:22:50 gibi dansmith: and it hides the outer scope list
14:23:14 dansmith but replaces it with the terrible hack of modifying the runtime objects instead of keeping that fully within the test!
14:23:17 efried and when there are multiple calls to the same method nested inside whatever's happening inside the context manager, it gives you a way to introspect them individually
14:23:31 gibi dansmith: and it remembers that we have to use new= instead of side_effect if the mocked function is bound method
14:23:55 efried with mock.patch:
14:23:56 efried do_thing calls foo() N times with different args or whatever
14:23:56 efried do_thing()
14:23:57 openstackgerrit Matt Riedemann proposed openstack/nova master: Set Instance AZ from Selection AZ during migrate reschedule https://review.opendev.org/686050
14:23:57 openstackgerrit Matt Riedemann proposed openstack/nova master: Set Instance AZ from Selection AZ during build reschedule https://review.opendev.org/686047
14:23:57 openstackgerrit Matt Riedemann proposed openstack/nova master: Add Selection.availability_zone field https://review.opendev.org/685807
14:23:57 openstack bug 1781286 in OpenStack Compute (nova) "CantStartEngineError in cell conductor during reschedule - get_host_availability_zone up-call" [Medium,In progress] https://launchpad.net/bugs/1781286 - Assigned to Matt Riedemann (mriedem)
14:23:57 openstackgerrit Matt Riedemann proposed openstack/nova master: Add functional regression test for migrate part of bug 1781286 https://review.opendev.org/686017
14:23:58 openstackgerrit Matt Riedemann proposed openstack/nova master: Update cells v2 up-call caveats doc https://review.opendev.org/686053
14:24:42 efried thing, which puts creation of the tracker container back in the hands of the caller.
14:24:42 efried spy.claim_calls_on_node = getattr(spy, 'claim_calls_on_node', [])
14:24:42 efried dansmith: I also suggested a way we don't have to do the
14:25:07 efried same #LOC, just a little more scrutable
14:25:13 dansmith efried: yes, initializing that "context" elsewhere would be a lot better
14:25:59 dansmith anyway, it seems to do a lot for you and gibi, that's all that matters. it's something else I have to learn to comprehend the test, but...
14:26:09 efried gibi: I can work that ^ up if you like, at least for demonstration purposes.
14:26:19 efried maybe it'll read more intuitively to dansmith
14:26:59 dansmith no, not more intuitively, it'll just make the handler less gross
14:28:37 mriedem i found out yesterday that we patch mock.patch globally using something in oslotest for fixing the autospec'ing stuff - what claudiub worked on a long time ago. i saw those changes merge but didn't realize we were patching the mock library.
14:28:47 mriedem i would not be surprised when we go to a new version and that blows up in our faces
14:29:06 openstackgerrit Merged openstack/nova stable/train: Reduce scope of 'path' query parameter to noVNC consoles https://review.opendev.org/686066
14:31:46 gibi efried, dansmith: fine. I'm out of steam. I think I get that we cannot remove the full uglyness of these tests. I think we can move and contain some the uglyness. But I also get that it makes the test less explicit. so meh
14:32:28 dansmith gibi: I appreciate your efforts to improve :)
14:32:32 mriedem gibi: if it makes you feel better, i was able to use mock.patch context manager in that migrate reschedule functional test :)
14:32:44 gibi mriedem: I haven't tried mock.patch(wraps=) recently. so it might work
14:32:58 mriedem using wraps= is awkward in my experience
14:33:09 mriedem side_effect is more straightforward, but maybe that's just b/c i have more experience with it
14:33:17 gibi mriedem: and patching the mock library feels baaaad
14:33:42 mriedem we'll find out about ^ but this is what we call https://github.com/openstack/oslotest/blob/master/oslotest/mock_fixture.py#L187
14:34:10 gibi dansmith: I guess I will go and fined another tech debt and work on that a bit :)
14:34:21 gibi s/fined/find/
14:36:24 gibi mriedem: that patching might even be relevant to my problem with side_effect and bound methods as it touches 'self'
14:36:44 mriedem too much touching of self can lead to problems
14:36:47 mriedem i've heard
14:36:48 mriedem ...
14:36:52 gibi ...
14:38:05 dansmith mriedem: comment here: https://review.opendev.org/#/c/686017/3
14:39:52 mriedem i can probably keep a handle to the mock and assert it's called, but if you remove the mock the test fails so i'm not sure it's worthwhile
14:41:07 mriedem actually as written i should confirm that statement,
14:41:23 mriedem because it probably fails anyway since the stub always raises right not, not just on the first host
14:41:29 mriedem *now
14:42:43 mriedem dansmith: replied inline with options
14:42:58 dansmith mriedem: that's why I'm asking.. just seems like since we can't see the actual failure in the fault, we should make sure it's hitting what we think it is
14:44:00 dansmith mriedem: I'd rather the explicit assertCalled personally, but it was just a suggestion anyway
14:50:43 mriedem ok i can tinker with it, gonna be a bit since i'm in a meeting in 10 minutes
14:52:26 dansmith ack, I dropped a few other comments on that series,
14:52:45 dansmith but overall..thanks for closing that loop.. it always seemed like a big deal so I never wanted to even start, but you made it look easy
14:59:32 mnaser sean-k-mooney: cpu/libvirt-y related question, is it possible that not all flags are passed down to the vm even with host-passthrough ?
15:00:56 sean-k-mooney i belive that yes there are 1 or 2 instruction that are not passsed but in general no
15:01:52 mnaser ok i have way more than 1 or 2 that arent passed through in this case..
15:02:01 sean-k-mooney the ones that are not passed releated to things like system management mode/model specic registers
15:02:20 sean-k-mooney mnaser: which ones?
15:03:38 mnaser https://www.irccloud.com/pastebin/MAa6LBVo/
15:03:40 mnaser sean-k-mooney: ^
15:04:12 mnaser wait sorry, thats what is exposed
15:04:16 mnaser what _doesnt_ get exposed is
15:04:22 sean-k-mooney they are the one not passed through?
15:04:28 sean-k-mooney ok that makes more sense
15:04:33 mnaser https://www.irccloud.com/pastebin/6qVAPHMx/
15:04:35 mnaser those ^
15:04:45 sean-k-mooney i was like how is it working at all with out a floating point unit
15:04:52 mnaser loll
15:05:41 sean-k-mooney i need to jump on an internal call for an hour mind if we come back to this after
15:06:03 sean-k-mooney but first glance several of them are not verutaliable or are cpu bug flags
15:06:30 mnaser yeah it looked like a lot of "management" things
15:06:34 mnaser like seeing stuff like hw_pstate
15:07:43 sean-k-mooney ya the svm ones are amd nested virt flags which need to be enabled in the kvm module sperately
15:08:06 sean-k-mooney ht is hyper treading and is only expose dif you configre the guest to have 2 thrads
15:08:14 sean-k-mooney e.g. hw:cpu_threads=2
15:09:18 openstackgerrit Balazs Gibizer proposed openstack/nova master: Refine comments about move_allocations https://review.opendev.org/686182
15:09:18 sean-k-mooney "cat" "cdp" and "rdt" freatures cant be viertualsed
15:09:39 mnaser okay so it looks like things are okay
15:09:45 sean-k-mooney yep
15:10:12 sean-k-mooney ill look at the list again later but first glance i dont see anygin alarming
15:10:42 kashyap mnaser: Ideally all flags from host will be passed-through as-is
15:10:58 kashyap (I haven't read the whole scrollback, though)
15:11:07 mnaser kashyap: that was my theory but i figured there might some blacklist of "things i will not pass because a vm shouldnt have them"
15:11:19 mnaser https://www.irccloud.com/pastebin/6qVAPHMx/
15:11:23 mnaser ^ the list that didnt show up
15:13:06 kashyap mnaser: What's the host processor?
15:13:19 mnaser kashyap: AMD EPYC 7402
15:13:43 openstackgerrit Stephen Finucane proposed openstack/nova master: nova-net: Remove explicit 'USE_NEUTRON = True' https://review.opendev.org/685937
15:14:02 openstackgerrit Stephen Finucane proposed openstack/nova master: nova-net: Remove explicit 'USE_NEUTRON = True' https://review.opendev.org/685937
15:16:17 kashyap mnaser: Maybe silly question, did you cold-reboot the guest?
15:16:23 kashyap (A bit slow, as on a meeting)

Earlier   Later