| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-05-21 | |||
| 16:04:05 | sean-k-mooney | ya that is true but i acutlly want it for other reasons like avoiding my host that have gpus when you did not ask for them that sort of thing | |
| 16:04:48 | sean-k-mooney | well that is what nova has always done to this point but ya im aware of the new spec too | |
| 16:05:27 | sean-k-mooney | personally i just use host-passthough and say screw it ill use host aggrates to seperate things if i need too | |
| 16:06:11 | sean-k-mooney | actully i used ot use AZs at intel for that and ould have one AZ per generation but that was just so i could select the generation simplely when lanuching a vm | |
| 16:06:13 | mtreinish | mriedem: do I need to review something? | |
| 16:06:37 | mdbooth | mtreinish: https://review.opendev.org/#/c/659317/ please? | |
| 16:15:29 | sean-k-mooney | efried: thanks for reviewing the video models stuff ill adress your commets after dinner | |
| 16:20:12 | mriedem | kashyap: https://bugs.launchpad.net/nova/+bug/1829896 for that guest not stopping thing, i checked the libvirtd logs and the power off requests all return with 0 | |
| 16:20:13 | openstack | Launchpad bug 1829896 in OpenStack Compute (nova) "libvirt: "Instance failed to shutdown in 60 seconds." in the gate" [Undecided,New] | |
| 16:20:28 | mriedem | not urgent but is concerning | |
| 16:22:03 | sean-k-mooney | i wonder if the qemu monitor process had hung? | |
| 16:22:21 | sean-k-mooney | i would expect a non 0 return in that case but i dont know if that is the case | |
| 16:41:07 | ganso | mriedem: hi Matt! Just a heads up in case you haven't seen it and have some free time to look at: https://review.opendev.org/#/c/659338 Thanks in advance! =) | |
| 16:41:38 | kashyap | sean-k-mooney: Yeah, I'd expect this to return non-zero here, in of a hung QEMU mithe case of hung QEMU: 2019-05-16 19:47:19.996+0000: 10502: debug : virThreadJobClear:121 : Thread 10502 (virNetServerHandleJob) finished job remoteDispatchDomainShutdown with ret=0 | |
| 16:46:52 | kashyap | mriedem: It seems to fail with the "MigrationsAdmin" test (but your bug descriptino says: 'snapshot'), whose purpose is "Add os-migration tests for Nova v2 API" (added in 2014). | |
| 16:48:20 | mriedem | kashyap: if you look at the logstash results it's on pretty much everything | |
| 16:48:56 | mriedem | migrate_disk_and_power_off in a cold migration / resize will also power off the guest | |
| 16:49:11 | mriedem | which is where i noticed it while debugging a cold migration test today | |
| 16:52:25 | kashyap | Hmm. So it is the actual guest | |
| 16:53:16 | kashyap | Damned if I could put a finger on why. Let me ask the libvirt folks, if they can think of anything | |
| 16:53:45 | kashyap | Wish I could reproduce the issue. Damn sure it won't if I try locally | |
| 16:54:47 | kashyap | < aw> kashyap: is it the VM or the guest OS? I've had fedora fail to shutdown in a timely way for all sorts of reasons, some that I still haven't figured out | |
| 16:54:47 | kashyap | One dev asks: | |
| 16:55:14 | mriedem | don't know, but these test vms in the gate runs are just cirros images | |
| 16:55:19 | kashyap | (I've told it's CirrOS; and it's the VM, aka. QEMU process) | |
| 17:01:49 | openstackgerrit | Elod Illes proposed openstack/nova stable/ocata: Fix incompatible version handling in BuildRequest https://review.opendev.org/660456 | |
| 17:02:00 | kashyap | Can anyone please remind, where is the guest boot log from the serial console here: http://logs.openstack.org/56/656656/12/check/nova-multi-cell/6a403ba/compute1/logs/ | |
| 17:09:07 | sean-k-mooney | i think i just spent an hour debuging a unit test failure because teh unit tests in osc are wrong an i coppied them to create my own... | |
| 17:15:21 | sean-k-mooney | nope i am wrong it was 2 hours but now they pass | |
| 17:30:31 | sean-k-mooney | aspiers: mriedem i just updated https://review.opendev.org/#/c/643578/ if there are any other changes you would like let me know with a comment :) | |
| 17:30:44 | aspiers | sean-k-mooney: ok will take a look thanks! | |
| 17:53:28 | aspiers | efried / anyone got any thoughts on whether it still makes sense to use nova.virt.libvirt.utils.file_open for mocking file opens? | |
| 17:53:42 | openstackgerrit | Merged openstack/nova master: Add detection of SEV support from QEMU/AMD-SP/libvirt on AMD hosts https://review.opendev.org/633855 | |
| 17:53:50 | openstackgerrit | Merged openstack/nova master: Extract provider tree functional tests into new file. https://review.opendev.org/660157 | |
| 17:54:02 | aspiers | \o/ first real SEV patch lands in nova! | |
| 17:55:15 | aspiers | file_open() was added in 2011 by Soren with the original framework for testing the libvirt driver | |
| 17:55:57 | aspiers | https://opendev.org/openstack/nova/commit/bb622e6d7c921894fd0e7697a0003630989d4f35 | |
| 17:56:25 | aspiers | he wrote "I hope eventually to make it similar to fakelibvirt in style (e.g. keep track of files created and deleted and attempts to open a file that it doesn't know about, you'll get proper exceptions with proper errnos set and whatnot)." | |
| 17:57:06 | aspiers | meanwhile, a ton of other tests just use mock.mock_open() | |
| 17:57:21 | aspiers | sean-k-mooney: any thoughts? | |
| 18:00:16 | efried | aspiers: The reason it exists is so that fake_libvirt_utils can mock it | |
| 18:00:24 | efried | and fake_libvirt_utils is dying in a fire. | |
| 18:00:29 | efried | https://review.opendev.org/#/c/642558/8 | |
| 18:00:48 | efried | IMO there's no reason for it to exist; we should just use mock_open | |
| 18:00:49 | aspiers | efried: my point was that a whole bunch of stuff needs to (and does) mock open, not just that | |
| 18:00:57 | aspiers | and it seems we have two different ways of doing it right now | |
| 18:01:01 | efried | yup | |
| 18:01:18 | efried | If you want to do the cleanup to kill file_open, I would support that. | |
| 18:01:42 | efried | meanwhile, definitely don't do anything that relies of fake_libvirt_utils.file_open | |
| 18:01:53 | aspiers | file_open seems much less popular right now than mock_open | |
| 18:01:55 | efried | and save yourself having more cleanup by avoiding mocking file_open - just mock open. | |
| 18:02:00 | efried | yeah. | |
| 18:02:15 | efried | Nice intention, but clearly nobody in the last eight years has cared enough to make it go. | |
| 18:02:23 | aspiers | having said that, Soren's original intention does make sense ... right | |
| 18:02:50 | efried | Just be wary of side effects. I had a situation recently where mocking open at the test method level was too broad and effed up, like, a dynamic import or something. | |
| 18:03:23 | aspiers | but file_open is actually also defined in nova/virt/libvirt/utils.py | |
| 18:03:40 | efried | if f == 'thing_i_care_about': | |
| 18:03:40 | efried | def fake_open(f, ...): | |
| 18:03:40 | efried | orig_open = builtins.open | |
| 18:03:40 | efried | so I had to do the | |
| 18:03:41 | efried | return orig_open(f, ...) | |
| 18:03:41 | efried | else: | |
| 18:03:41 | efried | do_the_mock_thing | |
| 18:03:42 | efried | thing. | |
| 18:04:18 | aspiers | isn't that what mock_open is supposed to help avoid? | |
| 18:04:22 | efried | yes, it's defined in fake_libvirt_utils because it was defined in libvirt.utils and apparently whoever set that up didn't know how to mock (or maybe mock wasn't properly powerful back then, dunno). | |
| 18:05:03 | efried | aforementioned series is getting rid of fake_libvirt_utils and just mocking the utils directly. | |
| 18:05:14 | aspiers | ah | |
| 18:05:24 | aspiers | makes sense, since a bunch of stuff is doing @mock.patch('nova.virt.libvirt.utils.file_open', ... | |
| 18:05:40 | aspiers | I'll stare at that review until I understand it | |
| 18:06:38 | efried | aspiers: re "isn't that what mock_open is supposed to help avoid?" -- not that I know of. Is there a way to make the mock automatically trigger conditionally based on the inputs? | |
| 18:07:07 | aspiers | I *think* there is, IIRC | |
| 18:07:17 | aspiers | I may have even done it recently, but would have to check | |
| 18:07:48 | efried | I thought mock_open was just good for abstracting the context manager awkwardness and providing an easy way to specify "output". | |
| 18:08:08 | efried | I don't see any conditionally-use-the-real-open mentioned in https://docs.python.org/3.3/library/unittest.mock.html#mock-open | |
| 18:10:03 | aspiers | Yeah I'm probably thinking of something else | |
| 18:11:02 | aspiers | mock is very powerful, but I find the API pretty damn confusing | |
| 18:11:15 | aspiers | I don't think it's the best designed API in the world | |
| 18:11:33 | aspiers | rspec is quite a bit easier to use | |
| 18:13:48 | aspiers | efried: maybe I was just thinking of side_effect, which would presumably at least avoid the need for orig_open | |
| 18:13:52 | efried | aspiers: We should totally use mox | |
| 18:14:29 | efried | I don't see how side_effect would avoid needing orig_open | |
| 18:14:54 | aspiers | Ah, yeah maybe not | |
| 18:15:01 | efried | side_effect lets you replace the generic no-op method with a method of your own design. | |
| 18:15:17 | efried | that method would still have to have a condition to orig_open if that was a thing you needed. | |
| 18:15:36 | efried | no need for that, this is good discussion. | |
| 18:15:45 | aspiers | ;) | |
| 18:15:48 | efried | helps clarify it in my mind if nothing else | |
| 18:15:59 | aspiers | I was thinking that inside the side_effect context, the built-in open would still be available | |
| 18:16:05 | aspiers | but maybe it's not | |
| 18:16:05 | efried | ah. Yeah, no. | |
| 18:16:17 | efried | you would recurse into your mock. | |
| 18:16:21 | aspiers | Right | |
| 18:16:28 | efried | and then other kinds of curse | |
| 18:16:37 | aspiers | That's necessary to support re-entrancy cases, I guess | |
| 18:16:52 | efried | also because it was the simplest to implement :) | |
| 18:18:03 | aspiers | I guess you were already well aware of https://governance.openstack.org/tc/goals/rocky/mox_removal.html | |
| 18:18:35 | aspiers | I never tried mox, so it's a shame if that's nicer to work with, but I guess consistency trumps niceness | |
| 18:20:20 | efried | aspiers: I was totally kidding. We've been working on removing mox for years now. | |
| 18:20:39 | efried | I find it very difficult to understand, but that's probably mostly because I'm not used to it. | |