| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-12-11 | |||
| 19:02:51 | smatzek | mriedem: https://bugs.launchpad.net/nova/+bug/1737599 | |
| 19:02:52 | openstack | Launchpad bug 1737599 in OpenStack Compute (nova) "Instance resize with attach volume fails" [Undecided,New] | |
| 19:03:38 | mriedem | i can see what's different about this flow from the bfv one | |
| 19:04:19 | mriedem | we seem to lose the connection_info during some of the much complicated bdm object -> driver bdm -> dict block_device_info conversions... | |
| 19:04:28 | mriedem | or, it was just never set in the first place | |
| 19:04:35 | mriedem | my guess is the latter | |
| 19:05:08 | mriedem | i can definitely see where it blows up though yeah | |
| 19:10:51 | openstackgerrit | Ed Leafe proposed openstack/nova master: Change RPC for select_destinations() https://review.openstack.org/516707 | |
| 19:10:52 | openstackgerrit | Ed Leafe proposed openstack/nova master: Make conductor pass and use host_lists https://review.openstack.org/511358 | |
| 19:10:52 | openstackgerrit | Ed Leafe proposed openstack/nova master: Move the claim_resources method to scheduler utils https://review.openstack.org/511357 | |
| 19:10:53 | openstackgerrit | Ed Leafe proposed openstack/nova master: Change compute RPC to use alternates for resize https://review.openstack.org/526436 | |
| 19:11:25 | edleafe | mriedem: cleaned up ^^ | |
| 19:18:49 | openstackgerrit | Chris Dent proposed openstack/nova master: [placement] Add cache headers to placement api requests https://review.openstack.org/521640 | |
| 19:18:50 | openstackgerrit | Chris Dent proposed openstack/nova master: [placement] Add info about last-modified to contrib docs https://review.openstack.org/526084 | |
| 19:20:15 | cdent | mriedem, edleafe ^ that fixes the several suggestions matt had | |
| 19:20:50 | mriedem | smatzek: ildikov: i think i have the resize thing figured out | |
| 19:23:25 | smatzek | mriedem: great, thanks | |
| 19:24:23 | mriedem | i'll post a patch and you can push a trove depends-on to see if it works | |
| 19:28:59 | smatzek | would the thing you found possibly cause the mountpoint to be lost in Cinder's attachments during rebuild? | |
| 19:29:19 | smatzek | mriedem: ^ | |
| 19:29:52 | ildikov | mriedem: cool, that was quick! | |
| 19:30:54 | mriedem | smatzek: don't know, one problem at a time | |
| 19:31:02 | mriedem | smatzek: but maybe | |
| 19:31:52 | openstackgerrit | Murali Annamneni proposed openstack/nova master: Enables MySQL Cluster Support for Nova https://review.openstack.org/446643 | |
| 19:31:54 | mriedem | smatzek: there is a good chance it is though | |
| 19:41:40 | ildikov | smatzek: that log indicates that there might be a step missing from rebuild | |
| 19:42:12 | ildikov | smatzek: will check what mriedem has as a fix and then we can compare whether we covered everything or not | |
| 19:42:55 | ericyoung | Any idea when https://review.openstack.org/#/c/407440/ will get some eyes on it? | |
| 19:43:08 | ericyoung | It is the patch for ScaleIO ephemeral/image volumes | |
| 19:48:30 | openstackgerrit | Matt Riedemann proposed openstack/nova master: WIP: Save driver_volume_type when refreshing attachment connection_info https://review.openstack.org/527228 | |
| 19:48:31 | mriedem | smatzek: ildikov: ^ see if that fixes the resize issue in trove; needs unit tests | |
| 19:51:30 | mriedem | actually, i think that will fix the VolumeDriverNotFound but we'll still fail to connect the volume on the destination host | |
| 19:51:39 | mriedem | because the new attachment connection_info doesn't have 'data' set | |
| 19:52:32 | smatzek | so should I hold off on pushing a depends-on test to Trove? | |
| 19:52:43 | mriedem | just start that depends-on to make sure it blows up where i think it is now | |
| 19:52:51 | mriedem | and we'll need it anyway to verify the eventual fix | |
| 19:54:10 | smatzek | mriedem: https://review.openstack.org/#/c/527230. It looks like the depends-on linked correctly. To be honest I've never done one cross-projects so hopefully this is right. | |
| 19:54:11 | smatzek | https://review.openstack.org/#/c/527230 | |
| 19:54:29 | mriedem | yes that should work | |
| 20:06:34 | mriedem | ok this is a bit shittier than i thought, but i think i know of another relatively painless fix | |
| 20:06:51 | mriedem | to avoid doing crazy crap in the virt driver(s) or nova/virt/block_device.py code | |
| 20:07:16 | ildikov | mriedem: hmm, we are explicitly transforming 'driver_volume_type' so it is supposed to come from Cinder already | |
| 20:07:35 | mriedem | ildikov: it's pulled out of the connection_info from the attachment, | |
| 20:07:43 | mriedem | which is empty for the new attachment which is not yet connected on the dest host | |
| 20:07:58 | mriedem | i.e. we don't call attachment_update in the cold migrate flow | |
| 20:08:21 | mriedem | for cold migration, the drivers just do all the work on the hosts with connecting the volume | |
| 20:08:40 | mriedem | and the old init/terminate_connection calls | |
| 20:08:42 | ildikov | hmm, but shouldn't we call attachment_update with the new flow now? | |
| 20:08:42 | mriedem | on the volume | |
| 20:08:50 | mriedem | yes - but we aren't | |
| 20:08:57 | mriedem | so that's what i think i need to add | |
| 20:09:23 | ildikov | sorry, I just got confused with the changes in your patch in mind | |
| 20:09:43 | ildikov | as the log looks like we have a missing attahcment_update call somewhere | |
| 20:09:58 | ildikov | and then we need to check whether attachment_complete happens there or not | |
| 20:10:44 | mriedem | yes my patch does'nt fix it | |
| 20:10:51 | mriedem | i think i know what needs to happen | |
| 20:10:54 | mriedem | but making coffee first | |
| 20:13:00 | ildikov | fair enough | |
| 20:31:08 | openstackgerrit | Ed Leafe proposed openstack/nova master: Make conductor pass and use host_lists https://review.openstack.org/511358 | |
| 20:31:09 | openstackgerrit | Ed Leafe proposed openstack/nova master: Change compute RPC to use alternates for resize https://review.openstack.org/526436 | |
| 20:38:55 | _ix | More high praise if you're interested. I think that newish Pike feature in disabling nodes saved me about half a day of troubleshooting. | |
| 20:39:06 | _ix | Thank you! | |
| 20:41:04 | mriedem | thanks for the feedback | |
| 20:41:15 | mriedem | always nice to know when people actually "like" something | |
| 20:42:43 | cdent | 👍 | |
| 20:43:42 | mriedem | dtantsur|afk: do you have a run on https://review.openstack.org/#/c/508555/ yet? | |
| 20:44:06 | mriedem | TheJulia: do you know where ^ is being tested to see if it unblocks the ironic multinode job? | |
| 20:44:50 | mriedem | dtantsur|afk: TheJulia: nevermind, found it https://review.openstack.org/#/c/527075/ | |
| 20:45:11 | TheJulia | :) | |
| 20:45:29 | mriedem | is ironic-tempest-dsvm-ipa-wholedisk-agent_ipmitool-tinyipa-multinode the job to care about? | |
| 20:51:48 | TheJulia | mriedem: yes | |
| 20:56:54 | mriedem | dansmith: comments in john's rebalance patch - https://review.openstack.org/#/c/508555/ - i think it looks better, but we could maybe drop the new object query method too - i think there is an existing one that could be re-used in there | |
| 21:00:15 | dansmith | mriedem: yep, looks right | |
| 21:06:41 | openstackgerrit | Matt Riedemann proposed openstack/nova master: WIP: Update and complete volume attachments during resize https://review.openstack.org/527228 | |
| 21:06:55 | mriedem | smatzek: ildikov: ^ should do it for resize | |
| 21:08:06 | mriedem | smatzek: the rebuild fix will be different, so do you have a separate bug for that? | |
| 21:08:09 | mriedem | with links to the logs | |
| 21:08:41 | smatzek | I don't, but I'll open one. | |
| 21:09:28 | mriedem | dansmith: ok, ironic multinode job passed and given dmitry and john are gone, i could update quick and get another run, or just wait for john to do it - but i think he's probably on paternity | |
| 21:09:56 | mriedem | maybe it's fine to wait another day assuming this has been broken since pike | |
| 21:10:13 | dansmith | yeah I think he is.. up to you, I'm around to +2 if you want, or I can do it | |
| 21:10:16 | mriedem | TheJulia: is this a latent bug or regression in pike? | |
| 21:10:18 | dansmith | if you update I think jaypipes and I can get it | |
| 21:10:23 | mriedem | dansmith: ok i'll update quick | |
| 21:12:30 | TheJulia | mriedem: don't have enough context to say either way | |
| 21:13:15 | ildikov | mriedem: so resize is using a different flow and not calling bdm.attach(), right? | |
| 21:13:26 | mriedem | ildikov: correct | |
| 21:14:02 | mriedem | doing it the way i'm doing it is a lot easier than trying to re-plumb how things work for connecting the volume during a resize | |
| 21:14:24 | mriedem | since in the old flow, we never needed to call bdm.attach because the volume was already attached | |
| 21:14:41 | mriedem | the new flow is all about the attachment though, so we have to do things differently | |
| 21:15:20 | mriedem | we could call bdm.attach and pass do_driver_attach=False, but then we have to do that before calling driver.finish_migration and it seems wrong to 'complete' the attachment before we've actually connected it on the host | |
| 21:16:39 | smatzek | mriedem: I can open a bug for the rebuild one if you want. If you know there are changes needed there and want a bug to work them. However, I'm not convinced there is a bug in rebuild itself after digging through the code this afternoon. | |
| 21:20:15 | smatzek | Trove does this with cinderclient before calling nova rebuild, and at this point the device/mountpoint is /dev/na, which means it was lost previous to rebuild. https://github.com/openstack/trove/blob/master/trove/taskmanager/models.py#L1369-L1371 | |
| 21:20:53 | smatzek | this probably needs more digging on my part to see what was done in the test suite to that instance before getting to this point. | |
| 21:21:24 | ildikov | mriedem: ok, that makes sense, I don't have a Devstack running at the moment, will set up one and check these flows as I always have to re-understand the whole thing if I just try to follow this code by reading... | |
| 21:22:08 | mriedem | smatzek: ok i'll leave rebuild up to you | |
| 21:22:28 | smatzek | mriedem: I need to take off to get the kids from school and run a few errands so I'll dig into it later. | |
| 21:22:37 | mriedem | ildikov: we definitely have a todo to add a test to tempest for resize with a volume attached (non-bfv) | |
| 21:24:07 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Re-use existing ComputeNode on ironic rebalance https://review.openstack.org/508555 | |
| 21:24:08 | mriedem | dansmith: ^ i'll recheck the ironic canary patch | |