Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-07-12
13:56:17 stephenfin gibi: tbh, most of it. I guess I need to model the whole flow from API call to instance creation. I'm not sure how much of that functional tests case do for us
13:56:25 stephenfin Also, simple things like faking nova.conf
13:56:29 melwitt gmann: ack, thanks
13:58:06 mriedem efried: alex_xu: thanks will check after the meeting
13:58:22 gibi stephenfin: functional test is fairly easy if the code under test is not in (or triggered by) the virt driver
13:59:08 gibi stephenfin: there is a lot of server create and move operation tests in https://github.com/openstack/nova/blob/master/nova/tests/functional/test_servers.py
13:59:23 mriedem stephenfin: you'll need a fixture for the host inventory stuff for numa
13:59:51 mriedem probably some kind of simple fixture data struct that you can insert inventory into and then run scenarios against
14:00:35 stephenfin mriedem, gibi: OK, cool. The only functional tests I'd worked
14:01:00 stephenfin *I've worked on before are the PCI ones but they don't seem to be E2E tests. This will be fun, heh https://github.com/openstack/nova/blob/master/nova/tests/functional/libvirt/test_pci_sriov_servers.py
14:02:57 mriedem so start simple with writing a fixture that can model some numa cell hardware on a host with no physnets or tunneled set,
14:03:05 mriedem run a server create with numa requirements against that and make sure it's ok,
14:03:19 mriedem then start folding in wrinkles like phsynet inventory and requirements, tunneled, etc
14:03:27 gibi stephenfin: I don't know about functional tests that are end2end in a sense that it trigger something on the API and uses real libvirt driver code (by mocking the libvirt interface)
14:03:47 mriedem you can create the numa topology for the host in the db
14:03:52 mriedem for a single compute node created in the test
14:05:52 gibi mriedem, stephenfin: I wouldn't touch db form a functional test, maybe a FakeVirtDriver that reports numa topology
14:06:45 mriedem depends on how complicated you want to start with,
14:06:56 mriedem creating stuff in the db to start might be easiest and then later convert to a fake virt driver
14:07:04 mriedem once you know all the scenarios
14:08:28 openstackgerrit Radoslav Gerganov proposed openstack/nova master: VMware: Live migration of instances https://review.openstack.org/270116
14:08:29 openstackgerrit Radoslav Gerganov proposed openstack/nova master: VMware: ensure that live migration attaches to correct interface https://review.openstack.org/478797
14:08:47 gibi mriedem: sure that could work
14:10:02 stephenfin mriedem: Sounds good. Lemme give that a shot
14:34:05 openstackgerrit Merged openstack/nova master: Add policy to InstanceGroup object https://review.openstack.org/563375
14:45:55 stephenfin mriedem: RE: [1], NUMACell.network_metadata is not nullable. However, that means I need to update everything that uses NUMACell to set something for that field, right? [1] https://review.openstack.org/#/c/564439/13/nova/objects/numa.py@58
14:46:15 stephenfin That's a _lot_ of changes, if so
14:47:32 dansmith stephenfin: it can be unset regardless of the nullability
14:47:47 dansmith I think his point was that you should just not set it instead of always forcing it to None
14:49:11 stephenfin dansmith: That makes sense, but I found a piece I'd missed here https://github.com/openstack/nova/blob/master/nova/virt/hardware.py#L1692-L1695
14:49:41 stephenfin (that was where I was seeing the "setting in the initializer vs. setting as an attribute issue, btw)
14:50:24 stephenfin If I'm assuming network_metadata is always set in production and it's a bug if not, then I need to update any unit test which calls the function
14:50:45 dansmith I'm not sure what you mean
14:51:32 stephenfin dansmith: I need http://paste.openstack.org/show/725721/
14:52:13 dansmith sounds like you need to conditionally set it
14:52:17 dansmith like:
14:52:37 dansmith if "network_metadata" in hostcell: newcell.network_metadata = hostcell.network_metadata
14:52:38 dansmith right?
14:53:09 stephenfin Indeed, and that's what I had done but I saw the aforementioned issue
14:53:49 dansmith you never really told me what you saw.. I'm assuming you meant that setting it after initialization seemed not to stick?
14:53:59 dansmith that would, of course, be strange behavior we'd want to nail down
14:54:07 dansmith because _obviously_ you should be able to do that r:)
14:56:05 mriedem stephenfin: dansmith: the issue i pointed out later in the series once i got to the api part, was that if the user explicitly specifies networks='none' it means network_metadata will be None,
14:56:21 mriedem so we can't blindly set that on the RequestSpec in the api because it will blow up if the field is not nullable,
14:56:30 mriedem so we just need to be conscious of that,
14:56:33 stephenfin Yeah, I was just concerned that even with that, we'd have the potential to hide some bugs so it wouldn't be acceptable
14:56:44 stephenfin i.e. if, for some reason, we weren't setting the attribute somewhere when we should be
14:56:45 mriedem but it also means the consuming code in the scheduler filter (and hardware.py) needs to be aware the field might not be set
14:56:49 mriedem which hardware.py is already doing i believe
14:56:51 dansmith mriedem: if there is a valid case where it should be None, then it should be nullable=true, however, we shouldn't ever set it to None when we don't know the value
14:57:07 stephenfin mriedem: I don't think that's an issue - we always set NetworkMetadata but the physnets/tunneled attributes might not be set
14:57:12 mriedem networks='none' is the case where we know it should be None
14:57:23 mriedem stephenfin: bzzt,
14:57:26 mriedem let me link you
14:57:33 mriedem (to your code)
14:57:58 dansmith mriedem: should that be network_metadata=None, or just physnets=[],tunneled=False ?
14:58:03 mriedem https://review.openstack.org/#/c/564444/14/nova/network/neutronv2/api.py@1633
14:58:11 mriedem that depends on ^
14:58:43 mriedem i have a whole buttload of comments in https://review.openstack.org/#/c/564452/ related to this
14:58:55 mriedem https://review.openstack.org/#/c/564452/17/nova/compute/api.py@889
14:59:02 stephenfin mriedem: Sorry, you said RequestSpec and I read NUMACell https://review.openstack.org/#/c/564441/21/nova/virt/libvirt/driver.py@6338
15:00:04 stephenfin mriedem: Yeah, I should just be setting that to NetworkMetadata(physnets=set(), tunneled=False)
15:01:08 mriedem well also see https://review.openstack.org/#/c/564444/14/nova/tests/unit/network/test_neutronv2.py@4963
15:01:11 mriedem similar concern
15:01:44 mriedem anyway, you have options
15:02:34 mriedem either use an 'empty' NetworkMetadata object, or support None, or handle unset
15:02:59 mriedem at this point i kind of think nullable=True makes sense for when we know we don't have networking
15:03:19 mriedem but it's like a 5% "makes sense"
15:04:15 stephenfin heh
15:04:23 stephenfin OK, I'll explore that
15:04:36 stephenfin and try to push something to demonstrate this possible o.vo bug
15:09:44 mriedem anyone have any idea why we wouldn't cache the host az here? https://github.com/openstack/nova/blob/da16690f4db6172659ffa4f804296100b5ba24d6/nova/availability_zones.py#L93
15:10:04 mriedem we do it when getting the az for an instance that has a host set https://github.com/openstack/nova/blob/da16690f4db6172659ffa4f804296100b5ba24d6/nova/availability_zones.py#L194
15:12:41 mriedem dansmith: i think i have a fix for the cinder/cross_az_attach=False up-call issue
15:12:44 mriedem 1 line fix
15:13:03 jmlowe dansmith: do you happen to know off the top of your head where the ceph free space is queried?
15:13:20 mriedem jmlowe: should be down in libvirt driver update_available_resource
15:13:30 mriedem get_available_resource?
15:13:42 jmlowe those are grepable, thanks
15:13:45 mriedem yeah _get_local_gb_info
15:13:53 mriedem elif CONF.libvirt.images_type == 'rbd':
15:13:53 mriedem info = LibvirtDriver._get_rbd_driver().get_pool_info()
15:20:02 mriedem https://bugs.launchpad.net/nova/+bug/1781421
15:20:03 openstack Launchpad bug 1781421 in OpenStack Compute (nova) "CantStartEngineError due to host aggregate up-call when boot from volume and [cinder]/cross_az_attach=False" [Medium,Triaged]
15:20:16 mriedem - if not host:
15:20:16 mriedem + if not host or (host and instance.availability_zone):
15:20:17 mriedem fixed!
15:25:59 openstackgerrit Matt Riedemann proposed openstack/nova master: Mention osc-placement for managing traits in docs https://review.openstack.org/582173
15:28:43 melwitt jmlowe: and here's a proposal for another way to query free space that I've not fixed up with tests yet https://review.openstack.org/#/c/556692/1/nova/virt/libvirt/storage/rbd_utils.py
15:31:51 dansmith note that in placement, we don't report free space, but infer it from total - (allocated + reserved)
15:32:42 melwitt oh, okay
15:32:58 mriedem melwitt: you were +2 on this before jaypipes pointed out that we couldn't actually update consumer project/user info in placement, but that bug is fixed now and his -2 is gone, and i've added a functional test so can you review this again? https://review.openstack.org/#/c/574488/ - it's the heal_allocations thing for healing incomplete consumers
15:33:02 mriedem and would close out that bp
15:33:29 melwitt mriedem: ah yup, thanks for the heads up
15:33:32 melwitt will revisit that
15:39:55 stephenfin Pro tip #1: 'tox -e py27 -- --failing' runs all the tests that failed on a previous iteration
15:40:29 stephenfin Pro tip #2: 'tox -e py27 -- -n nova/tests/unit/virt/test_hardware.py' let's you run tests without needing to convert to a Python path
15:42:20 mriedem efried: alex_xu: arvindn05: karimull: on its way to the gate of horrors https://review.openstack.org/#/c/569498/ - i'll do a follow up for my comments
15:42:35 efried mriedem: cool mon
15:42:54 mriedem pass the dutchie

Earlier   Later