| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-07-25 | |||
| 18:02:02 | openstackgerrit | Stephen Finucane proposed openstack/nova master: WIP: Add additional functional tests for NUMA networks https://review.openstack.org/585385 | |
| 18:17:47 | stephenfin | sean-k-mooney, dansmith, mriedem_away: A'ight, I've got a bus to Dublin in 6 hours and still need to pack. Gotta wrap this up :( | |
| 18:19:03 | dansmith | stephenfin: does that test work? | |
| 18:19:44 | stephenfin | dansmith: Nope. I think sean-k-mooney and I have the tests mostly right. Remaining blocker is that instance.info_cache.network_info isn't being updated. That happens when running IRL so I suspect a mocking issue but I've at a loss to figure out what I'm missing | |
| 18:20:11 | dansmith | okay that's the part I'm most suspicious about | |
| 18:20:13 | dansmith | you're saying if you're running devstack or something it works? | |
| 18:20:25 | stephenfin | Yeah | |
| 18:20:38 | stephenfin | (a) boot an instance with a physnet network attached | |
| 18:21:02 | stephenfin | (b) attach an interface from a tunneled network | |
| 18:21:09 | stephenfin | (c) rebuild and see NoValidHost | |
| 18:21:33 | dansmith | that's your IRL test? | |
| 18:22:02 | stephenfin | For lack of anything better, yes | |
| 18:22:12 | sean-k-mooney | dansmith: the periodic_tasks dont run in the functional tests so the instance.info_cache.network_info never gets updated. | |
| 18:22:19 | stephenfin | Given that's where I was going with the functional test until this afternoon | |
| 18:22:38 | dansmith | sean-k-mooney: that's how it gets _refreshed_ not how it ever gets set ever | |
| 18:23:15 | stephenfin | Naturally if I skip (b) or attach an interface from a network associated with the same NUMA node, it rebuilds just fine | |
| 18:23:29 | dansmith | stephenfin: you must be rebuilding with a new image right? | |
| 18:23:50 | dansmith | er, wait, I forget where we left the asking-scheduler-during-rebuild thing | |
| 18:23:53 | stephenfin | yes, to make sure the scheduler/claims code gets called | |
| 18:23:58 | dansmith | right okay | |
| 18:25:29 | stephenfin | The only things I can find that result in the network_info attribute getting re-built are attaching fixed/floating IPs, but I haven't been able to simulate that | |
| 18:25:41 | artom | dansmith, we left it in a working state, normally. But only some filters are run, as specified in the creatively named RUN_ON_REBUILD attribute ;) | |
| 18:26:04 | dansmith | artom: right and numa does run on rebuild right? | |
| 18:26:22 | stephenfin | dansmith: Yup, mriedem_away and I checked that yesterday | |
| 18:26:22 | dansmith | but what I'm not remembering is if we asking the scheduler _at_all_ if we aren't changing images | |
| 18:26:24 | artom | dansmith, ya | |
| 18:26:28 | sean-k-mooney | dansmith: ya im localy chaging the image and hitting the schduoler but the filter sees the instacne with only one interface because the info_cash only has one vif instead of 2 | |
| 18:26:37 | stephenfin | dansmith: That ^ | |
| 18:26:44 | artom | dansmith, I believe it's only if there's a new image | |
| 18:26:56 | dansmith | sean-k-mooney: yeah, doing this with an attach seems more complicated than the simplest case | |
| 18:26:58 | dansmith | artom: right okay | |
| 18:27:09 | stephenfin | I can see it called here https://review.openstack.org/#/c/585385/7/nova/tests/functional/libvirt/test_numa_servers.py@546 | |
| 18:27:15 | stephenfin | That's without an attach | |
| 18:27:43 | stephenfin | but, like sean-k-mooney, the cache isn't being refreshed | |
| 18:27:47 | sean-k-mooney | dansmith: i also just figured out how to turn on the periodic task and ya didnt help. im going to see if i can force update the cache next but not sure how | |
| 18:28:01 | dansmith | sean-k-mooney: periodic tasks are not the solution to this for sure :) | |
| 18:28:18 | dansmith | really, we shouldn't need attach in the mix here | |
| 18:28:35 | stephenfin | and it's not that my physical_network/tunneled attributes aren't being set: there are entire VIFs missing, as sean-k-mooney notes | |
| 18:28:53 | dansmith | fwiw, I'm talking about the migrate test, which is more important than the rebuild one, IMHO | |
| 18:29:06 | dansmith | why are we mocking out the host_passes of the filter? | |
| 18:29:19 | stephenfin | Yup, that's what I've been focusing on | |
| 18:29:31 | stephenfin | We're not mocking it per se, we're "watching"? | |
| 18:29:34 | dansmith | oh just so we can spy on the behavior or something? | |
| 18:29:40 | stephenfin | yeah, spying | |
| 18:29:44 | dansmith | functional tests really shouldn't do that, IMHO | |
| 18:30:28 | stephenfin | They shouldn't, but it's already done there and the easiest way to test the thing | |
| 18:30:36 | dansmith | also note that none of the test_servers functionals need to mock out the libvirt migrate_disk_and_power_off internal | |
| 18:30:50 | stephenfin | None of them are using fake libvirt | |
| 18:31:31 | mriedem | probably using fake virt drivers that already stub out that method to be a noop | |
| 18:31:32 | dansmith | okay, fair, do we have to use fake libvirt for this to test all the scheduler and manager stuff? | |
| 18:31:36 | stephenfin | fwict, this is the second time anyone's attempted live migration functional tests with the libvirt functional driver, the first being mdbooth earlier this week | |
| 18:31:51 | dansmith | stephenfin: this is cold migration | |
| 18:31:57 | stephenfin | sorry, cold migration | |
| 18:32:03 | stephenfin | (which is what mdbooth did too) | |
| 18:32:46 | dansmith | okay, well, I'm missing what the attach and info_cache stale-ness has to do with this resize test | |
| 18:34:23 | stephenfin | dansmith: I need the information in my RequestSpec, which is what I'm doing here https://review.openstack.org/#/c/585339/5/nova/conductor/tasks/migrate.py@224 | |
| 18:34:45 | stephenfin | However, in the functional test, when I get there instance.info_cache is stale | |
| 18:34:49 | dansmith | stephenfin: right, I know you need it, but is your problem that it's stale, even without an attach? | |
| 18:35:05 | dansmith | stephenfin: and your IRL test, have you tried migration or just rebuild? | |
| 18:35:38 | stephenfin | Yeah, correct | |
| 18:35:52 | dansmith | because the random fishing of data from info_cache is exactly how I expected this to fail | |
| 18:36:02 | dansmith | so I'm just wondering if it's really working IRL | |
| 18:36:19 | stephenfin | and just rebuild, yeah, so it's possible something is off, aye | |
| 18:37:46 | dansmith | so this test passes all the way to the spy verification where we check .physnets, right? | |
| 18:37:58 | stephenfin | yup | |
| 18:40:26 | dansmith | stephenfin: are you sure the info was there during the initial boot? | |
| 18:40:54 | dansmith | you're checking that it wasn't for the second schedule, but.. since you can't validate based on where it landed, do you know it was present when you went to place it initially? | |
| 18:41:22 | dansmith | actually | |
| 18:41:36 | dansmith | er, yeah, your spy is just there for the reschedule so you don't have the data I think | |
| 18:42:20 | stephenfin | Yeah, I limited its scope there since it was already tested in other tests | |
| 18:42:41 | stephenfin | But yeah, I still need to get home, eat, pack and sleep. There's no more I can do on this | |
| 18:43:10 | dansmith | so, I moved the boot into the spy mock, and left it checking the first (which would be the boot), and it still fails | |
| 18:43:27 | dansmith | er, on something else, hang on | |
| 18:43:56 | dansmith | okay, nm, so the boot has it right | |
| 18:45:01 | stephenfin | Unless something drastic changes tomorrow, this is either (a) punt, (b) apply for a feature freeze exception, or (c) merge without functional tests. If (c), someone needs to test cold migrate this IRL on a two-node deployment | |
| 18:45:35 | dansmith | I'm -2 on (c) | |
| 18:45:49 | stephenfin | I'll let mriedem, melwitt et all debate that, but I do really need to go. Sorry :( | |
| 18:45:55 | dansmith | I put some debugs in the neutronapi code and they're not getting called | |
| 18:47:16 | mriedem | (d) mriedem spends the rest of the day figuring it out and fixes it | |
| 18:47:36 | mriedem | i just go back online like 5 minutes ago so i'm getting caught up, but maybe i can sort something out here this afternoon | |
| 18:47:42 | mriedem | *got | |
| 18:47:49 | dansmith | mriedem: sean-k-mooney: I put a raise Exception() here and nothing changes: https://review.openstack.org/#/c/585339/5/nova/network/neutronv2/api.py@2657 | |
| 18:48:12 | mriedem | dansmith: that's b/c we're not using that code with the NeutronFixture... | |
| 18:48:22 | dansmith | oh? | |
| 18:48:33 | mriedem | the neutron fixture stubs out nova/network/neutronv2/api | |
| 18:48:37 | mriedem | well, | |
| 18:48:45 | mriedem | at least the python-neutronclient in there, looking | |
| 18:48:51 | dansmith | the fixture doesn't do any of this meta stuff, which would explain why it's empty if what you say is right | |
| 18:49:01 | mriedem | https://github.com/openstack/nova/blob/master/nova/tests/fixtures.py#L1111 | |
| 18:49:04 | dansmith | well, that I can see | |
| 18:49:19 | sean-k-mooney | dansmith: ill put a breakpoint there to confirm | |
| 18:49:24 | mriedem | https://github.com/openstack/nova/blob/master/nova/tests/fixtures.py#L1267 | |
| 18:50:07 | mriedem | should still have something in the instance.info_cache from that, | |
| 18:50:20 | mriedem | otherwise our versioned notification samples wouldn't have address info in them (which they do | |
| 18:50:34 | mriedem | the neutron fixture is using stubbed networks and ports and such though | |
| 18:50:39 | dansmith | mriedem: I think there's something in there, it's just not augmented with the extra stuff from the link I pasted above | |
| 18:50:51 | dansmith | and that's why they think it's stale | |
| 18:51:00 | sean-k-mooney | mriedem: we have something in the info cache. its just the vaule form when the instace was booted. not the updated vaules form after the attach | |
| 18:51:02 | mriedem | stephen added a NUMAAffinityNeutronFixture or whatever that had fake networks with those values | |
| 18:51:14 | mriedem | oh i didn't know you were doing attach stuff in these tests... | |