Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-07-19
15:41:48 alex_xu can we check the src and dst service version in the conductor?
15:42:14 openstackgerrit Artom Lifshitz proposed openstack/nova master: libvirt: move checking CONF.my_ip to init_host() https://review.opendev.org/671471
15:42:18 sean-k-mooney we can but why would we
15:42:22 alex_xu emm...I guess we need another param for the rpc
15:42:36 artom alex_xu, that's what I had at some point, Dan talked me out of it
15:42:53 sean-k-mooney we need the data that is pass back from can live migrate on dest
15:42:59 artom dansmith, efried ^^ (the CONF.my_ip to init_host() thing)
15:43:01 sean-k-mooney to be able to generate the updated xml
15:43:04 efried sean-k-mooney: Is https://review.opendev.org/#/c/638680/ still waiting on https://review.opendev.org/#/c/670189/ ?
15:43:45 sean-k-mooney kind of. im not changeing the public interface of the function it calls
15:44:03 sean-k-mooney but i suspect that if i chekcout that patch locally it will still expode
15:44:23 sean-k-mooney i can check that quickly.
15:46:37 efried artom: would you mind adding a teeny unit test that asserts _check_my_ip is called during init_host?
15:48:53 artom efried, ack, makes sense
15:49:25 efried artom: I recognize that a unit test for init_host as a whole is missing, and would be an appropriate add, but unrelated...
15:49:58 artom efried, hrmm, yeah, went looking, didn't find anything
15:50:04 efried yeah
15:50:08 artom So, maybe as a separate fup patch on top?
15:50:25 artom The full unit test, I mean
15:50:35 efried sure, that would be great, though I would also just not notice if it never happened
15:50:44 efried There are lots of gaps in unit test in libvirt/test_driver
15:50:49 efried lots
15:51:28 artom My thinking it, given how much I'll have to mock to just assert this one thing being called, might as well do the full thing
15:51:40 artom *is
15:51:41 sean-k-mooney efried: its not failing because my kernel does not support sev. so https://review.opendev.org/#/c/638680/27/nova/virt/libvirt/host.py@1066 prevent the call to get_domain_capablities
15:52:20 efried artom: There are other tests that call init_host but don't mock a zillion things. I didn't look too deeply into how they're doing that, but...
15:53:03 efried probably because of FakeLibvirtDriver
15:53:24 artom Ah, indeed
15:53:31 efried naw, even the ones that use real LibvirtDriver with FakeVirtAPI
15:53:39 efried so, it should be possible to do it without too much mocking.
15:54:38 sean-k-mooney if i comment out the if i get this trace back
15:54:40 sean-k-mooney http://paste.openstack.org/show/754594/
15:55:11 efried though that leads me to wonder how you're getting away with not mocking netifaces.interfaces() and .ifaddresses()
15:55:33 artom get_machines_ips gets mocked out entirely
15:55:34 efried artom: Is your test actually running netifaces.interfaces() on whatever system you're invoking it on??
15:55:47 efried yeah, I mean all the other tests that invoke init_host() :)
15:56:00 artom Oh, hah
15:56:05 artom I guess they do?
15:56:22 efried that's... not great
15:56:39 efried unpredictable results on linux, sure failure on other platforms, nah?
15:56:48 artom It's supposed to be cross-platform
15:56:50 artom But yeah
15:56:54 efried Not that I would really expect test_driver to work on another platform anyway, but...
15:57:04 sean-k-mooney artom: it should not be actully calling it however
15:57:27 efried so perhaps you need to fixture out get_machine_ips (or netifaces.interfaces) at a higher level
15:57:41 sean-k-mooney you shoudl be able to set up a mock in the setup function once
15:57:52 artom Yep
15:57:54 artom ;_:
15:58:05 artom And here I was thinking I'd get to do a pure removal patch :(
15:58:21 efried sorry artom. Your karma ran over your dogma.
15:58:51 artom Is that like my kar running over my dog, just with my ma involved?
16:00:27 efried just so
16:03:08 efried sean-k-mooney: I may need some help, or you should bring in a SME, reviewing that get_dowain_capabilities patch. kashyap?
16:05:17 sean-k-mooney sure. i could harden it more then i did but that at least fixes the orignial issue
16:05:56 sean-k-mooney im still not catching the libvirt exection if we ligitetly misconfigur the config
16:07:00 sean-k-mooney but we wont explode any more with the defaults
16:07:52 sean-k-mooney efried: do you know if aspires is still working on openstack?
16:08:08 sean-k-mooney i have not seen them involded in the sev work lately
16:08:09 efried I haven't seen him in quite a while, but I hope so.
16:08:14 efried yeah
16:08:24 openstackgerrit Artom Lifshitz proposed openstack/nova master: libvirt: move checking CONF.my_ip to init_host() https://review.opendev.org/671471
16:08:52 sean-k-mooney maybe i should add Boris Bobrov to the review too since they seam to have taken over the sev stuff
16:11:33 efried artom: That rev wfm. Just one other thing: do functional tests have the possibility of hitting netifaces now?
16:12:03 artom OK, this isn't a rabbit whole anywhere, it's the pit of hell
16:12:18 artom *hole
16:12:26 artom efried, but yeah, they do, good thought
16:12:56 artom Maybe right up in nova/test.py?
16:13:09 efried artom: do any func tests use the real libvirt driver?
16:13:14 efried if they use fake, this is n/a
16:13:30 efried no, I don't want a libvirt-specific fixture in nova/test.py
16:13:33 artom They use the fake libvirt API, but real driver IIRC, no?
16:13:53 efried (don't tell me if there are already libvirt-isms dirtying up the base class)
16:14:09 efried artom: I don't know, I'm still pretty green on all this libvirt stuff.
16:14:11 sean-k-mooney artom: i dont think so
16:14:40 sean-k-mooney the unit test in generall dont use the drivers at all
16:14:48 sean-k-mooney the functional test defualt to the fake dirver
16:14:58 sean-k-mooney execpt the function test under the libvirt folder
16:15:10 sean-k-mooney which do as you said
16:15:22 sean-k-mooney e.g. fake libvirt api + real libvirt driver
16:16:00 sean-k-mooney you could add a test in here somewhare https://github.com/openstack/nova/tree/master/nova/tests/functional/libvirt
16:16:01 artom efried, well, wait a second, we had that before, no?
16:16:20 efried had what before?
16:16:26 artom Any migration test that didn't mock out get_host_ip_addr
16:16:33 artom It ended up calling down to netifaces eventually
16:16:43 efried seems likely, yeah
16:16:43 sean-k-mooney we only have 2 migration tests i think
16:16:44 artom Same for functional tests
16:17:27 sean-k-mooney https://github.com/openstack/nova/blob/master/nova/tests/functional/compute/test_live_migration.py
16:18:07 artom I don't think live migration hit that
16:18:14 efried let's settle this...
16:18:14 efried I'm going to write a DNM with a poison fixture that will blow up any test that hits netifaces.interfaces()
16:18:14 efried and let the gate tell me which tests are affected.
16:19:06 efried which is almost, but not quite, unrelated to your patch.
16:19:19 efried since you've expanded the number of code paths in which it could be getting called
16:19:31 artom I'd be OK waiting for that to report before proceeding
16:19:39 artom Even rebasing on top
16:19:48 efried Well
16:19:55 efried I'm not sure I want to *keep* the poison fixture
16:20:00 efried although I guess that wouldn't be an awful thing.
16:20:19 artom Well, we're either OK with listing the host interfaces, or we're not :)

Earlier   Later