Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-11-13
17:53:18 openstackgerrit Stephen Finucane proposed openstack/nova master: nova-net: Remove 'networks' quota https://review.opendev.org/686812
17:53:18 openstackgerrit Stephen Finucane proposed openstack/nova master: Remove nova-manage network, floating commands https://review.opendev.org/686813
17:53:19 openstackgerrit Stephen Finucane proposed openstack/nova master: nova-net: Remove associate, disassociate network APIs https://review.opendev.org/686814
17:53:19 openstackgerrit Stephen Finucane proposed openstack/nova master: nova-net: Remove 'nova-dhcpbridge' binary https://review.opendev.org/686815
17:53:20 openstackgerrit Stephen Finucane proposed openstack/nova master: nova-net: Remove 'nova-network' binary https://review.opendev.org/686816
17:53:20 openstackgerrit Stephen Finucane proposed openstack/nova master: docs: Blast most references to nova-network https://review.opendev.org/686817
17:53:21 openstackgerrit Stephen Finucane proposed openstack/nova master: WIP https://review.opendev.org/686818
20:08:05 openstackgerrit Dan Smith proposed openstack/nova-specs master: Virtual instance rescue with stable disk devices https://review.opendev.org/693849
20:11:32 openstackgerrit Matt Riedemann proposed openstack/nova master: Create instance action when burying in cell0 https://review.opendev.org/694165
20:27:17 openstackgerrit Matt Riedemann proposed openstack/nova master: Create instance action when burying in cell0 https://review.opendev.org/694165
20:31:50 openstackgerrit Matt Riedemann proposed openstack/nova master: doc: add troubleshooting guide for cleaning up orphaned allocations https://review.opendev.org/691427
21:41:02 openstackgerrit François Palin proposed openstack/nova master: Add retry to cinder api calls related to volume detach https://review.opendev.org/669674
21:44:59 openstackgerrit Merged openstack/nova master: Rename Claims resources to compute_node https://review.opendev.org/679470
21:45:06 openstackgerrit Merged openstack/nova master: Clear instance.launched_on when build fails https://review.opendev.org/683725
21:49:12 melwitt so... after helping a colleague unwedge a failed resize for a customer, I've learned that we intentionally don't roll back port bindings to the source when finish_resize fails https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L5274
21:49:51 melwitt and the reasoning is because the assumption is "everything is fine" on the dest and that only the virt guest needs to be created
21:50:54 melwitt but, when finish_resize fails because update of volume attachments fails (error had to do with "duplicate connectors") it doesn't seem like the dest should really be considered an ok place for the instance to be
21:57:39 melwitt I wonder if fixing this might be as simple as moving the volume attach update before the network setup
21:57:46 melwitt on the dest
21:58:29 mriedem i feel like i recently added that comment but maybe not
21:58:56 mriedem we have a lot of "cleanup networking and cleanup volumes" in live and cold migrate/resize where none of it is really atomic
21:59:36 mriedem i made a note of that in the revert code for cross-cell resize here https://review.opendev.org/#/c/637647/51/nova/compute/manager.py@4690
22:00:19 mriedem i'm not sure what that duplicate connectors error is
22:00:55 mriedem ah that comment is relatively new https://review.opendev.org/#/c/635349/
22:01:22 melwitt yeah we're going to be investigating the root cause of the duplicate connectors thing, never seen that before
22:01:49 melwitt error came back from cinder
22:01:59 mriedem what was the volume type?
22:02:51 melwitt I dunno yet. we only focused on recovery for now, it's late at night for those who were fixing the wedged resize
22:03:40 mriedem so migrate_instance_finish updated the port bindings to point at the dest host and then _update_volume_attachments blew up right?
22:03:43 melwitt we (they) moved the port bindings back to the source manually and then we hard rebooted the instance to get it going for the customer again. they had only wanted to resize to a bigger flavor
22:03:47 melwitt yes
22:04:03 mriedem still, if you get to finish_resize the instance.host already points at the dest
22:04:12 mriedem so they would have had to do more than change port bindings
22:04:17 mriedem to get the guest to reboot on the source
22:04:32 mriedem resize_instance on the source updates the instance.host/node before casting to finish_resize on the dest
22:04:32 melwitt and it started up fine with the larger flavor once the port binding was put back on the source. and prior to that yes I had them change the host/node back to the source
22:04:38 melwitt yeah sorry
22:05:28 melwitt they'll have to fix the allocations too but the host is full so they're gonna move other stuff off and then fix once there's room
22:06:23 mriedem so even if we swapped migrate_instance_finish and _update_volume_attachments for this very specific fail case, the operator still has manual stuff they have to do, like updating the instance to point back at the source host
22:07:17 mriedem and if you do swap those, you likely should revert the instance host/node so hard rebooting on the source host works without manual db surgery
22:07:58 melwitt yeah, fair, but doing that is far easier than the port binding update. that was pretty involved and I personally didn't know how to do it. involved updating the virtual_interfaces table too I think
22:07:59 mriedem hell, even if finish_migration i'm not sure why we wouldn't roll everything back to the source host, but we've already updated port bindings and volume attachments by then which is likely why we say "you're on the dest now"
22:08:33 mriedem the virtual_interfaces table shouldn't have anything to do with it
22:08:40 mriedem unless they created new ports
22:09:05 melwitt ok, maybe they just looked for a uuid or something. I wasn't really understanding
22:10:31 melwitt looks like the volume type is "null" which would be the default
22:10:37 melwitt and I don't remember what the default is
22:10:40 mriedem well, if we changed the resize/cold migrate flow to use the multi-port binding api like we do for live migratoin since rocky thing would be a bit simpler to cleanup, because you'd have 2 port binding resources in neutron rather than one, one is active and one is inactive
22:11:04 mriedem but that's not a backportable change
22:11:18 mriedem and trying to rollback automatically from everything that could go wrong during finish_resize is likely to be pretty hairy
22:11:18 melwitt yeah, if port bindings AND volume attach succeed then saying you're on the dest now is fine
22:11:27 melwitt but if volumes fails, I don't see how dest could be ok
22:12:19 melwitt I was thinking of swapping the network setup and volume update only, in _finish_resize
22:13:44 melwitt so that if volume update fails, set the host/node back to the source like you said and bail. leave the port bindings alone
22:14:02 mriedem i don't think that gets you out of the woods,
22:14:31 mriedem because resize_instance on the source, before casting to finish_resize on the dest, deletes the old volume attachment with the source host connector and creates a new 'empty' volume attachment that gets updated on the dest with the dest host connector
22:14:37 mriedem see _terminate_volume_connections
22:14:44 melwitt O.o
22:14:52 melwitt dammit
22:15:10 mriedem so even if you go back to the source, the volume attachment has to be updated with the source host connector to re-connect the volumes on the source host
22:16:18 melwitt gah, this is over my head. I let them know they might have more to do to fix that VM
22:16:21 mriedem i want to say that at some point booth had a patch which would automatically call revert_resize (dest) from finish_resize (dest) if finish_resize failed
22:17:13 mriedem but i very much doubt that revert_resize is idempotent
22:17:18 melwitt that sounds familiar
22:17:51 mriedem i.e. i would not be surprised if just calling revert_resize from finish_resize fails in some weird way because it has some implicit preconditions that aren't setup when it's called to rollback
22:18:27 melwitt yeah
22:18:32 mriedem now if this all used task flow tasks with built in rollbacks...then we'd be cooking!
22:19:01 mriedem i kid, but that's why i used granular conductor tasks for the cross-cell stuff so we can rollback at certain points in the flow
22:20:01 melwitt task flow did come to mind
22:20:35 mriedem i was hoping efried had it as a keyword and he'd just appear
22:20:50 efried *poof*
22:21:06 efried you should ask johnsom, shiny new taskflow core. Been a while since I touched it.
22:21:31 efried though it's pretty simple when used simply.
22:22:06 efried and you're already doing most of the work, defining `execute` and `rollback` methods in your classes. Make those inherit from Task and do a little plumbing on the engine side, and presto.
22:22:21 johnsom Hi, happy to chat about taskflow, but I'm in a meeting right now. Ping me in 30 if you need something
22:22:44 efried jroll: There's generally a master switch to enable a feature like swtpm. I'm just defining that. I was starting to make it boolean, but it occurred to me that someone might care to say "enable vtpm {1.2 and/or 2.0}". Do you?
22:23:12 jroll efried: I'm not sure if we care
22:23:22 jroll that seems reasonable, though, 2.0 isn't backward compat
22:23:32 jroll (from an application perspective)
22:23:58 efried yeah, rn there's support for both or neither, conf opt notwithstanding.
22:24:12 efried so really it would just allow you to disable one or the other.
22:24:26 melwitt mriedem: how do we find the source host connector? is it in our db or somewhere else?
22:24:52 efried jroll: Easier for me if you don't care. Since you didn't immediately, I'll code it up bool, and we can discuss it in review if necessary.
22:24:54 jroll efried: in the 'both' case, how is it decided which version is presented to a vm?
22:25:01 efried jroll: flavor
22:25:07 jroll ah
22:25:13 efried you are required to ask for one version or the other
22:25:16 jroll yeah, +1 for discuss in review
22:25:20 efried because, as you say, they're not compat.
22:28:53 mriedem melwitt: the host connector is retrieved from the driver
22:29:27 melwitt argh.. ok, I was just seeing that in the code
22:29:27 mriedem cinder does stash the host connector in the volume attachment record when we update the attachment, but as noted in resize_instance we blow away the source host attachment and create a new empty attachment for the dest host to update
22:30:10 mriedem having said that, we (nova) do stash the last host connector used in the bdm.connection_info i think
22:30:14 melwitt :'(
22:30:26 melwitt cray cray
22:30:49 mriedem that might only be when using initialize_connection for legacy attachments though
22:32:14 mriedem i don't think we do that pack rat stash for new style volume attachments because cinder stores that information per volume attachment
22:32:39 mriedem whereas before with initialize_connection we worked with a single attachment record and duplicated it in our bdm.connection_info
22:33:04 melwitt looks like we're ok, this is using ceph and somehow it's connected to the correct host (source)
22:33:16 mriedem in this very specific scenario

Earlier   Later