| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-07-25 | |||
| 17:26:59 | stephenfin | We obviously shouldn't be doing that in the functional test itself | |
| 17:27:32 | sean-k-mooney | looks like we can call get_instance_nw_info and it will update the cache | |
| 17:29:49 | openstackgerrit | melanie witt proposed openstack/nova stable/ocata: [stable only] Always commit quota decrement if build request found https://review.openstack.org/582413 | |
| 17:42:57 | jangutter | Wow, looks like the killbot is getting quicker! | |
| 17:52:06 | sean-k-mooney | stephenfin: the periodic tasks are disable on the compute manager that is created by self._test_create_server_with_networks | |
| 17:54:17 | openstackgerrit | Merged openstack/os-vif master: Add vif_plug_noop to setup.cfg packages https://review.openstack.org/585530 | |
| 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 | dansmith | but what I'm not remembering is if we asking the scheduler _at_all_ if we aren't changing images | |
| 18:26:22 | stephenfin | dansmith: Yup, mriedem_away and I checked that yesterday | |
| 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 | |