Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-07-25
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
19:11:58 mriedem ServersTestBase
19:12:03 mriedem fake_network.set_stub_network_methods(self)
19:12:17 mriedem thar she blar
19:13:13 mriedem so if stephenfin's test calls unset_stub_network_methods we should hit the cache rebuilder
19:14:00 sean-k-mooney so _nw_info_build_network is being called when weer are booting
19:14:00 mriedem i'm not sure why someone didn't figure this all out hours ago :P
19:14:19 mriedem sean-k-mooney: yes, at the end of allocate_for_instance,
19:14:26 mriedem but we don't get that far b/c of set_stub_network_methods
19:14:27 sean-k-mooney yes
19:14:37 sean-k-mooney its not called in the rebuild
19:15:01 mriedem it doesn't need to be,
19:15:14 mriedem b/c after the server is created, the nw info cache is persisted in the db with the instance
19:15:21 mriedem assuming we had something in the db, but we don't b/c of set_stub_network_methods
19:15:22 dansmith yep, that fixes it, I'll push this up
19:15:26 mriedem WOOT
19:15:49 sean-k-mooney dansmith: awsome :)
19:16:22 openstackgerrit Dan Smith proposed openstack/nova master: WIP: Add additional functional tests for NUMA networks https://review.openstack.org/585385
19:23:10 dansmith melwitt: mriedem: I'm assuming no cells meeting with the crunch and all
19:23:20 melwitt +1
19:23:35 mriedem agree
19:31:49 mriedem oh sweet irony as soon as we approve artom's test_tagged_attachment debug patch, it fails on that test
19:31:53 mriedem http://logs.openstack.org/32/584032/5/check/nova-next/caee4e1/logs/testr_results.html.gz
19:39:06 mriedem doesn't show any of artom's new debug messages for the instance or volume
19:41:47 sean-k-mooney ok my server says i loged into it 12 hour and 21 minutes ago. my brain is fried so im going to call it a day. i have the functional test for dansmith updated version running local. if test_rebuild_server_network_changes still fails perhaps we should remove it given cold migrate and test_rebuild_server_no_network_changes pass
19:42:31 dansmith yeah the rebuild tests don't work,
19:42:35 mriedem dansmith: what do you want me to do re ^? otherwise i'm just watching zuul and need to book a flight to china. do we have more tests that need to be written and/or cleaned up in the patch?
19:43:15 dansmith I think we need the negative test
19:43:41 mriedem i'm not sure i know how to do that
19:43:50 mriedem that's where the nodes on the host are all claimed?
19:44:01 dansmith no, there's an easier way

Earlier   Later