| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-03-06 | |||
| 16:51:57 | stephenfin | dansmith: If I was using DynamicJSON, yeah | |
| 16:52:00 | dansmith | stephenfin: config_drive literally just imports the python module that generates the metadata response and uses it to dump things into a disk image | |
| 16:52:08 | dansmith | stephenfin: then yeah, nova-compute would need that config | |
| 16:52:16 | sean-k-mooney | artom: you can add force_config drive or something but you can also just enable config drive in the flavor/image | |
| 16:52:22 | stephenfin | dansmith++ ta | |
| 16:52:54 | stephenfin | I ended up down a rabbithole yesterday and ended up doing this https://review.openstack.org/#/c/640730/ and now I'm more confused than ever. Back to reviews | |
| 16:56:13 | mriedem | https://bugs.launchpad.net/nova/+bug/1818873 | |
| 16:56:14 | openstack | Launchpad bug 1818873 in OpenStack Compute (nova) "When post_live_migration_at_destination fails the instance is not put into ERROR/None vm_state/task_state" [Medium,Triaged] | |
| 16:56:26 | mriedem | so if post_live_migration_at_destination fails early, the guest is on the dest but the instance db state is all f'ed up | |
| 16:56:46 | mriedem | mdbooth: don't you have a similar bug to this ^? | |
| 16:57:41 | sean-k-mooney | mriedem: yes | |
| 16:58:07 | mdbooth | The one I recall was the other way round. The issue is when post_live_migration fails on the *source* | |
| 16:58:18 | artom | mriedem, maybe https://bugs.launchpad.net/nova/+bug/1628606? | |
| 16:58:20 | openstack | Launchpad bug 1628606 in OpenStack Compute (nova) "live migration does not clean up at target node if a failure occurs during post migration" [Low,In progress] - Assigned to Artom Lifshitz (notartom) | |
| 16:58:25 | sean-k-mooney | i brought that up in https://review.openstack.org/#/c/603844/ | |
| 16:59:00 | artom | mriedem, I've been meaning to refactor the flow to make sure we save the instance state correctly in all cases | |
| 16:59:16 | artom | I addressed the Cinder API fail case, there are more (like Neutron) | |
| 16:59:29 | sean-k-mooney | artom: didnt you partial fix this already. we just 1 addional try finally block effectily to make sure we always set the instance host | |
| 16:59:36 | openstackgerrit | Merged openstack/nova master: Support server create with ports having resource request https://review.openstack.org/636360 | |
| 16:59:41 | sean-k-mooney | artom: you didnt address all the cinder issues | |
| 16:59:44 | openstackgerrit | Merged openstack/nova master: objects: Store InstancePCIRequest.numa_policy in DB https://review.openstack.org/624444 | |
| 16:59:48 | artom | sean-k-mooney, yeah https://review.openstack.org/#/c/611093/ | |
| 17:00:28 | sean-k-mooney | you missed on place where we call cinder that can raise an exception | |
| 17:00:50 | artom | sean-k-mooney, yeah, it's entirely possible | |
| 17:01:15 | artom | sean-k-mooney, it was meant as a bandaid until a more correct fix is done | |
| 17:02:29 | mriedem | this is clearly not...enterprise ready | |
| 17:02:38 | mriedem | one could spend days cleaning up this mess | |
| 17:03:10 | sean-k-mooney | artom: ya i know | |
| 17:03:12 | sean-k-mooney | https://github.com/openstack/nova/blob/38c96993fd7163afe1a7c5f380af0d57a1bd2903/nova/compute/manager.py#L6736 | |
| 17:03:16 | mdbooth | It seems more intuitive to me to update instance.host to the destination *immediately* we're actually running there, i.e. from the source at the top of _post_live_migration, but IIRC there are weird gotchas with that. | |
| 17:03:22 | sean-k-mooney | that is that call im thinking of | |
| 17:03:24 | mriedem | another clunker is we pass the Migration object around via the MigrateData which is pretty brittle, | |
| 17:03:29 | artom | mriedem, what mdbooth said | |
| 17:03:30 | mriedem | i'd like to just partial that like what mdbooth did with source_bdms | |
| 17:04:01 | sean-k-mooney | it will hit the cinder attachments api and can raise an exception which means the vm is running on the dest but we never update the db | |
| 17:04:06 | artom | I think just making sure we always update instance state the moment it's irreversibly on the destination is a good first step | |
| 17:04:49 | sean-k-mooney | ya i wrote it down on my whiteboard after reviewing the other chage earlier in the week | |
| 17:06:41 | mdbooth | You know how sometimes you don't understand a thing, but you remember understanding a thing while somebody was explaining it? I vaguely recall dansmith explaining why instance.host needs to be updated later. | |
| 17:07:08 | dansmith | mdbooth: during migration? | |
| 17:07:14 | mdbooth | dansmith: Yeah. | |
| 17:07:38 | sean-k-mooney | dansmith: later in post_live_migration_at_destination to be percise | |
| 17:07:40 | dansmith | you know how sometimes you don't understand a thing, but you remember that every time someone asks about it, you go re-re-re-research it and re-remind yourself why that thing is true? | |
| 17:07:49 | mdbooth | dansmith: haha | |
| 17:08:21 | artom | Presumably things count on instance.host still being the source | |
| 17:08:38 | mdbooth | artom: Right. dansmith can be very convincing at times. | |
| 17:08:42 | sean-k-mooney | it was releated to cleaning up the cinder and neutron port properly on the souces | |
| 17:09:21 | sean-k-mooney | in anycase we can likely resolve that and make sure the db is consitent if we spend a little itme on it | |
| 17:09:45 | artom | sean-k-mooney, heh, that's true for literally *all* of the bugs :D | |
| 17:10:00 | sean-k-mooney | a good first step is proably jsut to break the fucntion into smaller functions so its eaier to grock | |
| 17:10:11 | artom | sean-k-mooney, that too | |
| 17:10:19 | artom | It's actually on our downstream squad backlog | |
| 17:10:53 | sean-k-mooney | ya i know its on my personal white board backlog too for after i get done with sriov. | |
| 17:11:10 | sean-k-mooney | i didnt wnat to go messing with the post live migrtation stuff in parallel | |
| 17:19:25 | openstackgerrit | Theodoros Tsioutsias proposed openstack/nova master: Add requested_networks to RequestSpec https://review.openstack.org/570201 | |
| 17:19:26 | openstackgerrit | Theodoros Tsioutsias proposed openstack/nova master: Enable rebuild for instances in cell0 https://review.openstack.org/570203 | |
| 17:31:04 | sean-k-mooney | adrianc: mind if i fix https://review.openstack.org/#/c/620115/27/nova/compute/manager.py@89 and rebase everyting | |
| 17:33:45 | openstackgerrit | Matt Riedemann proposed openstack/nova master: De-cruft compute manager live migration https://review.openstack.org/641449 | |
| 17:42:03 | sean-k-mooney | is it my imaginaiton or does devstack no longer delete exsiting libvirt instances when it stacks? | |
| 17:42:43 | sean-k-mooney | i keep hitting domain already exists errors lately if i dont delete instance before i unstack and stack | |
| 17:48:07 | cdent | sean-k-mooney: I haven't experienced that, I rarely created actual vms from devstack, but I'm not surprised if it is happening: maintenance of devstack as a _dev_ tool, rather than the thing that happens in the gate is getting nowhere near the attention it used to | |
| 17:49:18 | sean-k-mooney | i think someone removed it | |
| 17:50:01 | sean-k-mooney | ill check the logs but after 5+ years of knowing that stack will clean up any vms you forgot about its hard to unlearn | |
| 17:50:07 | dansmith | sean-k-mooney: I experienced that a while ago.. probably more than a year | |
| 17:50:44 | dansmith | tbh, I'm not sure why it would do that for you, and it'd need to be real careful about any non-nova vms that were there | |
| 17:50:46 | sean-k-mooney | well i always found it strange it did it on stack not unstack but i have only noticed it lately | |
| 17:50:56 | dansmith | unstack is really not maintained, AFAIK | |
| 17:50:58 | melwitt | meanwhile, I never knew it cleaned up anything because I always deleted mine or destroyed the entire env and start over. not much unstack/stack | |
| 17:51:02 | dansmith | it's missing a ton of crap | |
| 17:51:32 | sean-k-mooney | dansmith: it used to use the instance name prefix that you have cofigred to delete any vm that matched | |
| 17:51:52 | dansmith | sean-k-mooney: okay, that's surprising to me, but ... | |
| 17:51:54 | sean-k-mooney | dansmith: ya i have a todo to fix wsgi config cleanup | |
| 17:52:16 | sean-k-mooney | there are a few other unstack/clean things i need to do. | |
| 17:52:28 | sean-k-mooney | dansmith: it woudl be nice to keep it but make it conditional | |
| 17:52:32 | bauzas | sean-k-mooney: yup, I saw the same behaviour, even with a ./clean.sh | |
| 17:53:28 | sean-k-mooney | one of the hack we did in the intel nfv ci to make it more reliable back when i was invoved in runing it is we never called unstack | |
| 17:54:07 | sean-k-mooney | nodepool was going to delete the vm so it saved time and sometime the cinder loop device clean woudl randomly fail | |
| 17:55:03 | sean-k-mooney | if i find a commit that removed it for a reason i might revert and make it condional but off by default. | |
| 17:55:50 | openstackgerrit | guang-yee proposed openstack/nova master: pass endpoint interface to Ironic client https://review.openstack.org/640879 | |
| 17:57:11 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Refactor pre-live-migration work out of _do_live_migration https://review.openstack.org/641453 | |
| 17:57:34 | openstackgerrit | Ken'ichi Ohmichi proposed openstack/nova master: doc: cleanup pci.alias references https://review.openstack.org/640882 | |
| 17:59:29 | openstackgerrit | Matt Riedemann proposed openstack/nova master: doc: cleanup pci.alias references https://review.openstack.org/640882 | |
| 17:59:49 | mriedem | efried: you want to just fast approve ^ since ken'ichi updated it and +2ed but his updated need tweaking | |
| 18:04:05 | sean-k-mooney | huh its still there https://github.com/openstack-dev/devstack/blob/master/lib/nova#L230-L239 | |
| 18:04:16 | sean-k-mooney | i must be broken | |
| 18:05:29 | dansmith | heh that's a pretty brute-force thing.. would be wrong if you ever redefined the pattern to not be a prefix | |
| 18:05:34 | dansmith | not that anyone would, but.. | |
| 18:06:15 | sean-k-mooney | well people sometimes change the pattern to just use the uuid but ya its pretty dumb code | |
| 18:06:45 | sean-k-mooney | that sad its 2-7 years old and mostly just worked so its not terible | |
| 18:06:57 | melwitt | mriedem: I'm going to review the rbd volume extend patch. anything else you'd recommend for review today? | |
| 18:07:00 | sean-k-mooney | ill debug this later. | |
| 18:07:03 | dansmith | I mean unlikely to change it on devstack | |
| 18:07:47 | sean-k-mooney | right operators change it for reasons but devs dont really have a need too | |
| 18:08:22 | eandersson | frickler, thanks will double check | |
| 18:08:59 | sean-k-mooney | i was getting migration failure because i had stoped domains and got an error saying the domain already existed | |
| 18:09:35 | sean-k-mooney | nova rolled back the live-migration and everything was fine | |
| 18:21:18 | sean-k-mooney | ok finished testing the sriov migration stuff again. im going rebase it all to the head of master but it seams to work locally | |
| 18:40:46 | mriedem | melwitt: not really, i'm just waiting for another run of the ceph change on that one | |
| 18:41:46 | eandersson | frickler, that was indeed it | |
| 18:43:01 | melwitt | mriedem: ok, thanks. I just went through it, also lgtm | |
| 18:45:22 | mriedem | melwitt: the rebuild from cell0 stuff could probably use more eyes but i've already been over it in parts and continued to find issues, so i'm just not really comfortable with pushing for that on the last day | |