| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-07-25 | |||
| 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 | |
| 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 | |