| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-02-21 | |||
| 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 | |
| 15:30:36 | openstackgerrit | Dan Smith proposed openstack/nova master: Avoid exploding if guest refuses to detach a volume https://review.openstack.org/546423 | |
| 15:30:54 | sean-k-mooney | mriedem: its inteaded for live stream to things like twitch or youtube but it support local recoding too | |
| 15:31:22 | mriedem | ok cool, if i can't figure out the audio issues on SSR i'll try this out | |
| 15:31:58 | dansmith | mriedem: you just need to narrate over your existing video, right? | |
| 15:32:28 | dansmith | you can just record yourself to an audio file while watching the video and then mux in the audio with something like avidemux2 | |
| 15:32:47 | kashyap | Or you can add subtitles file, if you have the patience | |
| 15:33:10 | openstackgerrit | Mohammed Naser proposed openstack/nova master: Check if attachment_id exists before deleting it https://review.openstack.org/546398 | |
| 15:33:19 | sean-k-mooney | dansmith: im guessing mriedem is also would prefer not to have to do any video editing. but ya you could use audacity to recored audio the mux them together as dan said | |
| 15:33:31 | mnaser | mriedem: ^ much cleaner fix, thats for your suggestions | |
| 15:33:34 | sean-k-mooney | * as you said | |
| 15:33:43 | dansmith | sean-k-mooney: I think he already has a video that demos what he wants, is my point | |
| 15:33:50 | mnaser | oh i saw some comments you left after, let me address them | |
| 15:34:12 | mriedem | i'd prefer to not have to do the audio separately, | |
| 15:34:21 | mriedem | the actual demo itself isn't hard to run through | |
| 15:34:54 | mriedem | anyway, this will be my day now.. | |
| 15:35:43 | sean-k-mooney | mnaser: so ya back to the force cold migration i agree if the schduler elminated the node it should not be ignored or bad things can happen. the use case was for maintenance however so i dont know why you could not disable the host you wanted to do mantenance on then just migrate there instance to any other nodes | |
| 15:35:53 | mnaser | mriedem: ^ :) | |
| 15:36:00 | mnaser | that's probably for ya | |
| 15:39:05 | sean-k-mooney | mnaser: hehe ya sorry | |
| 15:39:48 | sean-k-mooney | i asked the question in the spec. but i dont think there is a good answer/justificaiton for why you should bypass the schduler. | |
| 15:44:46 | mriedem | dansmith: one nit on that patch, let's throw the volume_id into the warning message, then i'm +2 | |
| 15:46:06 | openstackgerrit | Mohammed Naser proposed openstack/nova master: Ensure attachment_id always exists for block device mapping https://review.openstack.org/546398 | |
| 15:47:37 | openstackgerrit | Dan Smith proposed openstack/nova master: Avoid exploding if guest refuses to detach a volume https://review.openstack.org/546423 | |
| 15:47:37 | mnaser | there.. that should be it .. for now :> | |
| 15:47:40 | dansmith | mriedem: ^ | |
| 15:51:28 | cfriesen | mriedem: given your comment about the force flag for evacuate and live migrate...do we want to deprecate that? :) Or is that just a case of it being stuck with it since we'll have to keep the code around until/unless we bump the minimum API version? | |
| 15:56:02 | mriedem | mnaser: ok +2 on that, with some musing comments in there for order of fixes | |
| 15:56:17 | mriedem | cfriesen: the latter | |
| 16:00:12 | mnaser | mriedem: cool, thanks, once this merges ill reorder to get this fix in stable/pike just before yours so functional tests pass for all patches :> | |
| 16:03:17 | mriedem | mnaser: sounds good, and that also takes care of my 'window of failure' worry in there | |
| 16:03:48 | mnaser | mriedem: oh you're right | |
| 16:05:47 | mriedem | so yeah we likely want your fix here before my fix in the series of backports | |
| 16:05:54 | openstackgerrit | Bence Romsics proposed openstack/osc-placement master: WIP skip (v1.13) https://review.openstack.org/546674 | |
| 16:05:55 | openstackgerrit | Bence Romsics proposed openstack/osc-placement master: Add nested resource providers (v1.14) https://review.openstack.org/546675 | |
| 16:21:41 | openstackgerrit | Surya Seetharaman proposed openstack/nova-specs master: Support disabling a cell https://review.openstack.org/546684 | |