| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-12-12 | |||
| 17:42:29 | smcginnis | mriedem: Why does that need to change? | |
| 17:42:30 | mriedem | the connector dict is host detail stuff from brick | |
| 17:42:47 | mriedem | smcginnis: see https://review.openstack.org/527468 | |
| 17:43:11 | mriedem | it doesn't *have* to change, but it's really clunky API behavior the way it is | |
| 17:45:12 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Pass mountpoint to volume attachment_create with connector https://review.openstack.org/527479 | |
| 17:45:14 | mriedem | and ^ | |
| 17:47:01 | dansmith | gibi: are you planning to hit this? https://review.openstack.org/#/c/517119/ | |
| 17:47:24 | dansmith | aside from the comment jaypipes is going to add, it looks okay to me, but I'm admittedly glossing over a lot of the database details | |
| 17:51:01 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Pass mountpoint to volume attachment_update https://review.openstack.org/527468 | |
| 17:51:01 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Update and complete volume attachments during resize https://review.openstack.org/527228 | |
| 17:51:02 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Pass bdms to versioned notifications during finish_revert_resize https://review.openstack.org/527446 | |
| 17:51:02 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Pass mountpoint to volume attachment_create with connector https://review.openstack.org/527479 | |
| 17:55:56 | jaypipes | dansmith: I already added my comment. | |
| 17:56:13 | dansmith | jaypipes: not that I see | |
| 17:56:47 | jaypipes | dansmith: it's inline in https://review.openstack.org/#/c/517119/17/nova/objects/resource_provider.py | |
| 17:58:25 | dansmith | jaypipes: your last comment on that patch asks if you can just add a note to satisfy the concern.. doesn't that mean you're going to respin and, you know, add a note? | |
| 17:58:34 | dansmith | L3111 | |
| 17:59:48 | jaypipes | dansmith: heh, sorry, I thought you were referring to my review comment as "the comment jaypipes is going to add". lol, sorry | |
| 18:00:10 | dansmith | ...no :) | |
| 18:00:57 | jaypipes | dansmith: I understand ya now. I havent' updated the series yet because I already recheck'd a couple in the series and didn't want to needlessly reset the gates for a small thing like that | |
| 18:01:25 | dansmith | I think the gate is fairly toast at the moment.. it's failing most status calls for me at least | |
| 18:02:15 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Remove unnecessary connector stash in attachment_update https://review.openstack.org/527484 | |
| 18:02:38 | melwitt | mriedem: patches look good to me, just waiting on test results. added a comment on the top patch about using a helper function | |
| 18:03:09 | mriedem | yeah i thought about that, but figured i'd do a refactor later on top if i'm going to do one | |
| 18:03:28 | mriedem | test results will probably be a few days coming... | |
| 18:03:38 | mriedem | i will kill some time with lunch | |
| 18:05:34 | jgriffith | mriedem: not sure why that's confusing? | |
| 18:12:54 | jgriffith | mriedem: added notes/questions to the bug report | |
| 18:24:54 | cdent | dansmith: do you have any feelings, pro or con, on starting to use the new POST /allocations functionality? now or rocky? | |
| 18:26:21 | dansmith | cdent: which new post behavior? the atomic multiple thing? | |
| 18:26:31 | cdent | aye | |
| 18:26:39 | dansmith | why would we wait? | |
| 18:27:01 | cdent | mostly because of coding and reviewing bandwidth, I guess? | |
| 18:28:12 | dansmith | cdent: well, we have a race right now, so I'd tend to think we should be fixing that and not delaying longer than necessary | |
| 18:29:30 | cdent | fine by me | |
| 18:30:08 | cdent | persistent | |
| 18:32:01 | cburgess | dansmith: Random object question for you if you are around. | |
| 18:32:15 | dansmith | cburgess: random object answer readied | |
| 18:32:56 | cburgess | dansmith Let me make a gist.. that will be simpler to show you. | |
| 18:36:33 | cburgess | dansmith I have found a script someone wrote (lets not ask to many question) that does something like the following... https://gist.github.com/cburgess/bbedb9038a79e8dfc92375b383b1fc4a | |
| 18:37:20 | dansmith | uhm, okay | |
| 18:37:32 | cburgess | So they are importing the objects directly. My question is... is that going direct to the DB, or making a call to conductor in this case? | |
| 18:37:41 | ildikov | melwitt: is your question on shelve offload about the case when we shelve an instance with volume attached? | |
| 18:38:17 | dansmith | cburgess: this is the magic that causes object operations to go via some other service (i.e. conductor): https://github.com/openstack/nova/blob/master/nova/cmd/compute.py#L56 | |
| 18:38:27 | dansmith | cburgess: without that, they always default to direct | |
| 18:38:38 | cburgess | Where direct = DB? | |
| 18:38:44 | dansmith | cburgess: yes | |
| 18:38:55 | cburgess | dansmith I'm going to go cry in the corner now. Thank you. | |
| 18:39:06 | dansmith | cburgess: uh, why? you wanted via conductor? | |
| 18:39:39 | cburgess | dansmith Well conductor would be better then the DB yes. Though they shouldn't be loading the objects directly like that. They should be using the API. | |
| 18:39:42 | melwitt | ildikov: yes, I think so. it's not really related to that patch but something I noticed during review | |
| 18:40:22 | dansmith | cburgess: why would conductor be better? you'd have to setup all the rpc stuff to do that thing | |
| 18:40:28 | cburgess | dansmith As in this isn't code in nova some place. This is something entirely external to nova that wants some data and its choosing to do so loading the objects directly. | |
| 18:40:33 | dansmith | cburgess: but agree, if you can get it efficiently from the api that's better | |
| 18:40:34 | ildikov | melwitt: this is how it looks like for attaching a volume to a shelved offloaded instance: https://github.com/openstack/nova/blob/master/nova/compute/api.py#L3720 | |
| 18:41:17 | cburgess | dansmith My real issue is they should not be loading the objects at all. That needs to be via the API. Clients of the cloud don't get to load internal object code and muck about in the DB. They should talk to the API. | |
| 18:41:18 | dansmith | cburgess: ack, you're crying because someone who is paying you money is doing a scary thing and you have to figure out how to fix that and keep getting paid, is that right? | |
| 18:41:26 | ildikov | melwitt: we call attachment_create as part of the check_attach_and_reserve_volume call and in addition we call attachment_complete in _attach_volume_shelved_offloaded to put the volume to 'in-use' | |
| 18:41:29 | dansmith | cburgess: ack, yes | |
| 18:41:31 | cburgess | dansmith Something like that. | |
| 18:42:05 | dansmith | cburgess: just wanted to make sure you weren't complaining that my objects don't default to rpc or something :P | |
| 18:42:09 | ildikov | melwitt: it's honestly a workaround for now as the volume would be perfectly fine in 'reserved' state, but that's not how the old flow worked, so changing that needs an API microversion | |
| 18:42:34 | cburgess | dansmith Nope I love you objects, and the conductor. | |
| 18:42:42 | dansmith | hah | |
| 18:43:03 | ildikov | melwitt: when we do the offload the part when we call attachment_create is correct as attachment_delete detaches the volume completely | |
| 18:43:23 | ildikov | melwitt: what I'm unsure at the moment is whether we call attachment_complete in that case as well | |
| 18:43:39 | melwitt | ildikov: what I mean is, when we go to shelve an instance, we detach the volume. but with that recent change it's not only deleting the attachment, it creates a new attachment. which I didn't think we would want while an instance is shelved-offloaded | |
| 18:44:15 | melwitt | oh really? so we want to keep an attachment (create a new one) while shelve offloaded? | |
| 18:44:38 | mriedem | melwitt: yes, | |
| 18:44:42 | mriedem | that keeps the volume reserved | |
| 18:44:45 | ildikov | melwitt: but with shelve the old flow only removes the connection on the host but the volume is still appearing to be attached to that instance on the Cinder side | |
| 18:44:47 | mriedem | while the instance is shelved | |
| 18:45:08 | melwitt | okay, good then. sorry for the noise | |
| 18:45:12 | mriedem | the empty vol attachment keeps the volume reserved but that vol attachment isn't actually connected to any host | |
| 18:45:16 | ildikov | melwitt: so when you call attachment_delete in terminate_connections that moves to volume back to 'available' | |
| 18:45:33 | mriedem | unless there is another attachment on the volume already | |
| 18:45:34 | ildikov | mriedem: do we call attachment_complete in every bit of shelve, where we need to? | |
| 18:45:42 | mriedem | once the attachment count is 0, then the volume is 'available' again | |
| 18:46:10 | mriedem | ildikov: you mean unshelve? | |
| 18:46:28 | mriedem | ildikov: yes via https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L4783 | |
| 18:46:34 | mriedem | same as normal server create | |
| 18:46:53 | mriedem | happens down in the DriverVolumeBlockDevice.attach magic cauldron | |
| 18:47:15 | mriedem | magic cauldron of attachitude | |
| 18:47:24 | mriedem | henceforth shall it be known | |
| 18:47:35 | ildikov | mriedem: I meant whether we have actions as shelve and attaching a volume to a shelved instance and which call inside the Nova code covers these | |
| 18:47:56 | ildikov | mriedem: as we want the volume to be 'in-use' with the new flow as well so we added the additional attachment_complete call | |
| 18:48:05 | mriedem | attaching to a shelved offloaded instance happens in the api and we complete the attachment there too as you pointed out | |
| 18:48:37 | mriedem | remember that we can complete an empty attachment, | |
| 18:48:41 | mriedem | and update a completed attachment | |
| 18:48:48 | mriedem | weird as that may be | |
| 18:48:53 | mriedem | that's why unshelve works | |
| 18:49:07 | mriedem | *that's why unshelve works for the case that the volume is attached while the instance is shelved offloaded | |
| 18:51:49 | ildikov | mriedem: I understand that part | |
| 18:52:08 | ildikov | mriedem: I was just wondering whether we do all the steps when we shelve offload an instance wit volume attached | |
| 18:52:13 | ildikov | as I guess we can do that too | |
| 18:52:46 | ildikov | which might be the same set of calls under the hood, just wanted to double check after melwitt's confusion :) | |
| 18:54:33 | mriedem | yes, _prep_block_device happens it | |
| 18:54:36 | mriedem | on unshelve | |
| 18:54:40 | mriedem | *handles | |
| 18:56:24 | ildikov | ok, cool | |