| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-02-02 | |||
| 14:19:57 | mdbooth | mriedem: i.e. we can just pass in volume_id | |
| 14:20:14 | lyarwood | nack, lets fix the bug and then talk about that refactor | |
| 14:20:16 | fried_rice | cdent: I thought I had dealt with that issue... | |
| 14:20:23 | mriedem | mdbooth: with 6 days to RC1, i want to just fix the bug | |
| 14:20:40 | mdbooth | It's really not a big refactor, just shuffling some argument | |
| 14:20:57 | giblet | ildikov, mriedem: I left some comments in https://review.openstack.org/#/c/532689 | |
| 14:20:59 | mdbooth | And it will be more robust as you're no longer beholden to the vagiaries of connection_info | |
| 14:21:26 | lyarwood | mriedem's suggested change isn't | |
| 14:21:30 | mdbooth | With 6 days to RC1, personally I'd just buckle down and do it. No heuristics means no late surprises. | |
| 14:22:25 | mriedem | with 6 days to RC1 how about we just revert https://review.openstack.org/#/c/523958/ ? | |
| 14:22:28 | mriedem | would that help? | |
| 14:22:32 | bauwser | FWIW, we have more than 50 open bugs | |
| 14:22:36 | bauwser | 6 days before RC1 | |
| 14:22:41 | bauwser | that's what it tramples me | |
| 14:22:55 | bauwser | 50 "new" bugs, my bad | |
| 14:23:12 | bauwser | and I don't have so much time reviewing those bug reports | |
| 14:23:25 | mriedem | lee answered my question so i'll just make this fix and we can move on | |
| 14:23:30 | bauwser | so, if anyone can help me on that, many thanks | |
| 14:23:33 | lyarwood | please, thanks :) | |
| 14:23:45 | mriedem | bauwser: several of those new bugs are old-ish at this point | |
| 14:23:59 | mriedem | like, "should instance group policies be empty?" | |
| 14:24:05 | bauwser | several, but I don't know if we have regressions yet | |
| 14:24:11 | mriedem | i don't know if that's a bug or what so i didn't know how to triage it | |
| 14:24:21 | bauwser | okay, I'll really need to help you | |
| 14:24:22 | mdbooth | Meh, this is how we play virt driver Jenga :( | |
| 14:24:32 | bauwser | ... once I'm done with other internal work :) | |
| 14:24:56 | mriedem | bauwser: i went through these after the meeting yesterday, there is only one new one in there since then | |
| 14:25:04 | bauwser | cool then | |
| 14:25:21 | bauwser | anyway, let's not diverge | |
| 14:26:59 | ildikov | giblet: tnx | |
| 14:28:36 | mriedem | lyarwood: looks like there were no tests for that pre_live_migration code | |
| 14:28:45 | mriedem | i was thinking it would be in https://review.openstack.org/#/c/523958/18/nova/tests/unit/virt/libvirt/test_migration.py but those don't actually call the driver from what i can tell | |
| 14:28:56 | mriedem | i added an exception to where volume_id was set in that block in pre_live_migratoin and nothing failed | |
| 14:29:46 | mriedem | will see if i can wrangle some existing test to add that condition | |
| 14:29:55 | cdent | fried_rice: this one is fun too https://bugs.launchpad.net/nova/+bug/1747003 | |
| 14:29:56 | openstack | Launchpad bug 1747003 in OpenStack Compute (nova) "A bad _RC_CACHE can rarely cause unit tests to fail" [Undecided,New] | |
| 14:30:03 | lyarwood | mriedem: is there nothing in _test_pre_live_migration_works_correctly_mocked ? | |
| 14:31:10 | fried_rice | cdent: Are you proposing fixes, or setting me up to review them? | |
| 14:31:11 | mriedem | no | |
| 14:32:02 | mriedem | i'll update https://review.openstack.org/#/c/523958/18/nova/tests/unit/virt/libvirt/test_migration.py | |
| 14:32:08 | mriedem | oops, test_pre_live_migration_vol_backed_works_correctly_mocked | |
| 14:32:26 | cdent | fried_rice: I'm making conversation. I'll propose fixes at some point. The RC_CACHE one is easy, the other one somewhat more effortful. | |
| 14:32:54 | Spazmotic | And just in time for my pre-sleepytime begging session to bother the first shift folks - If any core gets a chance to review this Live Migration XenAPI commit i'd apprecaite it. https://review.openstack.org/#/c/538415/ | |
| 14:33:07 | fried_rice | cdent: Okay. I wasn't ragging on you, just didn't want to duplicate effort if I decided to go off and work on one of 'em. | |
| 14:33:10 | Spazmotic | I know ya'lls busy, but just figured i'd throw it out there while I eat cereal :D | |
| 14:33:16 | openstackgerrit | Ed Leafe proposed openstack/nova master: Fix invalid UUIDs in test_compute.py https://review.openstack.org/531479 | |
| 14:33:53 | lyarwood | mriedem: re the connection_info.get('volume_id') lookup, I'm pretty sure that actually works, I can't find where but I recall one of the bdm layers using connection_info to store connection_info['data'] | |
| 14:34:47 | openstackgerrit | Ed Leafe proposed openstack/nova master: Escalate UUID validation warning to error in test https://review.openstack.org/540386 | |
| 14:35:36 | mriedem | connection_info['data'] != connection_info['volume_id'] | |
| 14:35:55 | mriedem | lyarwood: i'm assuming you meant that line of code to be the same as the others that looked for connection_info['data']['volume_id'] | |
| 14:36:02 | mriedem | it was probably a typo, that wasn't tested so it wasn't noticed | |
| 14:40:50 | lyarwood | mriedem: right, I'm pretty confused how the tempest tests I've been working on have passed without this | |
| 14:41:03 | mriedem | lyarwood: volume-backed live migratoin isn't enabled in tempest by default | |
| 14:41:08 | mriedem | so the tests likely aren't getting run | |
| 14:41:15 | lyarwood | mriedem: it is in my DNM change | |
| 14:41:18 | lyarwood | mriedem: and locally | |
| 14:41:24 | lyarwood | mriedem: where these tests have been passing | |
| 14:41:47 | lyarwood | http://logs.openstack.org/50/536350/5/check/legacy-tempest-dsvm-multinode-live-migration/539c51d/job-output.txt.gz for example | |
| 14:44:26 | mriedem | idk, i don't see the lvm volume driver setting connection_info['volume_id'] | |
| 14:45:33 | lyarwood | mriedem: right, anyway, ignore that, I'll look into how these actually passed now, it doesn't change the fix and lack of unit tests | |
| 14:46:01 | lyarwood | mriedem: happy to take over the updating the fix if you don't have time today btw | |
| 14:46:10 | lyarwood | mriedem: this is my mess, I should really be the one to clean it up | |
| 14:47:04 | mriedem | i've got it, just updating test_pre_live_migration_vol_backed_works_correctly_mocked so it doesn't fake out the bdm stuff that the compute manager is doing, | |
| 14:47:13 | mriedem | i.e. actually going to make that test go through driver.get_block_device_info | |
| 14:47:42 | mriedem | bauwser: question in https://review.openstack.org/#/c/540168/ | |
| 14:47:49 | openstackgerrit | Chris Dent proposed openstack/nova master: Reset the _RC_CACHE between tests https://review.openstack.org/540404 | |
| 14:48:34 | leakypipes | lyarwood: on Fridays, I always see your IRC nick and think of pirates. "Yarrr" wood is here. | |
| 14:50:51 | lyarwood | leakypipes: haha, yeah you're not the first to suggest that, had a guy in consulting who would scream that across the office while I was on the phone back in my support days | |
| 14:51:24 | leakypipes | lyarwood: you should wear a pirate hat on Fridays. just sayin'. | |
| 14:51:33 | leakypipes | :) yay! \o/ | |
| 14:51:58 | Spazmotic | I've started putting commits into my monthly planner since I have such a backlog and don't want to merge conflict the poop out of xenapi.vmops hehe | |
| 14:53:21 | bauwser | mriedem: I'm not a specialist on VIF plugging and unplugging, so I could be wrong | |
| 14:53:56 | bauwser | mriedem: but from what I can see, we destroy and cleanup the instance (hence the VIF unplugs) when we hard reboot | |
| 14:54:25 | bauwser | what happens behind the VIF unplug call is unbound territory to me | |
| 14:54:45 | bauwser | I'm just saying that the change melwitt provides makes it consistent with all the other server actions | |
| 14:57:28 | bauwser | 2 mins for making me coffee before an internal meeting, I can do | |
| 14:58:06 | openstackgerrit | Chris Dent proposed openstack/nova master: Optional separate database for placement API https://review.openstack.org/362766 | |
| 15:03:40 | openstackgerrit | rahul bardia proposed openstack/python-novaclient master: nova limits ERROR (Exception): Field names must be unique https://review.openstack.org/527922 | |
| 15:04:12 | melwitt | mriedem: when I read your original comment and when I rewrote it I was thinking 'locally' meant plugging vifs through os-vif only and not making any neutron api calls. plugging locally on the libvirt domain | |
| 15:09:07 | mriedem | we don't make any neutron api calls when plugging/unplugging vifs | |
| 15:09:22 | mriedem | i'm looking back on https://github.com/openstack/nova/commit/e4e1c6a2f060a91899549b9643132abde27d033e | |
| 15:09:44 | mriedem | "this is the explanation I've come up with, albeit not a great one, and indicates a gap in some of the nova/neutron integration story." | |
| 15:09:45 | mriedem | heh | |
| 15:09:48 | mriedem | thanks past me | |
| 15:11:08 | melwitt | mriedem: I know, I mean plugging a vif with no contact with neutron at all. on instance boot we create a port. honestly, I don't really know but from the other code comments, we've run into this before it looks like, and it must work at least *some* of the time otherwise reboot would be 100% broken right now. we really need a neutron expert to explain when/how events are/aren't emitted | |
| 15:11:14 | bauwser | mriedem: the fact that we don't call Neutron when we unplug is understandable, but don't we emit something about the port itself ? | |
| 15:11:41 | bauwser | because if we don't, why should we wait for Neutron ? it's a non-sense | |
| 15:11:45 | mriedem | no we don't update the port | |
| 15:11:50 | mriedem | we plug/unplug, | |
| 15:11:58 | bauwser | so, definitely no need to wait for Neutron, right? | |
| 15:12:01 | mriedem | then neutron agent picks up that change, tells the server, and the server sends an event to nova when it's done | |
| 15:12:01 | fried_rice | cdent: If this isn't already on your radar... https://review.openstack.org/#/c/539712/ | |
| 15:12:10 | bauwser | ah-ha | |
| 15:12:15 | melwitt | the neutron agent doesn't always pick up the change, but we don't know why not | |
| 15:12:21 | melwitt | I assume some kind of race or something | |
| 15:12:26 | mriedem | then we likely refresh out nw info cache on the instance to say if the port is gone or not or up/down | |
| 15:12:35 | cdent | fried_rice: yeah, it's in my queue, but may not be seen until monday | |
| 15:12:36 | mriedem | *our | |
| 15:12:40 | fried_rice | ight | |
| 15:12:57 | fried_rice | Hey, what's the lib I need to install to get around this pcre thing in py35? | |