| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-04-21 | |||
| 13:44:39 | johnthetubaguy | mriedem: the problem is to automate that, you need to filer all create server requests, to make sure you just in time provision the extra hosts they might need, but maybe thats not really how it works, people probably have to buy the hosts via some other system. | |
| 13:51:47 | stvnoyes | mriedem: johnthetubaguy: re: https://review.openstack.org/#/c/438750 - did you guys come to an agreement on whether something needs to done about the concern on begin_detaching? I looked thru the flows and i *think* it's ok for both the new flow and old flow. | |
| 13:52:27 | johnthetubaguy | stvnoyes: I thought the previous consensus was its not allowed in the new flow, as it doesn't work for multi-attach, etc | |
| 13:53:00 | mriedem | johnthetubaguy: ildikov was supposed to talk to jgriffith about that | |
| 13:53:10 | johnthetubaguy | ah, right, that was the action | |
| 13:53:25 | mriedem | but we did seem to get two answers on whether or not it would work in the new API model during the meeting yesterday | |
| 13:54:04 | stvnoyes | would that mean that detaching would no longer ever be a valid status? | |
| 13:54:15 | mriedem | i'd think it could be at least for a single attachment | |
| 13:54:18 | mriedem | i.e. not multiattach | |
| 13:54:29 | stvnoyes | ok got it | |
| 13:54:32 | johnthetubaguy | mriedem: maybe that... not sure | |
| 13:54:37 | mriedem | maybe the concern is that with multiattach you could go from in-use > detaching > in-use again | |
| 13:54:41 | johnthetubaguy | but we need a solution that works for multi-attach | |
| 13:54:43 | mriedem | if yo'ure going from 2 to 1 attachments | |
| 13:54:59 | mriedem | if you have one attachment, you go in-use > detaching > available | |
| 13:55:00 | johnthetubaguy | well the detaching status would have to be per attachment, I believe | |
| 13:55:09 | mriedem | i'm looking up their data model | |
| 13:55:24 | johnthetubaguy | well, its on the volume today, which I guess is the problem | |
| 13:55:33 | johnthetubaguy | but well worth a check | |
| 13:55:34 | ildikov | johnthetubaguy: I think begin_detaching is on another layer than the attachment calls TBH, therefore keeping it for that state change should be fine | |
| 13:55:38 | mriedem | having said this, when ildikov did multiattach before, begin_detaching worked, but it did in-use > detaching > in-use | |
| 13:55:56 | ildikov | johnthetubaguy: couldn't catch jgriffith yesterday, but I will try next week, he's out today | |
| 13:55:58 | openstackgerrit | sahid proposed openstack/nova master: numa: fix an uninitialized local variable https://review.openstack.org/458848 | |
| 13:56:44 | ildikov | mriedem: yep, we can say we don't want two detach happen at the same time, by which that state machine makes sense | |
| 13:56:47 | johnthetubaguy | mriedem: that works if we only allow people to detach one attachment at once, thats not terrible | |
| 13:57:00 | mriedem | does hemna know? | |
| 13:58:09 | stvnoyes | the new VolumeAttachmentNotFound exception in that change will be needed in other work. I may just put add that exception as a new review to remove the dependency so I can move forward | |
| 13:58:25 | mriedem | there is an attach_status on the volume_attachment table in cinder | |
| 13:58:37 | mriedem | stvnoyes: hold up for now | |
| 13:58:43 | johnthetubaguy | oh... | |
| 13:58:54 | stvnoyes | mriedem: ok | |
| 13:59:01 | mriedem | johnthetubaguy: given this is an open question and begin_detaching is in the api, and what this patch is doing is in the compute side, i don't think we need to hold it up | |
| 13:59:20 | mriedem | johnthetubaguy: this patch is at the bottom of the call chain and has to assume the api has done whatever it needed to do, right? | |
| 13:59:41 | johnthetubaguy | mriedem: yeah, I am not against leaving this as an open question, we do need to keep moving here | |
| 14:00:21 | mriedem | we also get the attach status back when showing details about a volume attachment | |
| 14:00:41 | ildikov | mriedem: I agree, I don't think we decided how to deprecate exactly what, begin_detaching can still fit as it does today | |
| 14:00:50 | johnthetubaguy | but we don't pass the attachment_id in begin_detaching? | |
| 14:00:56 | johnthetubaguy | its just the volume_id? | |
| 14:01:00 | mriedem | i think regardless of what happens, cinder api deals with the multiattach case if begin_detaching is called | |
| 14:01:01 | openstackgerrit | Eric Fried proposed openstack/nova master: PowerVM Driver: spawn/destroy #4: full flavor https://review.openstack.org/391288 | |
| 14:01:05 | stvnoyes | just the volume is passed in | |
| 14:01:07 | mriedem | johnthetubaguy: volume id | |
| 14:01:30 | johnthetubaguy | mriedem: you are right though, this is the API, its not our problem | |
| 14:02:12 | johnthetubaguy | mriedem: so didn't see your comment in the patch till now | |
| 14:02:32 | ildikov | volume_id should be fine for begin_detaching IMHO | |
| 14:02:54 | ildikov | at least from overall behavior perspective | |
| 14:03:48 | johnthetubaguy | well, it does disallow multiple detatch of the same volume, because we can't no which one it is | |
| 14:03:54 | johnthetubaguy | I mean, I am OK with that | |
| 14:03:59 | johnthetubaguy | its just a thing | |
| 14:04:19 | mriedem | we'd know which attachment to drop based on volume_id + instance_uuid | |
| 14:04:42 | johnthetubaguy | do we pass instance_uuid? | |
| 14:04:58 | mriedem | no | |
| 14:05:02 | mriedem | i'm talking future speak | |
| 14:05:06 | johnthetubaguy | oh right | |
| 14:05:09 | mriedem | hi, it's matt, from the future | |
| 14:05:10 | ildikov | johnthetubaguy: until we have multi-attach it's a valid behavior and while working towards multi-attach we can figure it out whether it's the level of lock we want or not | |
| 14:05:18 | johnthetubaguy | mriedem: you in australia? | |
| 14:05:29 | johnthetubaguy | ildikov: yep, agreed with that | |
| 14:05:34 | mriedem | johnthetubaguy: antarctica, it's warm and tropical now | |
| 14:05:39 | mriedem | in the future | |
| 14:05:57 | ildikov | mriedem: there are multiple ways to sort that out if that's what we want, like the one you mentioned | |
| 14:06:04 | ildikov | mriedem: how's the future? :) | |
| 14:06:06 | johnthetubaguy | mriedem: true, true. | |
| 14:06:08 | mriedem | bleak | |
| 14:06:09 | mriedem | very bleak | |
| 14:06:27 | johnthetubaguy | something tells me mriedem likes the snow | |
| 14:06:44 | mriedem | if we needed later on, we could hit a begin_detaching api on the attachments api in cinder, to change the status on a single attachment, which rolls up a status change in the volume itself, idk | |
| 14:06:49 | openstackgerrit | Lee Yarwood proposed openstack/nova master: libvirt: Always disconnect_volume after rebase failures https://review.openstack.org/458807 | |
| 14:06:50 | openstackgerrit | Lee Yarwood proposed openstack/nova master: libvirt: Remove is_job_complete polling after pivot https://review.openstack.org/458854 | |
| 14:06:53 | mriedem | but that seems to be the same as what we have now essentially | |
| 14:07:04 | mriedem | i guess you wouldn't aggregate the status | |
| 14:07:10 | johnthetubaguy | mriedem: its just the multi detach in parallel thing | |
| 14:07:14 | mriedem | the volume would be in-use, one attachment would be 'attached' and the other would be 'detaching' | |
| 14:07:23 | mriedem | so you could detach the other attachment, | |
| 14:07:27 | mriedem | but not the one that's 'detaching' | |
| 14:07:31 | mriedem | i think that's probably the answer | |
| 14:07:35 | johnthetubaguy | mriedem: I think that was in one proposal, the begin_detaching on the attachment, I don't remember why we dropped that now | |
| 14:07:45 | ildikov | I think we only have volume state right now | |
| 14:07:47 | mriedem | once both attachments are deleted, the volume status is 'available' | |
| 14:07:53 | johnthetubaguy | mriedem: yeah, overall its "in-use" until the last one is detaching, I gues... | |
| 14:08:12 | mriedem | ildikov: there is an attach_status column on the volume_attachment table in cinder | |
| 14:08:31 | mriedem | johnthetubaguy: yeah that sounds right, in-use until ==1 attachment is 'detaching', then the volume itself is 'detaching' | |
| 14:08:37 | mriedem | once all attachments are deleted, the volume is 'available' | |
| 14:08:46 | johnthetubaguy | anyways, I am +W on the detach patch, lets punt this, seems like its safe for now, appreciate stvnoyes for checking all that. | |
| 14:08:54 | mriedem | so yeah, probably long-term we need a begin_detaching action on the attachments API | |
| 14:08:57 | johnthetubaguy | mriedem: yeah, same for attaching I guess | |
| 14:08:57 | ildikov | mriedem: right, I remember that one, I just don't think we played that much with that state machine yet | |
| 14:09:18 | mriedem | until then you can only detach one attachment at a time | |
| 14:09:24 | johnthetubaguy | mriedem: +1 | |
| 14:09:24 | ildikov | johnthetubaguy: sounds good, thanks | |
| 14:09:27 | mriedem | which today is all you can do anyway b/c we don't support multiattac | |
| 14:09:32 | ildikov | mriedem: +1 | |
| 14:09:50 | mriedem | maybe we should write this down somewhere :) | |
| 14:09:56 | mriedem | b/c you know in 3 weeks we'll have forgotten | |
| 14:09:58 | johnthetubaguy | mriedem: you read my mind | |
| 14:10:03 | stvnoyes | thanks! moving on to the next one... | |
| 14:10:34 | ildikov | mriedem: johnthetubaguy: wiki page, devref, etherpad, love letter? | |
| 14:10:51 | mriedem | i haven't written a love letter since high school | |
| 14:10:57 | mriedem | when we didn't have cell phones | |
| 14:11:01 | gibi | mriedem: hi! thanks for the reviews and fixes on the https://review.openstack.org/#/q/topic:bp/additional-notification-fields-for-searchlight+status:open series. | |