| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-02-21 | |||
| 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 | |
| 15:28:44 | mriedem | the audio on this demo i did is super choppy, | |
| 15:28:54 | sean-k-mooney | mriedem: the force flag for cold migrate? can you rememebr why the instance had to be migrated to a specic host in this case and not jsut any other host | |
| 15:28:56 | mriedem | so figured i need to play around with compression levels | |
| 15:29:16 | mriedem | sean-k-mooney: his spec says, 'because the scheduler kicked out the target host the admin requested' | |
| 15:29:23 | sean-k-mooney | dansmith: probaly obs (open broadcasting studio) | |
| 15:29:26 | mriedem | the scheduler is likely kicking it out for good reason, force is a bad idea | |
| 15:29:30 | dansmith | sean-k-mooney: yeah, sounds right | |
| 15:29:59 | mriedem | https://obsproject.com/download ? | |
| 15:30:20 | sean-k-mooney | mriedem: yep | |