| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-07-12 | |||
| 13:49:11 | stephenfin | but I've spent the last two days on a case where doing that resulted in the attribute apparently not being set | |
| 13:49:55 | stephenfin | Lemme finish making sure this works and I'll push it up with a comment on the offending line, in case you're curious | |
| 13:50:00 | dansmith | okay | |
| 13:50:20 | stephenfin | Probably PEBKAC but we'll see :) | |
| 13:50:51 | dansmith | hopefully PEBKAC :) | |
| 13:51:31 | stephenfin | mriedem: After that fun, I totally agree on getting functional tests for numa-aware-vswitches. I'm going to need a little guidance/existing examples on how to actually approach it, if you have anything | |
| 13:52:03 | stephenfin | I have half-written Tempest tests but they won't be able to run upstream | |
| 13:55:03 | gibi | stephenfin: which part of the functional test you feel problematic? I might be able to help | |
| 13:55:11 | stephenfin | Also, forget about PCI, how we track all things NUMA is weird and I dislike it | |
| 13:55:25 | melwitt | nova meeting in 5 min | |
| 13:55:35 | alex_xu | efried: yea, +2 also, leave the +w to mriedem | |
| 13:56:01 | efried | Thanks alex_xu | |
| 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 | |