| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2020-08-21 | |||
| 12:50:45 | sean-k-mooney | i might however ill try that quickly i would prefer to have a regression test however | |
| 12:51:02 | artom | sean-k-mooney, yeah, long-term something merged in-tree would be better | |
| 12:51:05 | artom | Or a real CI job for it | |
| 12:51:18 | sean-k-mooney | im not standing up a contrial ci :P | |
| 12:52:06 | sean-k-mooney | live migration is really uncloudy we should maybe just remove it instead. it would be a lot less work and headaces then a contrail ci :) | |
| 12:53:24 | artom | lulz | |
| 12:53:43 | sean-k-mooney | it would "fix" all the live migration bugs :) | |
| 12:58:10 | sean-k-mooney | so there is a live migration test with the old microvertion that does not support it for bandwith | |
| 12:58:13 | sean-k-mooney | https://github.com/openstack/nova/blob/master/nova/tests/functional/test_servers.py#L5993-L6029 | |
| 12:58:41 | sean-k-mooney | but not the new one so ya i think i would need stephenfin patches to cover the gaps in the neutron fixture | |
| 12:59:38 | sean-k-mooney | oh there are more test for move operatoins later | |
| 13:01:03 | sean-k-mooney | ah ha https://github.com/openstack/nova/blob/master/nova/tests/functional/test_servers.py#L7299 | |
| 13:01:08 | sean-k-mooney | test_live_migrate_with_qos_port | |
| 13:02:43 | sean-k-mooney | so even without stephenfin patches there is technically enough of a fixture to do the migration with a port | |
| 13:02:49 | sean-k-mooney | just not sriov which is fine | |
| 13:03:23 | sean-k-mooney | i just need to mock the fixture or alter it in the regression test. | |
| 13:04:44 | sean-k-mooney | i need to redefine https://github.com/openstack/nova/blob/835440e3f9dace4074511036473ebdae22f72c25/nova/tests/fixtures.py#L1739-L1752 | |
| 13:05:15 | sean-k-mooney | which i think i can do with a mock | |
| 13:22:51 | stephenfin | you could totally modify the fixture to remove "support" for multiple port bindings | |
| 13:26:14 | sean-k-mooney | im just replaceing the list_extensions function on my instance of the fixture object before i activate it | |
| 13:26:31 | sean-k-mooney | it looks like that shoudl work and is trivial to do | |
| 13:26:56 | sean-k-mooney | self.neutron = nova_fixtures.NeutronFixture(self) | |
| 13:26:59 | sean-k-mooney | self.neutron.list_extensions = self.list_extensions | |
| 13:27:01 | sean-k-mooney | self.useFixture(self.neutron) | |
| 13:27:03 | sean-k-mooney | basicly that | |
| 13:27:38 | sean-k-mooney | im basically modifing https://github.com/openstack/nova/blob/master/nova/tests/functional/regressions/test_bug_1862633.py | |
| 13:27:41 | dansmith | sean-k-mooney: I saw the discussion, thanks for chasing that down | |
| 13:27:52 | sean-k-mooney | well not modifing just copying it and creating a new one | |
| 13:28:59 | sean-k-mooney | dansmith: it was not obvious at first but i knew the code path it was taking was for multipel portbindings | |
| 13:29:51 | sean-k-mooney | anyway hopefully ill have a functional regression test for this today and we can do the too patch thing were i assert the behavior is broken and then that the patch fixes it | |
| 13:30:05 | dansmith | sweet | |
| 15:41:41 | openstackgerrit | Takashi Natsume proposed openstack/nova master: Remove six.moves https://review.opendev.org/727224 | |
| 15:42:41 | openstackgerrit | Takashi Natsume proposed openstack/nova master: Remove six.iteritems/itervalues/iterkeys https://review.opendev.org/727757 | |
| 15:43:07 | openstackgerrit | Takashi Natsume proposed openstack/nova master: Remove six.byte2int/int2byte https://review.opendev.org/727777 | |
| 15:43:25 | openstackgerrit | Takashi Natsume proposed openstack/nova master: Remove six.binary_type/integer_types/string_types https://review.opendev.org/728094 | |
| 15:43:51 | openstackgerrit | Takashi Natsume proposed openstack/nova master: Remove six.text_type (1/2) https://review.opendev.org/728109 | |
| 15:44:08 | openstackgerrit | Takashi Natsume proposed openstack/nova master: Remove six.text_type (2/2) https://review.opendev.org/728117 | |
| 15:49:19 | openstackgerrit | Ghanshyam Mann proposed openstack/nova master: Few todo fixes for API new policies https://review.opendev.org/746610 | |
| 16:16:55 | sean-k-mooney | anyone know of the top of your head how i get a valid admin context in a functional test | |
| 16:19:13 | stephenfin | sean-k-mooney: nova.context.get_admin_context() | |
| 16:19:28 | stephenfin | look for 'self.ctxt' in nova/tests/functional | |
| 16:19:59 | sean-k-mooney | thanks | |
| 16:39:33 | openstackgerrit | Ghanshyam Mann proposed openstack/nova master: Few todo fixes for API new policies https://review.opendev.org/746610 | |
| 16:40:12 | sean-k-mooney | oh i need to use the libvirt driver in this regression test | |
| 16:40:27 | sean-k-mooney | that explains why its passing wehn it shoudl not be. | |
| 16:40:37 | sean-k-mooney | ok that is still doable | |
| 16:52:49 | openstackgerrit | Artom Lifshitz proposed openstack/nova master: Revert "Handle Neutron errors in _post_live_migration()" https://review.opendev.org/747443 | |
| 16:53:28 | artom | ... which I screwed up. | |
| 16:55:57 | sean-k-mooney | it certenly rivial some of the epics stephenfin has written lately | |
| 16:56:36 | sean-k-mooney | :) http://paste.openstack.org/show/797050/ | |
| 16:56:54 | sean-k-mooney | there we go repoduced it in a functional test | |
| 16:57:10 | stephenfin | not so fast | |
| 16:57:17 | artom | Noice. | |
| 16:57:50 | sean-k-mooney | i feel like im mocking more then i then i need to and could remove some of the fixture im using | |
| 16:57:52 | stephenfin | actually, yeah, that'd make sense for this bug | |
| 16:58:08 | sean-k-mooney | but on the other hand its working so.... | |
| 16:58:16 | stephenfin | was going to say that was failing for me because the NeutronFixture wasn't setting that field like neutron would in real-life | |
| 16:58:32 | stephenfin | but of course it'll only set it if the port-binding extension is present | |
| 16:58:49 | sean-k-mooney | actully that the correct behavior | |
| 16:58:56 | stephenfin | see the changes I made to https://review.opendev.org/#/c/746947/2/nova/conductor/tasks/live_migrate.py | |
| 16:59:10 | sean-k-mooney | currenlty we are setting migrate_data.vif unconditionally | |
| 16:59:15 | sean-k-mooney | im fixint that | |
| 16:59:25 | stephenfin | already fixed ^ | |
| 16:59:25 | openstackgerrit | Artom Lifshitz proposed openstack/nova master: Revert "Handle Neutron errors in _post_live_migration()" https://review.opendev.org/747443 | |
| 16:59:30 | sean-k-mooney | no that is not the fix | |
| 16:59:47 | sean-k-mooney | stephenfin: that code we determined can never be hit | |
| 17:00:03 | sean-k-mooney | the if will always be false | |
| 17:00:21 | artom | sean-k-mooney, that's just an argument to remove it, which stephenfin is doing | |
| 17:00:30 | sean-k-mooney | https://review.opendev.org/#/c/742180/6/nova/compute/manager.py | |
| 17:00:34 | sean-k-mooney | stephenfin: that id the fix | |
| 17:00:40 | sean-k-mooney | artom: ya i know | |
| 17:00:49 | artom | But yeah, they're different things | |
| 17:00:51 | sean-k-mooney | artom: also the do said to remove it in ussurit and its victoria | |
| 17:03:37 | artom | stephenfin, understandable. It took sean-k-mooney and me 2 hours on gmeet to nail it. We considered recording it and just putting the YouTube link in the commit messages. | |
| 17:04:45 | sean-k-mooney | so we coudl say you can trust us or here 2 hours of trailing through the code to fiture it out | |
| 17:07:32 | sean-k-mooney | stephenfin: your patch removing that is not wrong by the way we should do that anyway | |
| 17:11:27 | sean-k-mooney | but lets fix the other issue breaking live migation before merging it i want to try and keep the patches small for backproting | |
| 17:38:18 | openstackgerrit | Artom Lifshitz proposed openstack/nova master: post live migration: don't call Neutron needlessly https://review.opendev.org/747451 | |
| 17:38:42 | artom | I'm pretty sure I broke some unit tests, let's see what CI says. | |
| 17:46:51 | sean-k-mooney | there are a tone of places we saw that could also use the info from the infor case in pre livem migration too right | |
| 17:50:18 | artom | sean-k-mooney, I think they'd have to get double checked one by one... | |
| 17:50:35 | artom | That being said, maybe it wouldn't hurt to do them all in the same patch | |
| 17:50:40 | artom | Or least, more than 1 | |
| 17:50:58 | sean-k-mooney | i would do them in a followup | |
| 17:51:15 | sean-k-mooney | so just replace teh once case in post live migrate in that one | |
| 17:51:33 | sean-k-mooney | and then see if you could replace the other use in pre livemigate in a different patch | |
| 18:02:09 | openstackgerrit | sean mooney proposed openstack/nova master: [WIP] Set migrate_data.vifs only when using multiple port bindings https://review.opendev.org/742180 | |
| 18:02:11 | openstack | bug 188395 in pyTD "Strange Behavior of the game.creeps list" [Undecided,Fix released] https://launchpad.net/bugs/188395 | |
| 18:02:11 | openstackgerrit | sean mooney proposed openstack/nova master: add functional regression test for bug #188395 https://review.opendev.org/747454 | |
| 18:02:45 | sean-k-mooney | i still need to add unit tests and update the commit message but thats the repodcuer and fix | |
| 18:04:13 | sean-k-mooney | as a followup im also thinking of replacing all uses of | |
| 18:04:16 | sean-k-mooney | if 'vifs' in migrate_data and migrate_data.vifs: | |
| 18:04:41 | sean-k-mooney | with "if migrate_data.using_multiple_port_bindings():" | |
| 18:04:55 | artom | Would definitely be more readable, aye | |
| 18:05:15 | sean-k-mooney | i can do it as a property so its technically not an ovo change too | |
| 18:05:20 | sean-k-mooney | if we wanted to backport that | |
| 18:05:26 | sean-k-mooney | but it will be a spereate patch anyway | |
| 18:06:11 | sean-k-mooney | well it is an ovo change but not one that is visable when you serialise so its fine | |
| 18:11:14 | sean-k-mooney | apparently its only there 3 times | |
| 18:11:20 | sean-k-mooney | i tought it was more | |