Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-02-21
14:37:46 sean-k-mooney sahid: zuul seams to be happy with it so that is a good sign.
14:38:33 mriedem lyarwood: since sahid left, question in https://review.openstack.org/#/c/515008/
14:40:07 mriedem lyarwood: i mean, i guess the patch is ok in that if one thing fails the user has to retry (either updating the guest xml or the disconnect from the host) and either thing is now idempotent
14:40:19 mriedem just seems like you'd want to try the cleanup thing that's more likely to fail first
14:41:50 dansmith mriedem: trivial review to correct a stack trace in logs that confused multiple support people: https://review.openstack.org/#/c/546423/
14:42:19 jaypipes efried_omalley: I can shortcut that for you. "Edge", "NFV", "Hyperconverged", "Kubernetes". Rinse and repeat.
14:42:31 mriedem dansmith: timely given i'm asking about https://review.openstack.org/#/c/515008/
14:42:58 dansmith hah
14:43:04 efried_omalley jaypipes: Yeah, would be nice to have a list of checkboxes like Amazon does, where I could exclude those terms and filter down to the 20 or so talks that might interest me.
14:44:03 dansmith jaypipes: lol
14:44:10 dansmith jaypipes: early or late?
14:44:30 jaypipes dansmith: early of course.
14:44:38 dansmith jaypipes: good, just checking.
14:44:59 jaypipes dansmith: you know... pre-cloud.
14:45:16 mnaser wow i just realized this bug has 3 paths, < BFV_RESERVE_MIN_COMPUTE_VERSION --> _check_attach (reserves), >= BFV_RESERVE_MIN_COMPUTE_VERSION -> _check_attach_and_reserve_volume (can reserve if cinder version is too old), >= CINDER_V3_ATTACH_MIN_COMPUTE_VERSION -> _check_attach_and_reserve_volume (will create attachment)
14:45:18 dansmith jaypipes: even the ultra-modern starship days were pre-cloud, so I had to ask
14:45:29 sean-k-mooney jaypipes: im trying to think of something you missed but yep i think those 4 buzz words some it up well maybe add "serverless" of "FaaS"
14:45:38 mnaser so we reserve a volume in the _check_attach_and_reserve_volume path too
14:45:42 mnaser (or we could)
14:46:14 mnaser OH and also if the cinder api doesn't support attachments it will reserve too
14:46:27 jaypipes sean-k-mooney: I think you meant "FaaD".
14:51:32 dansmith mriedem: I think I answered your question on that patch
14:52:42 lyarwood mriedem: https://review.openstack.org/#/c/515008/13/nova/virt/libvirt/driver.py@1555 - otherwise we'd likely see QEMU getting into D state if the instance attempts to write to the underlying storage after we've disconnected it from the compute
14:53:05 dansmith lyarwood: if it even lets us disconnect
14:53:10 lyarwood right
14:53:17 dansmith I would expect it to always fail,
14:53:25 dansmith unless we're echoing into sdX/delete in sysfs
14:53:50 lyarwood yeah there are some backends in os-brick that force things to disconnect like that
14:53:55 dansmith ack
14:54:00 dansmith then that would be like bad and stuff :D
14:54:13 mriedem ok +2 on sahid's
14:54:15 mriedem -1 on dansmith's
14:55:14 dansmith mriedem: raising from there isn't putting the instance into error state,
14:55:19 dansmith but you're saying it does get a fault?
14:55:26 mriedem i never said it would be put into error state
14:55:36 mriedem but yes there is (1) a fault recorded and (2) the instance action event record is marked as failed
14:55:43 mriedem which are ways for an api user to track the detach volume operation in nova
14:55:47 dansmith I know you didn't, but I thought we didn't record a fault unless we put it into error state
14:55:52 mriedem nope
14:56:03 mriedem it's anytime a compute manager method raises an unhandled exception
14:56:12 dansmith okay, so the two people that hit this yesterday weren't looking at faults
14:56:18 dansmith it just never happened,
14:56:25 dansmith and then they saw the trace in the log
14:57:13 openstackgerrit sahid proposed openstack/nova stable/queens: libvirt: disconnect volume from host during detach https://review.openstack.org/546655
14:57:16 mriedem from what i'm reading in the bug, they are just saying, 'don't traceback, give me a useful message'
14:57:31 dansmith right
14:57:36 mriedem your patch is doing the useful message, but then saying the detach was successful
14:57:38 mriedem by returning
14:58:21 dansmith mriedem: returning isn't saying it's successful because the volume is still attached, but I see your point
14:58:39 mriedem well, success as in masking the failure
14:58:45 dansmith having to poll the volume status and then poll instance faults if it doesn't detach in a while is obscure
14:59:10 mriedem they can poll the action record
14:59:15 mriedem more than the fault
14:59:25 mriedem if the action fails, check the fault for why
14:59:42 dansmith it's just not what people do I think
14:59:48 mriedem although we also store that in the instance action event record, but you can only see the traceback there (like a fault) if you're an admin
14:59:49 dansmith they do a thing, they expect it to work
15:00:21 mriedem sure, but you see that with this patch, we fail, and return, we don't record a fault or say the action failed via the instance action record
15:00:24 mriedem whereas before we would
15:00:37 mriedem if the bug is just "don't log a traceback because that's not helpful" we can handle that
15:00:57 dansmith no, I get that this removes a fault that was being added before
15:01:05 dansmith I'm not arguing against fixing that,
15:01:28 dansmith I'm just ruminating about two separate support engineers (and me) not even thinking about polling for the action or fault
15:02:38 dansmith I guess this also deletes the bdm if we don't raise
15:02:50 mriedem which then means you can't try to detach it again..
15:02:55 dansmith yeah
15:03:02 mriedem lyarwood would say you'd be proper fucked
15:03:04 mriedem i assume
15:04:56 lyarwood rather
15:05:52 dansmith hmm, we have no tests for that driver_detach method?
15:06:10 mriedem in DriverVolumeBlockDevice?
15:06:19 dansmith yeah
15:06:22 mriedem they'd likely be in nova.tests.unit.virt.test_block_device, which is mox city
15:06:31 dansmith right, but I don't see any
15:06:56 mriedem could also be some tests in the old nova.tests.unit.compute.test_compute that didn't get ported,
15:07:04 mriedem that driver_detach code used to live in the compute manager
15:07:07 mriedem lyarwood moved it over
15:07:11 mriedem not sure all the tests made the journey
15:07:15 dansmith grep driver_detach -r nova/tests/unit/virt/
15:07:17 dansmith returns nothing
15:07:29 mriedem b/c it's a compute manager thing
15:07:32 mriedem it's not a virt driver thing
15:07:40 dansmith eh?
15:07:43 mriedem it's like everything else in nova.virt.block_device,
15:07:47 mriedem it's an extension of the compute manager
15:07:47 dansmith test_block_device is in there
15:08:22 dansmith # TODO(lyarwood): Test DriverVolumeBlockDevice.driver_detach in
15:08:23 mriedem i know, it's called that because that's what ndipanov called the driver-view bdm wrapper object things or whatever
15:08:28 mriedem bingo
15:08:34 dansmith I see no unmocked calls of it in compute/ either
15:09:52 mriedem you can be the first one
15:10:56 dansmith and I shall
15:12:39 openstackgerrit Surya Seetharaman proposed openstack/nova master: Purge comp_node and res_prvdr records during deletion of cells/hosts https://review.openstack.org/546660
15:14:17 mriedem jaypipes: dansmith: bauzas: i assume you all would be very much -1 on this also https://review.openstack.org/#/c/546482/
15:14:29 mriedem i'm actually pretty annoyed that's being proposed given it was already discussed in queens and we said no
15:14:37 mriedem http://lists.openstack.org/pipermail/openstack-dev/2017-August/121654.html
15:20:32 mriedem anyone ever used SimpleScreenRecord in ubuntu?
15:20:41 mriedem *SimpleScreenRecorder
15:27:32 openstackgerrit Dan Smith proposed openstack/nova master: Avoid exploding if guest refuses to detach a volume https://review.openstack.org/546423
15:28:02 dansmith mriedem: I haven't but sdague was using some broadcast studio thing to do some of that which might be useful

Earlier   Later