Earlier  
Posted Nick Remark
#openstack-nova - 2018-07-30
13:42:30 dansmith stephenfin: are you back?
13:50:04 openstackgerrit Merged openstack/nova master: Add documentation for emulator threads policy https://review.openstack.org/579956
13:51:50 kashyap dansmith: IIRC, he's still off til 01-Aug.
13:51:59 dansmith ugh okay
13:52:17 dansmith gibi: I replied here: https://review.openstack.org/#/c/585385/14
13:52:51 dansmith gibi: if you're happy with that I'd sure like to get that landed since the previous patches were predicated on those tests, which we're currently lacking in tree
13:56:28 openstackgerrit Takashi NATSUME proposed openstack/nova master: Add description for placement 1.26 https://review.openstack.org/587067
13:59:26 efried Scheduler meeting in #openstack-meeting-alt
14:02:08 kashyap sean-k-mooney: Yay! Now the 'nova-live-migration' job succeeds!
14:02:13 kashyap sean-k-mooney: http://zuul.openstack.org/stream.html?uuid=0707146f416940f68506d5d994038158&logfile=console.log
14:02:49 kashyap sean-k-mooney: Hmm, actually I jumped the gun toon soon :P It's "almost" success, because it's still in blue.
14:04:01 openstackgerrit Lee Yarwood proposed openstack/nova master: WIP compute: Recreate attachments during a reschedule https://review.openstack.org/587071
14:04:37 sean-k-mooney kashyap: nice
14:05:04 kashyap Not quite, I want to see the green label there :-)
14:05:35 sean-k-mooney has well it will go green when it uploads the logs
14:05:49 sean-k-mooney that said it is still running some of the tests
14:06:05 sean-k-mooney they are passing now however so thats good
14:06:06 kashyap But the failing test succeeded, though: tempest.api.compute.admin.test_live_migration.LiveMigrationTest.test_live_block_migration
14:06:09 kashyap Yep
14:07:32 gibi dansmith: looking...
14:10:21 gibi dansmith: I've removed the L96 fake_network.set_stub_network_methods(self) and removed L506 fake_network.unset_stub_network_methods(self) and the test started failing. Based on the code the two removed calls are reverse of each other so I expected that if I remove both then the test result is the same
14:11:25 dansmith gibi: okay I thought you meant the unstubbing part specifically
14:11:36 gibi dansmith: I can push that change in a separate commit top of stephenfin's if that helps
14:12:42 dansmith well, I just don't want to end up without functional tests for this stuff, but I understand why you think this isn't working
14:12:48 dansmith so let me dig in a bit I guess
14:14:46 gibi dansmith: I'm affraid we are missing something as this functional test is pretty complex.
14:15:05 dansmith I understand
14:16:04 gibi dansmith: If we find nothing then I can accept to merge this as there are new tests that seems OK
14:17:17 dansmith gibi: your comment about L96.. is that new test using that? I don't think it calls _run_build_test()
14:17:35 dansmith because I thought these were stubbed way lower, which is why mriedem suggested just unstubbing in these couple new tests
14:18:03 dansmith I also thought that this was stubbing neutronapi methods,
14:18:17 dansmith but the fake_stub_network_methods() on L96 is stubbing out compute stuff
14:18:28 dansmith unset will unset any that have been stubbed
14:19:03 gibi dansmith: that could explain what I see. Let me dig in after the scheduler meeting
14:19:38 gibi dansmith: in general I think these tests should work with non stubbed neutronv2/api
14:19:58 dansmith gibi: I'm not sure how, we still have to return fake responses from neutron
14:20:25 gibi dansmith: yeah, that is the only thing that need to be faked
14:22:17 dansmith well, it's using the neutronfixture, which seems like the right thing to do...
14:23:52 dansmith ah
14:23:55 dansmith gibi: https://github.com/openstack/nova/blob/master/nova/tests/functional/test_servers.py#L79-L79
14:24:02 dansmith that is the test this inherits from, which stubs things out
14:24:25 dansmith I think if we don't unstub them we don't call into our neutron fixture to return the augmented nw_info, which is why we don't see the new stuff unless we unstub
14:24:48 dansmith so NUMAServersTestWithNetworks->NUMAServersTestBase->ServersTestBase->the stubbing
14:25:40 gibi dansmith: OK, then my expectation is that if I move the unstub to the NUMAServersTestBase.setup() then everything should pass
14:26:18 dansmith do you mean NUMAServersWithNetworksTest?
14:26:29 dansmith I don't think this patch needs to change the other tests that are here
14:27:36 gibi dansmith: I theory both NUMAServersTestBase but I agree that in _this_ patch we should not change the other tests, so the NUMAServersTestBase change would be a followup
14:27:46 gibi s/both//
14:27:53 dansmith I just moved it to setup and I think all these new ones pass.. will run the whole file to be sure
14:28:06 sean-k-mooney dansmith: for the numa aware vswitch spec what is the expect behavior if you do not enable the numatopology fileter
14:28:26 dansmith TBH, I'm not sure I see why moving it to setup matters, but..
14:28:40 dansmith sean-k-mooney: are you asking?
14:29:18 sean-k-mooney dansmith: yes. with stephenfin code we do not validate the constratins on the compute node just in the schduler
14:29:32 sean-k-mooney at least that is what i am seeing locally.
14:29:33 dansmith sean-k-mooney: right, with the filter disabled this is just all ignored
14:29:47 sean-k-mooney ya is that correct however?
14:29:52 dansmith yes
14:30:18 sean-k-mooney ok cool
14:31:00 sean-k-mooney im guessing we dont want a late check like the anti affinity stuff to avoid retries?
14:31:01 dansmith gibi: another test fails if I do that
14:31:01 gibi dansmith: I thought originally that L96 was the stubbing for every NUMAServer tests (and I was wrong) and in general I want to have as small amount of stub in the functional tests as possible. Now that I know that we get the stubbing from a lot further base it means If I need to follow my principle then I have to do a bit more surgery _outside_ of the current patch
14:31:12 dansmith gibi: it's related to this series, but not this patch
14:31:25 dansmith so if I change this I will have to update that test too
14:31:46 dansmith gibi: totally agree with minimal stubbing in functional tests for sure
14:31:58 dansmith this is testing fake libvirt with fake neutron, so... lots of stubbing required regardless :/
14:32:01 gibi dansmith: OK, I don't think any more that is worth to blow up the current patch
14:32:26 dansmith gibi: I will put the move and the related changes in a separate patch on top then if that's okay with you
14:32:28 gibi dansmith: let's merge this as is and I will spend time on a followup to reduce the stubbing if possible
14:32:45 gibi dansmith: that totally works for me
14:38:20 mnaser dansmith: whenever you're free, if you can add this to your queue -- https://review.openstack.org/#/c/586824/ (and the patch above it is more of a poc) -- with some info in the bug
14:39:12 dansmith alright
14:39:51 kashyap mnaser: Maybe it's just me, but first paragraph in the commit message is confusing
14:40:52 kashyap (And the patch above needs to be rebased I think)
14:40:56 dansmith makes sense to me
14:42:06 kashyap You have more context, though :-) But yeah, reading the whole message makes it clearer.
14:44:58 mnaser kashyap: yeah, it's still very much early and needs reviews :)
14:45:08 mnaser it was more of a rough "hey how's this to solve a problem"
14:46:37 dansmith gibi: the fix for the other test was in the create_port() handler of the fixture, so it was touched by this test and part of your other feedback anyway,
14:46:47 dansmith gibi: so I'll just squash and update this patch
14:46:53 gibi dansmith: OK, cool
14:47:10 openstackgerrit Dan Smith proposed openstack/nova master: Add additional functional tests for NUMA networks https://review.openstack.org/585385
14:47:13 dansmith gibi: note that just moves it up one test level, not all the way to the top
14:48:14 kashyap mdbooth: Not sure if you'll be able to today, but this is ready and fixed all the tests (including the live migration job): https://review.openstack.org/#/c/567258/
14:48:33 mdbooth kashyap: ack
14:48:37 kashyap Gentle ping because you've reviewed it before. (And I'm off for 2 weeks from Wed.)
14:48:41 kashyap mdbooth: Thanks!
14:50:13 openstackgerrit Matt Riedemann proposed openstack/nova master: Updated AggregateImagePropertiesIsolation filter illustration https://review.openstack.org/586317
14:51:55 tetsuro_ gibi: I'm going to rebase https://review.openstack.org/#/c/585672/ on gibi's 1.28 support one.
14:52:00 mriedem lyarwood: looks like a backport to pike is using a method that doesn't even exist :( https://bugs.launchpad.net/nova/+bug/1783635
14:52:00 openstack Launchpad bug 1783635 in OpenStack Compute (nova) "Live migration failed.: AttributeError: 'Guest' object has no attribute 'migrate_configure_max_speed'" [Undecided,New]
14:52:09 tetsuro_ gibi: Before pushing them, I'm looking into the functional test in https://review.openstack.org/#/c/527728/18 to pass rebased on the two.
14:52:27 tetsuro_ gibi, lajos: Would you mind if I amend https://review.openstack.org/#/c/527728/18?
14:54:37 mriedem lyarwood: i guess you figured that out already https://review.openstack.org/#/c/586965/
14:54:39 gibi tetsuro_: thanks for the heads-up. I have nothing agains the rebase. I talked to lajos before and he let me take over https://review.openstack.org/#/c/527728 so I think I can let you in his name to amend it :)
14:56:08 tetsuro_ gibi: Thanks!
14:56:11 gibi tetsuro_: regarding the rebase the patch adding support for 1.29 top of 1.28. That is also cool with me
14:56:45 tetsuro_ Yup. That would be the right way to go.
14:56:49 gibi tetsuro_: I will try to keep continue progressing support for 1.28 as my time allows during the RC period so we can merge it soon in Stein
14:58:51 tetsuro_ gibi: Cool. I can help the "change unit tests to pass" part if I finish the rebase and test work earlier than I think.
15:00:00 gibi tetsuro_: I consider unit test secondary in 1.28 but thanks for any help with those. I'm more affraid of how to add proper functional coverage for the consumer generation conflict cases

Earlier   Later