| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-07-25 | |||
| 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... | |
| 18:51:16 | dansmith | not in fixtures that I see | |
| 18:51:20 | mriedem | why are we trying to test attach in this basic test? | |
| 18:51:29 | dansmith | mriedem: that's what I said, | |
| 18:51:38 | dansmith | mriedem: but we're talking about the migration test below the rebuilds | |
| 18:51:39 | mriedem | heh, let's get the dumb simple flows working first | |
| 18:51:43 | dansmith | right | |
| 18:51:48 | dansmith | but.. scroll down :) | |
| 18:51:52 | mriedem | before we try to land on the moon | |
| 18:51:59 | sean-k-mooney | the attahced interface is connected to a network which is mapped to a different numa node then the first network | |
| 18:52:21 | sean-k-mooney | the instance has 1 numa node requested so its invalid and should fail | |
| 18:53:14 | dansmith | mriedem: so, that fixture he added has neutron responses with those values, | |
| 18:53:36 | dansmith | but something has to copy them into the info cache's network_model, which is the neutronv2/api code, unless that's implemented somewhere else for this test | |
| 18:55:12 | dansmith | ah, I might know | |
| 18:56:27 | openstackgerrit | Merged openstack/nova master: Add nova-manage placement sync_aggregates https://review.openstack.org/575912 | |
| 18:58:41 | dansmith | hmm, | |
| 18:58:48 | dansmith | well the neutron fixture has a canned nw_info structure | |
| 18:58:54 | dansmith | which doesn't have the physnet stuff in it | |
| 18:59:23 | mriedem | but that's only used for add_fixed_ip_to_instance and remove | |
| 18:59:25 | dansmith | and it mocks out networkinfo.hydrate (!) to return that | |
| 18:59:28 | mriedem | which isn't the same as attach/detach | |
| 18:59:37 | dansmith | mriedem: there's no attach/detach in this test | |
| 19:00:02 | mriedem | still, we don't call add_fixed_ip_to_instance at all so that canned one shouldn't matter | |
| 19:00:17 | dansmith | it mocks out hydrate in that fixture | |
| 19:00:55 | mriedem | where? | |
| 19:01:08 | dansmith | oh sorry | |
| 19:01:20 | dansmith | I was reading it wrong, it mocks the thing you said and calls hydrate from the lambda | |
| 19:01:23 | dansmith | sorry | |
| 19:03:38 | dansmith | well, in that case, like I said, I'm not sure how we'll ever get those new things into the network model if we're not running the neutronv2/api code | |
| 19:06:58 | mriedem | i think we are but with the stubbed list_ports, list_networks, show_network stuff | |
| 19:07:15 | sean-k-mooney | dansmith: this is whats in instance.info_cache.network_info if it helps http://paste.openstack.org/show/726641/ | |
| 19:08:02 | mriedem | so that does have the physical_network and tunneled network meta | |
| 19:08:40 | mriedem | melwitt: https://review.openstack.org/#/c/517921/ and below need a final +@ | |
| 19:08:41 | mriedem | +2 | |
| 19:08:48 | dansmith | mriedem: we're not running the _nw_info_build_network code | |
| 19:09:08 | melwitt | mriedem: thanks, will look | |
| 19:09:17 | mriedem | dansmith: hmm | |
| 19:09:23 | mriedem | i'm not sure why we wouldn't, i don't see anything stubbing that out | |
| 19:09:27 | dansmith | mriedem: that was my point above | |
| 19:09:30 | dansmith | and you shat upon it | |
| 19:09:31 | mriedem | oh wait, | |
| 19:09:42 | mriedem | we do have other things that stub some shit out in the nw api | |
| 19:09:51 | mriedem | and i bet that is getting stubbed in the parent test base class | |
| 19:10:23 | mriedem | in the air like you just don't care!? | |
| 19:10:52 | dansmith | like jan brady when marcia suggests the same thing she just did and now everyone thinks it's cool | |